[PATCH v2 0/8] Fixes for multiprocess for FreeBSD's native target

[PATCH v2 0/8] Fixes for multiprocess for FreeBSD's native target

[John Baldwin]

While this series is not perfect, it does result in a net improvement
in testsuite results on FreeBSD/amd64 and is also a significantly
better experience debugging forking processes.  I'm less confident about
the last two patches but would like to get the first 6 patches into
GDB 14.

Changes since V1:

- A few early trivial patches were merged.

- The list of pending events are now stored as private class members
  of class fbsd_nat_target in Patch 1 along with various style fixes
  noted by Simon.  I also added an assertion that a pending event
  always has an associated inferior when it is added to the list.

- Patch 2 adds a bug trailer.

- Patch 3 is a bit simpler since the methods to work with the pending
  list are now class methods so the "this" pointer doesn't have to be
  passed around as often.  The ptid argument to resume is now renamed
  to scope_ptid.  I also added handling for TARGET_WAITKIND_IGNORE in
  the stop_process helper to avoid adding a bogus pending event if the
  process has disappeared (so wait() fails with ECHILD).  This case
  was caught by the new assertion in Patch 1.

- In patch 4, a workaround in pending_ptrace_events for a kernel bug
  is now conditional since I've merged an upstream fix to FreeBSD's
  kernel since posting the first version at
  https://cgit.freebsd.org/src/commit/?id=653738e895ba022be1179a95a85089e7bc66dbbe

- Patches 5 and 6 are unchanged.

- Patches 7 is new and replaces the ptid_t to identify single LWP vs
  entire process being resumed with an unordered_set<> of resumed LWPs
  permitting multiple individual LWPs in a process to be resumed while
  other LWPs are still suspended.

  Since FreeBSD can only PT_CONTINUE entire processes at a time, this
  means that the resume target method can no longer use ptrace
  directly, but instead stores the requested actions as a set of
  pending actions in the per-inferior data structure.  Later when
  either commit_resumed or wait is called, the target runs through all
  its non-exited inferiors comitting the cumulative results of earlier
  resume calls.

  Because that change is a bit hairy I've kept separate from the
  previous round of multiprocess fixes in patch 3 to make it easier to
  bisect in the future if there are regressions.

- Patch 8 is new and adds an implementation of the stop target method.
  It wasn't clear to me if this is really needed for an all-stop-only
  target, but at least one test in the testsuite was calling this
  method when I made it gdb_assert() instead of being the default.

John Baldwin (8):
  fbsd-nat: Add a list of pending events.
  fbsd-nat: Defer any ineligible events reported by wait.
  fbsd-nat: Fix resuming and waiting with multiple processes.
  fbsd-nat: Fix several issues with detaching.
  fbsd-nat: Fix thread_alive against a running thread.
  fbsd-nat: Stop a process if it is running before killing it.
  fbsd-nat: Defer resume of inferiors.
  fbsd-nat: Implement the target stop method.

 gdb/fbsd-nat.c | 971 ++++++++++++++++++++++++++++++++++++++++++-------
 gdb/fbsd-nat.h |  68 ++++
 2 files changed, 915 insertions(+), 124 deletions(-)

-- 
2.40.0

[PATCH v2 1/8] fbsd-nat: Add a list of pending events.

[John Baldwin]

The m_pending_events list stores a queue of deferred events that might
be reported by the next call to the target's wait method.  The set of
events that are eligible is filtered by the ptid passed to resume.

For now this just replaces the list of vfork_done events.  A
subsequent commit will reuse this to store other events.
---
 gdb/fbsd-nat.c | 128 ++++++++++++++++++++++++++++---------------------
 gdb/fbsd-nat.h |  45 +++++++++++++++++
 2 files changed, 118 insertions(+), 55 deletions(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index b0ca8b00699..f9d8632c0ef 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -47,13 +47,47 @@
 #include "fbsd-nat.h"
 #include "fbsd-tdep.h"
 
-#include <list>
-
 #ifndef PT_GETREGSET
 #define	PT_GETREGSET	42	/* Get a target register set */
 #define	PT_SETREGSET	43	/* Set a target register set */
 #endif
 
+/* See fbsd-nat.h.  */
+
+void
+fbsd_nat_target::add_pending_event (const ptid_t &ptid,
+				    const target_waitstatus &status)
+{
+  gdb_assert (find_inferior_ptid (this, ptid) != nullptr);
+  m_pending_events.emplace_back (ptid, status);
+}
+
+/* See fbsd-nat.h.  */
+
+bool
+fbsd_nat_target::have_pending_event (ptid_t filter)
+{
+  for (const pending_event &event : m_pending_events)
+    if (event.ptid.matches (filter))
+      return true;
+  return false;
+}
+
+/* See fbsd-nat.h.  */
+
+gdb::optional<fbsd_nat_target::pending_event>
+fbsd_nat_target::take_pending_event ()
+{
+  for (auto it = m_pending_events.begin (); it != m_pending_events.end (); it++)
+    if (it->ptid.matches (m_resume_ptid))
+      {
+	pending_event event = *it;
+	m_pending_events.erase (it);
+	return event;
+      }
+  return {};
+}
+
 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
 
@@ -1061,47 +1095,18 @@ fbsd_is_child_pending (pid_t pid)
 }
 
 #ifndef PTRACE_VFORK
-static std::forward_list<ptid_t> fbsd_pending_vfork_done;
-
 /* Record a pending vfork done event.  */
 
 static void
 fbsd_add_vfork_done (ptid_t pid)
 {
-  fbsd_pending_vfork_done.push_front (pid);
+  add_pending_event (ptid, target_waitstatus ().set_vfork_done ());
 
   /* If we're in async mode, need to tell the event loop there's
      something here to process.  */
   if (target_is_async_p ())
     async_file_mark ();
 }
-
-/* Check for a pending vfork done event for a specific PID.  */
-
-static int
-fbsd_is_vfork_done_pending (pid_t pid)
-{
-  for (auto it = fbsd_pending_vfork_done.begin ();
-       it != fbsd_pending_vfork_done.end (); it++)
-    if (it->pid () == pid)
-      return 1;
-  return 0;
-}
-
-/* Check for a pending vfork done event.  If one is found, remove it
-   from the list and return the PTID.  */
-
-static ptid_t
-fbsd_next_vfork_done (void)
-{
-  if (!fbsd_pending_vfork_done.empty ())
-    {
-      ptid_t ptid = fbsd_pending_vfork_done.front ();
-      fbsd_pending_vfork_done.pop_front ();
-      return ptid;
-    }
-  return null_ptid;
-}
 #endif
 #endif
 
