From: Pedro Alves Date: Mon, 11 Jan 2021 20:01:58 +0000 (+0000) Subject: detach in all-stop with threads running X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=408f66864a1a823591b26420410c982174c239a2;p=binutils-gdb.git detach in all-stop with threads running A following patch will add a testcase that has a number of threads constantly stepping over a breakpoint, and then has GDB detach the process, while threads are running. If we have more than one inferior running, and we detach from just one of the inferiors, we expect that the remaining inferior continues running. However, in all-stop, if GDB needs to pause the target for the detach, nothing is re-resuming the other inferiors after the detach. "info threads" shows the threads as running, but they really aren't. This fixes it. gdb/ChangeLog: * infcmd.c (detach_command): Hold strong reference to target, and if all-stop on entry, restart threads on exit. * infrun.c (switch_back_to_stepped_thread): Factor out bits to ... (restart_stepped_thread): ... this new function. Also handle trap_expected. (restart_after_all_stop_detach): New function. * infrun.h (restart_after_all_stop_detach): Declare. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 365d1375b9d..4a02a64e71b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2021-02-03 Pedro Alves + + * infcmd.c (detach_command): Hold strong reference to target, and + if all-stop on entry, restart threads on exit. + * infrun.c (switch_back_to_stepped_thread): Factor out bits to ... + (restart_stepped_thread): ... this new function. Also handle + trap_expected. + (restart_after_all_stop_detach): New function. + * infrun.h (restart_after_all_stop_detach): Declare. + 2021-02-03 Pedro Alves * infrun.c (struct step_over_info): Initialize fields. diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 6f0ed952de6..ebaf57592ef 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -2750,6 +2750,16 @@ detach_command (const char *args, int from_tty) disconnect_tracing (); + /* Hold a strong reference to the target while (maybe) + detaching the parent. Otherwise detaching could close the + target. */ + auto target_ref + = target_ops_ref::new_reference (current_inferior ()->process_target ()); + + /* Save this before detaching, since detaching may unpush the + process_stratum target. */ + bool was_non_stop_p = target_is_non_stop_p (); + target_detach (current_inferior (), from_tty); /* The current inferior process was just detached successfully. Get @@ -2766,6 +2776,9 @@ detach_command (const char *args, int from_tty) if (deprecated_detach_hook) deprecated_detach_hook (); + + if (!was_non_stop_p) + restart_after_all_stop_detach (as_process_stratum_target (target_ref.get ())); } /* Disconnect from the current target without resuming it (leaving it diff --git a/gdb/infrun.c b/gdb/infrun.c index 6585dddc6f8..f2abfe4a7c2 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7110,6 +7110,9 @@ process_event_stop_test (struct execution_control_state *ecs) keep_going (ecs); } +static bool restart_stepped_thread (process_stratum_target *resume_target, + ptid_t resume_ptid); + /* In all-stop mode, if we're currently stepping but have stopped in some other thread, we may need to switch back to the stepped thread. Returns true we set the inferior running, false if we left @@ -7120,8 +7123,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) { if (!target_is_non_stop_p ()) { - struct thread_info *stepping_thread; - /* If any thread is blocked on some internal breakpoint, and we simply need to step over that breakpoint to get it going again, do that first. */ @@ -7184,78 +7185,136 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs) if (!signal_program[ecs->event_thread->suspend.stop_signal]) ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; - /* Do all pending step-overs before actually proceeding with - step/next/etc. */ - if (start_step_over ()) + if (restart_stepped_thread (ecs->target, ecs->ptid)) { prepare_to_wait (ecs); return true; } - /* Look for the stepping/nexting thread. */ - stepping_thread = NULL; + switch_to_thread (ecs->event_thread); + } - for (thread_info *tp : all_non_exited_threads ()) - { - switch_to_thread_no_regs (tp); + return false; +} - /* Ignore threads of processes the caller is not - resuming. */ - if (!sched_multi - && (tp->inf->process_target () != ecs->target - || tp->inf->pid != ecs->ptid.pid ())) - continue; +/* Look for the thread that was stepping, and resume it. + RESUME_TARGET / RESUME_PTID indicate the set of threads the caller + is resuming. Return true if a thread was started, false + otherwise. */ - /* When stepping over a breakpoint, we lock all threads - except the one that needs to move past the breakpoint. - If a non-event thread has this set, the "incomplete - step-over" check above should have caught it earlier. */ - if (tp->control.trap_expected) - { - internal_error (__FILE__, __LINE__, - "[%s] has inconsistent state: " - "trap_expected=%d\n", - target_pid_to_str (tp->ptid).c_str (), - tp->control.trap_expected); - } +static bool +restart_stepped_thread (process_stratum_target *resume_target, + ptid_t resume_ptid) +{ + /* Do all pending step-overs before actually proceeding with + step/next/etc. */ + if (start_step_over ()) + return true; - /* Did we find the stepping thread? */ - if (tp->control.step_range_end) - { - /* Yep. There should only one though. */ - gdb_assert (stepping_thread == NULL); + for (thread_info *tp : all_threads_safe ()) + { + if (tp->state == THREAD_EXITED) + continue; + + if (tp->suspend.waitstatus_pending_p) + continue; - /* The event thread is handled at the top, before we - enter this loop. */ - gdb_assert (tp != ecs->event_thread); + /* Ignore threads of processes the caller is not + resuming. */ + if (!sched_multi + && (tp->inf->process_target () != resume_target + || tp->inf->pid != resume_ptid.pid ())) + continue; - /* If some thread other than the event thread is - stepping, then scheduler locking can't be in effect, - otherwise we wouldn't have resumed the current event - thread in the first place. */ - gdb_assert (!schedlock_applies (tp)); + if (tp->control.trap_expected) + { + infrun_debug_printf ("switching back to stepped thread (step-over)"); - stepping_thread = tp; - } + if (keep_going_stepped_thread (tp)) + return true; } + } + + for (thread_info *tp : all_threads_safe ()) + { + if (tp->state == THREAD_EXITED) + continue; + + if (tp->suspend.waitstatus_pending_p) + continue; - if (stepping_thread != NULL) + /* Ignore threads of processes the caller is not + resuming. */ + if (!sched_multi + && (tp->inf->process_target () != resume_target + || tp->inf->pid != resume_ptid.pid ())) + continue; + + /* Did we find the stepping thread? */ + if (tp->control.step_range_end) { - infrun_debug_printf ("switching back to stepped thread"); + infrun_debug_printf ("switching back to stepped thread (stepping)"); - if (keep_going_stepped_thread (stepping_thread)) - { - prepare_to_wait (ecs); - return true; - } + if (keep_going_stepped_thread (tp)) + return true; } - - switch_to_thread (ecs->event_thread); } return false; } +/* See infrun.h. */ + +void +restart_after_all_stop_detach (process_stratum_target *proc_target) +{ + /* Note we don't check target_is_non_stop_p() here, because the + current inferior may no longer have a process_stratum target + pushed, as we just detached. */ + + /* See if we have a THREAD_RUNNING thread that need to be + re-resumed. If we have any thread that is already executing, + then we don't need to resume the target -- it is already been + resumed. With the remote target (in all-stop), it's even + impossible to issue another resumption if the target is already + resumed, until the target reports a stop. */ + for (thread_info *thr : all_threads (proc_target)) + { + if (thr->state != THREAD_RUNNING) + continue; + + /* If we have any thread that is already executing, then we + don't need to resume the target -- it is already been + resumed. */ + if (thr->executing) + return; + + /* If we have a pending event to process, skip resuming the + target and go straight to processing it. */ + if (thr->resumed && thr->suspend.waitstatus_pending_p) + return; + } + + /* Alright, we need to re-resume the target. If a thread was + stepping, we need to restart it stepping. */ + if (restart_stepped_thread (proc_target, minus_one_ptid)) + return; + + /* Otherwise, find the first THREAD_RUNNING thread and resume + it. */ + for (thread_info *thr : all_threads (proc_target)) + { + if (thr->state != THREAD_RUNNING) + continue; + + execution_control_state ecs; + reset_ecs (&ecs, thr); + switch_to_thread (thr); + keep_going (&ecs); + return; + } +} + /* Set a previously stepped thread back to stepping. Returns true on success, false if the resume is not possible (e.g., the thread vanished). */ diff --git a/gdb/infrun.h b/gdb/infrun.h index 7160b60f136..e643c84dd80 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -269,4 +269,8 @@ extern void all_uis_check_sync_execution_done (void); started or re-started). */ extern void all_uis_on_sync_execution_starting (void); +/* In all-stop, restart the target if it had to be stopped to + detach. */ +extern void restart_after_all_stop_detach (process_stratum_target *proc_target); + #endif /* INFRUN_H */