Re: [PATCH v3 7/7] gdb, gdbserver: detach fork child when detaching from fork parent

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-12-03 18:44, Pedro Alves wrote:
> On 2021-12-01 14:45, Simon Marchi via Gdb-patches wrote:
> 
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c
>> @@ -0,0 +1,98 @@
>> +#include <assert.h>
> 
> Missing copyright header.

Done.

>> +int
>> +main (void)
>> +{
>> +  pthread_t forking_threads[NUM_FORKING_THREADS];
>> +  int ret;
>> +  struct sigaction sa;
>> +
>> +  /* 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 (int i = 0; i < NUM_FORKING_THREADS; ++i)
> 
> This will fail to compile against gcc 4.8, because that didn't default to C99 yet.
> 
> I.e., move the "int i" declaration out of the "for" loop.

Done.

>> +    {
>> +      ret = pthread_create (&forking_threads[i], NULL, forking_thread, NULL);
>> +      assert (ret == 0);
>> +    }
>> +
>> +  for (int i = 0; i < NUM_FORKING_THREADS; ++i)
>> +    {
>> +      ret = pthread_join (forking_threads[i], NULL);
>> +      assert (ret == 0);
>> +    }
>> +
>> +  printf("WRITING FILE\n");
> 
> Space before parens.

Woops, these printfs weren't supposed to make it in the final patch,
I'll remove them.  In my experience, having the inferior output things
can make tests flaky, as its output ends up the expect buffer
interleaved with GDB output, since they share a tty.

>> +  FILE *f = fopen (TOUCH_FILE_PATH, "w");
>> +  assert (f != NULL);
>> +  ret = fclose (f);
>> +  assert (ret == 0);
>> +  printf("KBYE\n");

Removing this one too.

>> +
>> +  return 0;
>> +}
> 
>> +
>> +proc do_test { } {
>> +    file delete $::touch_file_path
>> +    gdb_assert { ![file_exists $::touch_file_path] } "file does not exist before test"
> 
> These filesystem operations work on the build system's filesystem, while they should
> be working on the target filesystem.  "remote_file target delete" etc.

Hmm right.  I changed the operations to use remote_file.  I didn't test
with a remote target though, I don't have a setup for that on hand.

>> +
>> +    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 "continue &" ".*"
> 
> Is this garanteed to not print something immediately after the prompt?
> If not, then this is racy, and you should instead use gdb_test_multiple
> with a pattern that ends in "$gdb_prompt ", without the $ anchor.

Done.

>> +    sleep 2
>> +    gdb_test "detach" ".*"
>> +    remote_exec target "kill -USR1 ${pid}"
>> +    gdb_assert { [file_exists_with_timeout $::touch_file_path] } "file exists after detach"
>> +}
>> +
>> +do_test
>> +
> 
> Otherwise LGTM.
> 

Here's a new version, since there are some quite significant changes.
All you mentioned above, plus the consuming from GDBserver in the
-ns.exp test.



From 68778a034956cc815919a3142e6397b10f07f5c7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Wed, 1 Dec 2021 09:40:03 -0500
Subject: [PATCH] gdb, gdbserver: detach fork child when detaching from fork
 parent

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
---
 gdb/linux-nat.c                               |  53 ++++++-
 gdb/remote.c                                  |  39 ++++-
 .../pending-fork-event-detach-ns.c            | 114 ++++++++++++++
 .../pending-fork-event-detach-ns.exp          | 120 +++++++++++++++
 .../pending-fork-event-detach-touch-file.c    |  26 ++++
 ...rk-event.c => pending-fork-event-detach.c} |  10 +-
 .../gdb.threads/pending-fork-event-detach.exp | 139 ++++++++++++++++++
 .../gdb.threads/pending-fork-event.exp        |  79 ----------
 gdb/testsuite/lib/gdb.exp                     |  15 ++
 gdbserver/linux-low.cc                        |  11 ++
 gdbserver/linux-low.h                         |  27 ++++
 gdbserver/server.cc                           |  29 ++++
 gdbserver/target.cc                           |   6 +
 gdbserver/target.h                            |  10 ++
 14 files changed, 591 insertions(+), 87 deletions(-)
 create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c
 create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.exp
 create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-detach-touch-file.c
 rename gdb/testsuite/gdb.threads/{pending-fork-event.c => pending-fork-event-detach.c} (89%)
 create mode 100644 gdb/testsuite/gdb.threads/pending-fork-event-detach.exp
 delete mode 100644 gdb/testsuite/gdb.threads/pending-fork-event.exp

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 2f8cf498b73c..9d4b05a2167d 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,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)),
diff --git a/gdb/remote.c b/gdb/remote.c
index 2397ee587be5..67111187e001 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -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
@@ -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
@@ -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 ());
 }
 
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c b/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c
new file mode 100644
index 000000000000..5b9f9940f832
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.c
@@ -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;
+}
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.exp b/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.exp
new file mode 100644
index 000000000000..67c8058a12b3
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach-ns.exp
@@ -0,0 +1,120 @@
+# 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.
+	set ::pending_fork_detach_ns_stop 0
+	after [expr $wait_time_s * 1000] { set ::pending_fork_detach_ns_stop 1 }
+	gdb_test_multiple "" "" {
+	    -i $::server_spawn_id
+	    -re "Detaching from process $::decimal\r\n" {
+		if { !$::pending_fork_detach_ns_stop } {
+		    exp_continue
+		}
+	    }
+	}
+    } 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 "Detaching from process $::decimal\r\n" {
+		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
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event-detach-touch-file.c b/gdb/testsuite/gdb.threads/pending-fork-event-detach-touch-file.c
new file mode 100644
index 000000000000..5536381847b1
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach-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-detach.c
similarity index 89%
rename from gdb/testsuite/gdb.threads/pending-fork-event.c
rename to gdb/testsuite/gdb.threads/pending-fork-event-detach.c
index a39ca75a49ac..ecfed98fdfda 100644
--- a/gdb/testsuite/gdb.threads/pending-fork-event.c
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach.c
@@ -32,18 +32,22 @@ break_here (void)
 static void
 do_fork (void)
 {
-  pthread_barrier_wait (&barrier);
-
   while (!release_forking_thread);
 
   if (FORK_FUNCTION () == 0)
-    _exit (0);
+    {
+      /* We create the file in a separate program that we exec: if FORK_FUNCTION
+	 is vfork, we shouldn't do anything more than an exec.  */
+      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-detach.exp b/gdb/testsuite/gdb.threads/pending-fork-event-detach.exp
new file mode 100644
index 000000000000..433660a07ae3
--- /dev/null
+++ b/gdb/testsuite/gdb.threads/pending-fork-event-detach.exp
@@ -0,0 +1,139 @@
+# Copyright (C) 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/>.
+
+# Then, test that if we detach an inferior with a pending fork child, that
+# child is correctly detached and resumes execution normally.  There are two
+# kinds of "pending fork child" we test:
+#
+#   - resulting of a fork catchpoint: we stop at a fork catchpoint and detach.
+#   - resulting of an all-stop stop on top of a non-stop target, where a fork
+#     event is saved as a pending wait status.  To test this, we stepi a thread
+#     while another one forks.  The stepi generally completes at least as fast
+#     as the fork, so we have a chance that the stop due to the stepi being
+#     complete is shown to the user while the fork event is saved for later.
+#
+# To verify that the child process is detached and resumes execution, we have
+# it write a file on the filesystem.  If we don't see the file after a certain
+# delay, it means the child was likely not detached, and the test fails.
+#
+# At the same time, this tests that having this pending fork event does not
+# cause other problems in general.  For example, a buggy GDB / GDBserver combo
+# would notice the thread of the child process of the (still unprocessed) fork
+# event, and erroneously create a new inferior for it.  Once fixed, the child
+# process' thread is hidden by whoever holds the pending fork event.
+
+standard_testfile .c -touch-file.c
+
+set touch_file_bin $binfile-touch-file
+
+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]
+}
+
+set opts [list debug "additional_flags=-DTOUCH_FILE_PATH=\"$touch_file_path\""]
+if { [gdb_compile "$srcdir/$subdir/$srcfile2" $touch_file_bin executable $opts] != "" } {
+    return
+}
+
+proc do_test { target-non-stop who_forks fork_function stop_mode } {
+    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.
+    if { $who_forks == "main" } {
+	lappend opts "additional_flags=-DMAIN_THREAD_FORKS"
+	set this_binfile ${::binfile}-main-${fork_function}
+    } elseif { $who_forks == "other" } {
+	lappend opts "additional_flags=-DOTHER_THREAD_FORKS"
+	set this_binfile ${::binfile}-other-${fork_function}
+    } else {
+	error "invalid who_forks value: $who_forks"
+    }
+
+    if { [gdb_compile_pthreads "$::srcdir/$::subdir/$::srcfile" $this_binfile executable $opts] != "" } {
+	return
+    }
+
+    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 \"maintenance set target-non-stop ${target-non-stop}\""
+	clean_restart $this_binfile
+    }
+
+    if {![runto_main]} {
+	fail "could not run to main"
+	return
+    }
+
+    # Run until breakpoint in the second thread.
+    gdb_test "break break_here" "Breakpoint $::decimal.*"
+    gdb_continue_to_breakpoint "thread started"
+
+    # Delete the breakpoint so the thread doesn't do a step-over.
+    delete_breakpoints
+
+    # Let the forking thread make progress during the step.
+    gdb_test "p release_forking_thread = 1" " = 1"
+
+    # There are two "pending fork child" modes we can test here:
+    #
+    #   - catch: set up a "catch fork" / "catch vfork" and run to it.
+    #   - stepi: stepi the non-forking thread while the forking thread,
+    #     well, forks.
+    if { $stop_mode == "catch" } {
+	gdb_test "catch fork"
+	gdb_test "catch vfork"
+	gdb_test "continue" "hit Catchpoint $::decimal.*fork.*"
+    } elseif { $stop_mode == "stepi" } {
+	# stepi the non-forking thread.
+	gdb_test "stepi"
+    } else {
+	error "invalid stop_mode value: $stop_mode"
+    }
+
+    # Make sure there's still a single inferior.
+    gdb_test "info inferior" {\* 1 [^\r\n]+}
+
+    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 { [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
+    }
+}
+
+foreach_with_prefix target-non-stop { auto on off } {
+    foreach_with_prefix who_forks { main other } {
+	foreach_with_prefix fork_function { fork vfork } {
+	    foreach_with_prefix stop_mode { stepi catch } {
+		do_test ${target-non-stop} $who_forks $fork_function $stop_mode
+	    }
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.threads/pending-fork-event.exp b/gdb/testsuite/gdb.threads/pending-fork-event.exp
deleted file mode 100644
index 51af07f56bdd..000000000000
--- a/gdb/testsuite/gdb.threads/pending-fork-event.exp
+++ /dev/null
@@ -1,79 +0,0 @@
-# Copyright (C) 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/>.
-
-# Test that we handle well, in all-stop, a fork happening in a thread B while
-# doing a step-like command in a thread A.
-#
-# A buggy GDB / GDBserver combo would notice the thread of the child process
-# of the (still unprocessed) fork event, and erroneously create a new inferior
-# for it.  Once fixed, the child process' thread is hidden by whoever holds the
-# pending fork event.
-
-standard_testfile
-
-proc do_test { target-non-stop who_forks fork_function } {
-
-    set opts [list debug "additional_flags=-DFORK_FUNCTION=$fork_function"]
-
-    # 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.
-    if { $who_forks == "main" } {
-	lappend opts "additional_flags=-DMAIN_THREAD_FORKS"
-	set this_binfile ${::binfile}-main-${fork_function}
-    } elseif { $who_forks == "other" } {
-	lappend opts "additional_flags=-DOTHER_THREAD_FORKS"
-	set this_binfile ${::binfile}-other-${fork_function}
-    } else {
-	error "invalid who_forks value: $who_forks"
-    }
-
-    if { [gdb_compile_pthreads "$::srcdir/$::subdir/$::srcfile" $this_binfile executable $opts] != "" } {
-	return
-    }
-
-    save_vars { ::GDBFLAGS } {
-	append ::GDBFLAGS " -ex \"maintenance set target-non-stop ${target-non-stop}\""
-	clean_restart $this_binfile
-    }
-
-    if {![runto_main]} {
-	fail "could not run to main"
-	return
-    }
-
-    # Run until breakpoint in the second thread.
-    gdb_test "break break_here" "Breakpoint $::decimal.*"
-    gdb_continue_to_breakpoint "thread started"
-
-    # Delete the breakpoint so the thread doesn't do a step-over.
-    delete_breakpoints
-
-    # Let the forking thread make progress during the step.
-    gdb_test "p release_forking_thread = 1" " = 1"
-
-    # stepi the non-forking thread.
-    gdb_test "stepi"
-
-    # Make sure there's still a single inferior.
-    gdb_test "info inferior" {\* 1 [^\r\n]+}
-}
-
-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
-	}
-    }
-}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 70fa2b3a8013..8b7445b75819 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8317,5 +8317,20 @@ proc require { fn arg1 {arg2 ""} } {
     return -code return 0
 }
 
