[PATCH] Add basic event handling in the NetBSD target

[PATCH] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

Implement the following events:
 - single step (TRAP_TRACE)
 - software breakpoint (TRAP_DBREG)
 - exec() (TRAP_EXEC)
 - syscall entry/exit (TRAP_SCE / TRAP_SCX)

Add support for NetBSD specific ::wait () and ::resume ().

Instruct the generic code that exec and syscall events are supported.

Define an empty nbsd_get_syscall_number as it is prerequisite for
catching syscall entry and exit events, even if it is unused.
This function is used to detect whether the gdbarch supports the
'catch syscall' feature.

gdb/ChangeLog:

       * nbsd-nat.c: Include "sys/wait.h".
       * (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)
       (nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
       (nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-tdep.c (nbsd_get_syscall_number): Add.
       (nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
       `nbsd_get_syscall_number'.
---
 gdb/ChangeLog   |  15 ++++
 gdb/nbsd-nat.c  | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-nat.h  |   9 +++
 gdb/nbsd-tdep.c |  18 +++++
 4 files changed, 253 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0caeca04e79..9652daabbbd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2020-04-16  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "sys/wait.h".
+	* (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)
+	(nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
+	(nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-tdep.c (nbsd_get_syscall_number): Add.
+	(nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
+	`nbsd_get_syscall_number'.
+
 2020-04-16  Kamil Rytarowski  <n54@gmx.com>

 	* inf-ptrace.h (follow_fork, insert_fork_catchpoint)
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index d41cfc815d3..f9e85e10b16 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/wait.h>

 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
@@ -539,3 +540,213 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)

   return true;
 }
+
+/* Resume execution of thread PTID, or all threads if PTID is -1.  If
+   STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
+   that signal.  */
+
+void
+nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
+{
+  int request;
+
+  if (ptid.lwp_p ())
+    {
+      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
+      inferior *inf = find_inferior_ptid (this, ptid);
+
+      for (thread_info *tp : inf->non_exited_threads ())
+        {
+          if (tp->ptid.lwp () == ptid.lwp ())
+            request = PT_RESUME;
+          else
+            request = PT_SUSPEND;
+
+          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+            perror_with_name (("ptrace"));
+        }
+    }
+   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+          perror_with_name (("ptrace"));
+      ptid = inferior_ptid;
+    }
+
+  if (step)
+    {
+      for (thread_info *tp : all_non_exited_threads (this, ptid))
+       if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+         perror_with_name (("ptrace"));
+     }
+  else
+    {
+      for (thread_info *tp : all_non_exited_threads (this, ptid))
+       if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+         perror_with_name (("ptrace"));
+    }
+
+  if (minus_one_ptid == ptid)
+    /* Resume all threads.  Traditionally ptrace() only supports
+       single-threaded processes, so simply resume the inferior.  */
+    ptid = ptid_t (inferior_ptid.pid ());
+
+  if (catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
+  /* An address of (void *)1 tells ptrace to continue from
+     where it was.  If GDB wanted it to start some other way, we have
+     already written a new program counter value to the child.  */
+  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
+    perror_with_name (("ptrace"));
+}
+
+/* Implement the "update_thread_list" target_ops method.  */
+
+static ptid_t
+nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  pid_t pid;
+  int status;
+
+  set_sigint_trap ();
+
+  do
+    {
+      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
+      pid = waitpid (ptid.pid (), &status, 0);
+    }
+  while (pid == -1 && errno == EINTR);
+
+  clear_sigint_trap ();
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  store_waitstatus (ourstatus, status);
+  return ptid_t (pid);
+}
+
+/* Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
+
+ptid_t
+nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+		       int target_options)
+{
+  ptid_t wptid = nbsd_wait(ptid, ourstatus, target_options);
+
+  /* If the child stopped, keep investigating its status.  */
+  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
+    return wptid;
+
+  pid_t pid = wptid.pid ();
+
+  /* Extract the event and thread that received a signal.  */
+  ptrace_siginfo_t psi;
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  /* Pick child's siginfo_t.  */
+  siginfo_t *si = &psi.psi_siginfo;
+
+  int lwp = psi.psi_lwpid;
+
+  int signo = si->si_signo;
+  const int code = si->si_code;
+
+  /* Construct PTID with a specified thread that received the event.
+     If a signal was targeted to the whole process, lwp is 0.  */
+  wptid = ptid_t (pid, lwp, 0);
+
+  /* Bail out on non-debugger oriented signals..  */
+  if (signo != SIGTRAP)
+    return wptid;
+
+  /* Stop examining non-debugger oriented SIGTRAP codes.  */
+  if (code <= SI_USER || code == SI_NOINFO)
+    return wptid;
+
+  if (in_thread_list (this, ptid_t (pid)))
+    {
+      thread_change_ptid (this, ptid_t (pid), wptid);
+    }
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname = xstrdup (pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!catch_syscall_enabled () || !catching_syscall_number (sysnum))
+	{
+	  /* If the core isn't interested in this event, ignore it.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  return wptid;
+	}
+
+      ourstatus->kind =
+	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
+	TARGET_WAITKIND_SYSCALL_RETURN;
+      ourstatus->value.syscall_number = sysnum;
+      return wptid;
+    }
+
+  if (code == TRAP_BRKPT)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the "insert_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::insert_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "remove_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::remove_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "set_syscall_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::set_syscall_catchpoint (int pid, bool needed,
+                                         int any_count,
+                                         gdb::array_view<const int> syscall_counts)
+{
+  /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
+     will catch all system call entries and exits.  The system calls
+     are filtered by GDB rather than the kernel.  */
+  return 0;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 256db4b9017..6e14cbb889d 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -38,6 +38,15 @@ struct nbsd_nat_target : public inf_ptrace_target

   int find_memory_regions (find_memory_region_ftype func, void *data) override;
   bool info_proc (const char *, enum info_proc_what) override;
+
+  void resume (ptid_t, int, enum gdb_signal) override;
+  ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
+  int insert_exec_catchpoint (int pid) override;
+  int remove_exec_catchpoint (int pid) override;
+  int set_syscall_catchpoint (int pid, bool needed, int any_count,
+			      gdb::array_view<const int> syscall_counts)
+    override;
+
 };

 #endif /* nbsd-nat.h */
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 52e0640e35c..d5d1b7211c1 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
     }
 }

+/* Implement the "get_syscall_number" gdbarch method.  */
+
+static LONGEST
+nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
+{
+
+  /* FreeBSD doesn't use gdbarch_get_syscall_number since NetBSD
+     native targets fetch the system call number from the
+     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
+     However, system call catching requires this function to be
+     set.  */
+
+  internal_error (__FILE__, __LINE__, _("nbsd_get_sycall_number called"));
+}
+
 /* See nbsd-tdep.h.  */

 void
@@ -453,4 +468,7 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
   set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
+
+  /* `catch syscall' */
+  set_gdbarch_get_syscall_number (gdbarch, nbsd_get_syscall_number);
 }
--
2.25.0

Re: [PATCH] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 11718 bytes --]

Ping?