@@ -1110,21 +1115,18 @@ fbsd_next_vfork_done (void)
 void
 fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
 {
-#if defined(TDP_RFPPWAIT) && !defined(PTRACE_VFORK)
-  pid_t pid;
-
-  /* Don't PT_CONTINUE a process which has a pending vfork done event.  */
-  if (minus_one_ptid == ptid)
-    pid = inferior_ptid.pid ();
-  else
-    pid = ptid.pid ();
-  if (fbsd_is_vfork_done_pending (pid))
-    return;
-#endif
-
   fbsd_nat_debug_printf ("[%s], step %d, signo %d (%s)",
 			 target_pid_to_str (ptid).c_str (), step, signo,
 			 gdb_signal_to_name (signo));
+
+  /* Don't PT_CONTINUE a thread or process which has a pending event.  */
+  m_resume_ptid = ptid;
+  if (have_pending_event (ptid))
+    {
+      fbsd_nat_debug_printf ("found pending event");
+      return;
+    }
+
   if (ptid.lwp_p ())
     {
       /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
@@ -1257,14 +1259,6 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 
   while (1)
     {
-#ifndef PTRACE_VFORK
-      wptid = fbsd_next_vfork_done ();
-      if (wptid != null_ptid)
-	{
-	  ourstatus->set_vfork_done ();
-	  return wptid;
-	}
-#endif
       wptid = inf_ptrace_target::wait (ptid, ourstatus, target_options);
       if (ourstatus->kind () == TARGET_WAITKIND_STOPPED)
 	{
@@ -1473,16 +1467,26 @@ ptid_t
 fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		       target_wait_flags target_options)
 {
-  ptid_t wptid;
-
   fbsd_nat_debug_printf ("[%s], [%s]", target_pid_to_str (ptid).c_str (),
 			 target_options_to_string (target_options).c_str ());
 
+  /* If there is a valid pending event, return it.  */
+  gdb::optional<pending_event> event = take_pending_event ();
+  if (event.has_value ())
+    {
+      fbsd_nat_debug_printf ("returning pending event [%s], [%s]",
+			     target_pid_to_str (event->ptid).c_str (),
+			     event->status.to_string ().c_str ());
+      gdb_assert (event->ptid.matches (ptid));
+      *ourstatus = event->status;
+      return event->ptid;
+    }
+
   /* Ensure any subsequent events trigger a new event in the loop.  */
   if (is_async_p ())
     async_file_flush ();
 
-  wptid = wait_1 (ptid, ourstatus, target_options);
+  ptid_t wptid = wait_1 (ptid, ourstatus, target_options);
 
   /* If we are in async mode and found an event, there may still be
      another event pending.  Trigger the event pipe so that that the
@@ -1585,9 +1589,23 @@ fbsd_nat_target::create_inferior (const char *exec_file,
     (disable_randomization);
 #endif
 
+  /* Expect a wait for the new process.  */
+  m_resume_ptid = minus_one_ptid;
+  fbsd_nat_debug_printf ("setting resume_ptid to [%s]",
+			 target_pid_to_str (m_resume_ptid).c_str ());
   inf_ptrace_target::create_inferior (exec_file, allargs, env, from_tty);
 }
 
+void
+fbsd_nat_target::attach (const char *args, int from_tty)
+{
+  /* Expect a wait for the new process.  */
+  m_resume_ptid = minus_one_ptid;
+  fbsd_nat_debug_printf ("setting resume_ptid to [%s]",
+			 target_pid_to_str (m_resume_ptid).c_str ());
+  inf_ptrace_target::attach (args, from_tty);
+}
+
 #ifdef TDP_RFPPWAIT
 /* Target hook for follow_fork.  On entry and at return inferior_ptid is
    the ptid of the followed inferior.  */
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index a19bceaf5e4..e1d5b8efdf0 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -20,12 +20,15 @@
 #ifndef FBSD_NAT_H
 #define FBSD_NAT_H
 
+#include "gdbsupport/gdb_optional.h"
 #include "inf-ptrace.h"
 #include "regcache.h"
 #include "regset.h"
 #include <osreldate.h>
 #include <sys/proc.h>
 
+#include <list>
+
 /* FreeBSD kernels 11.3 and later report valid si_code values for
    SIGTRAP on all architectures.  Older FreeBSD kernels that supported
    TRAP_BRKPT did not report valid values for MIPS and sparc64.  Even
@@ -76,6 +79,8 @@ class fbsd_nat_target : public inf_ptrace_target
   void create_inferior (const char *, const std::string &,
 			char **, int) override;
 
+  void attach (const char *, int) override;
+
   void resume (ptid_t, int, enum gdb_signal) override;
 
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
@@ -217,6 +222,46 @@ class fbsd_nat_target : public inf_ptrace_target
     return store_regset (regcache, regnum, note, regset, regbase, &regs,
 			 sizeof (regs));
   }
+
+private:
+  /* If an event is triggered asynchronously (fake vfork_done events)
+     or occurs when the core is not expecting it, a pending event is
+     created.  This event is then returned by a future call to the
+     target wait method.  */
+
+  struct pending_event
+  {
+    pending_event (const ptid_t &_ptid, const target_waitstatus &_status) :
+      ptid (_ptid), status (_status) {}
+
+    ptid_t ptid;
+    target_waitstatus status;
+  };
+
+  /* Add a new pending event to the list.  */
+
+  void add_pending_event (const ptid_t &ptid, const target_waitstatus &status);
+
+  /* Return true if there is a pending event matching FILTER.  */
+
+  bool have_pending_event (ptid_t filter);
+
+  /* Helper method called by the target wait method.  Check if there
+     is a pending event matching M_RESUME_PTID.  If there is a
+     matching event, the event is removed from the pending list and
+     returned.  */
+
+  gdb::optional<pending_event> take_pending_event ();
+
+  /* List of pending events.  */
+
+  std::list<pending_event> m_pending_events;
+
+  /* Filter for ptid's allowed to report events from wait.  Normally
+     set in resume, but also reset to minus_one_ptid in
+     create_inferior and attach.  */
+
+  ptid_t m_resume_ptid;
 };
 
 /* Fetch the signal information for PTID and store it in *SIGINFO.
-- 
2.40.0

[PATCH v2 2/8] fbsd-nat: Defer any ineligible events reported by wait.

[John Baldwin]

If wait_1 finds an event for a thread or process that does not match
the set of threads and processes previously resumed, defer the event.
If the event is for a specific thread, suspend the thread and continue
the associated process before waiting for another event.

One specific example of such an event is if a thread is created while
another thread in the same process hits a breakpoint.  If the second
thread's event is reported first, the target resume method does not
yet "know" about the new thread and will not suspend it via
PT_SUSPEND.  When wait is called, it will probably return the event
from the first thread before the result of the step from second
thread.  This is the case reported in PR 21497.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=21497
---
 gdb/fbsd-nat.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index f9d8632c0ef..15da555ec3e 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1486,7 +1486,40 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   if (is_async_p ())
     async_file_flush ();
 
-  ptid_t wptid = wait_1 (ptid, ourstatus, target_options);
+  ptid_t wptid;
+  while (1)
+    {
+      wptid = wait_1 (ptid, ourstatus, target_options);
+
+      /* If no event was found, just return.  */
+      if (ourstatus->kind () == TARGET_WAITKIND_IGNORE
+	  || ourstatus->kind () == TARGET_WAITKIND_NO_RESUMED)
+	break;
+
+      /* If an event is reported for a thread or process while
+	 stepping some other thread, suspend the thread reporting the
+	 event and defer the event until it can be reported to the
+	 core.  */
+      if (!wptid.matches (m_resume_ptid))
+	{
+	  add_pending_event (wptid, *ourstatus);
+	  fbsd_nat_debug_printf ("deferring event [%s], [%s]",
+				 target_pid_to_str (wptid).c_str (),
+				 ourstatus->to_string ().c_str ());
+	  if (wptid.pid () == m_resume_ptid.pid ())
+	    {
+	      fbsd_nat_debug_printf ("suspending thread [%s]",
+				     target_pid_to_str (wptid).c_str ());
+	      if (ptrace (PT_SUSPEND, wptid.lwp (), NULL, 0) == -1)
+		perror_with_name (("ptrace (PT_SUSPEND)"));
+	      if (ptrace (PT_CONTINUE, wptid.pid (), (caddr_t) 1, 0) == -1)
+		perror_with_name (("ptrace (PT_CONTINUE)"));
+	    }
+	  continue;
+	}
+
+      break;
+    }
 
   /* If we are in async mode and found an event, there may still be
      another event pending.  Trigger the event pipe so that that the
-- 
2.40.0

[PATCH v2 3/8] fbsd-nat: Fix resuming and waiting with multiple processes.

[John Baldwin]

I did not fully understand the requirements of multiple process
support when I enabled it previously and several parts were broken.
In particular, the resume method was only resuming a single process,
and wait was not stopping other processes when reporting an event.

To support multiple running inferiors, add a new per-inferior
structure which trackes the number of existing and running LWPs for
each process.  The structure also stores a ptid_t describing the
set of LWPs currently resumed for each process.

For the resume method, iterate over all non-exited inferiors resuming
each process matching the passed in ptid rather than only resuming the
current inferior's process for a wildcard ptid.  If a resumed process
has a pending event, don't actually resume the process, but other
matching processes without a pending event are still resumed in case
the later call to the wait method requests an event from one of the
processes without a pending event.

For the wait method, stop other running processes before returning an
event to the core.  When stopping a process, first check to see if an
event is already pending.  If it is, queue the event to be reported
later.  If not, send a SIGSTOP to the process and wait for it to stop.
If the event reported by the wait is not for the SIGSTOP, queue the
event and remember to ignore a future SIGSTOP event for the process.

Note that, unlike the Linux native target, entire processes are
stopped rather than individual LWPs.  In FreeBSD one can only wait on
processes (via pid), not for an event from a specific thread.

Other changes in this commit handle bookkeeping for the per-inferior
data such as purging the data in the mourn_inferior method and
migrating the data to the new inferior in the follow_exec method.  The
per-inferior data is created in the attach, create_inferior, and
follow_fork methods.
---
 gdb/fbsd-nat.c | 394 ++++++++++++++++++++++++++++++++++++++-----------
 gdb/fbsd-nat.h |  23 +--
 2 files changed, 321 insertions(+), 96 deletions(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 15da555ec3e..53c680d9efb 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -52,6 +52,26 @@
 #define	PT_SETREGSET	43	/* Set a target register set */
 #endif
 
+/* Information stored about each inferior.  */
+struct fbsd_inferior_info
+{
+  /* Filter for resumed LWPs which can report events from wait.  */
+  ptid_t resumed_lwps = null_ptid;
+
+  /* Number of LWPs this process contains.  */
+  unsigned int num_lwps = 0;
+
+  /* Number of LWPs currently running.  */
+  unsigned int running_lwps = 0;
+
+  /* Have a pending SIGSTOP event that needs to be discarded.  */
+  bool pending_sigstop = false;
+};
+
+/* Per-inferior data key.  */
+
+static const registry<inferior>::key<fbsd_inferior_info> fbsd_inferior_data;
+
 /* See fbsd-nat.h.  */
 
 void
@@ -76,14 +96,19 @@ fbsd_nat_target::have_pending_event (ptid_t filter)
 /* See fbsd-nat.h.  */
 
 gdb::optional<fbsd_nat_target::pending_event>
-fbsd_nat_target::take_pending_event ()
+fbsd_nat_target::take_pending_event (ptid_t filter)
 {
   for (auto it = m_pending_events.begin (); it != m_pending_events.end (); it++)
-    if (it->ptid.matches (m_resume_ptid))
+    if (it->ptid.matches (filter))
       {
-	pending_event event = *it;
-	m_pending_events.erase (it);
-	return event;
+	inferior *inf = find_inferior_ptid (this, it->ptid);
+	fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+	if (it->ptid.matches (info->resumed_lwps))
+	  {
+	    pending_event event = *it;
+	    m_pending_events.erase (it);
+	    return event;
+	  }
       }
   return {};
 }
@@ -773,6 +798,8 @@ show_fbsd_nat_debug (struct ui_file *file, int from_tty,
 #define fbsd_nat_debug_printf(fmt, ...) \
   debug_prefixed_printf_cond (debug_fbsd_nat, "fbsd-nat", fmt, ##__VA_ARGS__)
 
+#define fbsd_nat_debug_start_end(fmt, ...) \
+  scoped_debug_start_end (debug_fbsd_nat, "fbsd-nat", fmt, ##__VA_ARGS__)
 
 /*
   FreeBSD's first thread support was via a "reentrant" version of libc
@@ -930,6 +957,9 @@ fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
   if (nlwps == -1)
     perror_with_name (("ptrace (PT_GETLWPLIST)"));
 
+  inferior *inf = find_inferior_ptid (target, ptid_t (pid));
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+  gdb_assert (info != nullptr);
   for (i = 0; i < nlwps; i++)
     {
       ptid_t ptid = ptid_t (pid, lwps[i]);
@@ -948,8 +978,14 @@ fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
 #endif
 	  fbsd_lwp_debug_printf ("adding thread for LWP %u", lwps[i]);
 	  add_thread (target, ptid);
+#ifdef PT_LWP_EVENTS
+	  info->num_lwps++;
+#endif
 	}
     }
+#ifndef PT_LWP_EVENTS
+  info->num_lwps = nlwps;
+#endif
 }
 
 /* Implement the "update_thread_list" target_ops method.  */
@@ -1110,92 +1146,117 @@ fbsd_add_vfork_done (ptid_t pid)
 #endif
 #endif
 
-/* Implement the "resume" target_ops method.  */
+/* Resume a single process.  */
 
 void
-fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
+fbsd_nat_target::resume_one_process (ptid_t ptid, int step,
+				     enum gdb_signal signo)
 {
   fbsd_nat_debug_printf ("[%s], step %d, signo %d (%s)",
 			 target_pid_to_str (ptid).c_str (), step, signo,
 			 gdb_signal_to_name (signo));
 
+  inferior *inf = find_inferior_ptid (this, ptid);
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+  info->resumed_lwps = ptid;
+  gdb_assert (info->running_lwps == 0);
+
   /* Don't PT_CONTINUE a thread or process which has a pending event.  */
-  m_resume_ptid = ptid;
   if (have_pending_event (ptid))
     {
       fbsd_nat_debug_printf ("found pending event");
       return;
     }
 
-  if (ptid.lwp_p ())
+  for (thread_info *tp : inf->non_exited_threads ())
     {
-      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
-      inferior *inf = find_inferior_ptid (this, ptid);
+      int request;
 
-      for (thread_info *tp : inf->non_exited_threads ())
-	{
-	  int request;
-
-	  if (tp->ptid.lwp () == ptid.lwp ())
-	    request = PT_RESUME;
-	  else
-	    request = PT_SUSPEND;
-
-	  if (ptrace (request, tp->ptid.lwp (), NULL, 0) == -1)
-	    perror_with_name (request == PT_RESUME ?
-			      ("ptrace (PT_RESUME)") :
-			      ("ptrace (PT_SUSPEND)"));
-	  if (request == PT_RESUME)
-	    low_prepare_to_resume (tp);
-	}
-    }
-  else
-    {
-      /* If ptid is a wildcard, resume all matching threads (they won't run
-	 until the process is continued however).  */
-      for (thread_info *tp : all_non_exited_threads (this, ptid))
+      /* If ptid is a specific LWP, suspend all other LWPs in the
+	 process, otherwise resume all LWPs in the process..  */
+      if (!ptid.lwp_p() || tp->ptid.lwp () == ptid.lwp ())
 	{
 	  if (ptrace (PT_RESUME, tp->ptid.lwp (), NULL, 0) == -1)
 	    perror_with_name (("ptrace (PT_RESUME)"));
 	  low_prepare_to_resume (tp);
+	  info->running_lwps++;
 	}
+      else
+	{
+	  if (ptrace (PT_SUSPEND, tp->ptid.lwp (), NULL, 0) == -1)
+	    perror_with_name (("ptrace (PT_SUSPEND)"));
+	}
+    }
+
+  if (ptid.pid () != inferior_ptid.pid ())
+    {
+      step = 0;
+      signo = GDB_SIGNAL_0;
+      gdb_assert (!ptid.lwp_p ());
+    }
+  else
+    {
       ptid = inferior_ptid;
-    }
-
 #if __FreeBSD_version < 1200052
-  /* When multiple threads within a process wish to report STOPPED
-     events from wait(), the kernel picks one thread event as the
-     thread event to report.  The chosen thread event is retrieved via
-     PT_LWPINFO by passing the process ID as the request pid.  If
-     multiple events are pending, then the subsequent wait() after
-     resuming a process will report another STOPPED event after
-     resuming the process to handle the next thread event and so on.
+      /* When multiple threads within a process wish to report STOPPED
+	 events from wait(), the kernel picks one thread event as the
+	 thread event to report.  The chosen thread event is retrieved
+	 via PT_LWPINFO by passing the process ID as the request pid.
+	 If multiple events are pending, then the subsequent wait()
+	 after resuming a process will report another STOPPED event
+	 after resuming the process to handle the next thread event
+	 and so on.
 
-     A single thread event is cleared as a side effect of resuming the
-     process with PT_CONTINUE, PT_STEP, etc.  In older kernels,
-     however, the request pid was used to select which thread's event
-     was cleared rather than always clearing the event that was just
-     reported.  To avoid clearing the event of the wrong LWP, always
-     pass the process ID instead of an LWP ID to PT_CONTINUE or
-     PT_SYSCALL.
+	 A single thread event is cleared as a side effect of resuming
+	 the process with PT_CONTINUE, PT_STEP, etc.  In older
+	 kernels, however, the request pid was used to select which
+	 thread's event was cleared rather than always clearing the
+	 event that was just reported.  To avoid clearing the event of
+	 the wrong LWP, always pass the process ID instead of an LWP
+	 ID to PT_CONTINUE or PT_SYSCALL.
 
-     In the case of stepping, the process ID cannot be used with
-     PT_STEP since it would step the thread that reported an event
-     which may not be the thread indicated by PTID.  For stepping, use
-     PT_SETSTEP to enable stepping on the desired thread before
-     resuming the process via PT_CONTINUE instead of using
-     PT_STEP.  */
-  if (step)
-    {
-      if (ptrace (PT_SETSTEP, get_ptrace_pid (ptid), NULL, 0) == -1)
-	perror_with_name (("ptrace (PT_SETSTEP)"));
-      step = 0;
-    }
-  ptid = ptid_t (ptid.pid ());
+	 In the case of stepping, the process ID cannot be used with
+	 PT_STEP since it would step the thread that reported an event
+	 which may not be the thread indicated by PTID.  For stepping,
+	 use PT_SETSTEP to enable stepping on the desired thread
+	 before resuming the process via PT_CONTINUE instead of using
+	 PT_STEP.  */
+      if (step)
+	{
+	  if (ptrace (PT_SETSTEP, get_ptrace_pid (ptid), NULL, 0) == -1)
+	    perror_with_name (("ptrace (PT_SETSTEP)"));
+	  step = 0;
+	}
+      ptid = ptid_t (ptid.pid ());
 #endif
+    }
+
   inf_ptrace_target::resume (ptid, step, signo);
 }
 
+/* Implement the "resume" target_ops method.  */
+
+void
+fbsd_nat_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
+{
+  fbsd_nat_debug_start_end ("[%s], step %d, signo %d (%s)",
+			    target_pid_to_str (scope_ptid).c_str (), step, signo,
+			    gdb_signal_to_name (signo));
+
+  gdb_assert (inferior_ptid.matches (scope_ptid));
+  gdb_assert (!scope_ptid.tid_p ());
+
+  if (scope_ptid == minus_one_ptid)
+    {
+      for (inferior *inf : all_non_exited_inferiors (this))
+	resume_one_process (ptid_t (inf->pid), step, signo);
+    }
+  else
+    {
+      resume_one_process (scope_ptid, step, signo);
+    }
+}
+
 #ifdef USE_SIGTRAP_SIGINFO
 /* Handle breakpoint and trace traps reported via SIGTRAP.  If the
    trap was a breakpoint or trace trap that should be reported to the
@@ -1263,10 +1324,7 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
       if (ourstatus->kind () == TARGET_WAITKIND_STOPPED)
 	{
 	  struct ptrace_lwpinfo pl;
-	  pid_t pid;
-	  int status;
-
-	  pid = wptid.pid ();
+	  pid_t pid = wptid.pid ();
 	  if (ptrace (PT_LWPINFO, pid, (caddr_t) &pl, sizeof pl) == -1)
 	    perror_with_name (("ptrace (PT_LWPINFO)"));
 
@@ -1282,6 +1340,13 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 				       pl.pl_siginfo.si_code);
 	    }
 
+	  /* There may not be an inferior for this pid if this is a
+	     PL_FLAG_CHILD event.  */
+	  inferior *inf = find_inferior_ptid (this, wptid);
+	  fbsd_inferior_info *info = inf == nullptr ? nullptr :
+	    fbsd_inferior_data.get (inf);
+	  gdb_assert (info != NULL || pl.pl_flags & PL_FLAG_CHILD);
+
 #ifdef PT_LWP_EVENTS
 	  if (pl.pl_flags & PL_FLAG_EXITED)
 	    {
@@ -1299,6 +1364,20 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 				target_pid_to_str (wptid).c_str ());
 		  low_delete_thread (thr);
 		  delete_thread (thr);
+		  info->num_lwps--;
+
+		  /* If this LWP was the only resumed LWP from the
+		     process, report an event to the core.  */
+		  if (wptid == info->resumed_lwps)
+		    {
+		      ourstatus->set_spurious ();
+		      return wptid;
+		    }
+
+		  /* During process exit LWPs that were not resumed
+		     will report exit events.  */
+		  if (wptid.matches (info->resumed_lwps))
+		    info->running_lwps--;
 		}
 	      if (ptrace (PT_CONTINUE, pid, (caddr_t) 1, 0) == -1)
 		perror_with_name (("ptrace (PT_CONTINUE)"));
@@ -1331,6 +1410,10 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 		  fbsd_lwp_debug_printf ("adding thread for LWP %u",
 					 pl.pl_lwpid);
 		  add_thread (this, wptid);
+		  info->num_lwps++;
+
+		  if (wptid.matches(info->resumed_lwps))
+		    info->running_lwps++;
 		}
 	      ourstatus->set_spurious ();
 	      return wptid;
@@ -1357,6 +1440,8 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 	      child_ptid = fbsd_is_child_pending (child);
 	      if (child_ptid == null_ptid)
 		{
+		  int status;
+
 		  pid = waitpid (child, &status, 0);
 		  if (pid == -1)
 		    perror_with_name (("waitpid"));
@@ -1458,11 +1543,104 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 		perror_with_name (("ptrace (PT_CONTINUE)"));
 	      continue;
 	    }
+
+	  /* If this is a pending SIGSTOP event from an earlier call
+	     to stop_process, discard the event and wait for another
+	     event.  */
+	  if (ourstatus->sig () == GDB_SIGNAL_STOP && info->pending_sigstop)
+	    {
+	      fbsd_nat_debug_printf ("ignoring SIGSTOP for pid %u", pid);
+	      info->pending_sigstop = false;
+	      if (ptrace (PT_CONTINUE, pid, (caddr_t) 1, 0) == -1)
+		perror_with_name (("ptrace (PT_CONTINUE)"));
+	      continue;
+	    }
 	}
