gdb, gdbserver: detach fork child when detaching from fork parent
authorSimon Marchi <simon.marchi@efficios.com>
Thu, 14 Oct 2021 19:19:49 +0000 (15:19 -0400)
committerSimon Marchi <simon.marchi@efficios.com>
Thu, 28 Oct 2021 20:32:06 +0000 (16:32 -0400)
FIXME: we should make target_ops::detach take a pid, not an inferior,
and have the core detach fork childs of pending events (stored in
thread_info's pending waitstatus).

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 if using that), but
with the core of GDB not knowing about it.  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.

I don't think there is anything the core of GDB can do
here, it's up to each target to detach a fork child if detaching a
thread with a pending fork event.  This affects the Linux native target
and the remote target (and probably the other targets that support fork,
but I haven't tried them).

Update the gdb.threads/pending-fork-event.exp to try to detach the
process while a thread has 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).

The 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).

Change-Id: I6d811a56f520e3cb92d5ea563ad38976f92e93dd

gdb/linux-nat.c
gdb/remote.c
gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c [new file with mode: 0644]
gdb/testsuite/gdb.threads/pending-fork-event.c
gdb/testsuite/gdb.threads/pending-fork-event.exp
gdbserver/server.cc

index f206b8749299f6b1fb041f47b427350335656a58..10fa8f57d4a28b5df028c4128fe41f99335e9506 100644 (file)
@@ -1302,7 +1302,16 @@ get_detach_signal (struct lwp_info *lp)
       if (target_is_non_stop_p () && !tp->executing)
        {
          if (tp->suspend.waitstatus_pending_p)
-           signo = tp->suspend.waitstatus.value.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->suspend.waitstatus.kind == TARGET_WAITKIND_STOPPED)
+               signo = tp->suspend.waitstatus.value.sig;
+             else
+               signo = GDB_SIGNAL_0;
+           }
          else
            signo = tp->suspend.stop_signal;
        }
