From 2ab76a181f3db93f051aaae66d65ff2733884d96 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 23 Dec 2020 00:34:54 +0000 Subject: [PATCH] Fix attaching in non-stop mode (PR gdb/27055) Attaching in non-stop mode currently misbehaves, like so: (gdb) attach 1244450 Attaching to process 1244450 [New LWP 1244453] [New LWP 1244454] [New LWP 1244455] [New LWP 1244456] [New LWP 1244457] [New LWP 1244458] [New LWP 1244459] [New LWP 1244461] [New LWP 1244462] [New LWP 1244463] No unwaited-for children left. At this point, GDB's stopped/running thread state is out of sync with the inferior: (gdb) info threads Id Target Id Frame * 1 LWP 1244450 "attach-non-stop" 0xf1b443bf in ?? () 2 LWP 1244453 "attach-non-stop" (running) 3 LWP 1244454 "attach-non-stop" (running) 4 LWP 1244455 "attach-non-stop" (running) 5 LWP 1244456 "attach-non-stop" (running) 6 LWP 1244457 "attach-non-stop" (running) 7 LWP 1244458 "attach-non-stop" (running) 8 LWP 1244459 "attach-non-stop" (running) 9 LWP 1244461 "attach-non-stop" (running) 10 LWP 1244462 "attach-non-stop" (running) 11 LWP 1244463 "attach-non-stop" (running) (gdb) (gdb) interrupt -a (gdb) *nothing* The problem is that attaching installs an inferior continuation, called when the target reports the initial attach stop, here, in inf-loop.c:inferior_event_handler: /* Do all continuations associated with the whole inferior (not a particular thread). */ if (inferior_ptid != null_ptid) do_all_inferior_continuations (0); However, currently in non-stop mode, inferior_ptid is still null_ptid when we get here. If you try to do "set debug infrun 1" to debug the problem, however, then the attach completes correctly, with GDB reporting a stop for each thread. The bug is that we're missing a switch_to_thread/context_switch call when handling the initial stop, here: if (stop_soon == STOP_QUIETLY_NO_SIGSTOP && (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_STOP || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP || ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_0)) { stop_print_frame = true; stop_waiting (ecs); ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0; return; } Note how the STOP_QUIETLY / STOP_QUIETLY_REMOTE case above that does call context_switch. And the reason "set debug infrun 1" "fixes" it, is that the debug path has a switch_to_thread call. This patch fixes it by moving the main context_switch call earlier. It also removes the: if (ecs->ptid != inferior_ptid) check at the same time because: #1 - that is half of what context_switch already does #2 - deprecated_context_hook is only used in Insight, and all it does is set an int. It won't care if we call it when the current thread hasn't actually changed. A testcase exercising this will be added in a following patch. gdb/ChangeLog: PR gdb/27055 * infrun.c (handle_signal_stop): Move main context_switch call earlier, before STOP_QUIETLY_NO_SIGSTOP. --- gdb/ChangeLog | 6 ++++++ gdb/infrun.c | 20 +++++--------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 209834a15fc..9b3b64dcf93 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2021-02-03 Pedro Alves + + PR gdb/27055 + * infrun.c (handle_signal_stop): Move main context_switch call + earlier, before STOP_QUIETLY_NO_SIGSTOP. + 2021-02-02 Lancelot SIX * NEWS (Changed commands): Add entry for the behavior change of diff --git a/gdb/infrun.c b/gdb/infrun.c index e070eff33d7..405b907856a 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -5735,13 +5735,16 @@ handle_signal_stop (struct execution_control_state *ecs) ecs->event_thread->suspend.stop_pc = regcache_read_pc (get_thread_regcache (ecs->event_thread)); + context_switch (ecs); + + if (deprecated_context_hook) + deprecated_context_hook (ecs->event_thread->global_num); + if (debug_infrun) { struct regcache *regcache = get_thread_regcache (ecs->event_thread); struct gdbarch *reg_gdbarch = regcache->arch (); - switch_to_thread (ecs->event_thread); - infrun_debug_printf ("stop_pc=%s", paddress (reg_gdbarch, ecs->event_thread->suspend.stop_pc)); @@ -5764,7 +5767,6 @@ handle_signal_stop (struct execution_control_state *ecs) stop_soon = get_inferior_stop_soon (ecs); if (stop_soon == STOP_QUIETLY || stop_soon == STOP_QUIETLY_REMOTE) { - context_switch (ecs); infrun_debug_printf ("quietly stopped"); stop_print_frame = true; stop_waiting (ecs); @@ -5802,18 +5804,6 @@ handle_signal_stop (struct execution_control_state *ecs) return; } - /* See if something interesting happened to the non-current thread. If - so, then switch to that thread. */ - if (ecs->ptid != inferior_ptid) - { - infrun_debug_printf ("context switch"); - - context_switch (ecs); - - if (deprecated_context_hook) - deprecated_context_hook (ecs->event_thread->global_num); - } - /* At this point, get hold of the now-current thread's frame. */ frame = get_current_frame (); gdbarch = get_frame_arch (frame); -- 2.30.2