+      else
+	fbsd_nat_debug_printf ("event [%s], [%s]",
+			       target_pid_to_str (wptid).c_str (),
+			       ourstatus->to_string ().c_str ());
       return wptid;
     }
 }
 
+/* Stop a given process.  If the process is already stopped, record
+   its pending event instead.  */
+
+void
+fbsd_nat_target::stop_process (inferior *inf)
+{
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+  gdb_assert (info != nullptr);
+
+  info->resumed_lwps = null_ptid;
+  if (info->running_lwps == 0)
+    return;
+
+  ptid_t ptid (inf->pid);
+  target_waitstatus status;
+  ptid_t wptid = wait_1 (ptid, &status, TARGET_WNOHANG);
+
+  if (wptid != minus_one_ptid)
+    {
+      /* Save the current event as a pending event.  */
+      add_pending_event (wptid, status);
+      info->running_lwps = 0;
+      return;
+    }
+
+  /* If a SIGSTOP is already pending, don't send a new one, but tell
+     wait_1 to report a SIGSTOP.  */
+  if (info->pending_sigstop)
+    {
+      fbsd_nat_debug_printf ("waiting for existing pending SIGSTOP for %u",
+			     inf->pid);
+      info->pending_sigstop = false;
+    }
+  else
+    {
+      /* Ignore errors from kill as process exit might race with kill.  */
+      fbsd_nat_debug_printf ("killing %u with SIGSTOP", inf->pid);
+      ::kill (inf->pid, SIGSTOP);
+    }
+
+  /* Wait for SIGSTOP (or some other event) to be reported.  */
+  wptid = wait_1 (ptid, &status, 0);
+
+  switch (status.kind ())
+    {
+    case TARGET_WAITKIND_EXITED:
+    case TARGET_WAITKIND_SIGNALLED:
+      /* If the process has exited, we aren't going to get an
+	 event for the SIGSTOP.  Save the current event and
+	 return.  */
+      add_pending_event (wptid, status);
+      break;
+    case TARGET_WAITKIND_IGNORE:
+      /* wait() failed with ECHILD meaning the process no longer
+	 exists.  This means a bug happened elsewhere, but at least
+	 the process is no longer running.  */
+      break;
+    case TARGET_WAITKIND_STOPPED:
+      /* If this is the SIGSTOP event, discard it and return
+	 leaving the process stopped.  */
+      if (status.sig () == GDB_SIGNAL_STOP)
+	break;
+
+      /* FALLTHROUGH */
+    default:
+      /* Some other event has occurred.  Save the current
+	 event.  */
+      add_pending_event (wptid, status);
+
+      /* Ignore the next SIGSTOP for this process.  */
+      fbsd_nat_debug_printf ("ignoring next SIGSTOP for %u", inf->pid);
+      info->pending_sigstop = true;
+      break;
+    }
+  info->running_lwps = 0;
+}
+
 ptid_t
 fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 		       target_wait_flags target_options)
