Skip to content

Commit

Permalink
gdb, gdbserver: detach fork child when detaching from fork parent
Browse files Browse the repository at this point in the history
While working with pending fork events, I wondered what would happen if
the user detached an inferior while a thread of that inferior had a
pending fork event.  What happens with the fork child, which is
ptrace-attached by the GDB process (or by GDBserver), but not known to
the core?  Sure enough, neither the core of GDB or the target detach the
child process, so GDB (or GDBserver) just stays ptrace-attached to the
process.  The result is that the fork child process is stuck, while you
would expect it to be detached and run.

Make GDBserver detach of fork children it knows about.  That is done in
the generic handle_detach function.  Since a process_info already exists
for the child, we can simply call detach_inferior on it.

GDB-side, make the linux-nat and remote targets detach of fork children
known because of pending fork events.  These pending fork events can be
stored in:

 - thread_info::pending_waitstatus, if the core has consumed the event
   but then saved it for later (for example, because it got the event
   while stopping all threads, to present an all-stop stop on top of a
   non-stop target)
 - thread_info::pending_follow: if we ran to a "catch fork" and we
   detach at that moment

Additionally, pending fork events can be in target-specific fields:

 - For linux-nat, they can be in lwp_info::status and
   lwp_info::waitstatus.
 - For the remote target, they could be stored as pending stop replies,
   saved in `remote_state::notif_state::pending_event`, if not
   acknowledged yet, or in `remote_state::stop_reply_queue`, if
   acknowledged.  I followed the model of remove_new_fork_children for
   this: call remote_notif_get_pending_events to process /
   acknowledge any unacknowledged notification, then look through
   stop_reply_queue.

Update the gdb.threads/pending-fork-event.exp test (and rename it to
gdb.threads/pending-fork-event-detach.exp) to try to detach the process
while it is stopped with a pending fork event.  In order to verify that
the fork child process is correctly detached and resumes execution
outside of GDB's control, make that process create a file in the test
output directory, and make the test wait $timeout seconds for that file
to appear (it happens instantly if everything goes well).

