diff options
author | Pedro Alves <pedro@palves.net> | 2020-07-04 20:51:36 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2020-07-06 19:57:57 +0100 |
commit | 367aff81be9d8fcdae383d72a555b14998aa3251 (patch) | |
tree | 5b5baf2f6b0aaf7dc6ce9e63d164717ab05ef512 | |
parent | 85dae3461286b416a19c7ab53681786be6965cc0 (diff) | |
download | gdb-367aff81be9d8fcdae383d72a555b14998aa3251.zip gdb-367aff81be9d8fcdae383d72a555b14998aa3251.tar.gz gdb-367aff81be9d8fcdae383d72a555b14998aa3251.tar.bz2 |
Make handle_no_resumed transfer terminal
Let's consider the same use case as in the previous commit:
Say you have two inferiors 1 and 2, each connected to a different
target, A and B.
Now say you set inferior 2 running, with "continue &".
Now you select a thread of inferior 1, say thread 1.2, and continue in
the foreground. All other threads of inferior 1 are left stopped.
Thread 1.2 exits, and thus target A has no other resumed thread, so it
reports TARGET_WAITKIND_NO_RESUMED.
At this point, because the threads of inferior 2 are still executing
the TARGET_WAITKIND_NO_RESUMED event is ignored.
Now, the user types Ctrl-C. Because GDB had previously put inferior 1
in the foreground, the kernel sends the SIGINT to that inferior.
However, no thread in that inferior is executing right now, so ptrace
never intercepts the SIGINT -- it is never dequeued by any thread.
The result is that GDB's CLI is stuck. There's no way to get back the
prompt (unless inferior 2 happens to report some event).
The fix in this commit is to make handle_no_resumed give the terminal
to some other inferior that still has threads executing so that a
subsequent Ctrl-C reaches that target first (and then GDB intercepts
the SIGINT). This is a bit hacky, but seems like the best we can do
with the current design.
I think that putting all native inferiors in their own session would
help fixing this in a clean way, since with that a Ctrl-C on GDB's
terminal will _always_ reach GDB first, and then GDB can decide how to
pause the inferior. But that's a much larger change.
The testcase added by the following patch needs this fix.
gdb/ChangeLog:
PR gdb/26199
* infrun.c (handle_no_resumed): Transfer terminal to inferior with
executing threads.
-rw-r--r-- | gdb/infrun.c | 66 |
1 files changed, 54 insertions, 12 deletions
diff --git a/gdb/infrun.c b/gdb/infrun.c index 0f2f45a..158b199 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5071,20 +5071,52 @@ handle_no_resumed (struct execution_control_state *ecs) the synchronous command and show "no unwaited-for " to the user. */ - { - scoped_restore_current_thread restore_thread; + inferior *curr_inf = current_inferior (); - for (auto *target : all_non_exited_process_targets ()) - { - switch_to_target_no_thread (target); - update_thread_list (); - } - } + scoped_restore_current_thread restore_thread; + + for (auto *target : all_non_exited_process_targets ()) + { + switch_to_target_no_thread (target); + update_thread_list (); + } + + /* If: + + - the current target has no thread executing, and + - the current inferior is native, and + - the current inferior is the one which has the terminal, and + - we did nothing, + + then a Ctrl-C from this point on would remain stuck in the + kernel, until a thread resumes and dequeues it. That would + result in the GDB CLI not reacting to Ctrl-C, not able to + interrupt the program. To address this, if the current inferior + no longer has any thread executing, we give the terminal to some + other inferior that has at least one thread executing. */ + bool swap_terminal = true; + + /* Whether to ignore this TARGET_WAITKIND_NO_RESUMED event, or + whether to report it to the user. */ + bool ignore_event = false; for (thread_info *thread : all_non_exited_threads ()) { - if (thread->executing - || thread->suspend.waitstatus_pending_p) + if (swap_terminal && thread->executing) + { + if (thread->inf != curr_inf) + { + target_terminal::ours (); + + switch_to_thread (thread); + target_terminal::inferior (); + } + swap_terminal = false; + } + + if (!ignore_event + && (thread->executing + || thread->suspend.waitstatus_pending_p)) { /* Either there were no unwaited-for children left in the target at some point, but there are now, or some target @@ -5094,9 +5126,19 @@ handle_no_resumed (struct execution_control_state *ecs) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_NO_RESUMED " "(ignoring: found resumed)\n"); - prepare_to_wait (ecs); - return 1; + + ignore_event = true; } + + if (ignore_event && !swap_terminal) + break; + } + + if (ignore_event) + { + switch_to_inferior_no_thread (curr_inf); + prepare_to_wait (ecs); + return 1; } /* Go ahead and report the event. */ |