@@ -1471,9 +1649,13 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 			 target_options_to_string (target_options).c_str ());
 
   /* If there is a valid pending event, return it.  */
-  gdb::optional<pending_event> event = take_pending_event ();
+  gdb::optional<pending_event> event = take_pending_event (ptid);
   if (event.has_value ())
     {
+      /* Stop any other inferiors currently running.  */
+      for (inferior *inf : all_non_exited_inferiors (this))
+	stop_process (inf);
+
       fbsd_nat_debug_printf ("returning pending event [%s], [%s]",
 			     target_pid_to_str (event->ptid).c_str (),
 			     event->status.to_string ().c_str ());
@@ -1496,28 +1678,38 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	  || ourstatus->kind () == TARGET_WAITKIND_NO_RESUMED)
 	break;
 
+      inferior *inf = find_inferior_ptid (this, wptid);
+      gdb_assert (inf != nullptr);
+      fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+      gdb_assert (info != nullptr);
+      gdb_assert (info->resumed_lwps != null_ptid);
+      gdb_assert (info->running_lwps > 0);
+
       /* If an event is reported for a thread or process while
 	 stepping some other thread, suspend the thread reporting the
 	 event and defer the event until it can be reported to the
 	 core.  */
-      if (!wptid.matches (m_resume_ptid))
+      if (!wptid.matches (info->resumed_lwps))
 	{
 	  add_pending_event (wptid, *ourstatus);
 	  fbsd_nat_debug_printf ("deferring event [%s], [%s]",
 				 target_pid_to_str (wptid).c_str (),
 				 ourstatus->to_string ().c_str ());
-	  if (wptid.pid () == m_resume_ptid.pid ())
-	    {
-	      fbsd_nat_debug_printf ("suspending thread [%s]",
-				     target_pid_to_str (wptid).c_str ());
-	      if (ptrace (PT_SUSPEND, wptid.lwp (), NULL, 0) == -1)
-		perror_with_name (("ptrace (PT_SUSPEND)"));
-	      if (ptrace (PT_CONTINUE, wptid.pid (), (caddr_t) 1, 0) == -1)
-		perror_with_name (("ptrace (PT_CONTINUE)"));
-	    }
+	  if (ptrace (PT_SUSPEND, wptid.lwp (), NULL, 0) == -1)
+	    perror_with_name (("ptrace (PT_SUSPEND)"));
+	  if (ptrace (PT_CONTINUE, wptid.pid (), (caddr_t) 1, 0) == -1)
+	    perror_with_name (("ptrace (PT_CONTINUE)"));
 	  continue;
 	}
 
+      /* This process is no longer running.  */
+      info->resumed_lwps = null_ptid;
+      info->running_lwps = 0;
+
+      /* Stop any other inferiors currently running.  */
+      for (inferior *inf : all_non_exited_inferiors (this))
+	stop_process (inf);
+
       break;
     }
 
@@ -1622,23 +1814,49 @@ fbsd_nat_target::create_inferior (const char *exec_file,
     (disable_randomization);
 #endif
 
-  /* Expect a wait for the new process.  */
-  m_resume_ptid = minus_one_ptid;
-  fbsd_nat_debug_printf ("setting resume_ptid to [%s]",
-			 target_pid_to_str (m_resume_ptid).c_str ());
+  fbsd_inferior_info *info = fbsd_inferior_data.emplace (current_inferior ());
+  info->resumed_lwps = minus_one_ptid;
+  info->num_lwps = 1;
+  info->running_lwps = 1;
   inf_ptrace_target::create_inferior (exec_file, allargs, env, from_tty);
 }
 
 void
 fbsd_nat_target::attach (const char *args, int from_tty)
 {
-  /* Expect a wait for the new process.  */
-  m_resume_ptid = minus_one_ptid;
-  fbsd_nat_debug_printf ("setting resume_ptid to [%s]",
-			 target_pid_to_str (m_resume_ptid).c_str ());
+  fbsd_inferior_info *info = fbsd_inferior_data.emplace (current_inferior ());
+  info->resumed_lwps = minus_one_ptid;
+  info->num_lwps = 1;
+  info->running_lwps = 1;
   inf_ptrace_target::attach (args, from_tty);
 }
 
+void
+fbsd_nat_target::mourn_inferior ()
+{
+  inferior *inf = current_inferior ();
+  gdb_assert (!have_pending_event (ptid_t (inf->pid)));
+  fbsd_inferior_data.clear (inf);
+  inf_ptrace_target::mourn_inferior ();
+}
+
+void
+fbsd_nat_target::follow_exec (inferior *follow_inf, ptid_t ptid,
+			      const char *execd_pathname)
+{
+  inferior *orig_inf = current_inferior ();
+
+  inf_ptrace_target::follow_exec (follow_inf, ptid, execd_pathname);
+
+  if (orig_inf != follow_inf)
+    {
+      /* Migrate the inferior info to the new inferior. */
+      fbsd_inferior_info *info = fbsd_inferior_data.get (orig_inf);
+      fbsd_inferior_data.set (orig_inf, nullptr);
+      fbsd_inferior_data.set (follow_inf, info);
+    }
+}
+
 #ifdef TDP_RFPPWAIT
 /* Target hook for follow_fork.  On entry and at return inferior_ptid is
    the ptid of the followed inferior.  */
@@ -1651,6 +1869,12 @@ fbsd_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
   inf_ptrace_target::follow_fork (child_inf, child_ptid, fork_kind,
 				  follow_child, detach_fork);
 