On 17.04.2020 16:45, Kamil Rytarowski wrote:
> Implement the following events:
>  - single step (TRAP_TRACE)
>  - software breakpoint (TRAP_DBREG)
>  - exec() (TRAP_EXEC)
>  - syscall entry/exit (TRAP_SCE / TRAP_SCX)
> 
> Add support for NetBSD specific ::wait () and ::resume ().
> 
> Instruct the generic code that exec and syscall events are supported.
> 
> Define an empty nbsd_get_syscall_number as it is prerequisite for
> catching syscall entry and exit events, even if it is unused.
> This function is used to detect whether the gdbarch supports the
> 'catch syscall' feature.
> 
> gdb/ChangeLog:
> 
>        * nbsd-nat.c: Include "sys/wait.h".
>        * (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)
>        (nbsd_nat_target::insert_exec_catchpoint)
>        (nbsd_nat_target::remove_exec_catchpoint)
>        (nbsd_nat_target::set_syscall_catchpoint): Add.
>        * nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
>        (nbsd_nat_target::insert_exec_catchpoint)
>        (nbsd_nat_target::remove_exec_catchpoint)
>        (nbsd_nat_target::set_syscall_catchpoint): Add.
>        * nbsd-tdep.c (nbsd_get_syscall_number): Add.
>        (nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
>        `nbsd_get_syscall_number'.
> ---
>  gdb/ChangeLog   |  15 ++++
>  gdb/nbsd-nat.c  | 211 ++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/nbsd-nat.h  |   9 +++
>  gdb/nbsd-tdep.c |  18 +++++
>  4 files changed, 253 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 0caeca04e79..9652daabbbd 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,18 @@
> +2020-04-16  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* nbsd-nat.c: Include "sys/wait.h".
> +	* (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)
> +	(nbsd_nat_target::insert_exec_catchpoint)
> +	(nbsd_nat_target::remove_exec_catchpoint)
> +	(nbsd_nat_target::set_syscall_catchpoint): Add.
> +	* nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
> +	(nbsd_nat_target::insert_exec_catchpoint)
> +	(nbsd_nat_target::remove_exec_catchpoint)
> +	(nbsd_nat_target::set_syscall_catchpoint): Add.
> +	* nbsd-tdep.c (nbsd_get_syscall_number): Add.
> +	(nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
> +	`nbsd_get_syscall_number'.
> +
>  2020-04-16  Kamil Rytarowski  <n54@gmx.com>
> 
>  	* inf-ptrace.h (follow_fork, insert_fork_catchpoint)
> diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
> index d41cfc815d3..f9e85e10b16 100644
> --- a/gdb/nbsd-nat.c
> +++ b/gdb/nbsd-nat.c
> @@ -28,6 +28,7 @@
>  #include <sys/types.h>
>  #include <sys/ptrace.h>
>  #include <sys/sysctl.h>
> +#include <sys/wait.h>
> 
>  /* Return the name of a file that can be opened to get the symbols for
>     the child process identified by PID.  */
> @@ -539,3 +540,213 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
> 
>    return true;
>  }
> +
> +/* Resume execution of thread PTID, or all threads if PTID is -1.  If
> +   STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
> +   that signal.  */
> +
> +void
> +nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
> +{
> +  int request;
> +
> +  if (ptid.lwp_p ())
> +    {
> +      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
> +      inferior *inf = find_inferior_ptid (this, ptid);
> +
> +      for (thread_info *tp : inf->non_exited_threads ())
> +        {
> +          if (tp->ptid.lwp () == ptid.lwp ())
> +            request = PT_RESUME;
> +          else
> +            request = PT_SUSPEND;
> +
> +          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +            perror_with_name (("ptrace"));
> +        }
> +    }
> +   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +          perror_with_name (("ptrace"));
> +      ptid = inferior_ptid;
> +    }
> +
> +  if (step)
> +    {
> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
> +       if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +         perror_with_name (("ptrace"));
> +     }
> +  else
> +    {
> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
> +       if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +         perror_with_name (("ptrace"));
> +    }
> +
> +  if (minus_one_ptid == ptid)
> +    /* Resume all threads.  Traditionally ptrace() only supports
> +       single-threaded processes, so simply resume the inferior.  */
> +    ptid = ptid_t (inferior_ptid.pid ());
> +
> +  if (catch_syscall_enabled () > 0)
> +    request = PT_SYSCALL;
> +  else
> +    request = PT_CONTINUE;
> +
> +  /* An address of (void *)1 tells ptrace to continue from
> +     where it was.  If GDB wanted it to start some other way, we have
> +     already written a new program counter value to the child.  */
> +  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
> +    perror_with_name (("ptrace"));
> +}
> +
> +/* Implement the "update_thread_list" target_ops method.  */
> +
> +static ptid_t
> +nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
> +{
> +  pid_t pid;
> +  int status;
> +
> +  set_sigint_trap ();
> +
> +  do
> +    {
> +      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
> +      pid = waitpid (ptid.pid (), &status, 0);
> +    }
> +  while (pid == -1 && errno == EINTR);
> +
> +  clear_sigint_trap ();
> +
> +  if (pid == -1)
> +    perror_with_name (_("Child process unexpectedly missing"));
> +
> +  store_waitstatus (ourstatus, status);
> +  return ptid_t (pid);
> +}
> +
> +/* Wait for the child specified by PTID to do something.  Return the
> +   process ID of the child, or MINUS_ONE_PTID in case of error; store
> +   the status in *OURSTATUS.  */
> +
> +ptid_t
> +nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
> +		       int target_options)
> +{
> +  ptid_t wptid = nbsd_wait(ptid, ourstatus, target_options);
> +
> +  /* If the child stopped, keep investigating its status.  */
> +  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
> +    return wptid;
> +
> +  pid_t pid = wptid.pid ();
> +
> +  /* Extract the event and thread that received a signal.  */
> +  ptrace_siginfo_t psi;
> +  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
> +    perror_with_name (("ptrace"));
> +
> +  /* Pick child's siginfo_t.  */
> +  siginfo_t *si = &psi.psi_siginfo;
> +
> +  int lwp = psi.psi_lwpid;
> +
> +  int signo = si->si_signo;
> +  const int code = si->si_code;
> +
> +  /* Construct PTID with a specified thread that received the event.
> +     If a signal was targeted to the whole process, lwp is 0.  */
> +  wptid = ptid_t (pid, lwp, 0);
> +
> +  /* Bail out on non-debugger oriented signals..  */
> +  if (signo != SIGTRAP)
> +    return wptid;
> +
> +  /* Stop examining non-debugger oriented SIGTRAP codes.  */
> +  if (code <= SI_USER || code == SI_NOINFO)
> +    return wptid;
> +
> +  if (in_thread_list (this, ptid_t (pid)))
> +    {
> +      thread_change_ptid (this, ptid_t (pid), wptid);
> +    }
> +
> +  if (code == TRAP_EXEC)
> +    {
> +      ourstatus->kind = TARGET_WAITKIND_EXECD;
> +      ourstatus->value.execd_pathname = xstrdup (pid_to_exec_file (pid));
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_TRACE)
> +    {
> +      /* Unhandled at this level.  */
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_SCE || code == TRAP_SCX)
> +    {
> +      int sysnum = si->si_sysnum;
> +
> +      if (!catch_syscall_enabled () || !catching_syscall_number (sysnum))
> +	{
> +	  /* If the core isn't interested in this event, ignore it.  */
> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +	  return wptid;
> +	}
> +
> +      ourstatus->kind =
> +	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
> +	TARGET_WAITKIND_SYSCALL_RETURN;
> +      ourstatus->value.syscall_number = sysnum;
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_BRKPT)
> +    {
> +      /* Unhandled at this level.  */
> +      return wptid;
> +    }
> +
> +  /* Unclassified SIGTRAP event.  */
> +  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +  return wptid;
> +}
> +
> +/* Implement the "insert_exec_catchpoint" target_ops method.  */
> +
> +int
> +nbsd_nat_target::insert_exec_catchpoint (int pid)
> +{
> +  /* Nothing to do.  */
> +  return 0;
> +}
> +
> +/* Implement the "remove_exec_catchpoint" target_ops method.  */
> +
> +int
> +nbsd_nat_target::remove_exec_catchpoint (int pid)
> +{
> +  /* Nothing to do.  */
> +  return 0;
> +}
> +
> +/* Implement the "set_syscall_catchpoint" target_ops method.  */
> +
> +int
> +nbsd_nat_target::set_syscall_catchpoint (int pid, bool needed,
> +                                         int any_count,
> +                                         gdb::array_view<const int> syscall_counts)
> +{
> +  /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
> +     will catch all system call entries and exits.  The system calls
> +     are filtered by GDB rather than the kernel.  */
> +  return 0;
> +}
> diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
> index 256db4b9017..6e14cbb889d 100644
> --- a/gdb/nbsd-nat.h
> +++ b/gdb/nbsd-nat.h
> @@ -38,6 +38,15 @@ struct nbsd_nat_target : public inf_ptrace_target
> 
>    int find_memory_regions (find_memory_region_ftype func, void *data) override;
>    bool info_proc (const char *, enum info_proc_what) override;
> +
> +  void resume (ptid_t, int, enum gdb_signal) override;
> +  ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
> +  int insert_exec_catchpoint (int pid) override;
> +  int remove_exec_catchpoint (int pid) override;
> +  int set_syscall_catchpoint (int pid, bool needed, int any_count,
> +			      gdb::array_view<const int> syscall_counts)
> +    override;
> +
>  };
> 
>  #endif /* nbsd-nat.h */
> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
> index 52e0640e35c..d5d1b7211c1 100644
> --- a/gdb/nbsd-tdep.c
> +++ b/gdb/nbsd-tdep.c
> @@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
>      }
>  }
> 
> +/* Implement the "get_syscall_number" gdbarch method.  */
> +
> +static LONGEST
> +nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
> +{
> +
> +  /* FreeBSD doesn't use gdbarch_get_syscall_number since NetBSD
> +     native targets fetch the system call number from the
> +     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
> +     However, system call catching requires this function to be
> +     set.  */
> +
> +  internal_error (__FILE__, __LINE__, _("nbsd_get_sycall_number called"));
> +}
> +
>  /* See nbsd-tdep.h.  */
> 
>  void
> @@ -453,4 +468,7 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
>    set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
>    set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
> +
> +  /* `catch syscall' */
> +  set_gdbarch_get_syscall_number (gdbarch, nbsd_get_syscall_number);
>  }
> --
> 2.25.0
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Add basic event handling in the NetBSD target

