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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 209834a15fc4403b9560ce97081725b4420a07f3..9b3b64dcf93e44101d115e7864045a2dc6b0838a 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2021-02-03  Pedro Alves  <pedro@palves.net>
+
+       PR gdb/27055
+       * infrun.c (handle_signal_stop): Move main context_switch call
+       earlier, before STOP_QUIETLY_NO_SIGSTOP.
+
 2021-02-02  Lancelot SIX  <lsix@lancelotsix.com>
 
        * NEWS (Changed commands): Add entry for the behavior change of

diff --git a/gdb/infrun.c b/gdb/infrun.c
index e070eff33d7a3b8618932262a731276917bae4be..405b907856ae96ce740523eac0aeb044cd28fb0c 100644 (file)
--- 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);