+  if (child_inf != nullptr)
+    {
+      fbsd_inferior_info *info = fbsd_inferior_data.emplace (child_inf);
+      info->num_lwps = 1;
+    }
+
   if (!follow_child && detach_fork)
     {
       pid_t child_pid = child_ptid.pid ();
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index e1d5b8efdf0..2736b0f0912 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -81,6 +81,8 @@ class fbsd_nat_target : public inf_ptrace_target
 
   void attach (const char *, int) override;
 
+  void mourn_inferior () override;
+
   void resume (ptid_t, int, enum gdb_signal) override;
 
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
@@ -92,6 +94,8 @@ class fbsd_nat_target : public inf_ptrace_target
   bool stopped_by_sw_breakpoint () override;
 #endif
 
+  void follow_exec (inferior *, ptid_t, const char *) override;
+
 #ifdef TDP_RFPPWAIT
   void follow_fork (inferior *, ptid_t, target_waitkind, bool, bool) override;
 
@@ -135,6 +139,10 @@ class fbsd_nat_target : public inf_ptrace_target
 private:
   ptid_t wait_1 (ptid_t, struct target_waitstatus *, target_wait_flags);
 
+  void resume_one_process (ptid_t, int, enum gdb_signal);
+
+  void stop_process (inferior *);
+
   /* Helper routines for use in fetch_registers and store_registers in
      subclasses.  These routines fetch and store a single set of
      registers described by REGSET.  The REGSET's 'regmap' field must
@@ -246,22 +254,15 @@ class fbsd_nat_target : public inf_ptrace_target
 
   bool have_pending_event (ptid_t filter);
 
-  /* Helper method called by the target wait method.  Check if there
-     is a pending event matching M_RESUME_PTID.  If there is a
-     matching event, the event is removed from the pending list and
-     returned.  */
+  /* Check if there is a pending event for a resumed process matching
+     FILTER.  If there is a matching event, the event is removed from
+     the pending list and returned.  */
 
-  gdb::optional<pending_event> take_pending_event ();
+  gdb::optional<pending_event> take_pending_event (ptid_t filter);
 
   /* List of pending events.  */
 
   std::list<pending_event> m_pending_events;
-
-  /* Filter for ptid's allowed to report events from wait.  Normally
-     set in resume, but also reset to minus_one_ptid in
-     create_inferior and attach.  */
-
-  ptid_t m_resume_ptid;
 };
 
 /* Fetch the signal information for PTID and store it in *SIGINFO.
-- 
2.40.0

[PATCH v2 4/8] fbsd-nat: Fix several issues with detaching.

[John Baldwin]

- Detach from any child processes implicitly attached to by the kernel
  due to fork following that have not yet been processed by GDB's
  core.

- Delete breakpoints before detaching.

  inf-ptrace::detach does not do this (somewhat surprisingly), so add
  an override to remove breakpoints from a process before detaching
  from it.

  This also requires explicitly draining any pending SIGTRAP events
  for software breakpoints before detaching.  In particular, threads
  may need their PC adjusted due to the software breakpoint before
  being resumed after detach.  On more modern systems using the si_code
  from SIGTRAP to identify software breakpoint traps, the PC is adjusted
  in ::wait_1 as a side effect of parsing the event.  To support older
  kernels, ::detach fixes up the PC for any SIGTRAP stop whose potential
  new PC matches an existing software breakpoint.
---
 gdb/fbsd-nat.c | 262 +++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-nat.h |  14 +++
 2 files changed, 276 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 53c680d9efb..ed69cb33e93 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1831,6 +1831,268 @@ fbsd_nat_target::attach (const char *args, int from_tty)
   inf_ptrace_target::attach (args, from_tty);
 }
 
+/* If this thread has a pending fork event, there is a child process
+   GDB is attached to that the core of GDB doesn't know about.
+   Detach from it.  */
+
+void
+fbsd_nat_target::detach_fork_children (thread_info *tp)
+{
+  /* Check in thread_info::pending_waitstatus.  */
+  if (tp->has_pending_waitstatus ())
+    {
+      const target_waitstatus &ws = tp->pending_waitstatus ();
+
+      if (ws.kind () == TARGET_WAITKIND_VFORKED
+	  || ws.kind () == TARGET_WAITKIND_FORKED)
+	{
+	  pid_t pid = ws.child_ptid ().pid ();
+	  fbsd_nat_debug_printf ("detaching from child %d", pid);
+	  (void) ptrace (PT_DETACH, pid, (caddr_t) 1, 0);
+	}
+    }
+
+  /* Check in thread_info::pending_follow.  */
+  if (tp->pending_follow.kind () == TARGET_WAITKIND_VFORKED
+      || tp->pending_follow.kind () == TARGET_WAITKIND_FORKED)
+    {
+      pid_t pid = tp->pending_follow.child_ptid ().pid ();
+      fbsd_nat_debug_printf ("detaching from child %d", pid);
+      (void) ptrace (PT_DETACH, pid, (caddr_t) 1, 0);
+    }
+}
+
+/* Detach from any child processes associated with pending fork events
+   for a stopped process.  Returns true if the process has terminated
+   and false if it is still alive.  */
+
+bool
+fbsd_nat_target::detach_fork_children (inferior *inf)
+{
+  /* Detach any child processes associated with pending fork events in
+     threads belonging to this process.  */
+  for (thread_info *tp : inf->non_exited_threads ())
+    detach_fork_children (tp);
+
+  /* Unwind state associated with any pending events.  Reset
+     info->resumed_lwps so that take_pending_event will harvest
+     events.  */
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+  ptid_t ptid = ptid_t (inf->pid);
+  info->resumed_lwps = ptid;
+
+  while (1)
+    {
+      gdb::optional<pending_event> event = take_pending_event (ptid);
+      if (!event.has_value ())
+	break;
+
+      switch (event->status.kind ())
+	{
+	case TARGET_WAITKIND_EXITED:
+	case TARGET_WAITKIND_SIGNALLED:
+	  return true;
+	case TARGET_WAITKIND_FORKED:
+	case TARGET_WAITKIND_VFORKED:
+	  {
+	    pid_t pid = event->status.child_ptid ().pid ();
+	    fbsd_nat_debug_printf ("detaching from child %d", pid);
+	    (void) ptrace (PT_DETACH, pid, (caddr_t) 1, 0);
+	  }
+	  break;
+	}
+    }
+  return false;
+}
+
+/* Scan all of the threads for a stopped process invoking the supplied
+   callback on the ptrace_lwpinfo object for threads other than the
+   thread which reported the current stop.  The callback can return
+   true to terminate the iteration early.  This function returns true
+   if the callback returned true, otherwise it returns false.  */
+
+typedef bool (ptrace_event_ftype) (const struct ptrace_lwpinfo &pl);
+
+static bool
+iterate_other_ptrace_events (pid_t pid,
+			     gdb::function_view<ptrace_event_ftype> callback)
+{
+  /* Fetch the LWP ID of the thread that just reported the last stop
+     and ignore that LWP in the following loop.  */
+  ptrace_lwpinfo pl;
+  if (ptrace (PT_LWPINFO, pid, (caddr_t) &pl, sizeof (pl)) != 0)
+    perror_with_name (("ptrace (PT_LWPINFO)"));
+  lwpid_t lwpid = pl.pl_lwpid;
+
+  int nlwps = ptrace (PT_GETNUMLWPS, pid, NULL, 0);
+  if (nlwps == -1)
+    perror_with_name (("ptrace (PT_GETLWPLIST)"));
+  if (nlwps == 1)
+    return false;
+
+  gdb::unique_xmalloc_ptr<lwpid_t[]> lwps (XCNEWVEC (lwpid_t, nlwps));
+
+  nlwps = ptrace (PT_GETLWPLIST, pid, (caddr_t) lwps.get (), nlwps);
+  if (nlwps == -1)
+    perror_with_name (("ptrace (PT_GETLWPLIST)"));
+
+  for (int i = 0; i < nlwps; i++)
+    {
+      if (lwps[i] == lwpid)
+	continue;
+
+      if (ptrace (PT_LWPINFO, lwps[i], (caddr_t) &pl, sizeof (pl)) != 0)
+	perror_with_name (("ptrace (PT_LWPINFO)"));
+
+      if (callback (pl))
+	return true;
+    }
+  return false;
+}
+
+/* True if there are any stopped threads with an interesting event.  */
+
+static bool
+pending_ptrace_events (inferior *inf)
+{
+  auto lambda = [] (const struct ptrace_lwpinfo &pl)
+  {
+#if defined(PT_LWP_EVENTS) && __FreeBSD_kernel_version < 1400090
+    if (pl.pl_flags == PL_FLAG_BORN)
+      return true;
+#endif
+#ifdef TDP_RFPPWAIT
+    if (pl.pl_flags & PL_FLAG_FORKED)
+      return true;
+#endif
+    if (pl.pl_event == PL_EVENT_SIGNAL)
+      {
+	if ((pl.pl_flags & PL_FLAG_SI) == 0)
+	  {
+	    /* Not sure which signal, assume it matters.  */
+	    return true;
+	  }
+	if (pl.pl_siginfo.si_signo == SIGTRAP)
+	  return true;
+      }
+    return false;
+  };
+  return iterate_other_ptrace_events (inf->pid,
+				      gdb::make_function_view (lambda));
+}
+
+void
+fbsd_nat_target::detach (inferior *inf, int from_tty)
+{
+  fbsd_nat_debug_start_end ("pid %d", inf->pid);
+
+  stop_process (inf);
+
+  remove_breakpoints_inf (inf);
+
+  if (detach_fork_children (inf)) {
+    /* No need to detach now.  */
+    target_announce_detach (from_tty);
+
+    detach_success (inf);
+    return;
+  }
+
+  /* If there are any pending events (SIGSTOP from stop_process or a
+     breakpoint hit that needs a PC fixup), drain events until the
+     process can be safely detached.  */
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+  ptid_t ptid = ptid_t (inf->pid);
+  if (info->pending_sigstop || pending_ptrace_events (inf))
+    {
+      bool pending_sigstop = info->pending_sigstop;
+      int sig = 0;
+
+      if (pending_sigstop)
+	fbsd_nat_debug_printf ("waiting for SIGSTOP");
+
+      /* Force wait_1 to report the SIGSTOP instead of swallowing it.  */
+      info->pending_sigstop = false;
+
+      /* Report event for all threads from wait_1.  */
+      info->resumed_lwps = ptid;
+
+      do
+	{
+	  if (ptrace (PT_CONTINUE, inf->pid, (caddr_t) 1, sig) != 0)
+	    perror_with_name (("ptrace(PT_CONTINUE)"));
+
+	  target_waitstatus ws;
+	  ptid_t wptid = wait_1 (ptid, &ws, 0);
+
+	  switch (ws.kind ())
+	    {
+	    case TARGET_WAITKIND_EXITED:
+	    case TARGET_WAITKIND_SIGNALLED:
+	      /* No need to detach now.  */
+	      target_announce_detach (from_tty);
+
+	      detach_success (inf);
+	      return;
+	    case TARGET_WAITKIND_FORKED:
+	    case TARGET_WAITKIND_VFORKED:
+	      {
+		pid_t pid = ws.child_ptid ().pid ();
+		fbsd_nat_debug_printf ("detaching from child %d", pid);
+		(void) ptrace (PT_DETACH, pid, (caddr_t) 1, 0);
+		sig = 0;
+	      }
+	      break;
+	    case TARGET_WAITKIND_STOPPED:
+	      sig = gdb_signal_to_host (ws.sig ());
+	      switch (sig)
+		{
+		case SIGSTOP:
+		  if (pending_sigstop)
+		    {
+		      sig = 0;
+		      pending_sigstop = false;
+		    }
+		  break;
+		case SIGTRAP:
+#ifndef USE_SIGTRAP_SIGINFO
+		  {
+		    /* Update PC from software breakpoint hit.  */
+		    struct regcache *regcache = get_thread_regcache (this, wptid);
+		    struct gdbarch *gdbarch = regcache->arch ();
+		    int decr_pc = gdbarch_decr_pc_after_break (gdbarch);
+
+		    if (decr_pc != 0)
+		      {
+			CORE_ADDR pc;
+
+			pc = regcache_read_pc (regcache);
+			if (breakpoint_inserted_here_p (regcache->aspace (),
+							pc - decr_pc))
+			  {
+			    fbsd_nat_debug_printf ("adjusted PC for LWP %ld",
+						   wptid.lwp ());
+			    regcache_write_pc (regcache, pc - decr_pc);
+			  }
+		      }
+		  }
+#endif
+		  sig = 0;
+		  break;
+		}
+	    }
+	}
+      while (pending_sigstop || pending_ptrace_events (inf));
+    }
+
+  target_announce_detach (from_tty);
+
+  if (ptrace (PT_DETACH, inf->pid, (caddr_t) 1, 0) == -1)
+	perror_with_name (("ptrace (PT_DETACH)"));
+
+  detach_success (inf);
+}
+
 void
 fbsd_nat_target::mourn_inferior ()
 {
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 2736b0f0912..3ae0a97a336 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -81,6 +81,8 @@ class fbsd_nat_target : public inf_ptrace_target
 
   void attach (const char *, int) override;
 
+  void detach (inferior *, int) override;
+
   void mourn_inferior () override;
 
   void resume (ptid_t, int, enum gdb_signal) override;
@@ -263,6 +265,18 @@ class fbsd_nat_target : public inf_ptrace_target
   /* List of pending events.  */
 
   std::list<pending_event> m_pending_events;
+
+  /* If this thread has a pending fork event, there is a child process
+     GDB is attached to that the core of GDB doesn't know about.
+     Detach from it.  */
+
+  void detach_fork_children (thread_info *tp);
+
+  /* Detach from any child processes associated with pending fork events
+     for a stopped process.  Returns true if the process has terminated
+     and false if it is still alive.  */
+
+  bool detach_fork_children (inferior *inf);
 };
 
 /* Fetch the signal information for PTID and store it in *SIGINFO.
-- 
2.40.0

[PATCH v2 5/8] fbsd-nat: Fix thread_alive against a running thread.

[John Baldwin]

FreeBSD's ptrace fails requests with EBUSY against a running process.
Report that the thread is alive instead of dead if ptrace fails with
EBUSY.

This fixes an internal error in the gdb.threads/detach-step-over.exp
test where one process was detached while a thread in a second process
was being stepped.  The core incorrectly assumed the stepping thread
had vanished and discarded the pending stepping state.  When the
thread later reported a SIGTRAP from completing the step, this
triggered an assertion.
---
 gdb/fbsd-nat.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index ed69cb33e93..6ce77c5cd16 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -839,7 +839,13 @@ fbsd_nat_target::thread_alive (ptid_t ptid)
 
       if (ptrace (PT_LWPINFO, ptid.lwp (), (caddr_t) &pl, sizeof pl)
 	  == -1)
-	return false;
+	{
+	  /* EBUSY means the associated process is running which means
+	     the LWP does exist and belongs to a running process.  */
+	  if (errno == EBUSY)
+	    return true;
+	  return false;
+	}
 #ifdef PL_FLAG_EXITED
       if (pl.pl_flags & PL_FLAG_EXITED)
 	return false;
