[PATCH 3/3] gdb, gdbserver: detach fork child when detaching from fork parent

[Simon Marchi <simon.marchi@efficios.com>]

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.  They can also be stored, for the
linux-nat target, in lwp_info::status and lwp_info::waitstatus.  For the
remote target, I suppose that a pending fork event could maybe be in the
stop reply queue, but I could generate a case where would could detach
while having pending events there, so I didn't handle that case in this
patch.

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

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
---
 gdb/linux-nat.c                               | 48 +++++++++++-
 gdb/remote.c                                  | 14 ++++
 .../pending-fork-event-touch-file.c           | 26 +++++++
 .../gdb.threads/pending-fork-event.c          |  6 +-
 .../gdb.threads/pending-fork-event.exp        | 76 +++++++++++++++++--
 gdbserver/server.cc                           | 29 +++++++
 6 files changed, 189 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 85ae68d8a6fa..b9e09e4fc0ce 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -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 ();
 	}
@@ -1367,6 +1376,43 @@ 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);
+    }
+
   if (lp->status != 0)
     linux_nat_debug_printf ("Pending %s for %s on detach.",
 			    strsignal (WSTOPSIG (lp->status)),
diff --git a/gdb/remote.c b/gdb/remote.c
index 724386e09164..677d54ccd5c9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5876,6 +5876,20 @@ 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 ())
+    {
+      if (!thread->has_pending_waitstatus ())
+	continue;
+
+      const target_waitstatus &ws = thread->pending_waitstatus ();
+      if (ws.kind () == TARGET_WAITKIND_FORKED
+	  || ws.kind () == TARGET_WAITKIND_VFORKED)
+	remote_detach_pid (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
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
index 000000000000..5536381847b1
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-touch-file.c
@@ -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;
+}
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event.c b/gdb/testsuite/gdb.threads/pending-fork-event.c
index a39ca75a49ac..bab4a99e5786 100644
--- a/gdb/testsuite/gdb.threads/pending-fork-event.c
+++ b/gdb/testsuite/gdb.threads/pending-fork-event.c
@@ -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)
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event.exp b/gdb/testsuite/gdb.threads/pending-fork-event.exp
index 6574b9abf5b7..cbe55c2f5e7c 100644
--- a/gdb/testsuite/gdb.threads/pending-fork-event.exp
+++ b/gdb/testsuite/gdb.threads/pending-fork-event.exp
@@ -28,11 +28,32 @@
 # 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
 	}
     }
 }
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 7963f2faf267..2f3d5341758f 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -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 known to
+     GDB, so 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, which is
+     the one saved by the safe iterator.  We will never delete 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)).c_str (),
+		 target_pid_to_str (thread->id).c_str ());
+    }
+
   if (detach_inferior (process) != 0)
     write_enn (own_buf);
   else
-- 
2.26.2