+# Wait up to ::TIMEOUT seconds for file PATH to exist on the target system.
+# Return 1 if it does exist, 0 otherwise.
+
+proc target_file_exists_with_timeout { path } {
+    for {set i 0} {$i < $::timeout} {incr i} {
+	if { [remote_file target exists $path] } {
+	    return 1
+	}
+
+	sleep 1
+    }
+
+    return 0
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 34991df449bf..87888044c1f8 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -7130,6 +7130,17 @@ linux_process_target::thread_pending_parent (thread_info *thread)
   return get_lwp_thread (parent);
 }
 
+thread_info *
+linux_process_target::thread_pending_child (thread_info *thread)
+{
+  lwp_info *child = get_thread_lwp (thread)->pending_child ();
+
+  if (child == nullptr)
+    return nullptr;
+
+  return get_lwp_thread (child);
+}
+
 /* Default implementation of linux_target_ops method "set_pc" for
    32-bit pc register which is literally named "pc".  */
 
diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
index 819f915ea9a3..b563537216a6 100644
--- a/gdbserver/linux-low.h
+++ b/gdbserver/linux-low.h
@@ -312,6 +312,7 @@ class linux_process_target : public process_stratum_target
 #endif
 
   thread_info *thread_pending_parent (thread_info *thread) override;