-- 
2.40.0

[PATCH v2 6/8] fbsd-nat: Stop a process if it is running before killing it.

[John Baldwin]

In addition, detach from any child processes implicitly attached to by
the kernel due to fork following that have not yet been processed by
GDB's core.
---
 gdb/fbsd-nat.c | 93 +++++++++++++++++++++++++++++++++++++++++---------
 gdb/fbsd-nat.h |  2 ++
 2 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 6ce77c5cd16..195c4feb49a 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1136,6 +1136,31 @@ fbsd_is_child_pending (pid_t pid)
   return null_ptid;
 }
 
+/* Wait for a child of a fork to report its stop.  Returns the PTID of
+   the new child process.  */
+
+static ptid_t
+fbsd_wait_for_fork_child (pid_t pid)
+{
+  ptid_t ptid = fbsd_is_child_pending (pid);
+  if (ptid != null_ptid)
+    return ptid;
+
+  int status;
+  pid_t wpid = waitpid (pid, &status, 0);
+  if (wpid == -1)
+    perror_with_name (("waitpid"));
+
+  gdb_assert (wpid == pid);
+
+  struct ptrace_lwpinfo pl;
+  if (ptrace (PT_LWPINFO, wpid, (caddr_t) &pl, sizeof pl) == -1)
+    perror_with_name (("ptrace (PT_LWPINFO)"));
+
+  gdb_assert (pl.pl_flags & PL_FLAG_CHILD);
+  return ptid_t (wpid, pl.pl_lwpid);
+}
+
 #ifndef PTRACE_VFORK
 /* Record a pending vfork done event.  */
 
@@ -1443,23 +1468,7 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 #endif
 
 	      /* Make sure the other end of the fork is stopped too.  */
-	      child_ptid = fbsd_is_child_pending (child);
-	      if (child_ptid == null_ptid)
-		{
-		  int status;
-
-		  pid = waitpid (child, &status, 0);
-		  if (pid == -1)
-		    perror_with_name (("waitpid"));
-
-		  gdb_assert (pid == child);
-
-		  if (ptrace (PT_LWPINFO, child, (caddr_t) &pl, sizeof pl) == -1)
-		    perror_with_name (("ptrace (PT_LWPINFO)"));
-
-		  gdb_assert (pl.pl_flags & PL_FLAG_CHILD);
-		  child_ptid = ptid_t (child, pl.pl_lwpid);
-		}
+	      child_ptid = fbsd_wait_for_fork_child (child);
 
 	      /* Enable additional events on the child process.  */
 	      fbsd_enable_proc_events (child_ptid.pid ());
@@ -2099,6 +2108,56 @@ fbsd_nat_target::detach (inferior *inf, int from_tty)
   detach_success (inf);
 }
 