[Simon Marchi]

Hi Kamil,

Check the indentation throughout the patch, some lines have 1 space indent instead of 2.

On 2020-04-17 10:45 a.m., Kamil Rytarowski wrote:
> Implement the following events:
>  - single step (TRAP_TRACE)
>  - software breakpoint (TRAP_DBREG)
>  - exec() (TRAP_EXEC)
>  - syscall entry/exit (TRAP_SCE / TRAP_SCX)
> 
> Add support for NetBSD specific ::wait () and ::resume ().
> 
> Instruct the generic code that exec and syscall events are supported.
> 
> Define an empty nbsd_get_syscall_number as it is prerequisite for
> catching syscall entry and exit events, even if it is unused.
> This function is used to detect whether the gdbarch supports the
> 'catch syscall' feature.
> 
> gdb/ChangeLog:
> 
>        * nbsd-nat.c: Include "sys/wait.h".
>        * (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)

Remove asterisk on last line.

> diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
> index d41cfc815d3..f9e85e10b16 100644
> --- a/gdb/nbsd-nat.c
> +++ b/gdb/nbsd-nat.c
> @@ -28,6 +28,7 @@
>  #include <sys/types.h>
>  #include <sys/ptrace.h>
>  #include <sys/sysctl.h>
> +#include <sys/wait.h>
> 
>  /* Return the name of a file that can be opened to get the symbols for
>     the child process identified by PID.  */
> @@ -539,3 +540,213 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
> 
>    return true;
>  }
> +
> +/* Resume execution of thread PTID, or all threads if PTID is -1.  If
> +   STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
> +   that signal.  */
> +
> +void
> +nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
> +{
> +  int request;
> +
> +  if (ptid.lwp_p ())
> +    {
> +      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
> +      inferior *inf = find_inferior_ptid (this, ptid);
> +
> +      for (thread_info *tp : inf->non_exited_threads ())
> +        {
> +          if (tp->ptid.lwp () == ptid.lwp ())
> +            request = PT_RESUME;
> +          else
> +            request = PT_SUSPEND;
> +
> +          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +            perror_with_name (("ptrace"));
> +        }

I know that you'll say it's because FreeBSD does it this way, but still it surprised
me.  If the core of GDB asks to resume one specific thread, it doesn't mean you should
suspend the other threads, they should just be left in whatever state they are.  Is there
a reason to stop the other threads here?

> +    }
> +   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +          perror_with_name (("ptrace"));
> +      ptid = inferior_ptid;

Can you explain this?  Ideally the resume method should not rely on inferior_ptid.  We
are working on reducing the dependencies on this global, in favor of passing the context
by parameters.  Here, the `ptid` parameter should be enough.

> +    }
> +
> +  if (step)
> +    {
> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
> +       if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +         perror_with_name (("ptrace"));
> +     }
> +  else
> +    {
> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
> +       if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> +         perror_with_name (("ptrace"));
> +    }
> +
> +  if (minus_one_ptid == ptid)
> +    /* Resume all threads.  Traditionally ptrace() only supports
> +       single-threaded processes, so simply resume the inferior.  */
> +    ptid = ptid_t (inferior_ptid.pid ());
> +
> +  if (catch_syscall_enabled () > 0)
> +    request = PT_SYSCALL;
> +  else
> +    request = PT_CONTINUE;
> +
> +  /* An address of (void *)1 tells ptrace to continue from
> +     where it was.  If GDB wanted it to start some other way, we have
> +     already written a new program counter value to the child.  */
> +  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
> +    perror_with_name (("ptrace"));
> +}
> +
> +/* Implement the "update_thread_list" target_ops method.  */
> +
> +static ptid_t
> +nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)

The comment doesn't match the function.

Since this function only returns a pid, I think it would be clearer if it returned
an int or pid_t.  At first glance, I didn't see that it returned a ptid_t with only
the pid field filled.  If the caller needs to transform it into a ptid, it can do that.

> +{
> +  pid_t pid;
> +  int status;
> +
> +  set_sigint_trap ();
> +
> +  do
> +    {
> +      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
> +      pid = waitpid (ptid.pid (), &status, 0);

What is crashing exactly?

> +    }
> +  while (pid == -1 && errno == EINTR);
> +
> +  clear_sigint_trap ();
> +
> +  if (pid == -1)
> +    perror_with_name (_("Child process unexpectedly missing"));
> +
> +  store_waitstatus (ourstatus, status);
> +  return ptid_t (pid);
> +}
> +
> +/* Wait for the child specified by PTID to do something.  Return the
> +   process ID of the child, or MINUS_ONE_PTID in case of error; store
> +   the status in *OURSTATUS.  */
> +
> +ptid_t
> +nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
> +		       int target_options)
> +{
> +  ptid_t wptid = nbsd_wait(ptid, ourstatus, target_options);

Missing space.

> +
> +  /* If the child stopped, keep investigating its status.  */
> +  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
> +    return wptid;
> +
> +  pid_t pid = wptid.pid ();
> +
> +  /* Extract the event and thread that received a signal.  */
> +  ptrace_siginfo_t psi;
> +  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
> +    perror_with_name (("ptrace"));
> +
> +  /* Pick child's siginfo_t.  */
> +  siginfo_t *si = &psi.psi_siginfo;
> +
> +  int lwp = psi.psi_lwpid;
> +
> +  int signo = si->si_signo;
> +  const int code = si->si_code;
> +
> +  /* Construct PTID with a specified thread that received the event.
> +     If a signal was targeted to the whole process, lwp is 0.  */
> +  wptid = ptid_t (pid, lwp, 0);
> +
> +  /* Bail out on non-debugger oriented signals..  */
> +  if (signo != SIGTRAP)
> +    return wptid;
> +
> +  /* Stop examining non-debugger oriented SIGTRAP codes.  */
> +  if (code <= SI_USER || code == SI_NOINFO)
> +    return wptid;
> +
> +  if (in_thread_list (this, ptid_t (pid)))
> +    {
> +      thread_change_ptid (this, ptid_t (pid), wptid);
> +    }

Curly braces not needed.

> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
> index 52e0640e35c..d5d1b7211c1 100644
> --- a/gdb/nbsd-tdep.c
> +++ b/gdb/nbsd-tdep.c
> @@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
>      }
>  }
> 
> +/* Implement the "get_syscall_number" gdbarch method.  */
> +
> +static LONGEST
> +nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
> +{
> +
> +  /* FreeBSD doesn't use gdbarch_get_syscall_number since NetBSD
> +     native targets fetch the system call number from the
> +     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
> +     However, system call catching requires this function to be
> +     set.  */

FreeBSD?

Simon

Re: [PATCH] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 10113 bytes --]

On 24.04.2020 03:33, Simon Marchi wrote:
> Hi Kamil,
> 
> Check the indentation throughout the patch, some lines have 1 space indent instead of 2.
> 

OK.

> On 2020-04-17 10:45 a.m., Kamil Rytarowski wrote:
>> Implement the following events:
>>  - single step (TRAP_TRACE)
>>  - software breakpoint (TRAP_DBREG)
>>  - exec() (TRAP_EXEC)
>>  - syscall entry/exit (TRAP_SCE / TRAP_SCX)
>>
>> Add support for NetBSD specific ::wait () and ::resume ().
>>
>> Instruct the generic code that exec and syscall events are supported.
>>
>> Define an empty nbsd_get_syscall_number as it is prerequisite for
>> catching syscall entry and exit events, even if it is unused.
>> This function is used to detect whether the gdbarch supports the
>> 'catch syscall' feature.
>>
>> gdb/ChangeLog:
>>
>>        * nbsd-nat.c: Include "sys/wait.h".
>>        * (nbsd_nat_target::resume, nbsd_wait, nbsd_nat_target::wait)
> 
> Remove asterisk on last line.
> 

OK.

>> diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
>> index d41cfc815d3..f9e85e10b16 100644
>> --- a/gdb/nbsd-nat.c
>> +++ b/gdb/nbsd-nat.c
>> @@ -28,6 +28,7 @@
>>  #include <sys/types.h>
>>  #include <sys/ptrace.h>
>>  #include <sys/sysctl.h>
>> +#include <sys/wait.h>
>>
>>  /* Return the name of a file that can be opened to get the symbols for
>>     the child process identified by PID.  */
>> @@ -539,3 +540,213 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
>>
>>    return true;
>>  }
>> +
>> +/* Resume execution of thread PTID, or all threads if PTID is -1.  If
>> +   STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
>> +   that signal.  */
>> +
>> +void
>> +nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
>> +{
>> +  int request;
>> +
>> +  if (ptid.lwp_p ())
>> +    {
>> +      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
>> +      inferior *inf = find_inferior_ptid (this, ptid);
>> +
>> +      for (thread_info *tp : inf->non_exited_threads ())
>> +        {
>> +          if (tp->ptid.lwp () == ptid.lwp ())
>> +            request = PT_RESUME;
>> +          else
>> +            request = PT_SUSPEND;
>> +
>> +          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
>> +            perror_with_name (("ptrace"));
>> +        }
> 
> I know that you'll say it's because FreeBSD does it this way, but still it surprised
> me.  If the core of GDB asks to resume one specific thread, it doesn't mean you should
> suspend the other threads, they should just be left in whatever state they are.  Is there
> a reason to stop the other threads here?
> 

In Linux there can be non-stop mode and threads are managed (stopped,
started, etc) individually. On NetBSD we can start and stop the whole
process, regardless of the number of threads in it.

We need to set the state of all threads before PT_CONTINUE (or assume
unchanged state of them).

If a thread is not expected to be suspended, we shall set its flag to
resume. The same for single-step (PT_STEP). This approach simplifies the
implementation as I don't need to track internally which thread has what
status and merely save a few syscalls.

>> +    }
>> +   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
>> +          perror_with_name (("ptrace"));
>> +      ptid = inferior_ptid;
> 
> Can you explain this?  Ideally the resume method should not rely on inferior_ptid.  We
> are working on reducing the dependencies on this global, in favor of passing the context
> by parameters.  Here, the `ptid` parameter should be enough.
> 

The caller can pass minus_one_ptid, which makes no sense for NetBSD and
certainly in the multi-target support.

Scenarios, as I can see them on NetBSD:

 - passing ptid_t (pid, 0, 0) -> resume the whole process with all the
threads
 - passing ptid_t (pid, lwp, 0) -> resume pid::lwp with all the other
threads suspended in pid (`set scheduler-lock')
 - passing ptid_t (-1, ?, ?) -> confusion to me so I try to find any
fallback

If we can abandon the -1 case, I can drop the inferior_ptid usage. If
the core no longer passes -1 (it could be), we shall cleanup existing
code that handles this as it could happen. If a refactoring can happen,
this shall be followed by addition of gdb_assert(ptid != minus_one_ptid).

>> +    }
>> +
>> +  if (step)
>> +    {
>> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
>> +       if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
>> +         perror_with_name (("ptrace"));
>> +     }
>> +  else
>> +    {
>> +      for (thread_info *tp : all_non_exited_threads (this, ptid))
>> +       if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
>> +         perror_with_name (("ptrace"));
>> +    }
>> +
>> +  if (minus_one_ptid == ptid)
>> +    /* Resume all threads.  Traditionally ptrace() only supports
>> +       single-threaded processes, so simply resume the inferior.  */
>> +    ptid = ptid_t (inferior_ptid.pid ());
>> +
>> +  if (catch_syscall_enabled () > 0)
>> +    request = PT_SYSCALL;
>> +  else
>> +    request = PT_CONTINUE;
>> +
>> +  /* An address of (void *)1 tells ptrace to continue from
>> +     where it was.  If GDB wanted it to start some other way, we have
>> +     already written a new program counter value to the child.  */
>> +  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
>> +    perror_with_name (("ptrace"));
>> +}
>> +
>> +/* Implement the "update_thread_list" target_ops method.  */
>> +
>> +static ptid_t
>> +nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
> 
> The comment doesn't match the function.
> 

I will fix.

> Since this function only returns a pid, I think it would be clearer if it returned
> an int or pid_t.  At first glance, I didn't see that it returned a ptid_t with only
> the pid field filled.  If the caller needs to transform it into a ptid, it can do that.
> 
>> +{
>> +  pid_t pid;
>> +  int status;
>> +
>> +  set_sigint_trap ();
>> +
>> +  do
>> +    {
>> +      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
>> +      pid = waitpid (ptid.pid (), &status, 0);
> 
> What is crashing exactly?
> 

The core code asks to waitpid() with WNOHANG, receives pid=0 (nobody)
and crashes.

It's a generic bug that shall be fixed... but it looks like nobody
really obeys the core and the 3rd argument is overwritten (compare:
rs6000-nat.c, obsd-nat.c, inf-ptrace.c or Linux...).

I've landed into more similar generic nuances during my effort on GDB,
that handling events promptly can crash the core and I need to change
the behavior to emulate other kernels.

It would be better to fix the core, but it is not NetBSD's fault and not
a blocker for NetBSD support.

If there is interest in fixing it, I can file a bug report
independently. I don't pledge to work on it myself (at least as long as
I can merely focus on one kernel support).

>> +    }
>> +  while (pid == -1 && errno == EINTR);
>> +
>> +  clear_sigint_trap ();
>> +
>> +  if (pid == -1)
>> +    perror_with_name (_("Child process unexpectedly missing"));
>> +
>> +  store_waitstatus (ourstatus, status);
>> +  return ptid_t (pid);
>> +}
>> +
>> +/* Wait for the child specified by PTID to do something.  Return the
>> +   process ID of the child, or MINUS_ONE_PTID in case of error; store
>> +   the status in *OURSTATUS.  */
>> +
>> +ptid_t
>> +nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>> +		       int target_options)
>> +{
>> +  ptid_t wptid = nbsd_wait(ptid, ourstatus, target_options);
> 
> Missing space.
> 

OK.

>> +
>> +  /* If the child stopped, keep investigating its status.  */
>> +  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
>> +    return wptid;
>> +
>> +  pid_t pid = wptid.pid ();
>> +
>> +  /* Extract the event and thread that received a signal.  */
>> +  ptrace_siginfo_t psi;
>> +  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
>> +    perror_with_name (("ptrace"));
>> +
>> +  /* Pick child's siginfo_t.  */
>> +  siginfo_t *si = &psi.psi_siginfo;
>> +
>> +  int lwp = psi.psi_lwpid;
>> +
>> +  int signo = si->si_signo;
>> +  const int code = si->si_code;
>> +
>> +  /* Construct PTID with a specified thread that received the event.
>> +     If a signal was targeted to the whole process, lwp is 0.  */
>> +  wptid = ptid_t (pid, lwp, 0);
>> +
>> +  /* Bail out on non-debugger oriented signals..  */
>> +  if (signo != SIGTRAP)
>> +    return wptid;
>> +
>> +  /* Stop examining non-debugger oriented SIGTRAP codes.  */
>> +  if (code <= SI_USER || code == SI_NOINFO)
>> +    return wptid;
>> +
>> +  if (in_thread_list (this, ptid_t (pid)))
>> +    {
>> +      thread_change_ptid (this, ptid_t (pid), wptid);
>> +    }
> 
> Curly braces not needed.
> 

OK.

>> diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
>> index 52e0640e35c..d5d1b7211c1 100644
>> --- a/gdb/nbsd-tdep.c
>> +++ b/gdb/nbsd-tdep.c
>> @@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
>>      }
>>  }
>>
>> +/* Implement the "get_syscall_number" gdbarch method.  */
>> +
>> +static LONGEST
>> +nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
>> +{
>> +
>> +  /* FreeBSD doesn't use gdbarch_get_syscall_number since NetBSD
>> +     native targets fetch the system call number from the
>> +     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
>> +     However, system call catching requires this function to be
>> +     set.  */
> 
> FreeBSD?
> 

I will fix it.

> Simon
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Add basic event handling in the NetBSD target

[Simon Marchi]

On 2020-04-24 6:09 a.m., Kamil Rytarowski wrote:
>> I know that you'll say it's because FreeBSD does it this way, but still it surprised
>> me.  If the core of GDB asks to resume one specific thread, it doesn't mean you should
>> suspend the other threads, they should just be left in whatever state they are.  Is there
>> a reason to stop the other threads here?
>>
> 
> In Linux there can be non-stop mode and threads are managed (stopped,
> started, etc) individually. On NetBSD we can start and stop the whole
> process, regardless of the number of threads in it.
> 
> We need to set the state of all threads before PT_CONTINUE (or assume
> unchanged state of them).
> 
> If a thread is not expected to be suspended, we shall set its flag to
> resume. The same for single-step (PT_STEP). This approach simplifies the
> implementation as I don't need to track internally which thread has what
> status and merely save a few syscalls.

Ok.

>>> +    }
>>> +   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
>>> +          perror_with_name (("ptrace"));
>>> +      ptid = inferior_ptid;
>>
>> Can you explain this?  Ideally the resume method should not rely on inferior_ptid.  We
>> are working on reducing the dependencies on this global, in favor of passing the context
>> by parameters.  Here, the `ptid` parameter should be enough.
>>
> 
> The caller can pass minus_one_ptid, which makes no sense for NetBSD and
> certainly in the multi-target support.
> 
> Scenarios, as I can see them on NetBSD:
> 
>  - passing ptid_t (pid, 0, 0) -> resume the whole process with all the
> threads
>  - passing ptid_t (pid, lwp, 0) -> resume pid::lwp with all the other
> threads suspended in pid (`set scheduler-lock')
>  - passing ptid_t (-1, ?, ?) -> confusion to me so I try to find any
> fallback
> 
> If we can abandon the -1 case, I can drop the inferior_ptid usage. If
> the core no longer passes -1 (it could be), we shall cleanup existing
> code that handles this as it could happen. If a refactoring can happen,
> this shall be followed by addition of gdb_assert(ptid != minus_one_ptid).

Or a 4th one: resume all threads of all processes that this target manages.

I don't know off-hand if the core of GDB today can pass minus_one_ptid.  You could
add such an assert, run all your tests, and if everything works, conclude it's not
needed.  If the assert is hit, you'll know why.

>>> +      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
>>> +      pid = waitpid (ptid.pid (), &status, 0);
>>
>> What is crashing exactly?
>>
> 
> The core code asks to waitpid() with WNOHANG, receives pid=0 (nobody)
> and crashes.
> 
> It's a generic bug that shall be fixed... but it looks like nobody
> really obeys the core and the 3rd argument is overwritten (compare:
> rs6000-nat.c, obsd-nat.c, inf-ptrace.c or Linux...).
> 
> I've landed into more similar generic nuances during my effort on GDB,
> that handling events promptly can crash the core and I need to change
> the behavior to emulate other kernels.
> 
> It would be better to fix the core, but it is not NetBSD's fault and not
> a blocker for NetBSD support.
> 
> If there is interest in fixing it, I can file a bug report
> independently. I don't pledge to work on it myself (at least as long as
> I can merely focus on one kernel support).

Ok, I'm fine with doing this and documenting it somewhere.

Simon

Re: [PATCH] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

> Sent: Sunday, April 26, 2020 at 11:14 PM
> From: "Simon Marchi" <simark@simark.ca>
> To: "Kamil Rytarowski" <n54@gmx.com>, gdb-patches@sourceware.org
> Subject: Re: [PATCH] Add basic event handling in the NetBSD target
>
> On 2020-04-24 6:09 a.m., Kamil Rytarowski wrote:
> >> I know that you'll say it's because FreeBSD does it this way, but still it surprised
> >> me.  If the core of GDB asks to resume one specific thread, it doesn't mean you should
> >> suspend the other threads, they should just be left in whatever state they are.  Is there
> >> a reason to stop the other threads here?
> >>
> >
> > In Linux there can be non-stop mode and threads are managed (stopped,
> > started, etc) individually. On NetBSD we can start and stop the whole
> > process, regardless of the number of threads in it.
> >
> > We need to set the state of all threads before PT_CONTINUE (or assume
> > unchanged state of them).
> >
> > If a thread is not expected to be suspended, we shall set its flag to
> > resume. The same for single-step (PT_STEP). This approach simplifies the
> > implementation as I don't need to track internally which thread has what
> > status and merely save a few syscalls.
>
> Ok.
>
> >>> +    }
> >>> +   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 (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
> >>> +          perror_with_name (("ptrace"));
> >>> +      ptid = inferior_ptid;
> >>
> >> Can you explain this?  Ideally the resume method should not rely on inferior_ptid.  We
> >> are working on reducing the dependencies on this global, in favor of passing the context
> >> by parameters.  Here, the `ptid` parameter should be enough.
> >>
> >
> > The caller can pass minus_one_ptid, which makes no sense for NetBSD and
> > certainly in the multi-target support.
> >
> > Scenarios, as I can see them on NetBSD:
> >
> >  - passing ptid_t (pid, 0, 0) -> resume the whole process with all the
> > threads
> >  - passing ptid_t (pid, lwp, 0) -> resume pid::lwp with all the other
> > threads suspended in pid (`set scheduler-lock')
> >  - passing ptid_t (-1, ?, ?) -> confusion to me so I try to find any
> > fallback
> >
> > If we can abandon the -1 case, I can drop the inferior_ptid usage. If
> > the core no longer passes -1 (it could be), we shall cleanup existing
> > code that handles this as it could happen. If a refactoring can happen,
> > this shall be followed by addition of gdb_assert(ptid != minus_one_ptid).
>
> Or a 4th one: resume all threads of all processes that this target manages.
>

OK. I will try to propose this solution.

> I don't know off-hand if the core of GDB today can pass minus_one_ptid.  You could
> add such an assert, run all your tests, and if everything works, conclude it's not
> needed.  If the assert is hit, you'll know why.
>

I've checked that the GDB core tries to resume minus_one_ptid.

#2  0x0000000000693e08 in dump_core () at utils.c:203
#3  0x00000000006984ba in internal_vproblem(internal_problem *, const char *, int, const char *, typedef __va_list_tag __va_list_tag *) (problem=problem@entry=0xd24840 <internal_error_problem>,
    file=<optimized out>, line=553, fmt=<optimized out>, ap=ap@entry=0x7f7fff3266a8) at utils.c:413
#4  0x0000000000698622 in internal_verror (file=<optimized out>, line=<optimized out>, fmt=<optimized out>, ap=ap@entry=0x7f7fff3266a8) at utils.c:438
#5  0x0000000000780497 in internal_error (file=file@entry=0x8cdc9c "nbsd-nat.c", line=line@entry=553, fmt=<optimized out>) at errors.cc:55
#6  0x00000000005b18f4 in nbsd_nat_target::resume (this=0xd1f9a0 <the_amd64_nbsd_nat_target>, ptid=..., step=0, signal=GDB_SIGNAL_0) at nbsd-nat.c:553
#7  0x000000000064ae0d in target_resume (ptid=..., step=step@entry=0, signal=signal@entry=GDB_SIGNAL_0) at target.c:2121
#8  0x0000000000657d29 in target_continue_no_signal (ptid=...) at target.c:3416
#9  0x00000000005aea79 in startup_inferior (proc_target=proc_target@entry=0xd1f9a0 <the_amd64_nbsd_nat_target>, pid=pid@entry=718, ntraps=ntraps@entry=1,
    last_waitstatus=last_waitstatus@entry=0x0, last_ptid=last_ptid@entry=0x0) at nat/fork-inferior.c:569
#10 0x0000000000520023 in gdb_startup_inferior (pid=pid@entry=718, num_traps=num_traps@entry=1) at fork-child.c:134
#11 0x0000000000555403 in inf_ptrace_target::create_inferior (During symbol reading: Duplicate PC 0x559029 for DW_TAG_call_site DIE 0x1999451 [in module /public/binutils-gdb/gdb/gdb]
this=0xd1f9a0 <the_amd64_nbsd_nat_target>, exec_file=<optimized out>, allargs=..., env=0x7626ff79ae00, from_tty=<optimized out>)
    at inf-ptrace.c:119
#12 0x000000000055b112 in run_command_1 (args=<optimized out>, from_tty=1, run_how=RUN_NORMAL) at infcmd.c:640
#13 0x0000000000481c96 in cmd_func (cmd=<optimized out>, args=<optimized out>, from_tty=<optimized out>) at cli/cli-decode.c:2004
#14 0x00000000006617b2 in execute_command (p=<optimized out>, p@entry=0x7626ffb1c020 "", from_tty=1) at top.c:655
#15 0x0000000000512d5d in command_handler (command=0x7626ffb1c020 "") at event-top.c:588
#16 0x000000000051303a in command_line_handler (rl=...) at event-top.c:773
#17 0x0000000000513576 in gdb_rl_callback_handler (rl=0x7626ffb1c1e0 "r") at event-top.c:219
#18 0x00000000006cae86 in rl_callback_read_char () at callback.c:281
#19 0x0000000000512317 in gdb_rl_callback_read_char_wrapper_noexcept () at event-top.c:177
#20 0x00000000005133a7 in gdb_rl_callback_read_char_wrapper (client_data=<optimized out>) at event-top.c:194
#21 0x00000000005121d0 in stdin_event_handler (error=<optimized out>, client_data=0x7626ffafe0c0) at event-top.c:516
#22 0x0000000000780b4b in gdb_wait_for_event (block=block@entry=1) at event-loop.cc:673
#23 0x0000000000780d3d in gdb_do_one_event () at event-loop.cc:215
#24 0x0000000000589ed0 in start_event_loop () at main.c:356
#25 captured_command_loop () at main.c:416
#26 0x000000000058bc5f in captured_main (data=0x7f7fff326eb0) at main.c:1254
#27 gdb_main (args=args@entry=0x7f7fff326ed0) at main.c:1269
#28 0x00000000007bcf1d in main (argc=<optimized out>, argv=<optimized out>) at gdb.c:32

> >>> +      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
> >>> +      pid = waitpid (ptid.pid (), &status, 0);
> >>
> >> What is crashing exactly?
> >>
> >
> > The core code asks to waitpid() with WNOHANG, receives pid=0 (nobody)
> > and crashes.
> >
> > It's a generic bug that shall be fixed... but it looks like nobody
> > really obeys the core and the 3rd argument is overwritten (compare:
> > rs6000-nat.c, obsd-nat.c, inf-ptrace.c or Linux...).
> >
> > I've landed into more similar generic nuances during my effort on GDB,
> > that handling events promptly can crash the core and I need to change
> > the behavior to emulate other kernels.
> >
> > It would be better to fix the core, but it is not NetBSD's fault and not
> > a blocker for NetBSD support.
> >
> > If there is interest in fixing it, I can file a bug report
> > independently. I don't pledge to work on it myself (at least as long as
> > I can merely focus on one kernel support).
>
> Ok, I'm fine with doing this and documenting it somewhere.
>
> Simon
>
>

[PATCH v2] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

[-- Attachment #1: Type: text/plain, Size: 184 bytes --]

Hello,

Please review new version submitted as an attachment.

I've applied all the requested changes and used a nonexited inferior iterator in ::resume(), whenever we receive PTID -1.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-basic-event-handling-in-the-NetBSD-target.patch --]
[-- Type: text/x-patch, Size: 11548 bytes --]

From fbdc02d2e3cf3080f8b12d4a8dab91256db08e61 Mon Sep 17 00:00:00 2001
From: Kamil Rytarowski <n54@gmx.com>
Date: Wed, 29 Apr 2020 01:57:38 +0200
Subject: [PATCH] Add basic event handling in the NetBSD target

Implement the following events:
 - single step (TRAP_TRACE)
 - software breakpoint (TRAP_DBREG)
 - exec() (TRAP_EXEC)
 - syscall entry/exit (TRAP_SCE / TRAP_SCX)

Add support for NetBSD specific ::wait () and ::resume ().

Instruct the generic code that exec and syscall events are supported.

Define an empty nbsd_get_syscall_number as it is prerequisite for
catching syscall entry and exit events, even if it is unused.
This function is used to detect whether the gdbarch supports the
'catch syscall' feature.

gdb/ChangeLog:

       * nbsd-nat.c: Include "sys/wait.h".
       (nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
       (nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
       (nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-tdep.c (nbsd_get_syscall_number): Add.
       (nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
       `nbsd_get_syscall_number'.
---
 gdb/ChangeLog   |  15 ++++
 gdb/nbsd-nat.c  | 222 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-nat.h  |   9 ++
 gdb/nbsd-tdep.c |  18 ++++
 4 files changed, 264 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3313f156c9..fb46a31b0a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2020-04-28  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "sys/wait.h".
+	(nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
+	(nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
+	(nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-tdep.c (nbsd_get_syscall_number): Add.
+	(nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
+	`nbsd_get_syscall_number'.
+
 2020-04-28  Kamil Rytarowski  <n54@gmx.com>

 	* syscalls/update-netbsd.sh: New file.
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index d41cfc815d..cf1e774820 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/wait.h>

 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
@@ -539,3 +540,224 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)

   return true;
 }
+
+/* Resume execution of a specified PTID, that points to a process or a thread
+   within a process.  If one thread is specified, all other threads are
+   suspended.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+static void
+nbsd_resume(nbsd_nat_target *target, ptid_t ptid, int step,
+	    enum gdb_signal signal)
+{
+  int request;
+
+  gdb_assert (minus_one_ptid != ptid);
+
+  if (ptid.lwp_p ())
+    {
+      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
+      inferior *inf = find_inferior_ptid (target, ptid);
+
+      for (thread_info *tp : inf->non_exited_threads ())
+        {
+          if (tp->ptid.lwp () == ptid.lwp ())
+            request = PT_RESUME;
+          else
+            request = PT_SUSPEND;
+
+          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+            perror_with_name (("ptrace"));
+        }
+    }
+  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 (target, ptid))
+        if (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+          perror_with_name (("ptrace"));
+    }
+
+  if (step)
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+  else
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+
+  if (catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
+  /* An address of (void *)1 tells ptrace to continue from
+     where it was.  If GDB wanted it to start some other way, we have
+     already written a new program counter value to the child.  */
+  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
+    perror_with_name (("ptrace"));
+}
+
+/* Resume execution of thread PTID, or all threads of all inferiors
+   if PTID is -1.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+void
+nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
+{
+  if (minus_one_ptid != ptid)
+    nbsd_resume (this, ptid, step, signal);
+  else
+    {
+      for (inferior *inf : all_non_exited_inferiors ())
+	nbsd_resume (this, ptid_t (inf->pid, 0, 0), step, signal);
+    }
+}
+
+/* Implement a safe wrapper around waitpid().  */
+
+static pid_t
+nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  pid_t pid;
+  int status;
+
+  set_sigint_trap ();
+
+  do
+    {
+      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
+      pid = waitpid (ptid.pid (), &status, 0);
+    }
+  while (pid == -1 && errno == EINTR);
+
+  clear_sigint_trap ();
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  store_waitstatus (ourstatus, status);
+  return pid;
+}
+
+/* Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
+
+ptid_t
+nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+		       int target_options)
+{
+  pid_t pid = nbsd_wait (ptid, ourstatus, target_options);
+  ptid_t wptid = ptid_t (pid);
+
+  /* If the child stopped, keep investigating its status.  */
+  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
+    return wptid;
+
+  /* Extract the event and thread that received a signal.  */
+  ptrace_siginfo_t psi;
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  /* Pick child's siginfo_t.  */
+  siginfo_t *si = &psi.psi_siginfo;
+
+  int lwp = psi.psi_lwpid;
+
+  int signo = si->si_signo;
+  const int code = si->si_code;
+
+  /* Construct PTID with a specified thread that received the event.
+     If a signal was targeted to the whole process, lwp is 0.  */
+  wptid = ptid_t (pid, lwp, 0);
+
+  /* Bail out on non-debugger oriented signals..  */
+  if (signo != SIGTRAP)
+    return wptid;
+
+  /* Stop examining non-debugger oriented SIGTRAP codes.  */
+  if (code <= SI_USER || code == SI_NOINFO)
+    return wptid;
+
+  if (in_thread_list (this, ptid_t (pid)))
+      thread_change_ptid (this, ptid_t (pid), wptid);
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname = xstrdup (pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!catch_syscall_enabled () || !catching_syscall_number (sysnum))
+	{
+	  /* If the core isn't interested in this event, ignore it.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  return wptid;
+	}
+
+      ourstatus->kind =
+	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
+	TARGET_WAITKIND_SYSCALL_RETURN;
+      ourstatus->value.syscall_number = sysnum;
+      return wptid;
+    }
+
+  if (code == TRAP_BRKPT)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the "insert_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::insert_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "remove_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::remove_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "set_syscall_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::set_syscall_catchpoint (int pid, bool needed,
+                                         int any_count,
+                                         gdb::array_view<const int> syscall_counts)
+{
+  /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
+     will catch all system call entries and exits.  The system calls
+     are filtered by GDB rather than the kernel.  */
+  return 0;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 256db4b901..6e14cbb889 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -38,6 +38,15 @@ struct nbsd_nat_target : public inf_ptrace_target

   int find_memory_regions (find_memory_region_ftype func, void *data) override;
   bool info_proc (const char *, enum info_proc_what) override;
+
+  void resume (ptid_t, int, enum gdb_signal) override;
+  ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
+  int insert_exec_catchpoint (int pid) override;
+  int remove_exec_catchpoint (int pid) override;
+  int set_syscall_catchpoint (int pid, bool needed, int any_count,
+			      gdb::array_view<const int> syscall_counts)
+    override;
+
 };

 #endif /* nbsd-nat.h */
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 52e0640e35..2ed16f6faf 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
     }
 }

+/* Implement the "get_syscall_number" gdbarch method.  */
+
+static LONGEST
+nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
+{
+
+  /* NetBSD doesn't use gdbarch_get_syscall_number since NetBSD
+     native targets fetch the system call number from the
+     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
+     However, system call catching requires this function to be
+     set.  */
+
+  internal_error (__FILE__, __LINE__, _("nbsd_get_sycall_number called"));
+}
+
 /* See nbsd-tdep.h.  */

 void
@@ -453,4 +468,7 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
   set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
+
+  /* `catch syscall' */
+  set_gdbarch_get_syscall_number (gdbarch, nbsd_get_syscall_number);
 }
--
2.25.2

[PATCH v2] Implement the following events: - single step (TRAP_TRACE) - software breakpoint (TRAP_DBREG) - exec() (TRAP_EXEC) - syscall entry/exit (TRAP_SCE / TRAP_SCX)

[Kamil Rytarowski]

Add support for NetBSD specific ::wait () and ::resume ().

Instruct the generic code that exec and syscall events are supported.

Define an empty nbsd_get_syscall_number as it is prerequisite for
catching syscall entry and exit events, even if it is unused.
This function is used to detect whether the gdbarch supports the
'catch syscall' feature.

gdb/ChangeLog:

       * nbsd-nat.c: Include "sys/wait.h".
       (nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
       (nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
       (nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-tdep.c (nbsd_get_syscall_number): Add.
       (nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
       `nbsd_get_syscall_number'.
---
 gdb/ChangeLog   |  15 ++++
 gdb/nbsd-nat.c  | 222 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-nat.h  |   9 ++
 gdb/nbsd-tdep.c |  18 ++++
 4 files changed, 264 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3313f156c9..fb46a31b0a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2020-04-28  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "sys/wait.h".
+	(nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
+	(nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
+	(nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-tdep.c (nbsd_get_syscall_number): Add.
+	(nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
+	`nbsd_get_syscall_number'.
+
 2020-04-28  Kamil Rytarowski  <n54@gmx.com>
 
 	* syscalls/update-netbsd.sh: New file.
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index d41cfc815d..cf1e774820 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/wait.h>
 
 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
@@ -539,3 +540,224 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
 
   return true;
 }
+
+/* Resume execution of a specified PTID, that points to a process or a thread
+   within a process.  If one thread is specified, all other threads are
+   suspended.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+static void
+nbsd_resume(nbsd_nat_target *target, ptid_t ptid, int step,
+	    enum gdb_signal signal)
+{
+  int request;
+
+  gdb_assert (minus_one_ptid != ptid);
+
+  if (ptid.lwp_p ())
+    {
+      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
+      inferior *inf = find_inferior_ptid (target, ptid);
+
+      for (thread_info *tp : inf->non_exited_threads ())
+        {
+          if (tp->ptid.lwp () == ptid.lwp ())
+            request = PT_RESUME;
+          else
+            request = PT_SUSPEND;
+
+          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+            perror_with_name (("ptrace"));
+        }
+    }
+  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 (target, ptid))
+        if (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+          perror_with_name (("ptrace"));
+    }
+
+  if (step)
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+  else
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+
+  if (catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
+  /* An address of (void *)1 tells ptrace to continue from
+     where it was.  If GDB wanted it to start some other way, we have
+     already written a new program counter value to the child.  */
+  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
+    perror_with_name (("ptrace"));
+}
+
+/* Resume execution of thread PTID, or all threads of all inferiors
+   if PTID is -1.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+void
+nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
+{
+  if (minus_one_ptid != ptid)
+    nbsd_resume (this, ptid, step, signal);
+  else
+    {
+      for (inferior *inf : all_non_exited_inferiors ())
+	nbsd_resume (this, ptid_t (inf->pid, 0, 0), step, signal);
+    }
+}
+
+/* Implement a safe wrapper around waitpid().  */
+
+static pid_t
+nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  pid_t pid;
+  int status;
+
+  set_sigint_trap ();
+
+  do
+    {
+      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
+      pid = waitpid (ptid.pid (), &status, 0);
+    }
+  while (pid == -1 && errno == EINTR);
+
+  clear_sigint_trap ();
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  store_waitstatus (ourstatus, status);
+  return pid;
+}
+
+/* Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
+
+ptid_t
+nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+		       int target_options)
+{
+  pid_t pid = nbsd_wait (ptid, ourstatus, target_options);
+  ptid_t wptid = ptid_t (pid);
+
+  /* If the child stopped, keep investigating its status.  */
+  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
+    return wptid;
+
+  /* Extract the event and thread that received a signal.  */
+  ptrace_siginfo_t psi;
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  /* Pick child's siginfo_t.  */
+  siginfo_t *si = &psi.psi_siginfo;
+
+  int lwp = psi.psi_lwpid;
+
+  int signo = si->si_signo;
+  const int code = si->si_code;
+
+  /* Construct PTID with a specified thread that received the event.
+     If a signal was targeted to the whole process, lwp is 0.  */
+  wptid = ptid_t (pid, lwp, 0);
+
+  /* Bail out on non-debugger oriented signals..  */
+  if (signo != SIGTRAP)
+    return wptid;
+
+  /* Stop examining non-debugger oriented SIGTRAP codes.  */
+  if (code <= SI_USER || code == SI_NOINFO)
+    return wptid;
+
+  if (in_thread_list (this, ptid_t (pid)))
+      thread_change_ptid (this, ptid_t (pid), wptid);
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname = xstrdup (pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!catch_syscall_enabled () || !catching_syscall_number (sysnum))
+	{
+	  /* If the core isn't interested in this event, ignore it.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  return wptid;
+	}
+
+      ourstatus->kind =
+	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
+	TARGET_WAITKIND_SYSCALL_RETURN;
+      ourstatus->value.syscall_number = sysnum;
+      return wptid;
+    }
+
+  if (code == TRAP_BRKPT)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the "insert_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::insert_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "remove_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::remove_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "set_syscall_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::set_syscall_catchpoint (int pid, bool needed,
+                                         int any_count,
+                                         gdb::array_view<const int> syscall_counts)
+{
+  /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
+     will catch all system call entries and exits.  The system calls
+     are filtered by GDB rather than the kernel.  */
+  return 0;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 256db4b901..6e14cbb889 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -38,6 +38,15 @@ struct nbsd_nat_target : public inf_ptrace_target
 
   int find_memory_regions (find_memory_region_ftype func, void *data) override;
   bool info_proc (const char *, enum info_proc_what) override;
+
+  void resume (ptid_t, int, enum gdb_signal) override;
+  ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
+  int insert_exec_catchpoint (int pid) override;
+  int remove_exec_catchpoint (int pid) override;
+  int set_syscall_catchpoint (int pid, bool needed, int any_count,
+			      gdb::array_view<const int> syscall_counts)
+    override;
+
 };
 
 #endif /* nbsd-nat.h */
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 52e0640e35..2ed16f6faf 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
     }
 }
 
+/* Implement the "get_syscall_number" gdbarch method.  */
+
+static LONGEST
+nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
+{
+
+  /* NetBSD doesn't use gdbarch_get_syscall_number since NetBSD
+     native targets fetch the system call number from the
+     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
+     However, system call catching requires this function to be
+     set.  */
+
+  internal_error (__FILE__, __LINE__, _("nbsd_get_sycall_number called"));
+}
+
 /* See nbsd-tdep.h.  */
 
 void
@@ -453,4 +468,7 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
   set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
+
+  /* `catch syscall' */
+  set_gdbarch_get_syscall_number (gdbarch, nbsd_get_syscall_number);
 }
-- 
2.25.2

[PATCH v2] Add basic event handling in the NetBSD target

[Kamil Rytarowski]

Implement the following events:
 - single step (TRAP_TRACE)
 - software breakpoint (TRAP_DBREG)
 - exec() (TRAP_EXEC)
 - syscall entry/exit (TRAP_SCE / TRAP_SCX)

Add support for NetBSD specific ::wait () and ::resume ().

Instruct the generic code that exec and syscall events are supported.

Define an empty nbsd_get_syscall_number as it is prerequisite for
catching syscall entry and exit events, even if it is unused.
This function is used to detect whether the gdbarch supports the
'catch syscall' feature.

gdb/ChangeLog:

       * nbsd-nat.c: Include "sys/wait.h".
       (nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
       (nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
       (nbsd_nat_target::insert_exec_catchpoint)
       (nbsd_nat_target::remove_exec_catchpoint)
       (nbsd_nat_target::set_syscall_catchpoint): Add.
       * nbsd-tdep.c (nbsd_get_syscall_number): Add.
       (nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
       `nbsd_get_syscall_number'.
---
 gdb/ChangeLog   |  15 ++++
 gdb/nbsd-nat.c  | 222 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/nbsd-nat.h  |   9 ++
 gdb/nbsd-tdep.c |  18 ++++
 4 files changed, 264 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3313f156c9..fb46a31b0a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2020-04-28  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "sys/wait.h".
+	(nbsd_resume, nbsd_nat_target::resume, nbsd_wait)
+	(nbsd_nat_target::wait, nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-nat.h (nbsd_nat_target::resume, nbsd_nat_target::wait)
+	(nbsd_nat_target::insert_exec_catchpoint)
+	(nbsd_nat_target::remove_exec_catchpoint)
+	(nbsd_nat_target::set_syscall_catchpoint): Add.
+	* nbsd-tdep.c (nbsd_get_syscall_number): Add.
+	(nbsd_init_abi): Call `set_gdbarch_get_syscall_number' and pass
+	`nbsd_get_syscall_number'.
+
 2020-04-28  Kamil Rytarowski  <n54@gmx.com>
 
 	* syscalls/update-netbsd.sh: New file.
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index d41cfc815d..cf1e774820 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/wait.h>
 
 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
@@ -539,3 +540,224 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
 
   return true;
 }
+
+/* Resume execution of a specified PTID, that points to a process or a thread
+   within a process.  If one thread is specified, all other threads are
+   suspended.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+static void
+nbsd_resume(nbsd_nat_target *target, ptid_t ptid, int step,
+	    enum gdb_signal signal)
+{
+  int request;
+
+  gdb_assert (minus_one_ptid != ptid);
+
+  if (ptid.lwp_p ())
+    {
+      /* If ptid is a specific LWP, suspend all other LWPs in the process.  */
+      inferior *inf = find_inferior_ptid (target, ptid);
+
+      for (thread_info *tp : inf->non_exited_threads ())
+        {
+          if (tp->ptid.lwp () == ptid.lwp ())
+            request = PT_RESUME;
+          else
+            request = PT_SUSPEND;
+
+          if (ptrace (request, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+            perror_with_name (("ptrace"));
+        }
+    }
+  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 (target, ptid))
+        if (ptrace (PT_RESUME, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+          perror_with_name (("ptrace"));
+    }
+
+  if (step)
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_SETSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+  else
+    {
+      for (thread_info *tp : all_non_exited_threads (target, ptid))
+	if (ptrace (PT_CLEARSTEP, tp->ptid.pid (), NULL, tp->ptid.lwp ()) == -1)
+	  perror_with_name (("ptrace"));
+    }
+
+  if (catch_syscall_enabled () > 0)
+    request = PT_SYSCALL;
+  else
+    request = PT_CONTINUE;
+
+  /* An address of (void *)1 tells ptrace to continue from
+     where it was.  If GDB wanted it to start some other way, we have
+     already written a new program counter value to the child.  */
+  if (ptrace (request, ptid.pid (), (void *)1, gdb_signal_to_host (signal)) == -1)
+    perror_with_name (("ptrace"));
+}
+
+/* Resume execution of thread PTID, or all threads of all inferiors
+   if PTID is -1.  If STEP is nonzero, single-step it.  If SIGNAL is nonzero,
+   give it that signal.  */
+
+void
+nbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
+{
+  if (minus_one_ptid != ptid)
+    nbsd_resume (this, ptid, step, signal);
+  else
+    {
+      for (inferior *inf : all_non_exited_inferiors ())
+	nbsd_resume (this, ptid_t (inf->pid, 0, 0), step, signal);
+    }
+}
+
+/* Implement a safe wrapper around waitpid().  */
+
+static pid_t
+nbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  pid_t pid;
+  int status;
+
+  set_sigint_trap ();
+
+  do
+    {
+      /* The common code passes WNOHANG that leads to crashes, overwrite it.  */
+      pid = waitpid (ptid.pid (), &status, 0);
+    }
+  while (pid == -1 && errno == EINTR);
+
+  clear_sigint_trap ();
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  store_waitstatus (ourstatus, status);
+  return pid;
+}
+
+/* Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
+
+ptid_t
+nbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+		       int target_options)
+{
+  pid_t pid = nbsd_wait (ptid, ourstatus, target_options);
+  ptid_t wptid = ptid_t (pid);
+
+  /* If the child stopped, keep investigating its status.  */
+  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
+    return wptid;
+
+  /* Extract the event and thread that received a signal.  */
+  ptrace_siginfo_t psi;
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  /* Pick child's siginfo_t.  */
+  siginfo_t *si = &psi.psi_siginfo;
+
+  int lwp = psi.psi_lwpid;
+
+  int signo = si->si_signo;
+  const int code = si->si_code;
+
+  /* Construct PTID with a specified thread that received the event.
+     If a signal was targeted to the whole process, lwp is 0.  */
+  wptid = ptid_t (pid, lwp, 0);
+
+  /* Bail out on non-debugger oriented signals..  */
+  if (signo != SIGTRAP)
+    return wptid;
+
+  /* Stop examining non-debugger oriented SIGTRAP codes.  */
+  if (code <= SI_USER || code == SI_NOINFO)
+    return wptid;
+
+  if (in_thread_list (this, ptid_t (pid)))
+      thread_change_ptid (this, ptid_t (pid), wptid);
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname = xstrdup (pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!catch_syscall_enabled () || !catching_syscall_number (sysnum))
+	{
+	  /* If the core isn't interested in this event, ignore it.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  return wptid;
+	}
+
+      ourstatus->kind =
+	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
+	TARGET_WAITKIND_SYSCALL_RETURN;
+      ourstatus->value.syscall_number = sysnum;
+      return wptid;
+    }
+
+  if (code == TRAP_BRKPT)
+    {
+      /* Unhandled at this level.  */
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the "insert_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::insert_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "remove_exec_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::remove_exec_catchpoint (int pid)
+{
+  /* Nothing to do.  */
+  return 0;
+}
+
+/* Implement the "set_syscall_catchpoint" target_ops method.  */
+
+int
+nbsd_nat_target::set_syscall_catchpoint (int pid, bool needed,
+                                         int any_count,
+                                         gdb::array_view<const int> syscall_counts)
+{
+  /* Ignore the arguments.  inf-ptrace.c will use PT_SYSCALL which
+     will catch all system call entries and exits.  The system calls
+     are filtered by GDB rather than the kernel.  */
+  return 0;
+}
diff --git a/gdb/nbsd-nat.h b/gdb/nbsd-nat.h
index 256db4b901..6e14cbb889 100644
--- a/gdb/nbsd-nat.h
+++ b/gdb/nbsd-nat.h
@@ -38,6 +38,15 @@ struct nbsd_nat_target : public inf_ptrace_target
 
   int find_memory_regions (find_memory_region_ftype func, void *data) override;
   bool info_proc (const char *, enum info_proc_what) override;
+
+  void resume (ptid_t, int, enum gdb_signal) override;
+  ptid_t wait (ptid_t, struct target_waitstatus *, int) override;
+  int insert_exec_catchpoint (int pid) override;
+  int remove_exec_catchpoint (int pid) override;
+  int set_syscall_catchpoint (int pid, bool needed, int any_count,
+			      gdb::array_view<const int> syscall_counts)
+    override;
+
 };
 
 #endif /* nbsd-nat.h */
diff --git a/gdb/nbsd-tdep.c b/gdb/nbsd-tdep.c
index 52e0640e35..2ed16f6faf 100644
--- a/gdb/nbsd-tdep.c
+++ b/gdb/nbsd-tdep.c
@@ -444,6 +444,21 @@ nbsd_info_proc_mappings_entry (int addr_bit, ULONGEST kve_start,
     }
 }
 
+/* Implement the "get_syscall_number" gdbarch method.  */
+
+static LONGEST
+nbsd_get_syscall_number (struct gdbarch *gdbarch, thread_info *thread)
+{
+
+  /* NetBSD doesn't use gdbarch_get_syscall_number since NetBSD
+     native targets fetch the system call number from the
+     'si_sysnum' member of siginfo_t in nbsd_nat_target::wait.
+     However, system call catching requires this function to be
+     set.  */
+
+  internal_error (__FILE__, __LINE__, _("nbsd_get_sycall_number called"));
+}
+
 /* See nbsd-tdep.h.  */
 
 void
@@ -453,4 +468,7 @@ nbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_gdb_signal_to_target (gdbarch, nbsd_gdb_signal_to_target);
   set_gdbarch_skip_solib_resolver (gdbarch, nbsd_skip_solib_resolver);
   set_gdbarch_auxv_parse (gdbarch, svr4_auxv_parse);
+
+  /* `catch syscall' */
+  set_gdbarch_get_syscall_number (gdbarch, nbsd_get_syscall_number);
 }
-- 
2.25.2

Re: [PATCH v2] Add basic event handling in the NetBSD target

[Simon Marchi]

On 2020-04-29 7:08 a.m., Kamil Rytarowski wrote:
> Hello,
> 
> Please review new version submitted as an attachment.
> 
> I've applied all the requested changes and used a nonexited inferior iterator in ::resume(), whenever we receive PTID -1.
> 

I think that makes sense.  Only thing it that you should probably use
`all_non_exited_inferiors (this)`, to avoid trying to resume inferiors
of other targets.

Simon