+  thread_info *thread_pending_child (thread_info *thread) override;
 
   bool supports_catch_syscall () override;
 
@@ -750,6 +751,32 @@ struct lwp_info
     return this->fork_relative;
   }
 
+  /* If this LWP is the parent of a fork child we haven't reported to GDB yet,
+     return that child, else nullptr.  */
+  lwp_info *pending_child () const
+  {
+    if (this->fork_relative == nullptr)
+      return nullptr;
+
+    gdb_assert (this->fork_relative->fork_relative == this);
+
+    /* In a fork parent/child relationship, the parent has a status pending and
+       the child does not, and a thread can only be in one such relationship
+       at most.  So we can recognize who is the parent based on which one has
+       a pending status.  */
+    gdb_assert (!!this->status_pending_p
+		!= !!this->fork_relative->status_pending_p);
+
+    if (!this->status_pending_p)
+      return nullptr;
+
+    const target_waitstatus &ws = this->waitstatus;
+    gdb_assert (ws.kind () == TARGET_WAITKIND_FORKED
+		|| ws.kind () == TARGET_WAITKIND_VFORKED);
+
+    return this->fork_relative;
+  }
+
   /* Backlink to the parent object.  */
   struct thread_info *thread = nullptr;
 
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 8dde6fb07295..27e2aba01214 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 GDB won't take care of detaching it.  We must do it here.
+
+     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.  */
+      thread_info *child = target_thread_pending_child (thread);
+      if (child == nullptr)
+	continue;
+
+      process_info *fork_child_process = get_thread_process (child);
+      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
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 136b5104de85..aa3d42462f52 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -841,6 +841,12 @@ process_stratum_target::thread_pending_parent (thread_info *thread)
   return nullptr;
 }
 
+thread_info *
+process_stratum_target::thread_pending_child (thread_info *thread)
+{
+  return nullptr;
+}
+
 bool
 process_stratum_target::supports_software_single_step ()
 {
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 1b0a1201d755..331a21aa57a3 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -492,6 +492,10 @@ class process_stratum_target
      else nullptr.  */
   virtual thread_info *thread_pending_parent (thread_info *thread);
 
+  /* If THREAD is the parent of a fork child that was not reported to GDB,
+     return this child, else nullptr.  */
+  virtual thread_info *thread_pending_child (thread_info *thread);
+
   /* Returns true if the target can software single step.  */
   virtual bool supports_software_single_step ();
 
@@ -708,6 +712,12 @@ target_thread_pending_parent (thread_info *thread)
   return the_target->thread_pending_parent (thread);
 }
 
+static inline thread_info *
+target_thread_pending_child (thread_info *thread)
+{
+  return the_target->thread_pending_child (thread);
+}
+
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
 
 int set_desired_thread ();
-- 
2.33.1