Cancel execution command on thread exit, when stepping, nexting, etc.

If your target has no support for TARGET_WAITKIND_NO_RESUMED events
(and no way to support them, such as the yet-unsubmitted AMDGPU
target), and you step over thread exit with scheduler-locking on, this
is what you get:

 (gdb) n
 [Thread ... exited]
 *hang*

Getting back the prompt by typing Ctrl-C may not even work, since no
inferior thread is running to receive the SIGINT.  Even if it works,
it seems unnecessarily harsh.  If you started an execution command for
which there's a clear thread of interest (step, next, until, etc.),
and that thread disappears, then I think it's more user friendly if
GDB just detects the situation and aborts the command, giving back the
prompt.

That is what this commit implements.  It does this by explicitly
requesting the target to report thread exit events whenever the main
resumed thread has a thread_fsm.  Note that unlike stepping over a
breakpoint, we don't need to enable clone events in this case.

With this patch, we get:

 (gdb) n
 [Thread 0x7ffff7d89700 (LWP 3961883) exited]
 Command aborted, thread exited.
 (gdb)

Change-Id: I901ab64c91d10830590b2dac217b5264635a2b95
This commit is contained in:
Pedro Alves
2022-06-21 18:05:19 +01:00
parent 9ba356a18f
commit 1a41079f04
2 changed files with 81 additions and 20 deletions

View File