This test catches a bug in linux-nat.c, also reported as PR 28512
("waitstatus.h:300: internal-error: gdb_signal target_waitstatus::sig()
const: Assertion `m_kind == TARGET_WAITKIND_STOPPED || m_kind ==
TARGET_WAITKIND_SIGNALLED' failed.).  When detaching a thread with a
pending event, get_detach_signal unconditionally fetches the signal
stored in the waitstatus (`tp->pending_waitstatus ().sig ()`).  However,
that is only valid if the pending event is of type
TARGET_WAITKIND_STOPPED, and this is now enforced using assertions (iit
would also be valid for TARGET_WAITKIND_SIGNALLED, but that would mean
the thread does not exist anymore, so we wouldn't be detaching it).  Add
a condition in get_detach_signal to access the signal number only if the
wait status is of kind TARGET_WAITKIND_STOPPED, and use GDB_SIGNAL_0
instead (since the thread was not stopped with a signal to begin with).

Add another test, gdb.threads/pending-fork-event-ns.exp, specifically to
verify that we consider events in pending stop replies in the remote
target.  This test has many threads constantly forking, and we detach
from the program while the program is executing.  That gives us some
chance that we detach while a fork stop reply is stored in the remote
target.  To verify that we correctly detach all fork children, we ask
the parent to exit by sending it a SIGUSR1 signal and have it write a
file to the filesystem before exiting.  Because the parent's main thread
joins the forking threads, and the forking threads wait for their fork
children to exit, if some fork child is not detach by GDB, the parent
will not write the file, and the test will time out.  If I remove the
new remote_detach_pid calls in remote.c, the test fails eventually if I
run it in a loop.

There is a known limitation: we don't remove breakpoints from the
children before detaching it.  So the children, could hit a trap
instruction after being detached and crash.  I know this is wrong, and
it should be fixed, but I would like to handle that later.  The current
patch doesn't fix everything, but it's a step in the right direction.

Change-Id: I6d811a56f520e3cb92d5ea563ad38976f92e93dd
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28512
  • Loading branch information
simark committed Dec 9, 2021
1 parent 577d216 commit df5ad10
Show file tree
Hide file tree
Showing 14 changed files with 592 additions and 87 deletions.
53 changes: 52 additions & 1 deletion gdb/linux-nat.c
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,16 @@ get_detach_signal (struct lwp_info *lp)
if (target_is_non_stop_p () && !tp->executing ())
{
if (tp->has_pending_waitstatus ())
signo = tp->pending_waitstatus ().sig ();
{
/* If the thread has a pending event, and it was stopped with a
signal, use that signal to resume it. If it has a pending
event of another kind, it was not stopped with a signal, so
resume it without a signal. */
if (tp->pending_waitstatus ().kind () == TARGET_WAITKIND_STOPPED)
signo = tp->pending_waitstatus ().sig ();
else
signo = GDB_SIGNAL_0;
}
else
signo = tp->stop_signal ();
}
Expand Down Expand Up @@ -1367,6 +1376,48 @@ detach_one_lwp (struct lwp_info *lp, int *signo_p)

gdb_assert (lp->status == 0 || WIFSTOPPED (lp->status));

/* If the lwp/thread we are about to detach has a pending fork event,
there is a process GDB is attached to that the core of GDB doesn't know
about. Detach from it. */

/* Check in lwp_info::status. */
if (WIFSTOPPED (lp->status) && linux_is_extended_waitstatus (lp->status))
{
int event = linux_ptrace_get_extended_event (lp->status);

if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
{
unsigned long child_pid;
int ret = ptrace (PTRACE_GETEVENTMSG, lp->ptid.lwp (), 0, &child_pid);
if (ret == 0)
detach_one_pid (child_pid, 0);
else
perror_warning_with_name (_("Failed to detach fork child"));
}
}

/* Check in lwp_info::waitstatus. */
if (lp->waitstatus.kind () == TARGET_WAITKIND_VFORKED
|| lp->waitstatus.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (lp->waitstatus.child_ptid ().pid (), 0);


/* Check in thread_info::pending_waitstatus. */
thread_info *tp = find_thread_ptid (linux_target, lp->ptid);
if (tp->has_pending_waitstatus ())
{
const target_waitstatus &ws = tp->pending_waitstatus ();

if (ws.kind () == TARGET_WAITKIND_VFORKED
|| ws.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (ws.child_ptid ().pid (), 0);
}

/* Check in thread_info::pending_follow. */
if (tp->pending_follow.kind () == TARGET_WAITKIND_VFORKED
|| tp->pending_follow.kind () == TARGET_WAITKIND_FORKED)
detach_one_pid (tp->pending_follow.child_ptid ().pid (), 0);

if (lp->status != 0)
linux_nat_debug_printf ("Pending %s for %s on detach.",
strsignal (WSTOPSIG (lp->status)),
Expand Down
39 changes: 35 additions & 4 deletions gdb/remote.c
Original file line number Diff line number Diff line change
Expand Up @@ -5968,6 +5968,32 @@ remote_target::remote_detach_1 (inferior *inf, int from_tty)
if (from_tty && !rs->extended && number_of_live_inferiors (this) == 1)
puts_filtered (_("Ending remote debugging.\n"));

/* See if any thread of the inferior we are detaching has a pending fork
status. In that case, we must detach from the child resulting from
that fork. */
for (thread_info *thread : inf->non_exited_threads ())
{
const target_waitstatus *ws = thread_pending_fork_status (thread);

if (ws == nullptr)
continue;

remote_detach_pid (ws->child_ptid ().pid ());
}

/* Check also for any pending fork events in the stop reply queue. */
remote_notif_get_pending_events (&notif_client_stop);
for (stop_reply_up &reply : rs->stop_reply_queue)
{
if (reply->ptid.pid () != pid)
continue;

if (!is_fork_status (reply->ws.kind ()))
continue;

remote_detach_pid (reply->ws.child_ptid ().pid ());
}

thread_info *tp = find_thread_ptid (this, inferior_ptid);

/* Check to see if we are detaching a fork parent. Note that if we
Expand Down Expand Up @@ -7371,11 +7397,11 @@ remote_target::discard_pending_stop_replies (struct inferior *inf)
/* Leave the notification pending, since the server expects that
we acknowledge it with vStopped. But clear its contents, so
that later on when we acknowledge it, we also discard it. */
remote_debug_printf
("discarding in-flight notification: ptid: %s, ws: %s\n",
reply->ptid.to_string().c_str(),
reply->ws.to_string ().c_str ());
reply->ws.set_ignore ();

if (remote_debug)
fprintf_unfiltered (gdb_stdlog,
"discarded in-flight notification\n");
}

/* Discard the stop replies we have already pulled with
Expand All @@ -7386,6 +7412,11 @@ remote_target::discard_pending_stop_replies (struct inferior *inf)
{
return event->ptid.pid () == inf->pid;
});
for (auto it = iter; it != rs->stop_reply_queue.end (); ++it)
remote_debug_printf
("discarding queued stop reply: ptid: %s, ws: %s\n",
reply->ptid.to_string().c_str(),
reply->ws.to_string ().c_str ());
rs->stop_reply_queue.erase (iter, rs->stop_reply_queue.end ());
}

Expand Down
114 changes: 114 additions & 0 deletions gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2021 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 <assert.h>
#include <errno.h>
#include <poll.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>
#include <unistd.h>

#define NUM_FORKING_THREADS 12

static pthread_barrier_t barrier;
static volatile int should_exit = 0;

static void
sigusr1_handler (int sig, siginfo_t *siginfo, void *context)
{
should_exit = 1;
}

static void *
forking_thread (void *arg)
{
/* Wait for all forking threads to have spawned before fork-spamming. */
pthread_barrier_wait (&barrier);

while (!should_exit)
{
pid_t pid = fork ();
if (pid == 0)
{
/* Child */
exit (8);
}
else
{
/* Parent */
int status;
int ret = waitpid (pid, &status, 0);
assert (ret == pid);
assert (WIFEXITED (status));
assert (WEXITSTATUS (status) == 8);
}
}

return NULL;
}

static void
break_here_first (void)
{
}

static pid_t my_pid;

int
main (void)
{
pthread_t forking_threads[NUM_FORKING_THREADS];
int ret;
struct sigaction sa;
int i;

/* Just to make sure we don't run for ever. */
alarm (30);

my_pid = getpid ();

break_here_first ();

pthread_barrier_init (&barrier, NULL, NUM_FORKING_THREADS);

memset (&sa, 0, sizeof (sa));
sa.sa_sigaction = sigusr1_handler;
ret = sigaction (SIGUSR1, &sa, NULL);
assert (ret == 0);

for (i = 0; i < NUM_FORKING_THREADS; ++i)
{
ret = pthread_create (&forking_threads[i], NULL, forking_thread, NULL);
assert (ret == 0);
}

for (i = 0; i < NUM_FORKING_THREADS; ++i)
{
ret = pthread_join (forking_threads[i], NULL);
assert (ret == 0);
}

FILE *f = fopen (TOUCH_FILE_PATH, "w");
assert (f != NULL);
ret = fclose (f);
assert (ret == 0);

return 0;
}
121 changes: 121 additions & 0 deletions gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# Copyright 2021 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/>.

# Detach a running program that constantly forks, verify that we correctly
# detach all fork children, for which events are pending.
#
# The strategy is:
#
# - Resume a program in background (continue &) with many threads that
# constantly fork and wait for their fork children to exit.
# - Detach the program. If testing against GDBserver, hope that the detach
# CLI command is processed while there is a stop reply pending in the
# remote target.
# - Signal the parent program to exit, by sending it a SIGUSR1 signal.
# - Have the parent write a flag file to the filesystem just before exiting.
# - If a pending fork child is mistakenly still attached, it will prevent its
# parent thread from waitpid'ing it, preventing the main thread from joining
# it, prevent it from writing the flag file, failing the test.

standard_testfile

if { [is_remote target] } {
# If the target is remote, write the file in whatever the current working
# directory is, with a somewhat unique name.
set touch_file_path ${testfile}-flag
} else {
set touch_file_path [standard_output_file flag]
}

if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
executable [list debug "additional_flags=-DTOUCH_FILE_PATH=\"$touch_file_path\""]] != "" } {
return
}

proc do_test { } {
remote_file target delete $::touch_file_path
gdb_assert { ![remote_file target exists $::touch_file_path] } "file does not exist before test"

save_vars { ::GDBFLAGS } {
append ::GDBFLAGS " -ex \"set non-stop on\""
clean_restart $::binfile
}

if { ![runto break_here_first] } {
return
}

set pid [get_integer_valueof "my_pid" -1]
if { $pid == -1 } {
error "could not get inferior pid"
}

gdb_test_no_output "set print inferior-events off"

gdb_test_multiple "continue &" "" {
-re "Continuing.\r\n$::gdb_prompt " {
pass $gdb_test_name
}
}

set wait_time_s 2

if { [info exists ::server_spawn_id] } {
# Let the program run for 2 seconds, during which it will fork many times.
# When running against GDBserver, this makes server print a ton of
# "Detaching from process X" message, to the point where its output buffer
# gets full and it hangs in a write to stdout. During these 2 seconds,
# drain the messages from GDBserver to keep that from happening.
gdb_test_multiple "" "flush server output" {
-timeout $wait_time_s
-i $::server_spawn_id
-re ".+" {
exp_continue -continue_timer
}

timeout {
pass $gdb_test_name
}
}
} else {
# Not using GDBserver, just sleep 2 seconds.
sleep $wait_time_s
}

gdb_test "detach" "Detaching from program: .*"

if { [info exists ::server_spawn_id] } {
# Drain GDBserver's output buffer, in the (unlikely) event that enough
# messages were output to fill the buffer between the moment we stopped
# consuming it and the moment GDBserver detached the process.
gdb_test_multiple "" "" {
-i $::server_spawn_id
-re ".+" {
exp_continue
}
-re "^$" {}
}
}

remote_exec target "kill -USR1 ${pid}"
gdb_assert { [target_file_exists_with_timeout $::touch_file_path] } "file exists after detach"

# Don't leave random files on the target system.
if { [is_remote target] } {
remote_file target delete $::touch_file_path
}
}

do_test
26 changes: 26 additions & 0 deletions gdb/testsuite/gdb.threads/pending-fork-event-detach-touch-file.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2021 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 <stdio.h>

int
main (void)
{
FILE *f = fopen (TOUCH_FILE_PATH, "w");
fclose (f);
return 0;
}
Loading

0 comments on commit df5ad10

Please sign in to comment.