From fd492bf1e20a0410189307c5063acc444c4bd5d3 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 17 Jul 2023 11:31:11 +0100 Subject: [PATCH] gdb: handle main thread exiting during detach 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 Approved-By: Kevin Buettner --- gdb/linux-nat.c | 24 ++- gdb/remote.c | 27 +++- .../main-thread-exit-during-detach.c | 61 ++++++++ .../main-thread-exit-during-detach.exp | 140 ++++++++++++++++++ gdbserver/mem-break.cc | 11 ++ 5 files changed, 255 insertions(+), 8 deletions(-) create mode 100644 gdb/testsuite/gdb.threads/main-thread-exit-during-detach.c create mode 100644 gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 9148dda4aad..1c9756c18bd 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -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); } diff --git a/gdb/remote.c b/gdb/remote.c index b58dbd4cb66..69271048da2 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -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 (¬if_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 index 00000000000..5335842cfe8 --- /dev/null +++ b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.c @@ -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 . */ + +#include +#include + +/* 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 index 00000000000..83f5e855fb0 --- /dev/null +++ b/gdb/testsuite/gdb.threads/main-thread-exit-during-detach.exp @@ -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 . + +# 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 +} diff --git a/gdbserver/mem-break.cc b/gdbserver/mem-break.cc index 897b9a2273c..3bee8bc8990 100644 --- a/gdbserver/mem-break.cc +++ b/gdbserver/mem-break.cc @@ -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); -- 2.30.2