@@ -1818,6 +1818,22 @@ displaced_step_prepare (thread_info *thread)
return status; return status;
} }
/* True if any thread of TARGET that matches RESUME_PTID requires
target_thread_events enabled. This assumes TARGET does not support
target thread options. */
static bool
any_thread_needs_target_thread_events (process_stratum_target *target,
ptid_t resume_ptid)
{
for (thread_info *tp : all_non_exited_threads (target, resume_ptid))
if (displaced_step_in_progress_thread (tp)
|| schedlock_applies (tp)
|| tp->thread_fsm () != nullptr)
return true;
return false;
}
/* Maybe disable thread-{cloned,created,exited} event reporting after /* Maybe disable thread-{cloned,created,exited} event reporting after
a step-over (either in-line or displaced) finishes. */ a step-over (either in-line or displaced) finishes. */
@@ -1841,9 +1857,10 @@ update_thread_events_after_step_over (thread_info *event_thread,
else else
{ {
/* We can only control the target-wide target_thread_events /* We can only control the target-wide target_thread_events
setting. Disable it, but only if other threads don't need it setting. Disable it, but only if other threads in the target
enabled. */ don't need it enabled. */
if (!displaced_step_in_progress_any_thread ()) process_stratum_target *target = event_thread->inf->process_target ();
if (!any_thread_needs_target_thread_events (target, minus_one_ptid))
target_thread_events (false); target_thread_events (false);
} }
} }
@@ -2418,12 +2435,25 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig)
else else
target_thread_events (true); target_thread_events (true);
} }
else if (tp->thread_fsm () != nullptr)
{
gdb_thread_options options = GDB_TO_EXIT;
if (target_supports_set_thread_options (options))
tp->set_thread_options (options);
else
target_thread_events (true);
}
else else
{ {
if (target_supports_set_thread_options (0)) if (target_supports_set_thread_options (0))
tp->set_thread_options (0); tp->set_thread_options (0);
else if (!displaced_step_in_progress_any_thread ()) else
target_thread_events (false); {
process_stratum_target *resume_target = tp->inf->process_target ();
if (!any_thread_needs_target_thread_events (resume_target,
resume_ptid))
target_thread_events (false);
}
} }
/* If we're resuming more than one thread simultaneously, then any /* If we're resuming more than one thread simultaneously, then any
@@ -5581,6 +5611,13 @@ handle_thread_exited (execution_control_state *ecs)
ecs->event_thread->stepping_over_breakpoint = 0; ecs->event_thread->stepping_over_breakpoint = 0;
ecs->event_thread->stepping_over_watchpoint = 0; ecs->event_thread->stepping_over_watchpoint = 0;
/* If the thread had an FSM, then abort the command. But only after
finishing the step over, as in non-stop mode, aborting this
thread's command should not interfere with other threads. We
must check this before finish_step over, however, which may
update the thread list and delete the event thread. */
bool abort_cmd = (ecs->event_thread->thread_fsm () != nullptr);
/* Maybe the thread was doing a step-over, if so release /* Maybe the thread was doing a step-over, if so release
resources and start any further pending step-overs. resources and start any further pending step-overs.
@@ -5594,6 +5631,13 @@ handle_thread_exited (execution_control_state *ecs)
the event thread has exited. */ the event thread has exited. */
gdb_assert (ret == 0); gdb_assert (ret == 0);
if (abort_cmd)
{
delete_thread (ecs->event_thread);
ecs->event_thread = nullptr;
return false;
}
/* If finish_step_over started a new in-line step-over, don't /* If finish_step_over started a new in-line step-over, don't
try to restart anything else. */ try to restart anything else. */
if (step_over_info_valid_p ()) if (step_over_info_valid_p ())
@@ -8978,7 +9022,8 @@ normal_stop (void)
if (inferior_ptid != null_ptid) if (inferior_ptid != null_ptid)
finish_ptid = ptid_t (inferior_ptid.pid ()); finish_ptid = ptid_t (inferior_ptid.pid ());
} }
else if (last.kind () != TARGET_WAITKIND_NO_RESUMED) else if (last.kind () != TARGET_WAITKIND_NO_RESUMED
&& last.kind () != TARGET_WAITKIND_THREAD_EXITED)
finish_ptid = inferior_ptid; finish_ptid = inferior_ptid;
gdb::optional<scoped_finish_thread_state> maybe_finish_thread_state; gdb::optional<scoped_finish_thread_state> maybe_finish_thread_state;
@@ -9022,7 +9067,8 @@ normal_stop (void)
&& target_has_execution () && target_has_execution ()
&& last.kind () != TARGET_WAITKIND_SIGNALLED && last.kind () != TARGET_WAITKIND_SIGNALLED
&& last.kind () != TARGET_WAITKIND_EXITED && last.kind () != TARGET_WAITKIND_EXITED
&& last.kind () != TARGET_WAITKIND_NO_RESUMED) && last.kind () != TARGET_WAITKIND_NO_RESUMED
&& last.kind () != TARGET_WAITKIND_THREAD_EXITED)
{ {
SWITCH_THRU_ALL_UIS () SWITCH_THRU_ALL_UIS ()
{ {
@@ -9034,7 +9080,8 @@ normal_stop (void)
previous_inferior_ptid = inferior_ptid; previous_inferior_ptid = inferior_ptid;
} }
if (last.kind () == TARGET_WAITKIND_NO_RESUMED) if (last.kind () == TARGET_WAITKIND_NO_RESUMED
|| last.kind () == TARGET_WAITKIND_THREAD_EXITED)
{ {
stop_print_frame = false; stop_print_frame = false;
@@ -9042,7 +9089,12 @@ normal_stop (void)
if (current_ui->prompt_state == PROMPT_BLOCKED) if (current_ui->prompt_state == PROMPT_BLOCKED)
{ {
target_terminal::ours_for_output (); target_terminal::ours_for_output ();
gdb_printf (_("No unwaited-for children left.\n")); if (last.kind () == TARGET_WAITKIND_NO_RESUMED)
gdb_printf (_("No unwaited-for children left.\n"));
else if (last.kind () == TARGET_WAITKIND_THREAD_EXITED)
gdb_printf (_("Command aborted, thread exited.\n"));
else
gdb_assert_not_reached ("unhandled");
} }
} }
@@ -9127,7 +9179,8 @@ normal_stop (void)
{ {
if (last.kind () != TARGET_WAITKIND_SIGNALLED if (last.kind () != TARGET_WAITKIND_SIGNALLED
&& last.kind () != TARGET_WAITKIND_EXITED && last.kind () != TARGET_WAITKIND_EXITED
&& last.kind () != TARGET_WAITKIND_NO_RESUMED) && last.kind () != TARGET_WAITKIND_NO_RESUMED
&& last.kind () != TARGET_WAITKIND_THREAD_EXITED)
/* Delete the breakpoint we stopped at, if it wants to be deleted. /* Delete the breakpoint we stopped at, if it wants to be deleted.
Delete any breakpoint that is to be deleted at the next stop. */ Delete any breakpoint that is to be deleted at the next stop. */
breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat); breakpoint_auto_delete (inferior_thread ()->control.stop_bpstat);

View File

@@ -29,7 +29,7 @@ if { [build_executable "failed to prepare" $testfile \
# NS_STOP_ALL is only used if testing "set non-stop on", and indicates # NS_STOP_ALL is only used if testing "set non-stop on", and indicates
# whether to have GDB explicitly stop all threads before continuing to # whether to have GDB explicitly stop all threads before continuing to
# thread exit. # thread exit.
proc test {displaced-stepping non-stop target-non-stop schedlock ns_stop_all} { proc test {displaced-stepping non-stop target-non-stop schedlock cmd ns_stop_all} {
if {${non-stop} == "off" && $ns_stop_all} { if {${non-stop} == "off" && $ns_stop_all} {
error "invalid arguments" error "invalid arguments"
} }
@@ -72,9 +72,15 @@ proc test {displaced-stepping non-stop target-non-stop schedlock ns_stop_all} {
gdb_test_no_output "set scheduler-locking ${schedlock}" gdb_test_no_output "set scheduler-locking ${schedlock}"
gdb_test "continue" \ if {$cmd == "continue"} {
"No unwaited-for children left." \ gdb_test "continue" \
"continue stops when thread exits" "No unwaited-for children left." \
"continue stops when thread exits"
} else {
gdb_test $cmd \
"Command aborted, thread exited\\." \
"command aborts when thread exits"
}
} else { } else {
gdb_test_no_output "set scheduler-locking ${schedlock}" gdb_test_no_output "set scheduler-locking ${schedlock}"
@@ -108,13 +114,15 @@ foreach_with_prefix displaced-stepping {off auto} {
} }
foreach_with_prefix schedlock {off on} { foreach_with_prefix schedlock {off on} {
if {${non-stop} == "on"} { foreach_with_prefix cmd {"next" "continue"} {
foreach_with_prefix ns_stop_all {0 1} { if {${non-stop} == "on"} {
test ${displaced-stepping} ${non-stop} ${target-non-stop} \ foreach_with_prefix ns_stop_all {0 1} {
${schedlock} ${ns_stop_all} test ${displaced-stepping} ${non-stop} ${target-non-stop} \
${schedlock} ${cmd} ${ns_stop_all}
}
} else {
test ${displaced-stepping} ${non-stop} ${target-non-stop} ${schedlock} ${cmd} 0
} }
} else {
test ${displaced-stepping} ${non-stop} ${target-non-stop} ${schedlock} 0
} }
} }
} }