+/* Implement the "kill" target method.  */
+
+void
+fbsd_nat_target::kill ()
+{
+  pid_t pid = inferior_ptid.pid ();
+  if (pid == 0)
+    return;
+
+  inferior *inf = current_inferior ();
+  stop_process (inf);
+
+  if (detach_fork_children (inf)) {
+    /* No need to kill now.  */
+    target_mourn_inferior (inferior_ptid);
+
+    return;
+  }
+
+#ifdef TDP_RFPPWAIT
+  /* If there are any threads that have forked a new child but not yet
+     reported it because other threads reported events first, detach
+     from the children before killing the parent.  */
+  auto lambda = [] (const struct ptrace_lwpinfo &pl)
+  {
+    if (pl.pl_flags & PL_FLAG_FORKED)
+      {
+	pid_t child = pl.pl_child_pid;
+
+	/* If the child hasn't reported its stop yet, wait for it to
+	   stop.  */
+	fbsd_wait_for_fork_child (child);
+
+	/* Detach from the child.  */
+	(void) ptrace (PT_DETACH, child, (caddr_t) 1, 0);
+      }
+    return false;
+  };
+  iterate_other_ptrace_events (pid, gdb::make_function_view (lambda));
+#endif
+
+  if (ptrace (PT_KILL, pid, NULL, 0) == -1)
+	perror_with_name (("ptrace (PT_KILL)"));
+
+  int status;
+  waitpid (pid, &status, 0);
+
+  target_mourn_inferior (inferior_ptid);
+}
+
 void
 fbsd_nat_target::mourn_inferior ()
 {
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 3ae0a97a336..7016cc0242a 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -83,6 +83,8 @@ class fbsd_nat_target : public inf_ptrace_target
 
   void detach (inferior *, int) override;
 
+  void kill () override;
+
   void mourn_inferior () override;
 
   void resume (ptid_t, int, enum gdb_signal) override;
-- 
2.40.0

[PATCH v2 7/8] fbsd-nat: Defer resume of inferiors.

[John Baldwin]

For at least some tests in the testsuite, the core can resume multiple
individual threads belonging to the same process before waiting.  To
support this, change the resume method to record resumption requests
and defer resuming processes until either the commit_resumed target
method is called or until wait is called.

This requires tracking a set of resumed LWPs in the per-inferior
data rather than a single ptid_t to describe the set of resumed LWPs.
---
 gdb/fbsd-nat.c | 292 +++++++++++++++++++++++++++++++++----------------
 gdb/fbsd-nat.h |  12 +-
 2 files changed, 205 insertions(+), 99 deletions(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 195c4feb49a..f114f88e649 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -47,6 +47,8 @@
 #include "fbsd-nat.h"
 #include "fbsd-tdep.h"
 
+#include <unordered_set>
+
 #ifndef PT_GETREGSET
 #define	PT_GETREGSET	42	/* Get a target register set */
 #define	PT_SETREGSET	43	/* Set a target register set */
@@ -55,17 +57,52 @@
 /* Information stored about each inferior.  */
 struct fbsd_inferior_info
 {
-  /* Filter for resumed LWPs which can report events from wait.  */
-  ptid_t resumed_lwps = null_ptid;
+  /* Needs to be resumed.  */
+  bool resume_pending = false;
+
+  /* Either all threads in a process are resumed, a subset of threads
+     are resumed, or the entire process is stopped.  For the first
+     case, resumed is true and resumed_lwps is empty.  For the second
+     case, resumed is false and resumed_lwps contains the set of LWPs
+     resumed.  For the third case, resumed is false and resumed_lwps
+     is empty.  */
+  bool resumed_all_lwps = false;
+  std::unordered_set<lwpid_t> resumed_lwps;
+
+  /* Thread to step on next resume.  -1 if no thread to step.  */
+  lwpid_t stepping_lwp = -1;
+
+  /* Signal to send on next resume.  */
+  enum gdb_signal resume_signal = GDB_SIGNAL_0;
 
   /* Number of LWPs this process contains.  */
   unsigned int num_lwps = 0;
 
-  /* Number of LWPs currently running.  */
+  /* Number of resumed LWPs currently running.  */
   unsigned int running_lwps = 0;
 
   /* Have a pending SIGSTOP event that needs to be discarded.  */
   bool pending_sigstop = false;
+
+  /* Return true if a specific LWP has been resumed.  */
+  bool is_lwp_resumed (lwpid_t lwpid)
+  {
+    return (resumed_all_lwps || resumed_lwps.count (lwpid) != 0);
+  }
+
+  /* Return true if any LWPs in the process have been resumed.  */
+  bool is_process_resumed ()
+  {
+    return (resumed_all_lwps || !resumed_lwps.empty ());
+  }
+
+  /* Mark the process as stopped with no threads resumed or running.  */
+  void mark_stopped ()
+  {
+    resumed_all_lwps = false;
+    resumed_lwps.clear ();
+    running_lwps = 0;
+  }
 };
 
 /* Per-inferior data key.  */
@@ -103,7 +140,7 @@ fbsd_nat_target::take_pending_event (ptid_t filter)
       {
 	inferior *inf = find_inferior_ptid (this, it->ptid);
 	fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
-	if (it->ptid.matches (info->resumed_lwps))
+	if (info->is_lwp_resumed (it->ptid.lwp ()))
 	  {
 	    pending_event event = *it;
 	    m_pending_events.erase (it);
@@ -798,7 +835,10 @@ show_fbsd_nat_debug (struct ui_file *file, int from_tty,
 #define fbsd_nat_debug_printf(fmt, ...) \
   debug_prefixed_printf_cond (debug_fbsd_nat, "fbsd-nat", fmt, ##__VA_ARGS__)
 
-#define fbsd_nat_debug_start_end(fmt, ...) \
+#define fbsd_nat_debug_enter_exit() \
+  scoped_debug_enter_exit (debug_fbsd_nat, "fbsd-nat")
+
+#define fbsd_nat_debug_start_end(fmt, ...)				\
   scoped_debug_start_end (debug_fbsd_nat, "fbsd-nat", fmt, ##__VA_ARGS__)
 
 /*
@@ -1177,11 +1217,11 @@ fbsd_add_vfork_done (ptid_t pid)
 #endif
 #endif
 
-/* Resume a single process.  */
+/* Mark a single process for resumption.  */
 
 void
-fbsd_nat_target::resume_one_process (ptid_t ptid, int step,
-				     enum gdb_signal signo)
+fbsd_nat_target::record_resume (ptid_t ptid, int step,
+				enum gdb_signal signo)
 {
   fbsd_nat_debug_printf ("[%s], step %d, signo %d (%s)",
 			 target_pid_to_str (ptid).c_str (), step, signo,
@@ -1189,80 +1229,39 @@ fbsd_nat_target::resume_one_process (ptid_t ptid, int step,
 
   inferior *inf = find_inferior_ptid (this, ptid);
   fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
-  info->resumed_lwps = ptid;
   gdb_assert (info->running_lwps == 0);
 
-  /* Don't PT_CONTINUE a thread or process which has a pending event.  */
-  if (have_pending_event (ptid))
+  if (!info->resume_pending)
     {
-      fbsd_nat_debug_printf ("found pending event");
-      return;
+      info->stepping_lwp = -1;
+      info->resume_signal = GDB_SIGNAL_0;
+      info->resume_pending = true;
     }
 
-  for (thread_info *tp : inf->non_exited_threads ())
+  if (ptid.lwp_p ())
     {
-      int request;
-
-      /* If ptid is a specific LWP, suspend all other LWPs in the
-	 process, otherwise resume all LWPs in the process..  */
-      if (!ptid.lwp_p() || tp->ptid.lwp () == ptid.lwp ())
-	{
-	  if (ptrace (PT_RESUME, tp->ptid.lwp (), NULL, 0) == -1)
-	    perror_with_name (("ptrace (PT_RESUME)"));
-	  low_prepare_to_resume (tp);
-	  info->running_lwps++;
-	}
-      else
-	{
-	  if (ptrace (PT_SUSPEND, tp->ptid.lwp (), NULL, 0) == -1)
-	    perror_with_name (("ptrace (PT_SUSPEND)"));
-	}
-    }
-
-  if (ptid.pid () != inferior_ptid.pid ())
-    {
-      step = 0;
-      signo = GDB_SIGNAL_0;
-      gdb_assert (!ptid.lwp_p ());
+      gdb_assert (!info->is_lwp_resumed (ptid.lwp ()));
+      info->resumed_lwps.insert (ptid.lwp ());
     }
   else
     {
-      ptid = inferior_ptid;
-#if __FreeBSD_version < 1200052
-      /* When multiple threads within a process wish to report STOPPED
-	 events from wait(), the kernel picks one thread event as the
-	 thread event to report.  The chosen thread event is retrieved
-	 via PT_LWPINFO by passing the process ID as the request pid.
-	 If multiple events are pending, then the subsequent wait()
-	 after resuming a process will report another STOPPED event
-	 after resuming the process to handle the next thread event
-	 and so on.
+      gdb_assert (!info->is_process_resumed ());
+      info->resumed_all_lwps = true;
+    }
 
-	 A single thread event is cleared as a side effect of resuming
-	 the process with PT_CONTINUE, PT_STEP, etc.  In older
-	 kernels, however, the request pid was used to select which
-	 thread's event was cleared rather than always clearing the
-	 event that was just reported.  To avoid clearing the event of
-	 the wrong LWP, always pass the process ID instead of an LWP
-	 ID to PT_CONTINUE or PT_SYSCALL.
-
-	 In the case of stepping, the process ID cannot be used with
-	 PT_STEP since it would step the thread that reported an event
-	 which may not be the thread indicated by PTID.  For stepping,
-	 use PT_SETSTEP to enable stepping on the desired thread
-	 before resuming the process via PT_CONTINUE instead of using
-	 PT_STEP.  */
+  if (ptid.pid () == inferior_ptid.pid ())
+    {
       if (step)
 	{
-	  if (ptrace (PT_SETSTEP, get_ptrace_pid (ptid), NULL, 0) == -1)
-	    perror_with_name (("ptrace (PT_SETSTEP)"));
-	  step = 0;
+	  gdb_assert (info->stepping_lwp == -1);
+	  info->stepping_lwp = inferior_ptid.lwp ();
+	}
+      if (signo != GDB_SIGNAL_0)
+	{
+	  gdb_assert (info->resume_signal == GDB_SIGNAL_0);
+	  info->resume_signal = signo;
 	}
-      ptid = ptid_t (ptid.pid ());
-#endif
     }
-
-  inf_ptrace_target::resume (ptid, step, signo);
 }
 
 /* Implement the "resume" target_ops method.  */
@@ -1280,14 +1279,94 @@ fbsd_nat_target::resume (ptid_t scope_ptid, int step, enum gdb_signal signo)
   if (scope_ptid == minus_one_ptid)
     {
       for (inferior *inf : all_non_exited_inferiors (this))
-	resume_one_process (ptid_t (inf->pid), step, signo);
+	record_resume (ptid_t (inf->pid), step, signo);
     }
   else
     {
-      resume_one_process (scope_ptid, step, signo);
+      record_resume (scope_ptid, step, signo);
     }
 }
 
+/* Resume a single process.  */
+
+void
+fbsd_nat_target::resume_one_process (inferior *inf)
+{
+  fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+
+  if (!info->resume_pending)
+    return;
+  info->resume_pending = false;
+
+  ptid_t ptid (inf->pid);
+  fbsd_nat_debug_printf ("[%s], step LWP %d, signo %d (%s)",
+			 target_pid_to_str (ptid).c_str (),
+			 info->stepping_lwp, info->resume_signal,
+			 gdb_signal_to_name (info->resume_signal));
+
+  /* Enable stepping if requested.  Note that if a different resumed
+     thread in the process has a pending event the step might not
+     occur until a future resume.  */
+  if (info->stepping_lwp != -1)
+    {
+      gdb_assert (!have_pending_event (ptid_t (inf->pid, info->stepping_lwp)));
+      if (ptrace (PT_SETSTEP, info->stepping_lwp, NULL, 0) == -1)
+	perror_with_name (("ptrace (PT_SETSTEP)"));
+    }
+
+  /* Don't PT_CONTINUE a thread or process which has a pending event.  */
+  if (info->resumed_all_lwps)
+    {
+      if (have_pending_event (ptid))
+	{
+	  fbsd_nat_debug_printf ("found pending event");
+	  gdb_assert (info->resume_signal == GDB_SIGNAL_0);
+	  return;
+	}
+    }
+  else
+    for (lwpid_t lwp : info->resumed_lwps)
+      {
+	if (have_pending_event (ptid_t (inf->pid, lwp)))
+	  {
+	    fbsd_nat_debug_printf ("found pending event");
+	    gdb_assert (info->resume_signal == GDB_SIGNAL_0);
+	    return;
+	  }
+      }
+
+  gdb_assert (info->running_lwps == 0);
+
+  /* Suspend or resume individual threads.  */
+  for (thread_info *tp : inf->non_exited_threads ())
+    {
+      if (info->is_lwp_resumed (tp->ptid.lwp ()))
+	{
+	  if (ptrace (PT_RESUME, tp->ptid.lwp (), NULL, 0) == -1)
+	    perror_with_name (("ptrace (PT_RESUME)"));
+	  low_prepare_to_resume (tp);
+	  info->running_lwps++;
+	}
+      else
+	{
+	  if (ptrace (PT_SUSPEND, tp->ptid.lwp (), NULL, 0) == -1)
+	    perror_with_name (("ptrace (PT_SUSPEND)"));
+	}
+    }
+
+  inf_ptrace_target::resume (ptid, 0, info->resume_signal);
+}
+
+/* Implement the "commit_resumed" target_ops method.  */
+
+void
+fbsd_nat_target::commit_resumed ()
+{
+  fbsd_nat_debug_enter_exit ();
+  for (inferior *inf : all_non_exited_inferiors (this))
+    resume_one_process (inf);
+}
+
 #ifdef USE_SIGTRAP_SIGINFO
 /* Handle breakpoint and trace traps reported via SIGTRAP.  If the
    trap was a breakpoint or trace trap that should be reported to the
@@ -1397,18 +1476,23 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 		  delete_thread (thr);
 		  info->num_lwps--;
 
-		  /* If this LWP was the only resumed LWP from the
-		     process, report an event to the core.  */
-		  if (wptid == info->resumed_lwps)
+		  /* During process exit LWPs that were suspended will
+		     report exit events which should be quietly
+		     ignored.  Only update accounting for exit events
+		     on resumed LWPs. */
+		  if (info->is_lwp_resumed (pl.pl_lwpid))
 		    {
-		      ourstatus->set_spurious ();
-		      return wptid;
-		    }
+		      /* If this LWP was the only resumed LWP from the
+			 process still running, report an event to the
+			 core.  */
+		      if (info->running_lwps == 1)
+			{
+			  ourstatus->set_spurious ();
+			  return wptid;
+			}
 
-		  /* During process exit LWPs that were not resumed
-		     will report exit events.  */
-		  if (wptid.matches (info->resumed_lwps))
-		    info->running_lwps--;
+		      info->running_lwps--;
+		    }
 		}
 	      if (ptrace (PT_CONTINUE, pid, (caddr_t) 1, 0) == -1)
 		perror_with_name (("ptrace (PT_CONTINUE)"));
@@ -1443,7 +1527,7 @@ fbsd_nat_target::wait_1 (ptid_t ptid, struct target_waitstatus *ourstatus,
 		  add_thread (this, wptid);
 		  info->num_lwps++;
 
-		  if (wptid.matches(info->resumed_lwps))
+		  if (info->resumed_all_lwps)
 		    info->running_lwps++;
 		}
 	      ourstatus->set_spurious ();
@@ -1588,9 +1672,16 @@ fbsd_nat_target::stop_process (inferior *inf)
   fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
   gdb_assert (info != nullptr);
 
-  info->resumed_lwps = null_ptid;
   if (info->running_lwps == 0)
-    return;
+    {
+      /* If a resume is pending cancel it.  The process might be
+	 resumed even without any running LWPs if it had a pending
+	 event when it was resumed.  */
+      info->resume_pending = false;
+      info->mark_stopped ();
+      return;
+    }
+  gdb_assert (info->resume_pending == false);
 
   ptid_t ptid (inf->pid);
   target_waitstatus status;
@@ -1600,7 +1691,7 @@ fbsd_nat_target::stop_process (inferior *inf)
     {
       /* Save the current event as a pending event.  */
       add_pending_event (wptid, status);
-      info->running_lwps = 0;
+      info->mark_stopped ();
       return;
     }
 
@@ -1653,7 +1744,7 @@ fbsd_nat_target::stop_process (inferior *inf)
       info->pending_sigstop = true;
       break;
     }
-  info->running_lwps = 0;
+  info->mark_stopped ();
 }
 
 ptid_t
@@ -1663,6 +1754,15 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   fbsd_nat_debug_printf ("[%s], [%s]", target_pid_to_str (ptid).c_str (),
 			 target_options_to_string (target_options).c_str ());
 
+  /* Resume any pending processes with a pending signal even if we are
+     going to return a pending event so as to not lose the signal.  */
+  for (inferior *inf : all_non_exited_inferiors (this))
+    {
+      fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
+      if (info->resume_pending && info->resume_signal != GDB_SIGNAL_0)
+	resume_one_process (inf);
+    }
+
   /* If there is a valid pending event, return it.  */
   gdb::optional<pending_event> event = take_pending_event (ptid);
   if (event.has_value ())
@@ -1683,6 +1783,9 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
   if (is_async_p ())
     async_file_flush ();
 
+  /* Resume any pending processes.  */
+  commit_resumed ();
+
   ptid_t wptid;
   while (1)
     {
@@ -1697,14 +1800,14 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
       gdb_assert (inf != nullptr);
       fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
       gdb_assert (info != nullptr);
-      gdb_assert (info->resumed_lwps != null_ptid);
+      gdb_assert (info->is_process_resumed ());
       gdb_assert (info->running_lwps > 0);
 
       /* If an event is reported for a thread or process while
 	 stepping some other thread, suspend the thread reporting the
 	 event and defer the event until it can be reported to the
 	 core.  */
-      if (!wptid.matches (info->resumed_lwps))
+      if (!info->is_lwp_resumed (wptid.lwp ()))
 	{
 	  add_pending_event (wptid, *ourstatus);
 	  fbsd_nat_debug_printf ("deferring event [%s], [%s]",
@@ -1718,8 +1821,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 	}
 
       /* This process is no longer running.  */
-      info->resumed_lwps = null_ptid;
-      info->running_lwps = 0;
+      info->mark_stopped ();
 
       /* Stop any other inferiors currently running.  */
       for (inferior *inf : all_non_exited_inferiors (this))
@@ -1830,7 +1932,7 @@ fbsd_nat_target::create_inferior (const char *exec_file,
 #endif
 
   fbsd_inferior_info *info = fbsd_inferior_data.emplace (current_inferior ());
-  info->resumed_lwps = minus_one_ptid;
+  info->resumed_all_lwps = true;
   info->num_lwps = 1;
   info->running_lwps = 1;
   inf_ptrace_target::create_inferior (exec_file, allargs, env, from_tty);
@@ -1840,7 +1942,7 @@ void
 fbsd_nat_target::attach (const char *args, int from_tty)
 {
   fbsd_inferior_info *info = fbsd_inferior_data.emplace (current_inferior ());
-  info->resumed_lwps = minus_one_ptid;
+  info->resumed_all_lwps = true;
   info->num_lwps = 1;
   info->running_lwps = 1;
   inf_ptrace_target::attach (args, from_tty);
@@ -1889,12 +1991,12 @@ fbsd_nat_target::detach_fork_children (inferior *inf)
   for (thread_info *tp : inf->non_exited_threads ())
     detach_fork_children (tp);
 
-  /* Unwind state associated with any pending events.  Reset
-     info->resumed_lwps so that take_pending_event will harvest
+  /* Unwind state associated with any pending events.  Set
+     info->resumed_all_lwps so that take_pending_event will harvest
      events.  */
   fbsd_inferior_info *info = fbsd_inferior_data.get (inf);
   ptid_t ptid = ptid_t (inf->pid);
-  info->resumed_lwps = ptid;
+  info->resumed_all_lwps = true;
 
   while (1)
     {
@@ -2030,7 +2132,7 @@ fbsd_nat_target::detach (inferior *inf, int from_tty)
       info->pending_sigstop = false;
 
       /* Report event for all threads from wait_1.  */
-      info->resumed_lwps = ptid;
+      info->resumed_all_lwps = true;
 
       do
 	{
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 7016cc0242a..b149217b71b 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -89,6 +89,8 @@ class fbsd_nat_target : public inf_ptrace_target
 
   void resume (ptid_t, int, enum gdb_signal) override;
 
+  void commit_resumed () override;
+
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
 
   void post_attach (int) override;
@@ -143,7 +145,9 @@ class fbsd_nat_target : public inf_ptrace_target
 private:
   ptid_t wait_1 (ptid_t, struct target_waitstatus *, target_wait_flags);
 
-  void resume_one_process (ptid_t, int, enum gdb_signal);
+  void record_resume (ptid_t, int, enum gdb_signal);
+
+  void resume_one_process (inferior *);
 
   void stop_process (inferior *);
 
@@ -258,9 +262,9 @@ class fbsd_nat_target : public inf_ptrace_target
 
   bool have_pending_event (ptid_t filter);
 
-  /* Check if there is a pending event for a resumed process matching
-     FILTER.  If there is a matching event, the event is removed from
-     the pending list and returned.  */
+  /* Check if there is a pending event for a resumed thread or process
+     matching FILTER.  If there is a matching event, the event is
+     removed from the pending list and returned.  */
 
   gdb::optional<pending_event> take_pending_event (ptid_t filter);
 
-- 
2.40.0

[PATCH v2 8/8] fbsd-nat: Implement the target stop method.

[John Baldwin]

Stop each requested process via the stop_process helper function.
---
 gdb/fbsd-nat.c | 19 +++++++++++++++++++
 gdb/fbsd-nat.h |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index f114f88e649..91d0841fb33 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1367,6 +1367,25 @@ fbsd_nat_target::commit_resumed ()
     resume_one_process (inf);
 }
 
+/* Implement the "stop" target method.  */
+
+void
+fbsd_nat_target::stop (ptid_t ptid)
+{
+  fbsd_nat_debug_printf ("[%s]", target_pid_to_str (ptid).c_str ());
+  if (ptid == minus_one_ptid)
+    {
+      for (inferior *inf : all_non_exited_inferiors (this))
+	stop_process (inf);
+    }
+  else
+    {
+      inferior *inf = find_inferior_ptid (this, ptid);
+      gdb_assert (inf != nullptr);
+      stop_process (inf);
+    }
+}
+
 #ifdef USE_SIGTRAP_SIGINFO
 /* Handle breakpoint and trace traps reported via SIGTRAP.  If the
    trap was a breakpoint or trace trap that should be reported to the
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index b149217b71b..65d0954d36a 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -91,6 +91,8 @@ class fbsd_nat_target : public inf_ptrace_target
 
   void commit_resumed () override;
 
+  void stop (ptid_t) override;
+
   ptid_t wait (ptid_t, struct target_waitstatus *, target_wait_flags) override;
 
   void post_attach (int) override;
-- 
2.40.0

Re: [PATCH v2 3/8] fbsd-nat: Fix resuming and waiting with multiple processes.

[Tom Tromey]

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John> +/* Information stored about each inferior.  */
John> +struct fbsd_inferior_info
John> +{
John> +  /* Filter for resumed LWPs which can report events from wait.  */
John> +  ptid_t resumed_lwps = null_ptid;
John> +
John> +  /* Number of LWPs this process contains.  */
John> +  unsigned int num_lwps = 0;
John> +
John> +  /* Number of LWPs currently running.  */
John> +  unsigned int running_lwps = 0;
John> +
John> +  /* Have a pending SIGSTOP event that needs to be discarded.  */
John> +  bool pending_sigstop = false;
John> +};
John> +
John> +/* Per-inferior data key.  */
John> +
John> +static const registry<inferior>::key<fbsd_inferior_info> fbsd_inferior_data;

It isn't obvious from the comment (something we ought to fix), but I
think the inferior::priv field is reserved for the process stratum
target to use.  So, you don't need a registry key, you can just derive
fbsd_inferior_info from 'private_inferior'.

Tom

Re: [PATCH v2 0/8] Fixes for multiprocess for FreeBSD's native target

[Tom Tromey]

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John> While this series is not perfect, it does result in a net improvement
John> in testsuite results on FreeBSD/amd64 and is also a significantly
John> better experience debugging forking processes.  I'm less confident about
John> the last two patches but would like to get the first 6 patches into
John> GDB 14.

I skimmed through this, but I don't really know anything about FreeBSD.
Anyway I didn't see any real red flags, and I sent the one comment I
had.

John> - Patch 8 is new and adds an implementation of the stop target method.
John>   It wasn't clear to me if this is really needed for an all-stop-only
John>   target, but at least one test in the testsuite was calling this
John>   method when I made it gdb_assert() instead of being the default.

That is interesting.  I had thought it was only used for non-stop.
windows-nat doesn't implement it... but then again, I've never managed
to run the test suite on Windows.

Tom

Re: [PATCH v2 3/8] fbsd-nat: Fix resuming and waiting with multiple processes.

[John Baldwin]

On 8/4/23 9:30 AM, Tom Tromey wrote:
>>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
> 
> John> +/* Information stored about each inferior.  */
> John> +struct fbsd_inferior_info
> John> +{
> John> +  /* Filter for resumed LWPs which can report events from wait.  */
> John> +  ptid_t resumed_lwps = null_ptid;
> John> +
> John> +  /* Number of LWPs this process contains.  */
> John> +  unsigned int num_lwps = 0;
> John> +
> John> +  /* Number of LWPs currently running.  */
> John> +  unsigned int running_lwps = 0;
> John> +
> John> +  /* Have a pending SIGSTOP event that needs to be discarded.  */
> John> +  bool pending_sigstop = false;
> John> +};
> John> +
> John> +/* Per-inferior data key.  */
> John> +
> John> +static const registry<inferior>::key<fbsd_inferior_info> fbsd_inferior_data;
> 
> It isn't obvious from the comment (something we ought to fix), but I
> think the inferior::priv field is reserved for the process stratum
> target to use.  So, you don't need a registry key, you can just derive
> fbsd_inferior_info from 'private_inferior'.

Ah, thanks.  I've updated the series to do this.

-- 
John Baldwin

Re: [PATCH v2 0/8] Fixes for multiprocess for FreeBSD's native target

[John Baldwin]

On 8/4/23 9:45 AM, Tom Tromey wrote:
>>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
> 
> John> While this series is not perfect, it does result in a net improvement
> John> in testsuite results on FreeBSD/amd64 and is also a significantly
> John> better experience debugging forking processes.  I'm less confident about
> John> the last two patches but would like to get the first 6 patches into
> John> GDB 14.
> 
> I skimmed through this, but I don't really know anything about FreeBSD.
> Anyway I didn't see any real red flags, and I sent the one comment I
> had.

Ok, thanks.  I'm going to push the first six patches (which I'm more confident
of) with your suggestion applied (using inferior::priv_field).

-- 
John Baldwin