gdb: handle main thread exiting during detach
authorAndrew Burgess <aburgess@redhat.com>
Mon, 17 Jul 2023 10:31:11 +0000 (11:31 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Thu, 26 Oct 2023 17:11:54 +0000 (18:11 +0100)
Overview
========

Consider the following situation, GDB is in non-stop mode, the main
thread is running while a second thread is stopped.  The user has the
second thread selected as the current thread and asks GDB to detach.
At the exact moment of detach the main thread exits.

This situation currently causes crashes, assertion failures, and
unexpected errors to be reported from GDB for both native and remote
targets.

This commit addresses this situation for native and remote targets.
There are a number of different fixes, but all are required in order
to get this functionality working correct for native and remote
targets.

Native Linux Target
===================

For the native Linux target, detaching is handled in the function
linux_nat_target::detach.  In here we call stop_wait_callback for each
thread, and it is this callback that will spot that the main thread
has exited.

GDB then detaches from everything except the main thread by calling
detach_callback.

After this the first problem is this assert:

  /* Only the initial process should be left right now.  */
  gdb_assert (num_lwps (pid) == 1);

The num_lwps call will return 0 as the main thread has exited and all
of the other threads have now been detached.  I fix this by changing
the assert to allow for 0 or 1 lwps at this point.  As the 0 case can
only happen in non-stop mode, the assert becomes:

  gdb_assert (num_lwps (pid) == 1
      || (target_is_non_stop_p () && num_lwps (pid) == 0));

The next problem is that we do:

  main_lwp = find_lwp_pid (ptid_t (pid));

and then proceed assuming that main_lwp is not nullptr.  In the case
that the main thread has exited though, main_lwp will be nullptr.

However, we only need main_lwp so that GDB can detach from the
thread.  If the main thread has exited, and GDB has already detached
from every other thread, then GDB has finished detaching, GDB can skip
the calls that try to detach from the main thread, and then tell the
user that the detach was a success.

For Remote Targets
==================

On remote targets there are two problems.

First is that when the exit occurs during the early phase of the
detach, we see the stop notification arrive while GDB is removing the
breakpoints ahead of the detach.  The 'set debug remote on' trace
looks like this:

  [remote] Sending packet: $z0,7f1648fe0241,1#35
  [remote]   Notification received: Stop:W0;process:2a0ac8
  # At this point an unpatched gdbserver segfaults, and the connection
  # is broken.  A patched gdbserver continues as below...
  [remote] Packet received: E01
  [remote] Sending packet: $z0,7f1648ff00a8,1#68
  [remote] Packet received: E01
  [remote] Sending packet: $z0,7f1648ff132f,1#6b
  [remote] Packet received: E01
  [remote] Sending packet: $D;2a0ac8#3e
  [remote] Packet received: E01

I was originally running into Segmentation Faults, from within
gdbserver/mem-break.cc, in the function find_gdb_breakpoint.  This
function calls current_process() and then dereferences the result to
find the breakpoint list.

However, in our case, the current process has already exited, and so
the current_process() call returns nullptr.  At the point of failure,
the gdbserver backtrace looks like this:

  #0  0x00000000004190e4 in find_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:982
  #1  0x000000000041930d in delete_gdb_breakpoint (z_type=48 '0', addr=4198762, kind=1) at ../../src/gdbserver/mem-break.cc:1093
  #2  0x000000000042d8db in process_serial_event () at ../../src/gdbserver/server.cc:4372
  #3  0x000000000042dcab in handle_serial_event (err=0, client_data=0x0) at ../../src/gdbserver/server.cc:4498
  ...

The problem is that, as a result non-stop being on, the process
exiting is only reported back to GDB after the request to remove a
breakpoint has been sent.  Clearly gdbserver can't actually remove
this breakpoint -- the process has already exited -- so I think the
best solution is for gdbserver just to report an error, which is what
I've done.

The second problem I ran into was on the gdb side, as the process has
already exited, but GDB has not yet acknowledged the exit event, the
detach -- the 'D' packet in the above trace -- fails.  This was being
reported to the user with a 'Can't detach process' error.  As the test
actually calls detach from Python code, this error was then becoming a
Python exception.

Though clearly the detach has returned an error, and so, maybe, having
GDB throw an error would be fine, I think in this case, there's a good
argument that the remote error can be ignored -- if GDB tries to
detach and gets back an error, and if there's a pending exit event for
the pid we tried to detach, then just ignore the error and pretend the
detach worked fine.

We could possibly check for a pending exit event before sending the
detach packet, however, I believe that it might be possible (in
non-stop mode) for the stop notification to arrive after the detach is
sent, but before gdbserver has started processing the detach.  In this
case we would still need to check for pending stop events after seeing
the detach fail, so I figure there's no point having two checks -- we
just send the detach request, and if it fails, check to see if the
process has already exited.

Testing
=======

In order to test this issue I needed to ensure that the exit event
arrives at the same time as the detach call.  The window of
opportunity for getting the exit to arrive is so small I've never
managed to trigger this in real use -- I originally spotted this issue
while working on another patch, which did manage to trigger this
issue.

However, if we trigger both the exit and the detach from a single
Python function then we never return to GDB's event loop, as such GDB
never processes the exit event, and so the first time GDB gets a
chance to see the exit is during the detach call.  And so that is the
approach I've taken for testing this patch.

Tested-By: Kevin Buettner <kevinb@redhat.com>
Approved-By: Kevin Buettner <kevinb@redhat.com>
gdb/linux-nat.c
gdb/remote.c
gdb/testsuite/gdb.threads/main-thread-exit-during-detach.c [new file with mode: 0644]
gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp [new file with mode: 0644]
gdbserver/mem-break.cc

index 9148dda4aada12caf3789bd3185f10e16b3d398c..1c9756c18bd7982e96e104c60f24d5cd10dea62b 100644 (file)
@@ -1420,10 +1420,12 @@ linux_nat_target::detach (inferior *inf, int from_tty)
 
   iterate_over_lwps (ptid_t (pid), detach_callback);
 
-  /* Only the initial process should be left right now.  */
-  gdb_assert (num_lwps (pid) == 1);
-
-  main_lwp = find_lwp_pid (ptid_t (pid));
+  /* We have detached from everything except the main thread now, so
+     should only have one thread left.  However, in non-stop mode the
+     main thread might have exited, in which case we'll have no threads
+     left.  */
+  gdb_assert (num_lwps (pid) == 1
+             || (target_is_non_stop_p () && num_lwps (pid) == 0));
 
   if (forks_exist_p ())
     {
@@ -1437,10 +1439,18 @@ linux_nat_target::detach (inferior *inf, int from_tty)
     {
       target_announce_detach (from_tty);
 
-      /* Pass on any pending signal for the last LWP.  */
-      int signo = get_detach_signal (main_lwp);
+      /* In non-stop mode it is possible that the main thread has exited,
+        in which case we don't try to detach.  */
+      main_lwp = find_lwp_pid (ptid_t (pid));
+      if (main_lwp != nullptr)
+       {
+         /* Pass on any pending signal for the last LWP.  */
+         int signo = get_detach_signal (main_lwp);
 
-      detach_one_lwp (main_lwp, &signo);
+         detach_one_lwp (main_lwp, &signo);
+       }
+      else
+       gdb_assert (target_is_non_stop_p ());
 
       detach_success (inf);
     }
index b58dbd4cb66e064be3d2e82f5c9d596b0d2a740c..69271048da24ddaea1556e1b0780410fe9d77977 100644 (file)
@@ -6165,7 +6165,32 @@ remote_target::remote_detach_pid (int pid)
   else if (rs->buf[0] == '\0')
     error (_("Remote doesn't know how to detach"));
   else
-    error (_("Can't detach process."));
+    {
+      /* It is possible that we have an unprocessed exit event for this
+        pid.  If this is the case then we can ignore the failure to detach
+        and just pretend that the detach worked, as far as the user is
+        concerned, the process exited immediately after the detach.  */
+      bool process_has_already_exited = false;
+      remote_notif_get_pending_events (&notif_client_stop);
+      for (stop_reply_up &reply : rs->stop_reply_queue)
+       {
+         if (reply->ptid.pid () != pid)
+           continue;
+
+         enum target_waitkind kind = reply->ws.kind ();
+         if (kind == TARGET_WAITKIND_EXITED
+             || kind == TARGET_WAITKIND_SIGNALLED)
+           {
+             process_has_already_exited = true;
+             remote_debug_printf
+               ("detach failed, but process already exited");
+             break;
+           }
+       }
+
+      if (!process_has_already_exited)
+       error (_("can't detach process: %s"), (char *) rs->buf.data ());
+    }
 }
 
 /* This detaches a program to which we previously attached, using
diff --git a/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.c b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.c
new file mode 100644 (file)
index 0000000..5335842
--- /dev/null
@@ -0,0 +1,61 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <unistd.h>
+
+/* This is set from GDB to allow the main thread to exit.  */
+
+volatile int dont_exit_just_yet = 1;
+
+/* Somewhere to place a breakpoint.  */
+
+void
+breakpt ()
+{
+  /* Just spin.  When the test is started under GDB we never enter the spin
+     loop, but when we attach, the worker thread will be spinning here.  */
+  while (dont_exit_just_yet)
+    sleep (1);
+}
+
+/* Thread function, doesn't do anything but hit a breakpoint.  */
+
+void *
+thread_worker (void *arg)
+{
+  breakpt ();
+  return NULL;
+}
+
+int
+main ()
+{
+  pthread_t thr;
+
+  alarm (300);
+
+  /* Create a thread.  */
+  pthread_create (&thr, NULL, thread_worker, NULL);
+  pthread_detach (thr);
+
+  /* Spin until GDB releases us.  */
+  while (dont_exit_just_yet)
+    sleep (1);
+
+  _exit (0);
+}
diff --git a/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp
new file mode 100644 (file)
index 0000000..83f5e85
--- /dev/null
@@ -0,0 +1,140 @@
+# Copyright 2023 Free Software Foundation, Inc.
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check for a race condition where in non-stop mode, the user might
+# have a thread other than the main (original) thread selected and use
+# the 'detach' command.
+#
+# As GDB tries to detach it is possible that the main thread might
+# exit, the main thread is still running due to non-stop mode.
+#
+# GDB used to assume that the main thread would always exist when
+# processing the detach, clearly this isn't the case, and this
+# assumption would lead to assertion failures and segfaults.
+#
+# Triggering the precise timing is pretty hard, we need the main
+# thread to exit after the user has entered the 'detach' command, but
+# before GDB enters the detach implementation and stops all threads,
+# the window of opportunity for this bug is actually tiny.
+#
+# However, we can trigger this bug 100% from Python, as GDB's
+# event-loop only kicks in once we return from a Python function.
+# Thus, if we have a single Python function that causes the main
+# thread to exit, and then calls detach GDB will not have a chance to
+# handle the main thread exiting before entering the detach code.
+
+standard_testfile
+
+require allow_python_tests
+
+if {[build_executable "failed to prepare" $testfile $srcfile \
+        {debug pthreads}] == -1} {
+    return -1
+}
+
+# Run the test.  When SPAWN_INFERIOR is true the inferior is started
+# as a separate process which GDB then attaches too.  When
+# SPAWN_INFERIOR is false the inferior is started directly within GDB.
+
+proc run_test { spawn_inferior } {
+    save_vars { ::GDBFLAGS } {
+       append ::GDBFLAGS " -ex \"set non-stop on\""
+       clean_restart $::binfile
+    }
+
+    # Setup the inferior.  When complete the main thread (#1) will
+    # still be running (due to non-stop mode), while the worker thread
+    # (#2) will be stopped.
+    #
+    # There are two setup modes, when SPAWN_INFERIOR is true we span a
+    # separate process and attach to it, after the attach both threads
+    # are stopped, so it is necessary to resume thread #1.
+    #
+    # When SPAWN_INFERIOR is false we just start the inferior within
+    # GDB, in this case we place a breakpoint that will be hit by
+    # thread #2.  When the breakpoint is hit thread #1 will remain
+    # running.
+    if {$spawn_inferior} {
+       set test_spawn_id [spawn_wait_for_attach $::binfile]
+       set testpid [spawn_id_get_pid $test_spawn_id]
+
+       set escapedbinfile  [string_to_regexp $::binfile]
+       gdb_test -no-prompt-anchor "attach $testpid" \
+           "Attaching to program.*`?$escapedbinfile'?, process $testpid.*" \
+           "attach to the inferior"
+
+       # Attaching to a multi-threaded application in non-stop mode
+       # can result in thread stops being reported after the prompt
+       # is displayed.
+       #
+       # Send a simple command now just to resync the command prompt.
+       gdb_test "p 1 + 2" " = 3"
+
+       # Set thread 1 (the current thread) running again.
+       gdb_test "continue&"
+    } else {
+       if {![runto_main]} {
+           return -1
+       }
+
+       gdb_breakpoint "breakpt"
+       gdb_continue_to_breakpoint "run to breakpoint"
+    }
+
+    # Switch to thread 2.
+    gdb_test "thread 2" \
+       [multi_line \
+            "Switching to thread 2\[^\r\n\]*" \
+            "#0\\s+.*"]
+
+    # Create a Python function that sets a variable in the inferior and
+    # then detaches.  Setting the variable in the inferior will allow the
+    # main thread to exit, we even sleep for a short while in order to
+    # give the inferior a chance to exit.
+    #
+    # However, we don't want GDB to notice the exit before we call detach,
+    # which is why we perform both these actions from a Python function.
+    gdb_test_multiline "Create worker function" \
+       "python" "" \
+       "import time" "" \
+       "def set_and_detach():" "" \
+       "   gdb.execute(\"set variable dont_exit_just_yet=0\")" "" \
+       "   time.sleep(1)" "" \
+       "   gdb.execute(\"detach\")" "" \
+       "end" ""
+
+    # The Python function performs two actions, the first causes the
+    # main thread to exit, while the second detaches from the inferior.
+    #
+    # In both cases the stop arrives while GDB is processing the
+    # detach, however, for remote targets GDB doesn't report the stop,
+    # while for local targets GDB does report the stop.
+    if {![gdb_is_target_remote]} {
+       set stop_re "\\\[Thread.*exited\\\]\r\n"
+    } else {
+       set stop_re ""
+    }
+    gdb_test "python set_and_detach()" \
+       "${stop_re}\\\[Inferior.*detached\\\]"
+}
+
+foreach_with_prefix spawn_inferior { true false } {
+    if {$spawn_inferior && ![can_spawn_for_attach]} {
+       # If spawning (and attaching too) a separate inferior is not
+       # supported for the current board, then skip this test.
+       continue
+    }
+
+    run_test $spawn_inferior
+}
index 897b9a2273c721b849a2f9f176aa0b7ac971988c..3bee8bc89900fc578e7ea69b0e4e1ffea5a12120 100644 (file)
@@ -976,6 +976,17 @@ static struct gdb_breakpoint *
 find_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
 {
   struct process_info *proc = current_process ();
+
+  /* In some situations the current process exits, we inform GDB, but
+     before GDB can acknowledge that the process has exited GDB tries to
+     detach from the inferior.  As part of the detach process GDB will
+     remove all breakpoints, which means we can end up here when the
+     current process has already exited and so PROC is nullptr.  In this
+     case just claim we can't find (and so delete) the breakpoint, GDB
+     will ignore this error during detach.  */
+  if (proc == nullptr)
+    return nullptr;
+
   struct breakpoint *bp;
   enum bkpt_type type = Z_packet_to_bkpt_type (z_type);