@@ -1354,6 +1363,46 @@ 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 for a pending fork event stored in the lwp_info structure
+     (not reported to the core).  */
+  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)
+           ptrace (PTRACE_DETACH, child_pid, 0, 0);
+         else
+           perror_warning_with_name (_("Failed to detach fork child"));
+       }
+    }
+
+  /* Check for a pending fork event stored in the thread_info structure
+     (reported to the core).  */
+  thread_info *thread = find_thread_ptid (linux_target, lp->ptid);
+  gdb_assert (thread != nullptr);
+  if (thread->suspend.waitstatus_pending_p)
+    {
+      const target_waitstatus &ws = thread->suspend.waitstatus;
+
+      if (ws.kind == TARGET_WAITKIND_FORKED
+         || ws.kind == TARGET_WAITKIND_VFORKED)
+       {
+         /* We have not created an lwp for this process.  */
+         gdb_assert (find_lwp_pid (ws.value.related_pid) == nullptr);
+
+         ptrace (PTRACE_DETACH, ws.value.related_pid.pid (), 0, 0);
+       }
+
+    }
+
   if (lp->status != 0)
     linux_nat_debug_printf ("Pending %s for %s on detach.",
                            strsignal (WSTOPSIG (lp->status)),
index f2271ad3b5066e5b14feae995b3e487d17be738c..c09d714087596fcfdcdc07f0c136120d9d98b687 100644 (file)
@@ -5835,6 +5835,8 @@ remote_target::remote_detach_pid (int pid)
 void
 remote_target::remote_detach_1 (inferior *inf, int from_tty)
 {
+  REMOTE_SCOPED_DEBUG_ENTER_EXIT;
+
   int pid = inferior_ptid.pid ();
   struct remote_state *rs = get_remote_state ();
   int is_fork_parent;
@@ -5857,6 +5859,24 @@ remote_target::remote_detach_1 (inferior *inf, int from_tty)
   /* Tell the remote target to detach.  */
   remote_detach_pid (pid);
 
+  for (thread_info *thread : inf->threads ())
+    {
+      if (!thread->suspend.waitstatus_pending_p)
+       continue;
+
+      const target_waitstatus &ws = thread->suspend.waitstatus;
+      if (ws.kind != TARGET_WAITKIND_FORKED
+         && ws.kind != TARGET_WAITKIND_VFORKED)
+       continue;
+
+      remote_debug_printf ("Detached thread %s has a pending fork event, "
+                          "detaching fork child %d",
+                          thread->ptid.to_string ().c_str (),
+                          ws.value.related_pid.pid ());
+
+      remote_detach_pid (ws.value.related_pid.pid ());
+    }
+
   /* Exit only if this is the only active inferior.  */
   if (from_tty && !rs->extended && number_of_live_inferiors (this) == 1)
     puts_filtered (_("Ending remote debugging.\n"));
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c b/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c
new file mode 100644 (file)
index 0000000..5536381
--- /dev/null
@@ -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;
+}
index a39ca75a49aca19469b0285da3b916b57950c5f9..bab4a99e57867d8e1f30b8aaa9f5decf09fd4570 100644 (file)
@@ -32,18 +32,18 @@ break_here (void)
 static void
 do_fork (void)
 {
-  pthread_barrier_wait (&barrier);
-
   while (!release_forking_thread);
 
   if (FORK_FUNCTION () == 0)
-    _exit (0);
+    execl (TOUCH_FILE_BIN, TOUCH_FILE_BIN, NULL);
 
 }
 
 static void *
 thread_func (void *p)
 {
+  pthread_barrier_wait (&barrier);
+
 #if defined(MAIN_THREAD_FORKS)
   break_here ();
 #elif defined(OTHER_THREAD_FORKS)
index 6574b9abf5b7ee15394db781513551f9fe431042..cbe55c2f5e7c7c69a6ed66cbcae9cd855a7001a0 100644 (file)
 # for it.  Once fixed, the child process' thread is hidden by whoever holds the
 # pending fork event.
 
-standard_testfile
+standard_testfile .c -touch-file.c
 
-proc do_test { target-non-stop who_forks fork_function } {
+set touch_file_bin $binfile-touch-file
+set touch_file_path [standard_output_file some-file]
 
-    set opts [list debug "additional_flags=-DFORK_FUNCTION=$fork_function"]
+set opts [list debug "additional_flags=-DTOUCH_FILE_PATH=\"$touch_file_path\""]
+if { [gdb_compile "$srcdir/$subdir/$srcfile2" $touch_file_bin executable $opts] != "" } {
+    return 0
+}
+
+# Run until execution is stopped and a thread has a pending fork event.
+#
+# WHO_FORKS tells which of the thread program threads does the fork ("main" or
+# "other").
+#
+# FORK_FUNCTION tells which function to call to fork ("fork" or "vfork").
+#
+# Return 1 on success, 0 otherwise.
+
+proc setup { target-non-stop who_forks fork_function } {
+    set this_binfile ${::binfile}-${fork_function}
+
+    set opts [list \
+       debug \
+       "additional_flags=-DFORK_FUNCTION=$fork_function" \
+       "additional_flags=-DTOUCH_FILE_BIN=\"$::touch_file_bin\""]
 
     # WHO_FORKS says which of the main or other thread calls (v)fork.  The
     # thread that does not call (v)fork is the one who tries to step.
@@ -47,7 +68,7 @@ proc do_test { target-non-stop who_forks fork_function } {
     }
 
     if { [gdb_compile_pthreads "$::srcdir/$::subdir/$::srcfile" $this_binfile executable $opts] != "" } {
-       return
+       return 0
     }
 
     save_vars { ::GDBFLAGS } {
@@ -57,7 +78,7 @@ proc do_test { target-non-stop who_forks fork_function } {
 
     if {![runto_main]} {
        fail "could not run to main"
-       return
+       return 0
     }
 
     # Run until breakpoint in the second thread.
@@ -75,12 +96,55 @@ proc do_test { target-non-stop who_forks fork_function } {
 
     # Make sure there's still a single inferior.
     gdb_test "info inferior" {\* 1 [^\r\n]+}
+
+    return 1
+}
+
+# Return 1 if file ::TOUCH_FILE_PATH exists, else 0.
+
+proc file_exists {} {
+    return [file exists $::touch_file_path]
+}
+
+# Wait up to ::TIMEOUT seconds for file ::TOUCH_FILE_PATH to exist.  Return
+# 1 if it does exist, 0 otherwise.
+
+proc file_exists_with_timeout {} {
+    for {set i 0} {$i < $::timeout} {incr i} {
+       if { [file_exists] } {
+           return 1
+       }
+
+       sleep 1
+    }
+
+    return 0
+}
+
+# Run until there exists (at least, likely exists) a pending fork event.
+# Detach the fork parent.  Verify that the fork child (which does not appear in
+# GDB, since the fork event is pending) is detached as well.
+
+proc_with_prefix do_test_detach { target-non-stop who_forks fork_function } {
+    file delete $::touch_file_path
+    gdb_assert { ![file_exists] } "file does not exist before test"
+
+    if { ![setup ${target-non-stop} $who_forks $fork_function] } {
+       return
+    }
+
+    gdb_test "detach"
+
+    # After being detached, the fork child creates file ::TOUCH_FILE_PATH.
+    # Seeing this file tells us the fork child was detached and executed
+    # successfully.
+    gdb_assert { [file_exists_with_timeout] } "file exists after detach"
 }
 
 foreach_with_prefix target-non-stop { auto on off } {
     foreach_with_prefix who_forks { main other } {
        foreach_with_prefix fork_function { fork vfork } {
-           do_test ${target-non-stop} $who_forks $fork_function
+           do_test_detach ${target-non-stop} $who_forks $fork_function
        }
     }
 }
index 0f9fa9ddbb5a7351772f3122207cf1017194b3e8..50465c39c2aa025845b99ced3124913cec42d471 100644 (file)
@@ -1250,6 +1250,35 @@ handle_detach (char *own_buf)
   /* We'll need this after PROCESS has been destroyed.  */
   int pid = process->pid;
 
+  /* If this process has an unreported fork child, that child is not know to
+     GDB, so we should detach it as well.
+
+     Here, we specifically don't want to use "safe iteration", as detaching
+     another process might delete the next thread in the iteration, that is
+     already saved by the safe iterator.  We will never detach the currently
+     iterated on thread, so standard iteration should be safe.  */
+  for (thread_info *thread : all_threads)
+    {
+      /* Only threads that are of the process we are detaching.  */
+      if (thread->id.pid () != pid)
+       continue;
+
+      /* Only threads that have a pending fork event.  */
+      if (thread->fork_child == nullptr)
+       continue;
+
+      process_info *fork_child_process
+       = find_process_pid (thread->fork_child->id.pid ());
+      gdb_assert (fork_child_process != nullptr);
+
+      int fork_child_pid = fork_child_process->pid;
+
+      if (detach_inferior (fork_child_process) != 0)
+       warning (_("Failed to detach fork child %s, child of %s"),
+                target_pid_to_str (ptid_t (fork_child_pid)),
+                target_pid_to_str (thread->id));
+    }
+
   if (detach_inferior (process) != 0)
     write_enn (own_buf);
   else
This page took 0.034742 seconds and 4 git commands to generate.