public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support
@ 2020-09-02 17:59 Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls Kamil Rytarowski
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

Extract reusable functionality from gdb/nbsd-nat.c into
gdb/nat/netbsd-nat.c and switch the gdb/nbsd-nat.c functions
(nbsd_nat_target::pid_to_exec_file, nbsd_nat_target::thread_alive,
nbsd_nat_target::thread_name, nbsd_nat_target::post_startup_inferior,
nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial,
nbsd_add_threads) to gdb/nat.

Add handle_eintr in gdbsupport to wrap syscalls and EINTR handling.

Add NetBSD/amd64 gdbserver support

Implement the following functionality: create_inferior,
post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
resume, wait, fetch_registers, store_registers, read_memory, write_memory,
request_interrupt, supports_read_auxv, read_auxv,
supports_hardware_single_step, sw_breakpoint_from_kind,
supports_z_point_type, insert_point, remove_point,
stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
supports_stopped_by_sw_breakpoint, supports_non_stop,
supports_multi_process, supports_fork_events, supports_vfork_events,
supports_exec_events, supports_disable_randomization,
supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
supports_pid_to_exec_file, pid_to_exec_file, thread_name,
supports_catch_syscall.

The only CPU architecture supported: x86_64.

Implement only support for hardware assisted single step and
software breakpoint.

Implement support only for regular X86 registers, thus no FPU.

Kamil Rytarowski (10):
  Add handle_eintr to wrap EINTR handling in syscalls
  Register a placeholder for NetBSD shared functions in gdb/nat
  Build nat/netbsd-nat.o for the NetBSD native target
  Add netbsd_nat::pid_to_exec_file
  Add gdb/nat common functions for listing threads
  Add netbsd_nat::enable_proc_events in gdb/nat
  Add a common utility function to read and write siginfo_t in inferior
  Avoid double free in startup_inferior
  Switch local native code to gdb/nat shared functions
  Add minimal and functional NetBSD/amd64 gdbserver

 gdb/ChangeLog                  |   47 ++
 gdb/configure.nat              |    2 +-
 gdb/nat/fork-inferior.c        |    5 +-
 gdb/nat/netbsd-nat.c           |  213 +++++
 gdb/nat/netbsd-nat.h           |   46 ++
 gdb/nbsd-nat.c                 |  147 +---
 gdbserver/ChangeLog            |    9 +
 gdbserver/Makefile.in          |    3 +
 gdbserver/configure.srv        |    7 +
 gdbserver/netbsd-low.cc        | 1352 ++++++++++++++++++++++++++++++++
 gdbserver/netbsd-low.h         |  157 ++++
 gdbserver/netbsd-x86_64-low.cc |  250 ++++++
 gdbsupport/ChangeLog           |    4 +
 gdbsupport/eintr.h             |   41 +
 14 files changed, 2147 insertions(+), 136 deletions(-)
 create mode 100644 gdb/nat/netbsd-nat.c
 create mode 100644 gdb/nat/netbsd-nat.h
 create mode 100644 gdbserver/netbsd-low.cc
 create mode 100644 gdbserver/netbsd-low.h
 create mode 100644 gdbserver/netbsd-x86_64-low.cc
 create mode 100644 gdbsupport/eintr.h

--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-03 14:17   ` Tom Tromey
  2020-09-02 17:59 ` [PATCH 02/10] Register a placeholder for NetBSD shared functions in gdb/nat Kamil Rytarowski
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

gdbsupport/ChangeLog:

	* eintr.h: Add handle_eintr.
---
 gdbsupport/ChangeLog |  4 ++++
 gdbsupport/eintr.h   | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 gdbsupport/eintr.h

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index a1960537384..7a429367941 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,7 @@
+2020-03-17  Kamil Rytarowski  <n54@gmx.com>
+
+	* eintr.h: Add handle_eintr.
+
 2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>

 	* selftest.h (run_tests): Change parameter to array_view.
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
new file mode 100644
index 00000000000..cb35d2ffc3d
--- /dev/null
+++ b/gdbsupport/eintr.h
@@ -0,0 +1,41 @@
+/* Utility for handling interrupted syscalls by signals.
+
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_EINTR_H
+#define GDBSUPPORT_EINTR_H
+
+#include <cerrno>
+
+namespace gdb
+{
+template <typename Fun, typename... Args>
+inline decltype (auto) handle_eintr (const Fun &F, const Args &... A)
+{
+  decltype (F (A...)) ret;
+  do
+    {
+      errno = 0;
+      ret = F (A...);
+    }
+  while (ret == -1 && errno == EINTR);
+  return ret;
+}
+}
+
+#endif /* GDBSUPPORT_EINTR_H */
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 02/10] Register a placeholder for NetBSD shared functions in gdb/nat
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 03/10] Build nat/netbsd-nat.o for the NetBSD native target Kamil Rytarowski
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:

	* netbsd-nat.h: New file.
	* netbsd-nat.c: New file.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 24 ++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h | 27 +++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)
 create mode 100644 gdb/nat/netbsd-nat.c
 create mode 100644 gdb/nat/netbsd-nat.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ceff808d829..40f01da33ba 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h: New file.
+	* netbsd-nat.c: New file.
+
 2020-08-25  Shahab Vahedi  <shahab@synopsys.com>

 	* MAINTAINERS: Add ARC target and maintainer.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
new file mode 100644
index 00000000000..2b5a4183e30
--- /dev/null
+++ b/gdb/nat/netbsd-nat.c
@@ -0,0 +1,24 @@
+/* Internal interfaces for the NetBSD code.
+
+   Copyright (C) 2006-2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "nat/netbsd-nat.h"
+
+namespace netbsd_nat
+{
+}
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
new file mode 100644
index 00000000000..5fa08746610
--- /dev/null
+++ b/gdb/nat/netbsd-nat.h
@@ -0,0 +1,27 @@
+/* Internal interfaces for the NetBSD code.
+
+   Copyright (C) 2006-2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef NAT_NETBSD_NAT_H
+#define NAT_NETBSD_NAT_H
+
+namespace netbsd_nat
+{
+}
+
+#endif
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 03/10] Build nat/netbsd-nat.o for the NetBSD native target
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 02/10] Register a placeholder for NetBSD shared functions in gdb/nat Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 04/10] Add netbsd_nat::pid_to_exec_file Kamil Rytarowski
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

---
 gdb/ChangeLog     | 4 ++++
 gdb/configure.nat | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 40f01da33ba..053cce76b3f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h: New file.
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 6ea25834954..3e94a064aeb 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -68,7 +68,7 @@ case ${gdb_host} in
 	LOADLIBES='-lkvm'
 	;;
     nbsd*)
-	NATDEPFILES='fork-child.o nat/fork-inferior.o inf-ptrace.o'
+	NATDEPFILES='fork-child.o nat/fork-inferior.o nat/netbsd-nat.o inf-ptrace.o'
 	HAVE_NATIVE_GCORE_HOST=1
 	;;
     obsd*)
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 04/10] Add netbsd_nat::pid_to_exec_file
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (2 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 03/10] Build nat/netbsd-nat.o for the NetBSD native target Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 05/10] Add gdb/nat common functions for listing threads Kamil Rytarowski
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:

	* netbsd-nat.h (netbsd_nat::pid_to_exec_file): Add.
	* netbsd-nat.c (netbsd_nat::pid_to_exec_file): Likewise.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
 gdb/nat/netbsd-nat.h |  5 +++++
 3 files changed, 28 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 053cce76b3f..2adfd3d79d1 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::pid_to_exec_file): Add.
+	* netbsd-nat.c (netbsd_nat::pid_to_exec_file): Likewise.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 2b5a4183e30..297188bb8b4 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -19,6 +19,24 @@

 #include "nat/netbsd-nat.h"

+#include <sys/types.h>
+#include <sys/sysctl.h>
+
 namespace netbsd_nat
 {
+
+/* Return the executable file name of a process specified by PID.  Returns the
+   string in a static buffer.  */
+
+char *
+pid_to_exec_file (pid_t pid)
+{
+  static char buf[PATH_MAX];
+  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
+  size_t buflen = sizeof (buf);
+  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
+    return NULL;
+  return buf;
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index 5fa08746610..a5f664d95f4 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -20,8 +20,13 @@
 #ifndef NAT_NETBSD_NAT_H
 #define NAT_NETBSD_NAT_H

+#include <unistd.h>
+
 namespace netbsd_nat
 {
+
+extern char *pid_to_exec_file (pid_t pid);
+
 }

 #endif
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 05/10] Add gdb/nat common functions for listing threads
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (3 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 04/10] Add netbsd_nat::pid_to_exec_file Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 06/10] Add netbsd_nat::enable_proc_events in gdb/nat Kamil Rytarowski
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

Add netbsd_nat::netbsd_thread_lister a generic thread lister, used
internally in netbsd-nat.c, copied from gdb/nbsd-nat.c.

Add public extern functions for listing threads:
 * netbsd_nat::thread_alive
 * netbsd_nat::thread_name
 * netbsd_nat::list_threads

gdb/ChangeLog:

        * netbsd-nat.h (netbsd_nat::thread_alive, netbsd_nat::thread_name)
        (netbsd_nat::list_threads): Add.
        * netbsd-nat.c (netbsd_nat::netbsd_thread_lister)
        (netbsd_nat::thread_alive, netbsd_nat::thread_name)
        (netbsd_nat::list_threads): Add.
---
 gdb/ChangeLog        |   8 +++
 gdb/nat/netbsd-nat.c | 123 +++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h |   8 +++
 3 files changed, 139 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2adfd3d79d1..8b9ca1e9b32 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::thread_alive, netbsd_nat::thread_name)
+	(netbsd_nat::list_threads): Add.
+	* netbsd-nat.c (netbsd_nat::netbsd_thread_lister)
+	(netbsd_nat::thread_alive, netbsd_nat::thread_name)
+	(netbsd_nat::list_threads): Add.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::pid_to_exec_file): Add.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 297188bb8b4..5f005c38bf2 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -17,11 +17,17 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

+#include "gdbsupport/common-defs.h"
 #include "nat/netbsd-nat.h"
+#include "gdbsupport/common-debug.h"

 #include <sys/types.h>
 #include <sys/sysctl.h>

+#include <cstring>
+
+#include "gdbsupport/function-view.h"
+
 namespace netbsd_nat
 {

@@ -39,4 +45,121 @@ pid_to_exec_file (pid_t pid)
   return buf;
 }

+/* Generic thread (LWP) lister within a specified process.  The callback
+   parameters is a C++ function that is called for each detected thread.  */
+
+static bool
+netbsd_thread_lister (const pid_t pid,
+		      gdb::function_view<bool (const struct kinfo_lwp *)>
+		      callback)
+{
+  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
+  size_t size;
+
+  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
+    perror_with_name (("sysctl"));
+
+  mib[4] = size / sizeof (size_t);
+
+  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
+    ((struct kinfo_lwp *) xcalloc (size, 1));
+
+  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
+      || size == 0)
+    perror_with_name (("sysctl"));
+
+  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
+    {
+      struct kinfo_lwp *l = &kl[i];
+
+      /* Return true if the specified thread is alive.  */
+      auto lwp_alive
+	= [] (struct kinfo_lwp *lwp)
+	  {
+	    switch (lwp->l_stat)
+	      {
+	      case LSSLEEP:
+	      case LSRUN:
+	      case LSONPROC:
+	      case LSSTOP:
+	      case LSSUSPENDED:
+		return true;
+	      default:
+		return false;
+	      }
+	  };
+
+      /* Ignore embryonic or demised threads.  */
+      if (!lwp_alive (l))
+	continue;
+
+      if (callback (l))
+	return true;
+    }
+
+  return false;
+}
+
+/* Return true if PTID is still active in the inferior.  */
+
+bool
+thread_alive (ptid_t ptid)
+{
+  pid_t pid = ptid.pid ();
+  lwpid_t lwp = ptid.lwp ();
+
+  auto fn
+    = [&lwp] (const struct kinfo_lwp *kl)
+      {
+        return kl->l_lid == lwp;
+      };
+
+  return netbsd_thread_lister (pid, fn);
+}
+
+/* Return the name assigned to a thread by an application.  Returns
+   the string in a static buffer.  */
+
+const char *
+thread_name (ptid_t ptid)
+{
+  pid_t pid = ptid.pid ();
+  lwpid_t lwp = ptid.lwp ();
+
+  static char buf[KI_LNAMELEN] = {};
+
+  auto fn
+    = [&lwp] (const struct kinfo_lwp *kl)
+      {
+	if (kl->l_lid == lwp)
+	  {
+	    xsnprintf (buf, sizeof buf, "%s", kl->l_name);
+	    return true;
+	  }
+	return false;
+      };
+
+  if (netbsd_thread_lister (pid, fn))
+    return buf;
+  else
+    return NULL;
+}
+
+/* A generic thread lister within a specific PID.  The CALLBACK parameter
+   is a C++ function that is called for each detected thread.  */
+
+void
+list_threads (pid_t pid, gdb::function_view<void (ptid_t)> callback)
+{
+  auto fn
+    = [&callback, &pid] (const struct kinfo_lwp *kl)
+      {
+	ptid_t ptid = ptid_t (pid, kl->l_lid, 0);
+	callback (ptid);
+	return false;
+      };
+
+  netbsd_thread_lister (pid, fn);
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index a5f664d95f4..b6885e1a9a6 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -20,6 +20,8 @@
 #ifndef NAT_NETBSD_NAT_H
 #define NAT_NETBSD_NAT_H

+#include "gdbsupport/function-view.h"
+
 #include <unistd.h>

 namespace netbsd_nat
@@ -27,6 +29,12 @@ namespace netbsd_nat

 extern char *pid_to_exec_file (pid_t pid);

+extern bool thread_alive (ptid_t ptid);
+
+extern const char *thread_name (ptid_t ptid);
+
+extern void list_threads (pid_t pid,
+			  gdb::function_view<void (ptid_t)> callback);
 }

 #endif
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 06/10] Add netbsd_nat::enable_proc_events in gdb/nat
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (4 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 05/10] Add gdb/nat common functions for listing threads Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 07/10] Add a common utility function to read and write siginfo_t in inferior Kamil Rytarowski
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

Add generic function to enable debugger events in a process.

gdb/ChangeLog:

        * netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
        * netbsd-nat.c (netbsd_nat::enable_proc_events): Likewise.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
 gdb/nat/netbsd-nat.h |  2 ++
 3 files changed, 25 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8b9ca1e9b32..06d3476c137 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
+	* netbsd-nat.c (netbsd_nat::enable_proc_events): Likewise.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::thread_alive, netbsd_nat::thread_name)
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 5f005c38bf2..88c70ebe1bb 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -22,6 +22,7 @@
 #include "gdbsupport/common-debug.h"

 #include <sys/types.h>
+#include <sys/ptrace.h>
 #include <sys/sysctl.h>

 #include <cstring>
@@ -162,4 +163,21 @@ list_threads (pid_t pid, gdb::function_view<void (ptid_t)> callback)
   netbsd_thread_lister (pid, fn);
 }

+/* Enable additional event reporting in a new process specified by PID.  */
+
+void
+enable_proc_events (pid_t pid)
+{
+  int events;
+
+  if (ptrace (PT_GET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
+    perror_with_name (("ptrace"));
+
+  events |= PTRACE_LWP_CREATE;
+  events |= PTRACE_LWP_EXIT;
+
+  if (ptrace (PT_SET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
+    perror_with_name (("ptrace"));
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index b6885e1a9a6..c93e5d4822d 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -35,6 +35,8 @@ extern const char *thread_name (ptid_t ptid);

 extern void list_threads (pid_t pid,
 			  gdb::function_view<void (ptid_t)> callback);
+
+extern void enable_proc_events (pid_t pid);
 }

 #endif
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 07/10] Add a common utility function to read and write siginfo_t in inferior
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (5 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 06/10] Add netbsd_nat::enable_proc_events in gdb/nat Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 08/10] Avoid double free in startup_inferior Kamil Rytarowski
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

gdb/ChangeLog:

        * netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
        * netbsd-nat.c (netbsd_nat::qxfer_siginfo): Likewise.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 30 ++++++++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h |  4 ++++
 3 files changed, 39 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 06d3476c137..1dcf5d0ca81 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
+	* netbsd-nat.c (netbsd_nat::qxfer_siginfo): Likewise.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 88c70ebe1bb..fef273a125a 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -180,4 +180,34 @@ enable_proc_events (pid_t pid)
     perror_with_name (("ptrace"));
 }

+/* Implement reading and writing of inferior's siginfo_t specified by PID.
+   Returns -1 on failure and the number of bytes on a successful transfer.  */
+
+int
+qxfer_siginfo (pid_t pid, const char *annex, unsigned char *readbuf,
+	       unsigned const char *writebuf, CORE_ADDR offset, int len)
+{
+  ptrace_siginfo_t psi;
+
+  if (offset > sizeof (siginfo_t))
+    return -1;
+
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    return -1;
+
+  if (offset + len > sizeof (siginfo_t))
+    len = sizeof (siginfo_t) - offset;
+
+  if (readbuf != NULL)
+    memcpy (readbuf, ((gdb_byte *) &psi.psi_siginfo) + offset, len);
+  else
+    {
+      memcpy (((gdb_byte *) &psi.psi_siginfo) + offset, writebuf, len);
+
+      if (ptrace (PT_SET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+	return -1;
+    }
+  return len;
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index c93e5d4822d..9bd4e19697e 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -37,6 +37,10 @@ extern void list_threads (pid_t pid,
 			  gdb::function_view<void (ptid_t)> callback);

 extern void enable_proc_events (pid_t pid);
+
+extern int qxfer_siginfo (pid_t pid, const char *annex, unsigned char *readbuf,
+			  unsigned const char *writebuf, CORE_ADDR offset,
+			  int len);
 }

 #endif
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 08/10] Avoid double free in startup_inferior
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (6 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 07/10] Add a common utility function to read and write siginfo_t in inferior Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 09/10] Switch local native code to gdb/nat shared functions Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver Kamil Rytarowski
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

Do not free the last execd pathname as it will be used in
prepare_resume_reply(), after attaching a client side.

gdb/ChangeLog:

	* fork-inferior.c (startup_inferior): Avoid double free.
---
 gdb/ChangeLog           | 4 ++++
 gdb/nat/fork-inferior.c | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1dcf5d0ca81..c57504a6ceb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* fork-inferior.c (startup_inferior): Avoid double free.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 1185ef8998b..94ab0b9cbc2 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,

 	  case TARGET_WAITKIND_EXECD:
 	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
-	    xfree (ws.value.execd_pathname);
+	    /* Do not free the last execd pathname as it will be used in
+	       prepare_resume_reply(), after attaching a client side.  */
+	    if (pending_execs != 1)
+	      xfree (ws.value.execd_pathname);
 	    resume_signal = GDB_SIGNAL_TRAP;
 	    switch_to_thread (proc_target, event_ptid);
 	    break;
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 09/10] Switch local native code to gdb/nat shared functions
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (7 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 08/10] Avoid double free in startup_inferior Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-02 17:59 ` [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver Kamil Rytarowski
  9 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

No functional change as the same functionality inlined in nbsd-nat.c
is offered in gdb/nat/netbsd-nat.c.

gdb/ChangeLog:

	* nbsd-nat.c: Include "nat/netbsd-nat.h".
	* (nbsd_nat_target::pid_to_exec_file)
	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
	(nbsd_nat_target::post_startup_inferior)
	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
	(nbsd_add_threads): Switch local code to common gdb/nat functions.
	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
	* (nbsd_thread_lister): Remove.
---
 gdb/ChangeLog  |  11 ++++
 gdb/nbsd-nat.c | 147 +++++--------------------------------------------
 2 files changed, 24 insertions(+), 134 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c57504a6ceb..897449a1313 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "nat/netbsd-nat.h".
+	* (nbsd_nat_target::pid_to_exec_file)
+	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
+	(nbsd_nat_target::post_startup_inferior)
+	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
+	(nbsd_add_threads): Switch local code to common gdb/nat functions.
+	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
+	* (nbsd_thread_lister): Remove.
+
 2020-08-13  Kamil Rytarowski  <n54@gmx.com>

 	* fork-inferior.c (startup_inferior): Avoid double free.
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index 52c4d185695..96af49de952 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -20,6 +20,7 @@
 #include "defs.h"

 #include "nbsd-nat.h"
+#include "nat/netbsd-nat.h"
 #include "gdbthread.h"
 #include "nbsd-tdep.h"
 #include "inferior.h"
@@ -36,13 +37,7 @@
 char *
 nbsd_nat_target::pid_to_exec_file (int pid)
 {
-  static char buf[PATH_MAX];
-  size_t buflen;
-  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
-  buflen = sizeof (buf);
-  if (sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
-    return NULL;
-  return buf;
+  return netbsd_nat::pid_to_exec_file (pid);
 }

 /* Return the current directory for the process identified by PID.  */
@@ -80,12 +75,12 @@ nbsd_pid_to_cmdline (int pid)
   int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_ARGV};

   size_t size = 0;
-  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
+  if (::sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
     return nullptr;

   gdb::unique_xmalloc_ptr<char[]> args (XNEWVAR (char, size));

-  if (sysctl (mib, ARRAY_SIZE (mib), args.get (), &size, NULL, 0) == -1
+  if (::sysctl (mib, ARRAY_SIZE (mib), args.get (), &size, NULL, 0) == -1
       || size == 0)
     return nullptr;

@@ -99,76 +94,12 @@ nbsd_pid_to_cmdline (int pid)
   return args;
 }

-/* Generic thread (LWP) lister within a specified process.  The callback
-   parameters is a C++ function that is called for each detected thread.  */
-
-static bool
-nbsd_thread_lister (const pid_t pid,
-		    gdb::function_view<bool (const struct kinfo_lwp *)>
-		    callback)
-{
-  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
-  size_t size;
-
-  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
-    perror_with_name (("sysctl"));
-
-  mib[4] = size / sizeof (size_t);
-
-  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
-    ((struct kinfo_lwp *) xcalloc (size, 1));
-
-  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
-      || size == 0)
-    perror_with_name (("sysctl"));
-
-  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
-    {
-      struct kinfo_lwp *l = &kl[i];
-
-      /* Return true if the specified thread is alive.  */
-      auto lwp_alive
-	= [] (struct kinfo_lwp *lwp)
-	  {
-	    switch (lwp->l_stat)
-	      {
-	      case LSSLEEP:
-	      case LSRUN:
-	      case LSONPROC:
-	      case LSSTOP:
-	      case LSSUSPENDED:
-		return true;
-	      default:
-		return false;
-	      }
-	  };
-
-      /* Ignore embryonic or demised threads.  */
-      if (!lwp_alive (l))
-	continue;
-
-      if (callback (l))
-	return true;
-    }
-
-  return false;
-}
-
 /* Return true if PTID is still active in the inferior.  */

 bool
 nbsd_nat_target::thread_alive (ptid_t ptid)
 {
-  pid_t pid = ptid.pid ();
-  int lwp = ptid.lwp ();
-
-  auto fn
-    = [&lwp] (const struct kinfo_lwp *kl)
-      {
-        return kl->l_lid == lwp;
-      };
-
-  return nbsd_thread_lister (pid, fn);
+  return netbsd_nat::thread_alive (ptid);
 }

 /* Return the name assigned to a thread by an application.  Returns
@@ -178,26 +109,7 @@ const char *
 nbsd_nat_target::thread_name (struct thread_info *thr)
 {
   ptid_t ptid = thr->ptid;
-  pid_t pid = ptid.pid ();
-  int lwp = ptid.lwp ();
-
-  static char buf[KI_LNAMELEN] = {};
-
-  auto fn
-    = [&lwp] (const struct kinfo_lwp *kl)
-      {
-	if (kl->l_lid == lwp)
-	  {
-	    xsnprintf (buf, sizeof buf, "%s", kl->l_name);
-	    return true;
-	  }
-	return false;
-      };
-
-  if (nbsd_thread_lister (pid, fn))
-    return buf;
-  else
-    return NULL;
+  return netbsd_nat::thread_name (ptid);
 }

 /* Implement the "post_attach" target_ops method.  */
@@ -206,9 +118,8 @@ static void
 nbsd_add_threads (nbsd_nat_target *target, pid_t pid)
 {
   auto fn
-    = [&target, &pid] (const struct kinfo_lwp *kl)
+    = [&target] (ptid_t ptid)
       {
-	ptid_t ptid = ptid_t (pid, kl->l_lid, 0);
 	if (!in_thread_list (target, ptid))
 	  {
 	    if (inferior_ptid.lwp () == 0)
@@ -216,27 +127,9 @@ nbsd_add_threads (nbsd_nat_target *target, pid_t pid)
 	    else
 	      add_thread (target, ptid);
 	  }
-	return false;
       };

-  nbsd_thread_lister (pid, fn);
-}
-
-/* Enable additional event reporting on new processes.  */
-
-static void
-nbsd_enable_proc_events (pid_t pid)
-{
-  int events;
-
-  if (ptrace (PT_GET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
-    perror_with_name (("ptrace"));
-
-  events |= PTRACE_LWP_CREATE;
-  events |= PTRACE_LWP_EXIT;
-
-  if (ptrace (PT_SET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
-    perror_with_name (("ptrace"));
+  netbsd_nat::list_threads (pid, fn);
 }

 /* Implement the "post_startup_inferior" target_ops method.  */
@@ -244,7 +137,7 @@ nbsd_enable_proc_events (pid_t pid)
 void
 nbsd_nat_target::post_startup_inferior (ptid_t ptid)
 {
-  nbsd_enable_proc_events (ptid.pid ());
+  netbsd_nat::enable_proc_events (ptid.pid ());
 }

 /* Implement the "post_attach" target_ops method.  */
@@ -252,7 +145,7 @@ nbsd_nat_target::post_startup_inferior (ptid_t ptid)
 void
 nbsd_nat_target::post_attach (int pid)
 {
-  nbsd_enable_proc_events (pid);
+  netbsd_nat::enable_proc_events (pid);
   nbsd_add_threads (this, pid);
 }

@@ -861,26 +754,12 @@ nbsd_nat_target::xfer_partial (enum target_object object,
     {
     case TARGET_OBJECT_SIGNAL_INFO:
       {
-	ptrace_siginfo_t psi;
+	len = netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset,
+					len);

-	if (offset > sizeof (siginfo_t))
+	if (len == -1)
 	  return TARGET_XFER_E_IO;

-	if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
-	  return TARGET_XFER_E_IO;
-
-	if (offset + len > sizeof (siginfo_t))
-	  len = sizeof (siginfo_t) - offset;
-
-	if (readbuf != NULL)
-	  memcpy (readbuf, ((gdb_byte *) &psi.psi_siginfo) + offset, len);
-	else
-	  {
-	    memcpy (((gdb_byte *) &psi.psi_siginfo) + offset, writebuf, len);
-
-	    if (ptrace (PT_SET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
-	      return TARGET_XFER_E_IO;
-	  }
 	*xfered_len = len;
 	return TARGET_XFER_OK;
       }
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
                   ` (8 preceding siblings ...)
  2020-09-02 17:59 ` [PATCH 09/10] Switch local native code to gdb/nat shared functions Kamil Rytarowski
@ 2020-09-02 17:59 ` Kamil Rytarowski
  2020-09-03 17:42   ` Aktemur, Tankut Baris
  2020-09-16 16:08   ` Tom Tromey
  9 siblings, 2 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-02 17:59 UTC (permalink / raw)
  To: gdb-patches

Implement the following functionality: create_inferior,
post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
resume, wait, fetch_registers, store_registers, read_memory, write_memory,
request_interrupt, supports_read_auxv, read_auxv,
supports_hardware_single_step, sw_breakpoint_from_kind,
supports_z_point_type, insert_point, remove_point,
stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
supports_stopped_by_sw_breakpoint, supports_non_stop,
supports_multi_process, supports_fork_events, supports_vfork_events,
supports_exec_events, supports_disable_randomization,
supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
supports_pid_to_exec_file, pid_to_exec_file, thread_name,
supports_catch_syscall.

The only CPU architecture supported: x86_64.

Implement only support for hardware assisted single step and
software breakpoint.

Implement support only for regular X86 registers, thus no FPU.

gdb/ChangeLog:

       * netbsd-low.cc: Add.
       * netbsd-low.h: Likewise.
       * netbsd-x86_64-low.cc: Likewise.
       * Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
       "netbsd-x86_64-low.cc".
       * configure.srv: Add x86_64-*-netbsd*.
---
 gdbserver/ChangeLog            |    9 +
 gdbserver/Makefile.in          |    3 +
 gdbserver/configure.srv        |    7 +
 gdbserver/netbsd-low.cc        | 1352 ++++++++++++++++++++++++++++++++
 gdbserver/netbsd-low.h         |  157 ++++
 gdbserver/netbsd-x86_64-low.cc |  250 ++++++
 6 files changed, 1778 insertions(+)
 create mode 100644 gdbserver/netbsd-low.cc
 create mode 100644 gdbserver/netbsd-low.h
 create mode 100644 gdbserver/netbsd-x86_64-low.cc

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index e437493b56d..c0732d1a28a 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,12 @@
+2020-08-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc: Add.
+	* netbsd-low.h: Likewise.
+	* netbsd-x86_64-low.cc: Likewise.
+	* Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
+	"netbsd-x86_64-low.cc".
+	* configure.srv: Add x86_64-*-netbsd*.
+
 2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>

 	* server.cc (captured_main): Accept multiple `--selftest=`
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 9d7687be534..1d597b14523 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -193,6 +193,9 @@ SFILES = \
 	$(srcdir)/linux-x86-low.cc \
 	$(srcdir)/linux-xtensa-low.cc \
 	$(srcdir)/mem-break.cc \
+	$(srcdir)/netbsd-low.cc \
+	$(srcdir)/netbsd-low.h \
+	$(srcdir)/netbsd-x86_64-low.cc \
 	$(srcdir)/proc-service.cc \
 	$(srcdir)/proc-service.list \
 	$(srcdir)/regcache.cc \
diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index 5e33bd9c54d..12f9d410b7c 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -349,6 +349,13 @@ case "${gdbserver_host}" in
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
 			;;
+  x86_64-*-netbsd*)	srv_regobj=""
+			srv_tgtobj="netbsd-low.o netbsd-x86_64-low.o fork-child.o"
+			srv_tgtobj="${srv_tgtobj} nat/fork-inferior.o"
+			srv_tgtobj="${srv_tgtobj} nat/netbsd-nat.o"
+			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
+			srv_netbsd=yes
+			;;

   xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
 			srv_tgtobj="$srv_linux_obj linux-xtensa-low.o"
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
new file mode 100644
index 00000000000..db9531c66d3
--- /dev/null
+++ b/gdbserver/netbsd-low.cc
@@ -0,0 +1,1352 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "server.h"
+#include "target.h"
+#include "netbsd-low.h"
+#include "nat/netbsd-nat.h"
+
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include <sys/ptrace.h>
+#include <sys/sysctl.h>
+
+#include <limits.h>
+#include <unistd.h>
+#include <signal.h>
+
+#include <elf.h>
+
+#include <type_traits>
+
+#include "gdbsupport/eintr.h"
+#include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/filestuff.h"
+#include "gdbsupport/common-inferior.h"
+#include "nat/fork-inferior.h"
+#include "hostio.h"
+
+int using_threads = 1;
+
+const struct target_desc *netbsd_tdesc;
+
+/* Call add_process with the given parameters, and initializes
+   the process' private data.  */
+
+static struct process_info *
+netbsd_add_process (int pid, int attached)
+{
+  struct process_info *proc = add_process (pid, attached);
+  proc->tdesc = netbsd_tdesc;
+  proc->priv = nullptr;
+
+  return proc;
+}
+
+/* Callback used by fork_inferior to start tracing the inferior.  */
+
+static void
+netbsd_ptrace_fun ()
+{
+  /* Switch child to its own process group so that signals won't
+     directly affect GDBserver. */
+  if (setpgid (0, 0) < 0)
+    trace_start_error_with_name (("setpgid"));
+
+  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
+    trace_start_error_with_name (("ptrace"));
+
+  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
+     stdout to stderr so that inferior i/o doesn't corrupt the connection.
+     Also, redirect stdin to /dev/null.  */
+  if (remote_connection_is_stdio ())
+    {
+      if (close (0) < 0)
+	trace_start_error_with_name (("close"));
+      if (open ("/dev/null", O_RDONLY) < 0)
+	trace_start_error_with_name (("open"));
+      if (dup2 (2, 1) < 0)
+	trace_start_error_with_name (("dup2"));
+      if (write (2, "stdin/stdout redirected\n",
+		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
+	{
+	  /* Errors ignored.  */;
+	}
+    }
+}
+
+/* Register threads in the process specified by PID.  */
+
+static void
+netbsd_add_threads (pid_t pid)
+{
+  auto fn
+    = [] (ptid_t ptid)
+      {
+	add_thread (ptid, nullptr);
+      };
+
+  netbsd_nat::list_threads (pid, fn);
+}
+
+/* Implement the create_inferior method of the target_ops vector.  */
+
+int
+netbsd_process_target::create_inferior (const char *program,
+					const std::vector<char *> &program_args)
+{
+  std::string str_program_args = construct_inferior_arguments (program_args);
+
+  pid_t pid = fork_inferior (program, str_program_args.c_str (),
+			     get_environ ()->envp (), netbsd_ptrace_fun,
+			     nullptr, nullptr, nullptr, nullptr);
+
+  netbsd_add_process (pid, 0);
+
+  post_fork_inferior (pid, program);
+
+  return pid;
+}
+
+/* Implement the post_create_inferior target_ops method.  */
+
+void
+netbsd_process_target::post_create_inferior ()
+{
+  pid_t pid = current_process ()->pid;
+  netbsd_nat::enable_proc_events (pid);
+}
+
+/* Implement the attach target_ops method.  */
+
+int
+netbsd_process_target::attach (unsigned long pid)
+{
+
+  if (ptrace (PT_ATTACH, pid, nullptr, 0) != 0)
+    error ("Cannot attach to process %lu: %s (%d)\n", pid,
+	    safe_strerror (errno), errno);
+
+  netbsd_add_process (pid, 1);
+  netbsd_nat::enable_proc_events (pid);
+  netbsd_add_threads (pid);
+
+  return 0;
+}
+
+/* Returns true if GDB is interested in any child syscalls.  */
+
+static bool
+gdb_catching_syscalls_p (pid_t pid)
+{
+  struct process_info *proc = find_process_pid (pid);
+  return !proc->syscalls_to_catch.empty ();
+}
+
+/* Implement the resume target_ops method.  */
+
+void
+netbsd_process_target::resume (struct thread_resume *resume_info, size_t n)
+{
+  ptid_t resume_ptid = resume_info[0].thread;
+  const int signal = resume_info[0].sig;
+  const bool step = resume_info[0].kind == resume_step;
+
+  if (resume_ptid == minus_one_ptid)
+    resume_ptid = ptid_of (current_thread);
+
+  const pid_t pid = resume_ptid.pid ();
+  const lwpid_t lwp = resume_ptid.lwp ();
+  regcache_invalidate_pid (pid);
+
+  auto fn
+    = [&] (ptid_t ptid)
+      {
+	if (step)
+	  {
+	    if (ptid.lwp () == lwp || n != 1)
+	      {
+		if (ptrace (PT_SETSTEP, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+		if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+	      }
+	    else
+	      {
+		if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+		if (ptrace (PT_SUSPEND, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+	      }
+	  }
+	else
+	  {
+	    if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
+	      perror_with_name (("ptrace"));
+	    if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
+	      perror_with_name (("ptrace"));
+	  }
+      };
+
+  netbsd_nat::list_threads (pid, fn);
+
+  int request = gdb_catching_syscalls_p (pid) ? PT_CONTINUE : PT_SYSCALL;
+
+  errno = 0;
+  ptrace (request, pid, (void *)1, signal);
+  if (errno)
+    perror_with_name (("ptrace"));
+}
+
+/* Returns true if GDB is interested in the reported SYSNO syscall.  */
+
+static bool
+netbsd_catch_this_syscall (int sysno)
+{
+  struct process_info *proc = current_process ();
+
+  if (proc->syscalls_to_catch.empty ())
+    return false;
+
+  if (proc->syscalls_to_catch[0] == ANY_SYSCALL)
+    return true;
+
+  for (int iter : proc->syscalls_to_catch)
+    if (iter == sysno)
+      return true;
+
+  return false;
+}
+
+/* Helper function for child_wait and the derivatives of child_wait.
+   HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
+   translation of that in OURSTATUS.  */
+static void
+netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
+{
+  if (WIFEXITED (hoststatus))
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXITED;
+      ourstatus->value.integer = WEXITSTATUS (hoststatus);
+    }
+  else if (!WIFSTOPPED (hoststatus))
+    {
+      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (hoststatus));
+    }
+  else
+    {
+      ourstatus->kind = TARGET_WAITKIND_STOPPED;
+      ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (hoststatus));
+    }
+}
+
+/* Implement a safe wrapper around waitpid().  */
+
+static pid_t
+netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  int status;
+
+  pid_t pid = gdb::handle_eintr (::waitpid, ptid.pid (), &status, options);
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  netbsd_store_waitstatus (ourstatus, status);
+  return pid;
+}
+
+
+/* Implement the wait target_ops method.
+
+   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.  */
+
+static ptid_t
+netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+	     int target_options)
+{
+  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
+  ptid_t wptid = ptid_t (pid);
+
+  if (pid == 0)
+    {
+      gdb_assert (target_options & TARGET_WNOHANG);
+      ourstatus->kind = TARGET_WAITKIND_IGNORE;
+      return null_ptid;
+    }
+
+  gdb_assert (pid != -1);
+
+  /* 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;
+
+  lwpid_t 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;
+
+  /* Process state for threading events */
+  ptrace_state_t pst = {};
+  if (code == TRAP_LWP)
+    {
+      if (ptrace (PT_GET_PROCESS_STATE, pid, &pst, sizeof (pst)) == -1)
+	perror_with_name (("ptrace"));
+    }
+
+  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_EXIT)
+    {
+      /* If GDB attaches to a multi-threaded process, exiting
+	 threads might be skipped during post_attach that
+	 have not yet reported their PTRACE_LWP_EXIT event.
+	 Ignore exited events for an unknown LWP.  */
+      thread_info *thr = find_thread_ptid (wptid);
+      if (thr == nullptr)
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+      else
+	{
+	  ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
+	  /* NetBSD does not store an LWP exit status.  */
+	  ourstatus->value.integer = 0;
+
+	  remove_thread (thr);
+	}
+      return wptid;
+    }
+
+  if (find_thread_ptid (ptid_t (pid)))
+    current_thread = find_thread_ptid (wptid);
+
+  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_CREATE)
+    {
+      /* If GDB attaches to a multi-threaded process, newborn
+	 threads might be added by nbsd_add_threads that have
+	 not yet reported their PTRACE_LWP_CREATE event.  Ignore
+	 born events for an already-known LWP.  */
+      if (find_thread_ptid (wptid))
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+      else
+	{
+	  add_thread (wptid, NULL);
+	  ourstatus->kind = TARGET_WAITKIND_THREAD_CREATED;
+	}
+      return wptid;
+    }
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname
+	= xstrdup (netbsd_nat::pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+    {
+      return wptid;
+    }
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!netbsd_catch_this_syscall(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)
+    {
+#ifdef PTRACE_BREAKPOINT_ADJ
+      CORE_ADDR pc;
+      struct reg r;
+      ptrace (PT_GETREGS, pid, &r, psi.psi_lwpid);
+      pc = PTRACE_REG_PC (&r);
+      PTRACE_REG_SET_PC (&r, pc - PTRACE_BREAKPOINT_ADJ);
+      ptrace (PT_SETREGS, pid, &r, psi.psi_lwpid);
+#endif
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the wait target_ops method.  */
+
+ptid_t
+netbsd_process_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+			     int target_options)
+{
+  while (true)
+    {
+      ptid_t wptid = netbsd_wait (ptid, ourstatus, target_options);
+
+      /* Register thread in the gdbcore if a thread was not reported earlier.
+	 This is required after ::create_inferior, when the gdbcore does not
+	 know about the first internal thread.
+	 This may also happen on attach, when an event is registered on a thread
+	 that was not fully initialized during the attach stage. */
+      if (wptid.lwp () != 0 && !find_thread_ptid (wptid)
+	  && ourstatus->kind != TARGET_WAITKIND_THREAD_EXITED)
+	add_thread (wptid, nullptr);
+
+      switch (ourstatus->kind)
+	{
+	case TARGET_WAITKIND_EXITED:
+	case TARGET_WAITKIND_STOPPED:
+	case TARGET_WAITKIND_SIGNALLED:
+	case TARGET_WAITKIND_FORKED:
+	case TARGET_WAITKIND_VFORKED:
+	case TARGET_WAITKIND_EXECD:
+	case TARGET_WAITKIND_VFORK_DONE:
+	case TARGET_WAITKIND_SYSCALL_ENTRY:
+	case TARGET_WAITKIND_SYSCALL_RETURN:
+	  /* Pass the result to the generic code.  */
+	  return wptid;
+	case TARGET_WAITKIND_THREAD_CREATED:
+	case TARGET_WAITKIND_THREAD_EXITED:
+	  /* The core needlessly stops on these events.  */
+	  /* FALLTHROUGH */
+	case TARGET_WAITKIND_SPURIOUS:
+	  /* Spurious events are unhandled by the gdbserver core.  */
+	  if (ptrace (PT_CONTINUE, current_process ()->pid, (void *) 1, 0)
+	      == -1)
+	    perror_with_name (("ptrace"));
+	  break;
+	default:
+	  error (("Unknwon stopped status"));
+	}
+    }
+}
+
+/* Implement the kill target_ops method.  */
+
+int
+netbsd_process_target::kill (process_info *process)
+{
+  pid_t pid = process->pid;
+  ptrace (PT_KILL, pid, nullptr, 0);
+
+  int status;
+  gdb::handle_eintr (::waitpid, pid, &status, 0);
+  mourn (process);
+  return 0;
+}
+
+/* Implement the detach target_ops method.  */
+
+int
+netbsd_process_target::detach (process_info *process)
+{
+  pid_t pid = process->pid;
+
+  ptrace (PT_DETACH, pid, (void *) 1, 0);
+  mourn (process);
+  return 0;
+}
+
+/* Implement the mourn target_ops method.  */
+
+void
+netbsd_process_target::mourn (struct process_info *proc)
+{
+  for_each_thread (proc->pid, remove_thread);
+
+  remove_process (proc);
+}
+
+/* Implement the join target_ops method.  */
+
+void
+netbsd_process_target::join (int pid)
+{
+  /* The PT_DETACH is sufficient to detach from the process.
+     So no need to do anything extra.  */
+}
+
+/* Implement the thread_alive target_ops method.  */
+
+bool
+netbsd_process_target::thread_alive (ptid_t ptid)
+{
+  return netbsd_nat::thread_alive (ptid);
+}
+
+/* Implement the fetch_registers target_ops method.  */
+
+void
+netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
+{
+  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  ptid_t inferior_ptid = ptid_of (current_thread);
+
+  while (regset->size >= 0)
+    {
+      char *buf = (char *) xmalloc (regset->size);
+      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+      if (res == -1)
+	perror_with_name (("ptrace"));
+      regset->store_function (regcache, buf);
+      free (buf);
+      regset++;
+    }
+}
+
+/* Implement the store_registers target_ops method.  */
+
+void
+netbsd_process_target::store_registers (struct regcache *regcache, int regno)
+{
+  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  ptid_t inferior_ptid = ptid_of (current_thread);
+
+  while (regset->size >= 0)
+    {
+      char *buf = (char *)xmalloc (regset->size);
+      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+      if (res == -1)
+	perror_with_name (("ptrace"));
+      if (res == 0)
+	{
+	  /* Then overlay our cached registers on that.  */
+	  regset->fill_function (regcache, buf);
+	  /* Only now do we write the register set.  */
+	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+	}
+      free (buf);
+      regset++;
+    }
+}
+
+/* Implement the read_memory target_ops method.  */
+
+int
+netbsd_process_target::read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
+				    int size)
+{
+  struct ptrace_io_desc io;
+  io.piod_op = PIOD_READ_D;
+  io.piod_len = size;
+
+  pid_t pid = current_process ()->pid;
+
+  int bytes_read = 0;
+
+  if (size == 0)
+    {
+      /* Zero length write always succeeds.  */
+      return 0;
+    }
+  do
+    {
+      io.piod_offs = (void *)(memaddr + bytes_read);
+      io.piod_addr = myaddr + bytes_read;
+
+      int rv = ptrace (PT_IO, pid, &io, 0);
+      if (rv == -1)
+	return errno;
+      if (io.piod_len == 0)
+	return 0;
+
+      bytes_read += io.piod_len;
+      io.piod_len = size - bytes_read;
+    }
+  while (bytes_read < size);
+
+  return 0;
+}
+
+/* Implement the write_memory target_ops method.  */
+
+int
+netbsd_process_target::write_memory (CORE_ADDR memaddr,
+				     const unsigned char *myaddr, int size)
+{
+  struct ptrace_io_desc io;
+  io.piod_op = PIOD_WRITE_D;
+  io.piod_len = size;
+
+  pid_t pid = current_process ()->pid;
+
+  int bytes_written = 0;
+
+  if (size == 0)
+    {
+      /* Zero length write always succeeds.  */
+      return 0;
+    }
+
+  do
+    {
+      io.piod_addr = (void *)(myaddr + bytes_written);
+      io.piod_offs = (void *)(memaddr + bytes_written);
+
+      int rv = ptrace (PT_IO, pid, &io, 0);
+      if (rv == -1)
+	return errno;
+      if (io.piod_len == 0)
+	return 0;
+
+      bytes_written += io.piod_len;
+      io.piod_len = size - bytes_written;
+    }
+  while (bytes_written < size);
+
+  return 0;
+}
+
+/* Implement the kill_request target_ops method.  */
+
+void
+netbsd_process_target::request_interrupt ()
+{
+  ptid_t inferior_ptid = ptid_of (get_first_thread ());
+
+  ::kill (inferior_ptid.pid(), SIGINT);
+}
+
+static size_t
+netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
+{
+  struct ptrace_io_desc pio;
+
+  pio.piod_op = PIOD_READ_AUXV;
+  pio.piod_offs = offs;
+  pio.piod_addr = addr;
+  pio.piod_len = len;
+
+  if (ptrace (PT_IO, pid, &pio, 0) == -1)
+    perror_with_name (("ptrace"));
+
+  return pio.piod_len;
+}
+
+/* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
+   to debugger memory starting at MYADDR.  */
+
+int
+netbsd_process_target::read_auxv (CORE_ADDR offset,
+				  unsigned char *myaddr, unsigned int len)
+{
+  pid_t pid = pid_of (current_thread);
+
+  return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
+}
+
+bool
+netbsd_process_target::supports_z_point_type (char z_type)
+{
+  switch (z_type)
+    {
+    case Z_PACKET_SW_BP:
+      return true;
+    case Z_PACKET_HW_BP:
+    case Z_PACKET_WRITE_WP:
+    case Z_PACKET_READ_WP:
+    case Z_PACKET_ACCESS_WP:
+    default:
+      return false; /* Not supported.  */
+    }
+}
+
+/* Insert {break/watch}point at address ADDR.  SIZE is not used.  */
+
+int
+netbsd_process_target::insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		     int size, struct raw_breakpoint *bp)
+{
+  switch (type)
+    {
+    case raw_bkpt_type_sw:
+      return insert_memory_breakpoint (bp);
+    case raw_bkpt_type_hw:
+    case raw_bkpt_type_write_wp:
+    case raw_bkpt_type_read_wp:
+    case raw_bkpt_type_access_wp:
+    default:
+      return 1; /* Not supported.  */
+    }
+}
+
+/* Remove {break/watch}point at address ADDR.  SIZE is not used.  */
+
+int
+netbsd_process_target::remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
+				     int size, struct raw_breakpoint *bp)
+{
+  switch (type)
+    {
+    case raw_bkpt_type_sw:
+      return remove_memory_breakpoint (bp);
+    case raw_bkpt_type_hw:
+    case raw_bkpt_type_write_wp:
+    case raw_bkpt_type_read_wp:
+    case raw_bkpt_type_access_wp:
+    default:
+      return 1; /* Not supported.  */
+    }
+}
+
+/* Implement the to_stopped_by_sw_breakpoint target_ops
+   method.  */
+
+bool
+netbsd_process_target::stopped_by_sw_breakpoint ()
+{
+  ptrace_siginfo_t psi;
+  pid_t pid = current_process ()->pid;
+
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  return psi.psi_siginfo.si_signo == SIGTRAP &&
+	 psi.psi_siginfo.si_code == TRAP_BRKPT;
+}
+
+/* Implement the to_supports_stopped_by_sw_breakpoint target_ops
+   method.  */
+
+bool
+netbsd_process_target::supports_stopped_by_sw_breakpoint ()
+{
+  return true;
+}
+
+/* Implement the supports_qxfer_siginfo target_ops method.  */
+
+bool
+netbsd_process_target::supports_qxfer_siginfo ()
+{
+  return true;
+}
+
+/* Implement the qxfer_siginfo target_ops method.  */
+
+int
+netbsd_process_target::qxfer_siginfo (const char *annex, unsigned char *readbuf,
+				      unsigned const char *writebuf,
+				      CORE_ADDR offset, int len)
+{
+  if (current_thread == nullptr)
+    return -1;
+
+  pid_t pid = current_process ()->pid;
+
+  return netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset, len);
+}
+
+/* Implement the supports_non_stop target_ops method.  */
+
+bool
+netbsd_process_target::supports_non_stop ()
+{
+  return false;
+}
+
+/* Implement the supports_multi_process target_ops method.  */
+
+bool
+netbsd_process_target::supports_multi_process ()
+{
+  return true;
+}
+
+/* Check if fork events are supported.  */
+
+bool
+netbsd_process_target::supports_fork_events ()
+{
+  return false;
+}
+
+/* Check if vfork events are supported.  */
+
+bool
+netbsd_process_target::supports_vfork_events ()
+{
+  return false;
+}
+
+/* Check if exec events are supported.  */
+
+bool
+netbsd_process_target::supports_exec_events ()
+{
+  return true;
+}
+
+/* Implement the supports_disable_randomization target_ops method.  */
+
+bool
+netbsd_process_target::supports_disable_randomization ()
+{
+  return false;
+}
+
+/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
+
+template <typename T>
+int get_phdr_phnum_from_proc_auxv (const pid_t pid,
+				   CORE_ADDR *phdr_memaddr, int *num_phdr)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Aux64Info, Aux32Info>::type auxv_type;
+  const size_t auxv_size = sizeof (auxv_type);
+  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
+
+  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
+
+  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
+
+  *phdr_memaddr = 0;
+  *num_phdr = 0;
+
+  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
+    {
+      auxv_type *const aux = (auxv_type *) buf;
+
+      switch (aux->a_type)
+	{
+	case AT_PHDR:
+	  *phdr_memaddr = aux->a_v;
+	  break;
+	case AT_PHNUM:
+	  *num_phdr = aux->a_v;
+	  break;
+	}
+
+      if (*phdr_memaddr != 0 && *num_phdr != 0)
+	break;
+    }
+
+  xfree (auxv_buf);
+
+  if (*phdr_memaddr == 0 || *num_phdr == 0)
+    {
+      warning ("Unexpected missing AT_PHDR and/or AT_PHNUM: "
+	       "phdr_memaddr = %ld, phdr_num = %d",
+	       (long) *phdr_memaddr, *num_phdr);
+      return 2;
+    }
+
+  return 0;
+}
+
+/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
+
+template <typename T>
+static CORE_ADDR
+get_dynamic (netbsd_process_target *target, const pid_t pid)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Elf64_Phdr, Elf32_Phdr>::type phdr_type;
+  const int phdr_size = sizeof (phdr_type);
+
+  CORE_ADDR phdr_memaddr;
+  int num_phdr;
+  if (get_phdr_phnum_from_proc_auxv<T> (pid, &phdr_memaddr, &num_phdr))
+    return 0;
+
+  std::vector<unsigned char> phdr_buf;
+  phdr_buf.resize (num_phdr * phdr_size);
+
+  if ((*target).read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
+    return 0;
+
+  /* Compute relocation: it is expected to be 0 for "regular" executables,
+     non-zero for PIE ones.  */
+  CORE_ADDR relocation = -1;
+  for (int i = 0; relocation == -1 && i < num_phdr; i++)
+    {
+      phdr_type *const p = (phdr_type *) (phdr_buf.data() + i * phdr_size);
+
+      if (p->p_type == PT_PHDR)
+	relocation = phdr_memaddr - p->p_vaddr;
+    }
+
+  if (relocation == -1)
+    {
+      /* PT_PHDR is optional, but necessary for PIE in general.  Fortunately
+	 any real world executables, including PIE executables, have always
+	 PT_PHDR present.  PT_PHDR is not present in some shared libraries or
+	 in fpc (Free Pascal 2.4) binaries but neither of those have a need for
+	 or present DT_DEBUG anyway (fpc binaries are statically linked).
+
+	 Therefore if there exists DT_DEBUG there is always also PT_PHDR.
+
+	 GDB could find RELOCATION also from AT_ENTRY - e_entry.  */
+
+      return 0;
+    }
+
+  for (int i = 0; i < num_phdr; i++)
+    {
+      phdr_type *const p = (phdr_type *) (phdr_buf.data () + i * phdr_size);
+
+      if (p->p_type == PT_DYNAMIC)
+	return p->p_vaddr + relocation;
+    }
+
+  return 0;
+}
+
+/* Return &_r_debug in the inferior, or -1 if not present.  Return value
+   can be 0 if the inferior does not yet have the library list initialized.
+   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
+   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
+
+template <typename T>
+static CORE_ADDR
+get_r_debug (netbsd_process_target *target, const int pid)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Elf64_Dyn, Elf32_Dyn>::type dyn_type;
+  const int dyn_size = sizeof (dyn_type);
+  unsigned char buf[sizeof (dyn_type)];  /* The larger of the two.  */
+  CORE_ADDR map = -1;
+
+  CORE_ADDR dynamic_memaddr = get_dynamic<T> (target, pid);
+  if (dynamic_memaddr == 0)
+    return map;
+
+  while ((*target).read_memory (dynamic_memaddr, buf, dyn_size) == 0)
+    {
+      dyn_type *const dyn = (dyn_type *) buf;
+#if defined DT_MIPS_RLD_MAP
+      union
+      {
+	T map;
+	unsigned char buf[sizeof (T)];
+      }
+      rld_map;
+
+      if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	{
+	  if (read_memory (dyn->d_un.d_val,
+			   rld_map.buf, sizeof (rld_map.buf)) == 0)
+	    return rld_map.map;
+	  else
+	    break;
+	}
+#endif  /* DT_MIPS_RLD_MAP */
+
+      if (dyn->d_tag == DT_DEBUG && map == -1)
+	map = dyn->d_un.d_val;
+
+      if (dyn->d_tag == DT_NULL)
+	break;
+
+      dynamic_memaddr += dyn_size;
+    }
+
+  return map;
+}
+
+/* Read one pointer from MEMADDR in the inferior.  */
+
+static int
+read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
+	      int ptr_size)
+{
+  /* Go through a union so this works on either big or little endian
+     hosts, when the inferior's pointer size is smaller than the size
+     of CORE_ADDR.  It is assumed the inferior's endianness is the
+     same of the superior's.  */
+
+  union
+  {
+    CORE_ADDR core_addr;
+    unsigned int ui;
+    unsigned char uc;
+  } addr;
+
+  int ret = (*target).read_memory (memaddr, &addr.uc, ptr_size);
+  if (ret == 0)
+    {
+      if (ptr_size == sizeof (CORE_ADDR))
+	*ptr = addr.core_addr;
+      else if (ptr_size == sizeof (unsigned int))
+	*ptr = addr.ui;
+      else
+	gdb_assert_not_reached ("unhandled pointer size");
+    }
+  return ret;
+}
+
+/* Construct qXfer:libraries-svr4:read reply.  */
+
+template <typename T>
+int
+netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
+			     const pid_t pid, const char *annex,
+			     unsigned char *readbuf,
+			     unsigned const char *writebuf,
+			     CORE_ADDR offset, int len)
+{
+  struct link_map_offsets
+  {
+    /* Offset and size of r_debug.r_version.  */
+    int r_version_offset;
+
+    /* Offset and size of r_debug.r_map.  */
+    int r_map_offset;
+
+    /* Offset to l_addr field in struct link_map.  */
+    int l_addr_offset;
+
+    /* Offset to l_name field in struct link_map.  */
+    int l_name_offset;
+
+    /* Offset to l_ld field in struct link_map.  */
+    int l_ld_offset;
+
+    /* Offset to l_next field in struct link_map.  */
+    int l_next_offset;
+
+    /* Offset to l_prev field in struct link_map.  */
+    int l_prev_offset;
+  };
+
+  static const struct link_map_offsets lmo_32bit_offsets =
+    {
+      0,     /* r_version offset. */
+      4,     /* r_debug.r_map offset.  */
+      0,     /* l_addr offset in link_map.  */
+      4,     /* l_name offset in link_map.  */
+      8,     /* l_ld offset in link_map.  */
+      12,    /* l_next offset in link_map.  */
+      16     /* l_prev offset in link_map.  */
+    };
+
+  static const struct link_map_offsets lmo_64bit_offsets =
+    {
+      0,     /* r_version offset. */
+      8,     /* r_debug.r_map offset.  */
+      0,     /* l_addr offset in link_map.  */
+      8,     /* l_name offset in link_map.  */
+      16,    /* l_ld offset in link_map.  */
+      24,    /* l_next offset in link_map.  */
+      32     /* l_prev offset in link_map.  */
+    };
+
+  CORE_ADDR lm_addr = 0, lm_prev = 0;
+  CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
+  int header_done = 0;
+
+  const struct link_map_offsets *lmo
+    = (sizeof (T) == sizeof (int64_t))
+    ? &lmo_64bit_offsets : &lmo_32bit_offsets;
+  int ptr_size = sizeof (T);
+
+  while (annex[0] != '\0')
+    {
+      const char *sep = strchr (annex, '=');
+      if (sep == nullptr)
+	break;
+
+      int name_len = sep - annex;
+      CORE_ADDR *addrp;
+      if (name_len == 5 && startswith (annex, "start"))
+	addrp = &lm_addr;
+      else if (name_len == 4 && startswith (annex, "prev"))
+	addrp = &lm_prev;
+      else
+	{
+	  annex = strchr (sep, ';');
+	  if (annex == nullptr)
+	    break;
+	  annex++;
+	  continue;
+	}
+
+      annex = decode_address_to_semicolon (addrp, sep + 1);
+    }
+
+  if (lm_addr == 0)
+    {
+      CORE_ADDR r_debug = get_r_debug<T> (target, pid);
+
+      /* We failed to find DT_DEBUG.  Such situation will not change
+	 for this inferior - do not retry it.  Report it to GDB as
+	 E01, see for the reasons at the GDB solib-svr4.c side.  */
+      if (r_debug == (CORE_ADDR) -1)
+	return -1;
+
+      if (r_debug != 0)
+	{
+	  if (read_one_ptr (target, r_debug + lmo->r_map_offset,
+			    &lm_addr, ptr_size) != 0)
+	    {
+	      warning ("unable to read r_map from 0x%lx",
+		       (long) r_debug + lmo->r_map_offset);
+	    }
+	}
+    }
+
+  std::string document = "<library-list-svr4 version=\"1.0\"";
+
+  while (lm_addr
+	 && read_one_ptr (target, lm_addr + lmo->l_name_offset,
+			  &l_name, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_addr_offset,
+			  &l_addr, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_ld_offset,
+			  &l_ld, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_prev_offset,
+			  &l_prev, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_next_offset,
+			  &l_next, ptr_size) == 0)
+    {
+      if (lm_prev != l_prev)
+	{
+	  warning ("Corrupted shared library list: 0x%lx != 0x%lx",
+		   (long) lm_prev, (long) l_prev);
+	  break;
+	}
+
+      /* Ignore the first entry even if it has valid name as the first entry
+	 corresponds to the main executable.  The first entry should not be
+	 skipped if the dynamic loader was loaded late by a static executable
+	 (see solib-svr4.c parameter ignore_first).  But in such case the main
+	 executable does not have PT_DYNAMIC present and this function already
+	 exited above due to failed get_r_debug.  */
+      if (lm_prev == 0)
+	string_appendf (document, " main-lm=\"0x%lx\"",
+			(unsigned long) lm_addr);
+      else
+	{
+	  unsigned char libname[PATH_MAX];
+
+	  /* Not checking for error because reading may stop before
+	     we've got PATH_MAX worth of characters.  */
+	  libname[0] = '\0';
+	  (*target).read_memory (l_name, libname, sizeof (libname) - 1);
+	  libname[sizeof (libname) - 1] = '\0';
+	  if (libname[0] != '\0')
+	    {
+	      if (!header_done)
+		{
+		  /* Terminate `<library-list-svr4'.  */
+		  document += '>';
+		  header_done = 1;
+		}
+
+	      string_appendf (document, "<library name=\"");
+	      xml_escape_text_append (&document, (char *) libname);
+	      string_appendf (document, "\" lm=\"0x%lx\" "
+			      "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
+			      (unsigned long) lm_addr, (unsigned long) l_addr,
+			      (unsigned long) l_ld);
+	    }
+	}
+
+      lm_prev = lm_addr;
+      lm_addr = l_next;
+    }
+
+  if (!header_done)
+    {
+      /* Empty list; terminate `<library-list-svr4'.  */
+      document += "/>";
+    }
+  else
+    document += "</library-list-svr4>";
+
+  int document_len = document.length ();
+  if (offset < document_len)
+    document_len -= offset;
+  else
+    document_len = 0;
+  if (len > document_len)
+    len = document_len;
+
+  memcpy (readbuf, document.data () + offset, len);
+
+  return len;
+}
+
+/* Return true if FILE is a 64-bit ELF file,
+   false if the file is not a 64-bit ELF file,
+   and error if the file is not accessible or doesn't exist.  */
+
+static bool
+elf_64_file_p (const char *file)
+{
+  int fd = gdb::handle_eintr (::open, file, O_RDONLY);
+  if (fd < 0)
+    perror_with_name (("open"));
+
+  Elf64_Ehdr header;
+  ssize_t ret = gdb::handle_eintr (::read, fd, &header, sizeof (header));
+  if (ret == -1)
+    perror_with_name (("read"));
+  gdb::handle_eintr (::close, fd);
+  if (ret != sizeof (header))
+    error ("Cannot read ELF file header: %s", file);
+
+  if (header.e_ident[EI_MAG0] != ELFMAG0
+      || header.e_ident[EI_MAG1] != ELFMAG1
+      || header.e_ident[EI_MAG2] != ELFMAG2
+      || header.e_ident[EI_MAG3] != ELFMAG3)
+    error ("Unrecognized ELF file header: %s", file);
+
+  return header.e_ident[EI_CLASS] == ELFCLASS64;
+}
+
+/* Construct qXfer:libraries-svr4:read reply.  */
+
+int
+netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
+					     unsigned char *readbuf,
+					     unsigned const char *writebuf,
+					     CORE_ADDR offset, int len)
+{
+  if (writebuf != nullptr)
+    return -2;
+  if (readbuf == nullptr)
+    return -1;
+
+  struct process_info *proc = current_process ();
+  pid_t pid = proc->pid;
+  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
+
+  if (is_elf64)
+    return netbsd_qxfer_libraries_svr4<int64_t> (this, pid, annex, readbuf,
+						 writebuf, offset, len);
+  else
+    return netbsd_qxfer_libraries_svr4<int32_t> (this, pid, annex, readbuf,
+						 writebuf, offset, len);
+}
+
+/* Implement the supports_qxfer_libraries_svr4 target_ops method.  */
+
+bool
+netbsd_process_target::supports_qxfer_libraries_svr4 ()
+{
+  return true;
+}
+
+/* Return the name of a file that can be opened to get the symbols for
+   the child process identified by PID.  */
+
+char *
+netbsd_process_target::pid_to_exec_file (pid_t pid)
+{
+  return netbsd_nat::pid_to_exec_file (pid);
+}
+
+/* Implementation of the target_ops method "supports_pid_to_exec_file".  */
+
+bool
+netbsd_process_target::supports_pid_to_exec_file ()
+{
+  return true;
+}
+
+/* Implementation of the target_ops method "supports_hardware_single_step".  */
+bool
+netbsd_process_target::supports_hardware_single_step ()
+{
+  return true;
+}
+
+/* Implementation of the target_ops method "sw_breakpoint_from_kind".  */
+
+const gdb_byte *
+netbsd_process_target::sw_breakpoint_from_kind (int kind, int *size)
+{
+  static gdb_byte brkpt[PTRACE_BREAKPOINT_SIZE];
+
+  *size = PTRACE_BREAKPOINT_SIZE;
+
+  memcpy (brkpt, PTRACE_BREAKPOINT, PTRACE_BREAKPOINT_SIZE);
+
+  return brkpt;
+}
+
+/* Implement the thread_name target_ops method.  */
+
+const char *
+netbsd_process_target::thread_name (ptid_t ptid)
+{
+  return netbsd_nat::thread_name (ptid);
+}
+
+/* Implement the supports_catch_syscall target_ops method.  */
+
+bool
+netbsd_process_target::supports_catch_syscall ()
+{
+  return true;
+}
+
+/* Implement the supports_read_auxv target_ops method.  */
+
+bool
+netbsd_process_target::supports_read_auxv ()
+{
+  return true;
+}
+
+/* The NetBSD target ops object.  */
+
+static netbsd_process_target the_netbsd_target;
+
+void
+initialize_low ()
+{
+  set_target_ops (&the_netbsd_target);
+  the_low_target.arch_setup ();
+}
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
new file mode 100644
index 00000000000..2bd57c51726
--- /dev/null
+++ b/gdbserver/netbsd-low.h
@@ -0,0 +1,157 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSERVER_NETBSD_LOW_H
+#define GDBSERVER_NETBSD_LOW_H
+
+struct regcache;
+struct target_desc;
+
+/*  Some information relative to a given register set.   */
+
+struct netbsd_regset_info
+{
+  /* The ptrace request needed to get/set registers of this set.  */
+  int get_request, set_request;
+  /* The size of the register set.  */
+  int size;
+  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
+  void (*fill_function) (struct regcache *regcache, char *buf);
+  /* Store the register value in BUF in the given REGCACHE.  */
+  void (*store_function) (struct regcache *regcache, const char *buf);
+};
+
+/* A list of regsets for the target being debugged, terminated by an entry
+   where the size is negative.
+
+   This list should be created by the target-specific code.  */
+
+extern struct netbsd_regset_info netbsd_target_regsets[];
+
+/* The target-specific operations for NetBSD support.  */
+
+struct netbsd_target_ops
+{
+  /* Architecture-specific setup.  */
+  void (*arch_setup) ();
+
+  /* Hook to support target specific qSupported.  */
+  void (*process_qsupported) (char **, int count);
+};
+
+/* Target ops definitions for a NetBSD target.  */
+
+class netbsd_process_target : public process_stratum_target
+{
+public:
+
+  int create_inferior (const char *program,
+		       const std::vector<char *> &program_args) override;
+
+  void post_create_inferior () override;
+
+  int attach (unsigned long pid) override;
+
+  int kill (process_info *proc) override;
+
+  int detach (process_info *proc) override;
+
+  void mourn (process_info *proc) override;
+
+  void join (int pid) override;
+
+  bool thread_alive (ptid_t pid) override;
+
+  void resume (thread_resume *resume_info, size_t n) override;
+
+  ptid_t wait (ptid_t ptid, target_waitstatus *status,
+	       int options) override;
+
+  void fetch_registers (regcache *regcache, int regno) override;
+
+  void store_registers (regcache *regcache, int regno) override;
+
+  int read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
+		   int len) override;
+
+  int write_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
+		    int len) override;
+
+  void request_interrupt () override;
+
+  bool supports_read_auxv () override;
+
+  int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
+		 unsigned int len) override;
+
+  bool supports_hardware_single_step () override;
+
+  const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
+
+  bool supports_z_point_type (char z_type) override;
+
+  int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		    int size, struct raw_breakpoint *bp) override;
+
+  int remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		    int size, struct raw_breakpoint *bp) override;
+
+  bool stopped_by_sw_breakpoint () override;
+
+  bool supports_qxfer_siginfo () override;
+
+  int qxfer_siginfo (const char *annex, unsigned char *readbuf,
+		     unsigned const char *writebuf, CORE_ADDR offset,
+		     int len) override;
+
+  bool supports_stopped_by_sw_breakpoint () override;
+
+  bool supports_non_stop () override;
+
+  bool supports_multi_process () override;
+
+  bool supports_fork_events () override;
+
+  bool supports_vfork_events () override;
+
+  bool supports_exec_events () override;
+
+  bool supports_disable_randomization () override;
+
+  bool supports_qxfer_libraries_svr4 () override;
+
+  int qxfer_libraries_svr4 (const char*, unsigned char*, const unsigned char*,
+			    CORE_ADDR, int) override;
+
+  bool supports_pid_to_exec_file () override;
+
+  char *pid_to_exec_file (int pid) override;
+
+  const char *thread_name (ptid_t thread) override;
+
+  bool supports_catch_syscall () override;
+};
+
+/* The inferior's target description.  This is a global because the
+   NetBSD ports support neither bi-arch nor multi-process.  */
+
+extern struct netbsd_target_ops the_low_target;
+
+/* XXX: multilib */
+extern const struct target_desc *netbsd_tdesc;
+
+#endif /* GDBSERVER_NETBSD_LOW_H */
diff --git a/gdbserver/netbsd-x86_64-low.cc b/gdbserver/netbsd-x86_64-low.cc
new file mode 100644
index 00000000000..c8679d9f153
--- /dev/null
+++ b/gdbserver/netbsd-x86_64-low.cc
@@ -0,0 +1,250 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/types.h>
+#include <sys/ptrace.h>
+#include <limits.h>
+
+#include "server.h"
+#include "netbsd-low.h"
+#include "gdbsupport/x86-xstate.h"
+#include "arch/amd64.h"
+#include "x86-tdesc.h"
+#include "tdesc.h"
+
+static int use_xml;
+
+/* The index of various registers inside the regcache.  */
+
+enum netbsd_x86_64_gdb_regnum
+{
+  AMD64_RAX_REGNUM,	     /* %rax */
+  AMD64_RBX_REGNUM,	     /* %rbx */
+  AMD64_RCX_REGNUM,	     /* %rcx */
+  AMD64_RDX_REGNUM,	     /* %rdx */
+  AMD64_RSI_REGNUM,	     /* %rsi */
+  AMD64_RDI_REGNUM,	     /* %rdi */
+  AMD64_RBP_REGNUM,	     /* %rbp */
+  AMD64_RSP_REGNUM,	     /* %rsp */
+  AMD64_R8_REGNUM,	      /* %r8 */
+  AMD64_R9_REGNUM,	      /* %r9 */
+  AMD64_R10_REGNUM,	     /* %r10 */
+  AMD64_R11_REGNUM,	     /* %r11 */
+  AMD64_R12_REGNUM,	     /* %r12 */
+  AMD64_R13_REGNUM,	     /* %r13 */
+  AMD64_R14_REGNUM,	     /* %r14 */
+  AMD64_R15_REGNUM,	     /* %r15 */
+  AMD64_RIP_REGNUM,	     /* %rip */
+  AMD64_EFLAGS_REGNUM,	  /* %eflags */
+  AMD64_CS_REGNUM,	      /* %cs */
+  AMD64_SS_REGNUM,	      /* %ss */
+  AMD64_DS_REGNUM,	      /* %ds */
+  AMD64_ES_REGNUM,	      /* %es */
+  AMD64_FS_REGNUM,	      /* %fs */
+  AMD64_GS_REGNUM,	      /* %gs */
+  AMD64_ST0_REGNUM = 24,     /* %st0 */
+  AMD64_ST1_REGNUM,	     /* %st1 */
+  AMD64_FCTRL_REGNUM = AMD64_ST0_REGNUM + 8,
+  AMD64_FSTAT_REGNUM = AMD64_ST0_REGNUM + 9,
+  AMD64_FTAG_REGNUM = AMD64_ST0_REGNUM + 10,
+  AMD64_XMM0_REGNUM = 40,   /* %xmm0 */
+  AMD64_XMM1_REGNUM,	    /* %xmm1 */
+  AMD64_MXCSR_REGNUM = AMD64_XMM0_REGNUM + 16,
+  AMD64_YMM0H_REGNUM,	   /* %ymm0h */
+  AMD64_YMM15H_REGNUM = AMD64_YMM0H_REGNUM + 15,
+  AMD64_BND0R_REGNUM = AMD64_YMM15H_REGNUM + 1,
+  AMD64_BND3R_REGNUM = AMD64_BND0R_REGNUM + 3,
+  AMD64_BNDCFGU_REGNUM,
+  AMD64_BNDSTATUS_REGNUM,
+  AMD64_XMM16_REGNUM,
+  AMD64_XMM31_REGNUM = AMD64_XMM16_REGNUM + 15,
+  AMD64_YMM16H_REGNUM,
+  AMD64_YMM31H_REGNUM = AMD64_YMM16H_REGNUM + 15,
+  AMD64_K0_REGNUM,
+  AMD64_K7_REGNUM = AMD64_K0_REGNUM + 7,
+  AMD64_ZMM0H_REGNUM,
+  AMD64_ZMM31H_REGNUM = AMD64_ZMM0H_REGNUM + 31,
+  AMD64_PKRU_REGNUM,
+  AMD64_FSBASE_REGNUM,
+  AMD64_GSBASE_REGNUM
+};
+
+/* The fill_function for the general-purpose register set.  */
+
+static void
+netbsd_x86_64_fill_gregset (struct regcache *regcache, char *buf)
+{
+  struct reg *r = (struct reg *)buf;
+
+#define netbsd_x86_64_collect_gp(regnum, fld) do {		\
+    collect_register (regcache, regnum, &r->regs[_REG_##fld]);	\
+  } while (0)
+
+  netbsd_x86_64_collect_gp (AMD64_RAX_REGNUM, RAX);
+  netbsd_x86_64_collect_gp (AMD64_RBX_REGNUM, RBX);
+  netbsd_x86_64_collect_gp (AMD64_RCX_REGNUM, RCX);
+  netbsd_x86_64_collect_gp (AMD64_RDX_REGNUM, RDX);
+  netbsd_x86_64_collect_gp (AMD64_RSI_REGNUM, RSI);
+  netbsd_x86_64_collect_gp (AMD64_RDI_REGNUM, RDI);
+  netbsd_x86_64_collect_gp (AMD64_RBP_REGNUM, RBP);
+  netbsd_x86_64_collect_gp (AMD64_RSP_REGNUM, RSP);
+  netbsd_x86_64_collect_gp (AMD64_R8_REGNUM, R8);
+  netbsd_x86_64_collect_gp (AMD64_R9_REGNUM, R9);
+  netbsd_x86_64_collect_gp (AMD64_R10_REGNUM, R10);
+  netbsd_x86_64_collect_gp (AMD64_R11_REGNUM, R11);
+  netbsd_x86_64_collect_gp (AMD64_R12_REGNUM, R12);
+  netbsd_x86_64_collect_gp (AMD64_R13_REGNUM, R13);
+  netbsd_x86_64_collect_gp (AMD64_R14_REGNUM, R14);
+  netbsd_x86_64_collect_gp (AMD64_R15_REGNUM, R15);
+  netbsd_x86_64_collect_gp (AMD64_RIP_REGNUM, RIP);
+  netbsd_x86_64_collect_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
+  netbsd_x86_64_collect_gp (AMD64_CS_REGNUM, CS);
+  netbsd_x86_64_collect_gp (AMD64_SS_REGNUM, SS);
+  netbsd_x86_64_collect_gp (AMD64_DS_REGNUM, DS);
+  netbsd_x86_64_collect_gp (AMD64_ES_REGNUM, ES);
+  netbsd_x86_64_collect_gp (AMD64_FS_REGNUM, FS);
+  netbsd_x86_64_collect_gp (AMD64_GS_REGNUM, GS);
+}
+
+/* The store_function for the general-purpose register set.  */
+
+static void
+netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
+{
+  struct reg *r = (struct reg *)buf;
+
+#define netbsd_x86_64_supply_gp(regnum, fld) do {		\
+    supply_register (regcache, regnum, &r->regs[_REG_##fld]);	\
+  } while(0)
+
+  netbsd_x86_64_supply_gp (AMD64_RAX_REGNUM, RAX);
+  netbsd_x86_64_supply_gp (AMD64_RBX_REGNUM, RBX);
+  netbsd_x86_64_supply_gp (AMD64_RCX_REGNUM, RCX);
+  netbsd_x86_64_supply_gp (AMD64_RDX_REGNUM, RDX);
+  netbsd_x86_64_supply_gp (AMD64_RSI_REGNUM, RSI);
+  netbsd_x86_64_supply_gp (AMD64_RDI_REGNUM, RDI);
+  netbsd_x86_64_supply_gp (AMD64_RBP_REGNUM, RBP);
+  netbsd_x86_64_supply_gp (AMD64_RSP_REGNUM, RSP);
+  netbsd_x86_64_supply_gp (AMD64_R8_REGNUM, R8);
+  netbsd_x86_64_supply_gp (AMD64_R9_REGNUM, R9);
+  netbsd_x86_64_supply_gp (AMD64_R10_REGNUM, R10);
+  netbsd_x86_64_supply_gp (AMD64_R11_REGNUM, R11);
+  netbsd_x86_64_supply_gp (AMD64_R12_REGNUM, R12);
+  netbsd_x86_64_supply_gp (AMD64_R13_REGNUM, R13);
+  netbsd_x86_64_supply_gp (AMD64_R14_REGNUM, R14);
+  netbsd_x86_64_supply_gp (AMD64_R15_REGNUM, R15);
+  netbsd_x86_64_supply_gp (AMD64_RIP_REGNUM, RIP);
+  netbsd_x86_64_supply_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
+  netbsd_x86_64_supply_gp (AMD64_CS_REGNUM, CS);
+  netbsd_x86_64_supply_gp (AMD64_SS_REGNUM, SS);
+  netbsd_x86_64_supply_gp (AMD64_DS_REGNUM, DS);
+  netbsd_x86_64_supply_gp (AMD64_ES_REGNUM, ES);
+  netbsd_x86_64_supply_gp (AMD64_FS_REGNUM, FS);
+  netbsd_x86_64_supply_gp (AMD64_GS_REGNUM, GS);
+}
+
+/* Implements the netbsd_target_ops.arch_setup routine.  */
+
+static void
+netbsd_x86_64_arch_setup (void)
+{
+  struct target_desc *tdesc
+    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
+
+  init_target_desc (tdesc, amd64_expedite_regs);
+
+  netbsd_tdesc = tdesc;
+}
+
+/* Update all the target description of all processes; a new GDB
+   connected, and it may or not support xml target descriptions.  */
+
+static void
+x86_64_netbsd_update_xmltarget (void)
+{
+  struct thread_info *saved_thread = current_thread;
+
+  /* Before changing the register cache's internal layout, flush the
+     contents of the current valid caches back to the threads, and
+     release the current regcache objects.  */
+  regcache_release ();
+
+  for_each_process ([] (process_info *proc) {
+		      int pid = proc->pid;
+
+		      /* Look up any thread of this process.  */
+		      current_thread = find_any_thread_of_pid (pid);
+
+		      the_low_target.arch_setup ();
+		    });
+
+  current_thread = saved_thread;
+}
+
+/* Process qSupported query, "xmlRegisters=". */
+
+static void
+netbsd_x86_64_process_qsupported (char **features, int count)
+{
+  /* Return if gdb doesn't support XML.  If gdb sends "xmlRegisters="
+     with "i386" in qSupported query, it supports x86 XML target
+     descriptions.  */
+  use_xml = 0;
+  for (int i = 0; i < count; i++)
+    {
+      const char *feature = features[i];
+
+      if (startswith (feature, "xmlRegisters="))
+	{
+	  char *copy = xstrdup (feature + 13);
+	  char *last;
+	  char *p;
+
+	  for (p = strtok_r (copy, ",", &last); p != NULL;
+	       p = strtok_r (NULL, ",", &last))
+	    {
+	      if (strcmp (p, "i386") == 0)
+		{
+		  use_xml = 1;
+		  break;
+		}
+	    }
+
+	  free (copy);
+	}
+    }
+  x86_64_netbsd_update_xmltarget ();
+}
+
+/* Description of all the x86-netbsd register sets.  */
+
+struct netbsd_regset_info netbsd_target_regsets[] =
+{
+ /* General Purpose Registers.  */
+ {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
+  netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
+ /* End of list marker.  */
+ {0, 0, -1, NULL, NULL }
+};
+
+/* The netbsd_target_ops vector for x86-netbsd.  */
+
+struct netbsd_target_ops the_low_target =
+{
+ netbsd_x86_64_arch_setup,
+ netbsd_x86_64_process_qsupported,
+};
--
2.28.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls
  2020-09-02 17:59 ` [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls Kamil Rytarowski
@ 2020-09-03 14:17   ` Tom Tromey
  2020-09-03 21:10     ` Kamil Rytarowski
  0 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2020-09-03 14:17 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

Kamil> +2020-03-17  Kamil Rytarowski  <n54@gmx.com>
Kamil> +
Kamil> +	* eintr.h: Add handle_eintr.

Kamil> +template <typename Fun, typename... Args>
Kamil> +inline decltype (auto) handle_eintr (const Fun &F, const Args &... A)

I like this, but from what I read it seems that "decltype(auto)" is
C++14 -- but gdb is still C++11.


Kamil> +{
Kamil> +  decltype (F (A...)) ret;
Kamil> +  do
Kamil> +    {
Kamil> +      errno = 0;
Kamil> +      ret = F (A...);
Kamil> +    }
Kamil> +  while (ret == -1 && errno == EINTR);

Also this seems to assume that "ret" is comparable to an int anyway.
So maybe just assuming int everywhere would be ok.

Tom

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-02 17:59 ` [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver Kamil Rytarowski
@ 2020-09-03 17:42   ` Aktemur, Tankut Baris
  2020-09-04  0:13     ` Kamil Rytarowski
  2020-09-16 16:08   ` Tom Tromey
  1 sibling, 1 reply; 19+ messages in thread
From: Aktemur, Tankut Baris @ 2020-09-03 17:42 UTC (permalink / raw)
  To: Kamil Rytarowski, gdb-patches

On September 2, 2020 7:59 PM, Kamil Rytarowski wrote:
> Implement the following functionality: create_inferior,
> post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
> resume, wait, fetch_registers, store_registers, read_memory, write_memory,
> request_interrupt, supports_read_auxv, read_auxv,
> supports_hardware_single_step, sw_breakpoint_from_kind,
> supports_z_point_type, insert_point, remove_point,
> stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
> supports_stopped_by_sw_breakpoint, supports_non_stop,
> supports_multi_process, supports_fork_events, supports_vfork_events,
> supports_exec_events, supports_disable_randomization,
> supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
> supports_pid_to_exec_file, pid_to_exec_file, thread_name,
> supports_catch_syscall.
> 
> The only CPU architecture supported: x86_64.
> 
> Implement only support for hardware assisted single step and
> software breakpoint.
> 
> Implement support only for regular X86 registers, thus no FPU.
> 
> gdb/ChangeLog:

This should be gdbserver/ChangeLog.

> 
>        * netbsd-low.cc: Add.
>        * netbsd-low.h: Likewise.
>        * netbsd-x86_64-low.cc: Likewise.
>        * Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
>        "netbsd-x86_64-low.cc".
>        * configure.srv: Add x86_64-*-netbsd*.
> ---
>  gdbserver/ChangeLog            |    9 +
>  gdbserver/Makefile.in          |    3 +
>  gdbserver/configure.srv        |    7 +
>  gdbserver/netbsd-low.cc        | 1352 ++++++++++++++++++++++++++++++++
>  gdbserver/netbsd-low.h         |  157 ++++
>  gdbserver/netbsd-x86_64-low.cc |  250 ++++++
>  6 files changed, 1778 insertions(+)
>  create mode 100644 gdbserver/netbsd-low.cc
>  create mode 100644 gdbserver/netbsd-low.h
>  create mode 100644 gdbserver/netbsd-x86_64-low.cc
> 
> diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
> index e437493b56d..c0732d1a28a 100644
> --- a/gdbserver/ChangeLog
> +++ b/gdbserver/ChangeLog
> @@ -1,3 +1,12 @@
> +2020-08-13  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* netbsd-low.cc: Add.
> +	* netbsd-low.h: Likewise.
> +	* netbsd-x86_64-low.cc: Likewise.
> +	* Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
> +	"netbsd-x86_64-low.cc".
> +	* configure.srv: Add x86_64-*-netbsd*.
> +
>  2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>
> 
>  	* server.cc (captured_main): Accept multiple `--selftest=`
> diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
> index 9d7687be534..1d597b14523 100644
> --- a/gdbserver/Makefile.in
> +++ b/gdbserver/Makefile.in
> @@ -193,6 +193,9 @@ SFILES = \
>  	$(srcdir)/linux-x86-low.cc \
>  	$(srcdir)/linux-xtensa-low.cc \
>  	$(srcdir)/mem-break.cc \
> +	$(srcdir)/netbsd-low.cc \
> +	$(srcdir)/netbsd-low.h \
> +	$(srcdir)/netbsd-x86_64-low.cc \
>  	$(srcdir)/proc-service.cc \
>  	$(srcdir)/proc-service.list \
>  	$(srcdir)/regcache.cc \
> diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
> index 5e33bd9c54d..12f9d410b7c 100644
> --- a/gdbserver/configure.srv
> +++ b/gdbserver/configure.srv
> @@ -349,6 +349,13 @@ case "${gdbserver_host}" in
>  			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
>  			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
>  			;;
> +  x86_64-*-netbsd*)	srv_regobj=""
> +			srv_tgtobj="netbsd-low.o netbsd-x86_64-low.o fork-child.o"
> +			srv_tgtobj="${srv_tgtobj} nat/fork-inferior.o"
> +			srv_tgtobj="${srv_tgtobj} nat/netbsd-nat.o"
> +			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
> +			srv_netbsd=yes

Is `srv_netbsd` used anywhere?

> +			;;
> 
>    xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
>  			srv_tgtobj="$srv_linux_obj linux-xtensa-low.o"
> diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
> new file mode 100644
> index 00000000000..db9531c66d3
> --- /dev/null
> +++ b/gdbserver/netbsd-low.cc
> @@ -0,0 +1,1352 @@
> +/* Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "server.h"
> +#include "target.h"
> +#include "netbsd-low.h"
> +#include "nat/netbsd-nat.h"
> +
> +#include <sys/param.h>
> +#include <sys/types.h>
> +
> +#include <sys/ptrace.h>
> +#include <sys/sysctl.h>
> +
> +#include <limits.h>
> +#include <unistd.h>
> +#include <signal.h>
> +
> +#include <elf.h>
> +
> +#include <type_traits>
> +
> +#include "gdbsupport/eintr.h"
> +#include "gdbsupport/gdb_wait.h"
> +#include "gdbsupport/filestuff.h"
> +#include "gdbsupport/common-inferior.h"
> +#include "nat/fork-inferior.h"
> +#include "hostio.h"
> +
> +int using_threads = 1;
> +
> +const struct target_desc *netbsd_tdesc;
> +
> +/* Call add_process with the given parameters, and initializes

To be consistent with the imperative "Call", "initializes" could read
"initialize".

> +   the process' private data.  */
> +
> +static struct process_info *
> +netbsd_add_process (int pid, int attached)

There are two uses of this function and in both the return value is ignored.
The function could as well be void.

> +{
> +  struct process_info *proc = add_process (pid, attached);
> +  proc->tdesc = netbsd_tdesc;
> +  proc->priv = nullptr;
> +
> +  return proc;
> +}
> +
> +/* Callback used by fork_inferior to start tracing the inferior.  */
> +
> +static void
> +netbsd_ptrace_fun ()
> +{
> +  /* Switch child to its own process group so that signals won't
> +     directly affect GDBserver. */
> +  if (setpgid (0, 0) < 0)
> +    trace_start_error_with_name (("setpgid"));
> +
> +  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
> +    trace_start_error_with_name (("ptrace"));
> +
> +  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
> +     stdout to stderr so that inferior i/o doesn't corrupt the connection.
> +     Also, redirect stdin to /dev/null.  */
> +  if (remote_connection_is_stdio ())
> +    {
> +      if (close (0) < 0)
> +	trace_start_error_with_name (("close"));
> +      if (open ("/dev/null", O_RDONLY) < 0)
> +	trace_start_error_with_name (("open"));
> +      if (dup2 (2, 1) < 0)
> +	trace_start_error_with_name (("dup2"));
> +      if (write (2, "stdin/stdout redirected\n",
> +		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
> +	{
> +	  /* Errors ignored.  */;

Any particular reason for the explicit ';'?

> +	}
> +    }
> +}
> +
> +/* Register threads in the process specified by PID.  */
> +
> +static void
> +netbsd_add_threads (pid_t pid)
> +{
> +  auto fn
> +    = [] (ptid_t ptid)
> +      {
> +	add_thread (ptid, nullptr);
> +      };
> +
> +  netbsd_nat::list_threads (pid, fn);

IMHO, the name "list_threads" gives the impression that it only lists the threads
and is side-effect free.  Maybe use something like "for_each_thread", as in gdbthread.h?

> +}
> +
> +/* Implement the create_inferior method of the target_ops vector.  */
> +
> +int
> +netbsd_process_target::create_inferior (const char *program,
> +					const std::vector<char *> &program_args)
> +{
> +  std::string str_program_args = construct_inferior_arguments (program_args);
> +
> +  pid_t pid = fork_inferior (program, str_program_args.c_str (),
> +			     get_environ ()->envp (), netbsd_ptrace_fun,
> +			     nullptr, nullptr, nullptr, nullptr);
> +
> +  netbsd_add_process (pid, 0);
> +
> +  post_fork_inferior (pid, program);
> +
> +  return pid;
> +}
> +
> +/* Implement the post_create_inferior target_ops method.  */
> +
> +void
> +netbsd_process_target::post_create_inferior ()
> +{
> +  pid_t pid = current_process ()->pid;
> +  netbsd_nat::enable_proc_events (pid);
> +}
> +
> +/* Implement the attach target_ops method.  */
> +
> +int
> +netbsd_process_target::attach (unsigned long pid)
> +{
> +
> +  if (ptrace (PT_ATTACH, pid, nullptr, 0) != 0)
> +    error ("Cannot attach to process %lu: %s (%d)\n", pid,
> +	    safe_strerror (errno), errno);
> +
> +  netbsd_add_process (pid, 1);
> +  netbsd_nat::enable_proc_events (pid);
> +  netbsd_add_threads (pid);
> +
> +  return 0;
> +}
> +
> +/* Returns true if GDB is interested in any child syscalls.  */
> +
> +static bool
> +gdb_catching_syscalls_p (pid_t pid)
> +{
> +  struct process_info *proc = find_process_pid (pid);
> +  return !proc->syscalls_to_catch.empty ();
> +}
> +
> +/* Implement the resume target_ops method.  */
> +
> +void
> +netbsd_process_target::resume (struct thread_resume *resume_info, size_t n)
> +{
> +  ptid_t resume_ptid = resume_info[0].thread;
> +  const int signal = resume_info[0].sig;
> +  const bool step = resume_info[0].kind == resume_step;
> +
> +  if (resume_ptid == minus_one_ptid)
> +    resume_ptid = ptid_of (current_thread);
> +
> +  const pid_t pid = resume_ptid.pid ();
> +  const lwpid_t lwp = resume_ptid.lwp ();
> +  regcache_invalidate_pid (pid);
> +
> +  auto fn
> +    = [&] (ptid_t ptid)
> +      {
> +	if (step)
> +	  {
> +	    if (ptid.lwp () == lwp || n != 1)
> +	      {
> +		if (ptrace (PT_SETSTEP, pid, NULL, ptid.lwp ()) == -1)
> +		  perror_with_name (("ptrace"));
> +		if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
> +		  perror_with_name (("ptrace"));
> +	      }
> +	    else
> +	      {
> +		if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
> +		  perror_with_name (("ptrace"));
> +		if (ptrace (PT_SUSPEND, pid, NULL, ptid.lwp ()) == -1)
> +		  perror_with_name (("ptrace"));
> +	      }
> +	  }
> +	else
> +	  {
> +	    if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
> +	      perror_with_name (("ptrace"));
> +	    if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
> +	      perror_with_name (("ptrace"));
> +	  }
> +      };
> +
> +  netbsd_nat::list_threads (pid, fn);
> +
> +  int request = gdb_catching_syscalls_p (pid) ? PT_CONTINUE : PT_SYSCALL;
> +
> +  errno = 0;
> +  ptrace (request, pid, (void *)1, signal);
> +  if (errno)
> +    perror_with_name (("ptrace"));
> +}
> +
> +/* Returns true if GDB is interested in the reported SYSNO syscall.  */
> +
> +static bool
> +netbsd_catch_this_syscall (int sysno)
> +{
> +  struct process_info *proc = current_process ();
> +
> +  if (proc->syscalls_to_catch.empty ())
> +    return false;
> +
> +  if (proc->syscalls_to_catch[0] == ANY_SYSCALL)
> +    return true;
> +
> +  for (int iter : proc->syscalls_to_catch)
> +    if (iter == sysno)
> +      return true;
> +
> +  return false;
> +}
> +
> +/* Helper function for child_wait and the derivatives of child_wait.
> +   HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
> +   translation of that in OURSTATUS.  */

A blank line here between the comment and the function header.

> +static void
> +netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
> +{
> +  if (WIFEXITED (hoststatus))
> +    {
> +      ourstatus->kind = TARGET_WAITKIND_EXITED;
> +      ourstatus->value.integer = WEXITSTATUS (hoststatus);
> +    }
> +  else if (!WIFSTOPPED (hoststatus))
> +    {
> +      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
> +      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (hoststatus));
> +    }
> +  else
> +    {
> +      ourstatus->kind = TARGET_WAITKIND_STOPPED;
> +      ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (hoststatus));
> +    }
> +}
> +
> +/* Implement a safe wrapper around waitpid().  */
> +
> +static pid_t
> +netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
> +{
> +  int status;
> +
> +  pid_t pid = gdb::handle_eintr (::waitpid, ptid.pid (), &status, options);
> +
> +  if (pid == -1)
> +    perror_with_name (_("Child process unexpectedly missing"));
> +
> +  netbsd_store_waitstatus (ourstatus, status);
> +  return pid;
> +}
> +
> +
> +/* Implement the wait target_ops method.
> +
> +   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.  */
> +
> +static ptid_t
> +netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
> +	     int target_options)
> +{
> +  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
> +  ptid_t wptid = ptid_t (pid);
> +
> +  if (pid == 0)
> +    {
> +      gdb_assert (target_options & TARGET_WNOHANG);
> +      ourstatus->kind = TARGET_WAITKIND_IGNORE;
> +      return null_ptid;
> +    }
> +
> +  gdb_assert (pid != -1);
> +
> +  /* 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;
> +
> +  lwpid_t 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..  */

Double periods.

> +  if (signo != SIGTRAP)
> +    return wptid;
> +
> +  /* Stop examining non-debugger oriented SIGTRAP codes.  */
> +  if (code <= SI_USER || code == SI_NOINFO)
> +    return wptid;
> +
> +  /* Process state for threading events */

End with dot-space-space?

> +  ptrace_state_t pst = {};
> +  if (code == TRAP_LWP)
> +    {
> +      if (ptrace (PT_GET_PROCESS_STATE, pid, &pst, sizeof (pst)) == -1)
> +	perror_with_name (("ptrace"));
> +    }

I think braces can be removed.

> +
> +  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_EXIT)
> +    {
> +      /* If GDB attaches to a multi-threaded process, exiting
> +	 threads might be skipped during post_attach that
> +	 have not yet reported their PTRACE_LWP_EXIT event.
> +	 Ignore exited events for an unknown LWP.  */
> +      thread_info *thr = find_thread_ptid (wptid);
> +      if (thr == nullptr)
> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +      else
> +	{
> +	  ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
> +	  /* NetBSD does not store an LWP exit status.  */
> +	  ourstatus->value.integer = 0;
> +
> +	  remove_thread (thr);
> +	}
> +      return wptid;
> +    }
> +
> +  if (find_thread_ptid (ptid_t (pid)))
> +    current_thread = find_thread_ptid (wptid);
> +
> +  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_CREATE)
> +    {
> +      /* If GDB attaches to a multi-threaded process, newborn
> +	 threads might be added by nbsd_add_threads that have
> +	 not yet reported their PTRACE_LWP_CREATE event.  Ignore
> +	 born events for an already-known LWP.  */
> +      if (find_thread_ptid (wptid))
> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +      else
> +	{
> +	  add_thread (wptid, NULL);
> +	  ourstatus->kind = TARGET_WAITKIND_THREAD_CREATED;
> +	}
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_EXEC)
> +    {
> +      ourstatus->kind = TARGET_WAITKIND_EXECD;
> +      ourstatus->value.execd_pathname
> +	= xstrdup (netbsd_nat::pid_to_exec_file (pid));
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_TRACE)
> +    {
> +      return wptid;
> +    }

Braces can be removed.

> +
> +  if (code == TRAP_SCE || code == TRAP_SCX)
> +    {
> +      int sysnum = si->si_sysnum;
> +
> +      if (!netbsd_catch_this_syscall(sysnum))
> +	{
> +	  /* If the core isn't interested in this event, ignore it.  */
> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +	  return wptid;
> +	}
> +
> +      ourstatus->kind =

The assignment operator should be on the next line.

> +	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
> +	TARGET_WAITKIND_SYSCALL_RETURN;
> +      ourstatus->value.syscall_number = sysnum;
> +      return wptid;
> +    }
> +
> +  if (code == TRAP_BRKPT)
> +    {
> +#ifdef PTRACE_BREAKPOINT_ADJ
> +      CORE_ADDR pc;
> +      struct reg r;
> +      ptrace (PT_GETREGS, pid, &r, psi.psi_lwpid);
> +      pc = PTRACE_REG_PC (&r);
> +      PTRACE_REG_SET_PC (&r, pc - PTRACE_BREAKPOINT_ADJ);
> +      ptrace (PT_SETREGS, pid, &r, psi.psi_lwpid);
> +#endif
> +      return wptid;
> +    }
> +
> +  /* Unclassified SIGTRAP event.  */
> +  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
> +  return wptid;
> +}
> +
> +/* Implement the wait target_ops method.  */
> +
> +ptid_t
> +netbsd_process_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
> +			     int target_options)
> +{
> +  while (true)
> +    {
> +      ptid_t wptid = netbsd_wait (ptid, ourstatus, target_options);
> +
> +      /* Register thread in the gdbcore if a thread was not reported earlier.
> +	 This is required after ::create_inferior, when the gdbcore does not
> +	 know about the first internal thread.
> +	 This may also happen on attach, when an event is registered on a thread
> +	 that was not fully initialized during the attach stage. */

Dot-space-space.

> +      if (wptid.lwp () != 0 && !find_thread_ptid (wptid)
> +	  && ourstatus->kind != TARGET_WAITKIND_THREAD_EXITED)
> +	add_thread (wptid, nullptr);
> +
> +      switch (ourstatus->kind)
> +	{
> +	case TARGET_WAITKIND_EXITED:
> +	case TARGET_WAITKIND_STOPPED:
> +	case TARGET_WAITKIND_SIGNALLED:
> +	case TARGET_WAITKIND_FORKED:
> +	case TARGET_WAITKIND_VFORKED:
> +	case TARGET_WAITKIND_EXECD:
> +	case TARGET_WAITKIND_VFORK_DONE:
> +	case TARGET_WAITKIND_SYSCALL_ENTRY:
> +	case TARGET_WAITKIND_SYSCALL_RETURN:
> +	  /* Pass the result to the generic code.  */
> +	  return wptid;
> +	case TARGET_WAITKIND_THREAD_CREATED:
> +	case TARGET_WAITKIND_THREAD_EXITED:
> +	  /* The core needlessly stops on these events.  */
> +	  /* FALLTHROUGH */
> +	case TARGET_WAITKIND_SPURIOUS:
> +	  /* Spurious events are unhandled by the gdbserver core.  */
> +	  if (ptrace (PT_CONTINUE, current_process ()->pid, (void *) 1, 0)
> +	      == -1)
> +	    perror_with_name (("ptrace"));
> +	  break;
> +	default:
> +	  error (("Unknwon stopped status"));

Typo "Unknwon".

> +	}
> +    }
> +}
> +
> +/* Implement the kill target_ops method.  */
> +
> +int
> +netbsd_process_target::kill (process_info *process)
> +{
> +  pid_t pid = process->pid;
> +  ptrace (PT_KILL, pid, nullptr, 0);
> +
> +  int status;
> +  gdb::handle_eintr (::waitpid, pid, &status, 0);

The "kill" target op is supposed to return -1 on failure.
So, maybe not ignore the return value of handle_eintr here?

> +  mourn (process);
> +  return 0;
> +}
> +
> +/* Implement the detach target_ops method.  */
> +
> +int
> +netbsd_process_target::detach (process_info *process)
> +{
> +  pid_t pid = process->pid;
> +
> +  ptrace (PT_DETACH, pid, (void *) 1, 0);
> +  mourn (process);
> +  return 0;
> +}
> +
> +/* Implement the mourn target_ops method.  */
> +
> +void
> +netbsd_process_target::mourn (struct process_info *proc)
> +{
> +  for_each_thread (proc->pid, remove_thread);
> +
> +  remove_process (proc);
> +}
> +
> +/* Implement the join target_ops method.  */
> +
> +void
> +netbsd_process_target::join (int pid)
> +{
> +  /* The PT_DETACH is sufficient to detach from the process.
> +     So no need to do anything extra.  */
> +}
> +
> +/* Implement the thread_alive target_ops method.  */
> +
> +bool
> +netbsd_process_target::thread_alive (ptid_t ptid)
> +{
> +  return netbsd_nat::thread_alive (ptid);
> +}
> +
> +/* Implement the fetch_registers target_ops method.  */
> +
> +void
> +netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
> +{
> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  ptid_t inferior_ptid = ptid_of (current_thread);
> +
> +  while (regset->size >= 0)
> +    {
> +      char *buf = (char *) xmalloc (regset->size);
> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +      if (res == -1)
> +	perror_with_name (("ptrace"));
> +      regset->store_function (regcache, buf);
> +      free (buf);
> +      regset++;
> +    }
> +}
> +
> +/* Implement the store_registers target_ops method.  */
> +
> +void
> +netbsd_process_target::store_registers (struct regcache *regcache, int regno)
> +{
> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  ptid_t inferior_ptid = ptid_of (current_thread);
> +
> +  while (regset->size >= 0)
> +    {
> +      char *buf = (char *)xmalloc (regset->size);
> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +      if (res == -1)
> +	perror_with_name (("ptrace"));
> +      if (res == 0)
> +	{
> +	  /* Then overlay our cached registers on that.  */
> +	  regset->fill_function (regcache, buf);
> +	  /* Only now do we write the register set.  */
> +	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +	}
> +      free (buf);
> +      regset++;
> +    }
> +}
> +
> +/* Implement the read_memory target_ops method.  */
> +
> +int
> +netbsd_process_target::read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
> +				    int size)
> +{
> +  struct ptrace_io_desc io;
> +  io.piod_op = PIOD_READ_D;
> +  io.piod_len = size;
> +
> +  pid_t pid = current_process ()->pid;
> +
> +  int bytes_read = 0;
> +
> +  if (size == 0)
> +    {
> +      /* Zero length write always succeeds.  */
> +      return 0;
> +    }
> +  do
> +    {
> +      io.piod_offs = (void *)(memaddr + bytes_read);
> +      io.piod_addr = myaddr + bytes_read;
> +
> +      int rv = ptrace (PT_IO, pid, &io, 0);
> +      if (rv == -1)
> +	return errno;
> +      if (io.piod_len == 0)
> +	return 0;
> +
> +      bytes_read += io.piod_len;
> +      io.piod_len = size - bytes_read;
> +    }
> +  while (bytes_read < size);
> +
> +  return 0;
> +}
> +
> +/* Implement the write_memory target_ops method.  */
> +
> +int
> +netbsd_process_target::write_memory (CORE_ADDR memaddr,
> +				     const unsigned char *myaddr, int size)
> +{
> +  struct ptrace_io_desc io;
> +  io.piod_op = PIOD_WRITE_D;
> +  io.piod_len = size;
> +
> +  pid_t pid = current_process ()->pid;
> +
> +  int bytes_written = 0;
> +
> +  if (size == 0)
> +    {
> +      /* Zero length write always succeeds.  */
> +      return 0;
> +    }
> +
> +  do
> +    {
> +      io.piod_addr = (void *)(myaddr + bytes_written);
> +      io.piod_offs = (void *)(memaddr + bytes_written);
> +
> +      int rv = ptrace (PT_IO, pid, &io, 0);
> +      if (rv == -1)
> +	return errno;
> +      if (io.piod_len == 0)
> +	return 0;
> +
> +      bytes_written += io.piod_len;
> +      io.piod_len = size - bytes_written;
> +    }
> +  while (bytes_written < size);
> +
> +  return 0;
> +}
> +
> +/* Implement the kill_request target_ops method.  */

"kill_request" -> "request_interrupt".

> +
> +void
> +netbsd_process_target::request_interrupt ()
> +{
> +  ptid_t inferior_ptid = ptid_of (get_first_thread ());
> +
> +  ::kill (inferior_ptid.pid(), SIGINT);
> +}
> +

I think a function comment is needed here.

> +static size_t
> +netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
> +{
> +  struct ptrace_io_desc pio;
> +
> +  pio.piod_op = PIOD_READ_AUXV;
> +  pio.piod_offs = offs;
> +  pio.piod_addr = addr;
> +  pio.piod_len = len;
> +
> +  if (ptrace (PT_IO, pid, &pio, 0) == -1)
> +    perror_with_name (("ptrace"));
> +
> +  return pio.piod_len;
> +}
> +
> +/* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
> +   to debugger memory starting at MYADDR.  */
> +
> +int
> +netbsd_process_target::read_auxv (CORE_ADDR offset,
> +				  unsigned char *myaddr, unsigned int len)
> +{
> +  pid_t pid = pid_of (current_thread);
> +
> +  return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
> +}
> +
> +bool
> +netbsd_process_target::supports_z_point_type (char z_type)
> +{
> +  switch (z_type)
> +    {
> +    case Z_PACKET_SW_BP:
> +      return true;
> +    case Z_PACKET_HW_BP:
> +    case Z_PACKET_WRITE_WP:
> +    case Z_PACKET_READ_WP:
> +    case Z_PACKET_ACCESS_WP:
> +    default:
> +      return false; /* Not supported.  */
> +    }
> +}
> +
> +/* Insert {break/watch}point at address ADDR.  SIZE is not used.  */
> +
> +int
> +netbsd_process_target::insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
> +		     int size, struct raw_breakpoint *bp)
> +{
> +  switch (type)
> +    {
> +    case raw_bkpt_type_sw:
> +      return insert_memory_breakpoint (bp);
> +    case raw_bkpt_type_hw:
> +    case raw_bkpt_type_write_wp:
> +    case raw_bkpt_type_read_wp:
> +    case raw_bkpt_type_access_wp:
> +    default:
> +      return 1; /* Not supported.  */
> +    }
> +}
> +
> +/* Remove {break/watch}point at address ADDR.  SIZE is not used.  */
> +
> +int
> +netbsd_process_target::remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
> +				     int size, struct raw_breakpoint *bp)
> +{
> +  switch (type)
> +    {
> +    case raw_bkpt_type_sw:
> +      return remove_memory_breakpoint (bp);
> +    case raw_bkpt_type_hw:
> +    case raw_bkpt_type_write_wp:
> +    case raw_bkpt_type_read_wp:
> +    case raw_bkpt_type_access_wp:
> +    default:
> +      return 1; /* Not supported.  */
> +    }
> +}
> +
> +/* Implement the to_stopped_by_sw_breakpoint target_ops
> +   method.  */

"to_stopped_by_sw_breakpoint" -> "stopped_by_sw_breakpoint".

> +
> +bool
> +netbsd_process_target::stopped_by_sw_breakpoint ()
> +{
> +  ptrace_siginfo_t psi;
> +  pid_t pid = current_process ()->pid;
> +
> +  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
> +    perror_with_name (("ptrace"));
> +
> +  return psi.psi_siginfo.si_signo == SIGTRAP &&
> +	 psi.psi_siginfo.si_code == TRAP_BRKPT;
> +}
> +
> +/* Implement the to_supports_stopped_by_sw_breakpoint target_ops
> +   method.  */

The "to_" prefix shall be removed here, too.

> +
> +bool
> +netbsd_process_target::supports_stopped_by_sw_breakpoint ()
> +{
> +  return true;
> +}
> +
> +/* Implement the supports_qxfer_siginfo target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_qxfer_siginfo ()
> +{
> +  return true;
> +}
> +
> +/* Implement the qxfer_siginfo target_ops method.  */
> +
> +int
> +netbsd_process_target::qxfer_siginfo (const char *annex, unsigned char *readbuf,
> +				      unsigned const char *writebuf,
> +				      CORE_ADDR offset, int len)
> +{
> +  if (current_thread == nullptr)
> +    return -1;
> +
> +  pid_t pid = current_process ()->pid;
> +
> +  return netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset, len);
> +}
> +
> +/* Implement the supports_non_stop target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_non_stop ()
> +{
> +  return false;
> +}
> +
> +/* Implement the supports_multi_process target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_multi_process ()
> +{
> +  return true;
> +}
> +
> +/* Check if fork events are supported.  */
> +
> +bool
> +netbsd_process_target::supports_fork_events ()
> +{
> +  return false;
> +}
> +
> +/* Check if vfork events are supported.  */
> +
> +bool
> +netbsd_process_target::supports_vfork_events ()
> +{
> +  return false;
> +}
> +
> +/* Check if exec events are supported.  */
> +
> +bool
> +netbsd_process_target::supports_exec_events ()
> +{
> +  return true;
> +}
> +
> +/* Implement the supports_disable_randomization target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_disable_randomization ()
> +{
> +  return false;
> +}

Some of the target ops above do not change the base class behavior.  The override
can be omitted for code simplicity.  I think it's also OK to retain them if you want
to make the definitions explicit.

> +
> +/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
> +
> +template <typename T>
> +int get_phdr_phnum_from_proc_auxv (const pid_t pid,
> +				   CORE_ADDR *phdr_memaddr, int *num_phdr)
> +{
> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
> +				    Aux64Info, Aux32Info>::type auxv_type;
> +  const size_t auxv_size = sizeof (auxv_type);
> +  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
> +
> +  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
> +
> +  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
> +
> +  *phdr_memaddr = 0;
> +  *num_phdr = 0;
> +
> +  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
> +    {
> +      auxv_type *const aux = (auxv_type *) buf;
> +
> +      switch (aux->a_type)
> +	{
> +	case AT_PHDR:
> +	  *phdr_memaddr = aux->a_v;
> +	  break;
> +	case AT_PHNUM:
> +	  *num_phdr = aux->a_v;
> +	  break;
> +	}
> +
> +      if (*phdr_memaddr != 0 && *num_phdr != 0)
> +	break;
> +    }
> +
> +  xfree (auxv_buf);
> +
> +  if (*phdr_memaddr == 0 || *num_phdr == 0)
> +    {
> +      warning ("Unexpected missing AT_PHDR and/or AT_PHNUM: "
> +	       "phdr_memaddr = %ld, phdr_num = %d",
> +	       (long) *phdr_memaddr, *num_phdr);

It might be better to use `core_addr_to_string` or `core_addr_to_string_nz`
to print phdr_memaddr.

> +      return 2;
> +    }
> +
> +  return 0;
> +}
> +
> +/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
> +
> +template <typename T>
> +static CORE_ADDR
> +get_dynamic (netbsd_process_target *target, const pid_t pid)
> +{
> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
> +				    Elf64_Phdr, Elf32_Phdr>::type phdr_type;
> +  const int phdr_size = sizeof (phdr_type);
> +
> +  CORE_ADDR phdr_memaddr;
> +  int num_phdr;
> +  if (get_phdr_phnum_from_proc_auxv<T> (pid, &phdr_memaddr, &num_phdr))
> +    return 0;
> +
> +  std::vector<unsigned char> phdr_buf;
> +  phdr_buf.resize (num_phdr * phdr_size);
> +
> +  if ((*target).read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))

Why not simply `target->read_memory`?
Also, it might be better to explicitly check != 0.

> +    return 0;
> +
> +  /* Compute relocation: it is expected to be 0 for "regular" executables,
> +     non-zero for PIE ones.  */
> +  CORE_ADDR relocation = -1;
> +  for (int i = 0; relocation == -1 && i < num_phdr; i++)
> +    {
> +      phdr_type *const p = (phdr_type *) (phdr_buf.data() + i * phdr_size);
> +
> +      if (p->p_type == PT_PHDR)
> +	relocation = phdr_memaddr - p->p_vaddr;
> +    }
> +
> +  if (relocation == -1)
> +    {
> +      /* PT_PHDR is optional, but necessary for PIE in general.  Fortunately
> +	 any real world executables, including PIE executables, have always
> +	 PT_PHDR present.  PT_PHDR is not present in some shared libraries or
> +	 in fpc (Free Pascal 2.4) binaries but neither of those have a need for
> +	 or present DT_DEBUG anyway (fpc binaries are statically linked).
> +
> +	 Therefore if there exists DT_DEBUG there is always also PT_PHDR.
> +
> +	 GDB could find RELOCATION also from AT_ENTRY - e_entry.  */
> +
> +      return 0;
> +    }
> +
> +  for (int i = 0; i < num_phdr; i++)
> +    {
> +      phdr_type *const p = (phdr_type *) (phdr_buf.data () + i * phdr_size);
> +
> +      if (p->p_type == PT_DYNAMIC)
> +	return p->p_vaddr + relocation;
> +    }
> +
> +  return 0;
> +}
> +
> +/* Return &_r_debug in the inferior, or -1 if not present.  Return value
> +   can be 0 if the inferior does not yet have the library list initialized.
> +   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
> +   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
> +
> +template <typename T>
> +static CORE_ADDR
> +get_r_debug (netbsd_process_target *target, const int pid)
> +{
> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
> +				    Elf64_Dyn, Elf32_Dyn>::type dyn_type;
> +  const int dyn_size = sizeof (dyn_type);
> +  unsigned char buf[sizeof (dyn_type)];  /* The larger of the two.  */
> +  CORE_ADDR map = -1;
> +
> +  CORE_ADDR dynamic_memaddr = get_dynamic<T> (target, pid);
> +  if (dynamic_memaddr == 0)
> +    return map;
> +
> +  while ((*target).read_memory (dynamic_memaddr, buf, dyn_size) == 0)

The `target->read_memory` comment here, too.

> +    {
> +      dyn_type *const dyn = (dyn_type *) buf;
> +#if defined DT_MIPS_RLD_MAP
> +      union
> +      {
> +	T map;
> +	unsigned char buf[sizeof (T)];
> +      }
> +      rld_map;
> +
> +      if (dyn->d_tag == DT_MIPS_RLD_MAP)
> +	{
> +	  if (read_memory (dyn->d_un.d_val,
> +			   rld_map.buf, sizeof (rld_map.buf)) == 0)
> +	    return rld_map.map;
> +	  else
> +	    break;
> +	}
> +#endif  /* DT_MIPS_RLD_MAP */
> +
> +      if (dyn->d_tag == DT_DEBUG && map == -1)
> +	map = dyn->d_un.d_val;
> +
> +      if (dyn->d_tag == DT_NULL)
> +	break;
> +
> +      dynamic_memaddr += dyn_size;
> +    }
> +
> +  return map;
> +}
> +
> +/* Read one pointer from MEMADDR in the inferior.  */
> +
> +static int
> +read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
> +	      int ptr_size)
> +{
> +  /* Go through a union so this works on either big or little endian
> +     hosts, when the inferior's pointer size is smaller than the size
> +     of CORE_ADDR.  It is assumed the inferior's endianness is the
> +     same of the superior's.  */
> +
> +  union
> +  {
> +    CORE_ADDR core_addr;
> +    unsigned int ui;
> +    unsigned char uc;
> +  } addr;
> +
> +  int ret = (*target).read_memory (memaddr, &addr.uc, ptr_size);

Here, too.

> +  if (ret == 0)
> +    {
> +      if (ptr_size == sizeof (CORE_ADDR))
> +	*ptr = addr.core_addr;
> +      else if (ptr_size == sizeof (unsigned int))
> +	*ptr = addr.ui;
> +      else
> +	gdb_assert_not_reached ("unhandled pointer size");
> +    }
> +  return ret;
> +}
> +
> +/* Construct qXfer:libraries-svr4:read reply.  */
> +
> +template <typename T>
> +int
> +netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
> +			     const pid_t pid, const char *annex,
> +			     unsigned char *readbuf,
> +			     unsigned const char *writebuf,
> +			     CORE_ADDR offset, int len)
> +{
> +  struct link_map_offsets
> +  {
> +    /* Offset and size of r_debug.r_version.  */
> +    int r_version_offset;
> +
> +    /* Offset and size of r_debug.r_map.  */
> +    int r_map_offset;
> +
> +    /* Offset to l_addr field in struct link_map.  */
> +    int l_addr_offset;
> +
> +    /* Offset to l_name field in struct link_map.  */
> +    int l_name_offset;
> +
> +    /* Offset to l_ld field in struct link_map.  */
> +    int l_ld_offset;
> +
> +    /* Offset to l_next field in struct link_map.  */
> +    int l_next_offset;
> +
> +    /* Offset to l_prev field in struct link_map.  */
> +    int l_prev_offset;
> +  };
> +
> +  static const struct link_map_offsets lmo_32bit_offsets =
> +    {
> +      0,     /* r_version offset. */
> +      4,     /* r_debug.r_map offset.  */
> +      0,     /* l_addr offset in link_map.  */
> +      4,     /* l_name offset in link_map.  */
> +      8,     /* l_ld offset in link_map.  */
> +      12,    /* l_next offset in link_map.  */
> +      16     /* l_prev offset in link_map.  */
> +    };
> +
> +  static const struct link_map_offsets lmo_64bit_offsets =
> +    {
> +      0,     /* r_version offset. */
> +      8,     /* r_debug.r_map offset.  */
> +      0,     /* l_addr offset in link_map.  */
> +      8,     /* l_name offset in link_map.  */
> +      16,    /* l_ld offset in link_map.  */
> +      24,    /* l_next offset in link_map.  */
> +      32     /* l_prev offset in link_map.  */
> +    };
> +
> +  CORE_ADDR lm_addr = 0, lm_prev = 0;
> +  CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
> +  int header_done = 0;
> +
> +  const struct link_map_offsets *lmo
> +    = (sizeof (T) == sizeof (int64_t))
> +    ? &lmo_64bit_offsets : &lmo_32bit_offsets;

An extra pair of parens can be used to better align '?'.  Like this:

  const struct link_map_offsets *lmo
    = ((sizeof (T) == sizeof (int64_t))
       ? &lmo_64bit_offsets : &lmo_32bit_offsets);

> +  int ptr_size = sizeof (T);
> +
> +  while (annex[0] != '\0')
> +    {
> +      const char *sep = strchr (annex, '=');
> +      if (sep == nullptr)
> +	break;
> +
> +      int name_len = sep - annex;
> +      CORE_ADDR *addrp;
> +      if (name_len == 5 && startswith (annex, "start"))
> +	addrp = &lm_addr;
> +      else if (name_len == 4 && startswith (annex, "prev"))
> +	addrp = &lm_prev;
> +      else
> +	{
> +	  annex = strchr (sep, ';');
> +	  if (annex == nullptr)
> +	    break;
> +	  annex++;
> +	  continue;
> +	}
> +
> +      annex = decode_address_to_semicolon (addrp, sep + 1);
> +    }
> +
> +  if (lm_addr == 0)
> +    {
> +      CORE_ADDR r_debug = get_r_debug<T> (target, pid);
> +
> +      /* We failed to find DT_DEBUG.  Such situation will not change
> +	 for this inferior - do not retry it.  Report it to GDB as
> +	 E01, see for the reasons at the GDB solib-svr4.c side.  */
> +      if (r_debug == (CORE_ADDR) -1)
> +	return -1;
> +
> +      if (r_debug != 0)
> +	{
> +	  if (read_one_ptr (target, r_debug + lmo->r_map_offset,
> +			    &lm_addr, ptr_size) != 0)
> +	    {
> +	      warning ("unable to read r_map from 0x%lx",
> +		       (long) r_debug + lmo->r_map_offset);

Again, `core_addr_to_string` could be used.

> +	    }
> +	}

Braces can be removed in both conditional statements above.  And, if you prefer,
the conditions can be merged via &&.

> +    }
> +
> +  std::string document = "<library-list-svr4 version=\"1.0\"";
> +
> +  while (lm_addr
> +	 && read_one_ptr (target, lm_addr + lmo->l_name_offset,
> +			  &l_name, ptr_size) == 0
> +	 && read_one_ptr (target, lm_addr + lmo->l_addr_offset,
> +			  &l_addr, ptr_size) == 0
> +	 && read_one_ptr (target, lm_addr + lmo->l_ld_offset,
> +			  &l_ld, ptr_size) == 0
> +	 && read_one_ptr (target, lm_addr + lmo->l_prev_offset,
> +			  &l_prev, ptr_size) == 0
> +	 && read_one_ptr (target, lm_addr + lmo->l_next_offset,
> +			  &l_next, ptr_size) == 0)
> +    {
> +      if (lm_prev != l_prev)
> +	{
> +	  warning ("Corrupted shared library list: 0x%lx != 0x%lx",
> +		   (long) lm_prev, (long) l_prev);
> +	  break;
> +	}
> +
> +      /* Ignore the first entry even if it has valid name as the first entry
> +	 corresponds to the main executable.  The first entry should not be
> +	 skipped if the dynamic loader was loaded late by a static executable
> +	 (see solib-svr4.c parameter ignore_first).  But in such case the main
> +	 executable does not have PT_DYNAMIC present and this function already
> +	 exited above due to failed get_r_debug.  */
> +      if (lm_prev == 0)
> +	string_appendf (document, " main-lm=\"0x%lx\"",
> +			(unsigned long) lm_addr);
> +      else
> +	{
> +	  unsigned char libname[PATH_MAX];
> +
> +	  /* Not checking for error because reading may stop before
> +	     we've got PATH_MAX worth of characters.  */
> +	  libname[0] = '\0';
> +	  (*target).read_memory (l_name, libname, sizeof (libname) - 1);
> +	  libname[sizeof (libname) - 1] = '\0';
> +	  if (libname[0] != '\0')
> +	    {
> +	      if (!header_done)
> +		{
> +		  /* Terminate `<library-list-svr4'.  */
> +		  document += '>';
> +		  header_done = 1;
> +		}
> +
> +	      string_appendf (document, "<library name=\"");
> +	      xml_escape_text_append (&document, (char *) libname);
> +	      string_appendf (document, "\" lm=\"0x%lx\" "
> +			      "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
> +			      (unsigned long) lm_addr, (unsigned long) l_addr,
> +			      (unsigned long) l_ld);
> +	    }
> +	}
> +
> +      lm_prev = lm_addr;
> +      lm_addr = l_next;
> +    }
> +
> +  if (!header_done)
> +    {
> +      /* Empty list; terminate `<library-list-svr4'.  */
> +      document += "/>";
> +    }
> +  else
> +    document += "</library-list-svr4>";
> +
> +  int document_len = document.length ();
> +  if (offset < document_len)
> +    document_len -= offset;
> +  else
> +    document_len = 0;
> +  if (len > document_len)
> +    len = document_len;
> +
> +  memcpy (readbuf, document.data () + offset, len);
> +
> +  return len;
> +}
> +
> +/* Return true if FILE is a 64-bit ELF file,
> +   false if the file is not a 64-bit ELF file,
> +   and error if the file is not accessible or doesn't exist.  */
> +
> +static bool
> +elf_64_file_p (const char *file)
> +{
> +  int fd = gdb::handle_eintr (::open, file, O_RDONLY);
> +  if (fd < 0)
> +    perror_with_name (("open"));
> +
> +  Elf64_Ehdr header;
> +  ssize_t ret = gdb::handle_eintr (::read, fd, &header, sizeof (header));
> +  if (ret == -1)
> +    perror_with_name (("read"));
> +  gdb::handle_eintr (::close, fd);
> +  if (ret != sizeof (header))
> +    error ("Cannot read ELF file header: %s", file);
> +
> +  if (header.e_ident[EI_MAG0] != ELFMAG0
> +      || header.e_ident[EI_MAG1] != ELFMAG1
> +      || header.e_ident[EI_MAG2] != ELFMAG2
> +      || header.e_ident[EI_MAG3] != ELFMAG3)
> +    error ("Unrecognized ELF file header: %s", file);
> +
> +  return header.e_ident[EI_CLASS] == ELFCLASS64;
> +}
> +
> +/* Construct qXfer:libraries-svr4:read reply.  */
> +
> +int
> +netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
> +					     unsigned char *readbuf,
> +					     unsigned const char *writebuf,
> +					     CORE_ADDR offset, int len)
> +{
> +  if (writebuf != nullptr)
> +    return -2;
> +  if (readbuf == nullptr)
> +    return -1;
> +
> +  struct process_info *proc = current_process ();
> +  pid_t pid = proc->pid;
> +  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
> +
> +  if (is_elf64)
> +    return netbsd_qxfer_libraries_svr4<int64_t> (this, pid, annex, readbuf,
> +						 writebuf, offset, len);
> +  else
> +    return netbsd_qxfer_libraries_svr4<int32_t> (this, pid, annex, readbuf,
> +						 writebuf, offset, len);
> +}
> +
> +/* Implement the supports_qxfer_libraries_svr4 target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_qxfer_libraries_svr4 ()
> +{
> +  return true;
> +}
> +
> +/* Return the name of a file that can be opened to get the symbols for
> +   the child process identified by PID.  */
> +
> +char *
> +netbsd_process_target::pid_to_exec_file (pid_t pid)
> +{
> +  return netbsd_nat::pid_to_exec_file (pid);
> +}
> +
> +/* Implementation of the target_ops method "supports_pid_to_exec_file".  */
> +
> +bool
> +netbsd_process_target::supports_pid_to_exec_file ()
> +{
> +  return true;
> +}
> +
> +/* Implementation of the target_ops method "supports_hardware_single_step".  */
> +bool
> +netbsd_process_target::supports_hardware_single_step ()
> +{
> +  return true;
> +}
> +
> +/* Implementation of the target_ops method "sw_breakpoint_from_kind".  */
> +
> +const gdb_byte *
> +netbsd_process_target::sw_breakpoint_from_kind (int kind, int *size)
> +{
> +  static gdb_byte brkpt[PTRACE_BREAKPOINT_SIZE];
> +
> +  *size = PTRACE_BREAKPOINT_SIZE;
> +
> +  memcpy (brkpt, PTRACE_BREAKPOINT, PTRACE_BREAKPOINT_SIZE);

Is there a need to memcpy each time the function is called?

> +
> +  return brkpt;
> +}
> +
> +/* Implement the thread_name target_ops method.  */
> +
> +const char *
> +netbsd_process_target::thread_name (ptid_t ptid)
> +{
> +  return netbsd_nat::thread_name (ptid);
> +}
> +
> +/* Implement the supports_catch_syscall target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_catch_syscall ()
> +{
> +  return true;
> +}
> +
> +/* Implement the supports_read_auxv target_ops method.  */
> +
> +bool
> +netbsd_process_target::supports_read_auxv ()
> +{
> +  return true;
> +}
> +
> +/* The NetBSD target ops object.  */
> +
> +static netbsd_process_target the_netbsd_target;
> +
> +void
> +initialize_low ()
> +{
> +  set_target_ops (&the_netbsd_target);
> +  the_low_target.arch_setup ();
> +}
> diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
> new file mode 100644
> index 00000000000..2bd57c51726
> --- /dev/null
> +++ b/gdbserver/netbsd-low.h
> @@ -0,0 +1,157 @@
> +/* Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDBSERVER_NETBSD_LOW_H
> +#define GDBSERVER_NETBSD_LOW_H
> +
> +struct regcache;
> +struct target_desc;
> +
> +/*  Some information relative to a given register set.   */
> +
> +struct netbsd_regset_info
> +{
> +  /* The ptrace request needed to get/set registers of this set.  */
> +  int get_request, set_request;
> +  /* The size of the register set.  */
> +  int size;
> +  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
> +  void (*fill_function) (struct regcache *regcache, char *buf);
> +  /* Store the register value in BUF in the given REGCACHE.  */
> +  void (*store_function) (struct regcache *regcache, const char *buf);

Leaving one blank line between each field may improve readability.

> +};
> +
> +/* A list of regsets for the target being debugged, terminated by an entry
> +   where the size is negative.
> +
> +   This list should be created by the target-specific code.  */
> +
> +extern struct netbsd_regset_info netbsd_target_regsets[];
> +
> +/* The target-specific operations for NetBSD support.  */
> +
> +struct netbsd_target_ops
> +{
> +  /* Architecture-specific setup.  */
> +  void (*arch_setup) ();
> +
> +  /* Hook to support target specific qSupported.  */
> +  void (*process_qsupported) (char **, int count);

Is the `process_qsupported` function called anywhere?

> +};

No strong opinion, but the low target could be a derived class of
netbsd_process_target, like the linux low targets.

> +
> +/* Target ops definitions for a NetBSD target.  */
> +
> +class netbsd_process_target : public process_stratum_target
> +{
> +public:
> +
> +  int create_inferior (const char *program,
> +		       const std::vector<char *> &program_args) override;
> +
> +  void post_create_inferior () override;
> +
> +  int attach (unsigned long pid) override;
> +
> +  int kill (process_info *proc) override;
> +
> +  int detach (process_info *proc) override;
> +
> +  void mourn (process_info *proc) override;
> +
> +  void join (int pid) override;
> +
> +  bool thread_alive (ptid_t pid) override;
> +
> +  void resume (thread_resume *resume_info, size_t n) override;
> +
> +  ptid_t wait (ptid_t ptid, target_waitstatus *status,
> +	       int options) override;
> +
> +  void fetch_registers (regcache *regcache, int regno) override;
> +
> +  void store_registers (regcache *regcache, int regno) override;
> +
> +  int read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
> +		   int len) override;
> +
> +  int write_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
> +		    int len) override;
> +
> +  void request_interrupt () override;
> +
> +  bool supports_read_auxv () override;
> +
> +  int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
> +		 unsigned int len) override;
> +
> +  bool supports_hardware_single_step () override;
> +
> +  const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> +
> +  bool supports_z_point_type (char z_type) override;
> +
> +  int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
> +		    int size, struct raw_breakpoint *bp) override;
> +
> +  int remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
> +		    int size, struct raw_breakpoint *bp) override;
> +
> +  bool stopped_by_sw_breakpoint () override;
> +
> +  bool supports_qxfer_siginfo () override;
> +
> +  int qxfer_siginfo (const char *annex, unsigned char *readbuf,
> +		     unsigned const char *writebuf, CORE_ADDR offset,
> +		     int len) override;
> +
> +  bool supports_stopped_by_sw_breakpoint () override;
> +
> +  bool supports_non_stop () override;
> +
> +  bool supports_multi_process () override;
> +
> +  bool supports_fork_events () override;
> +
> +  bool supports_vfork_events () override;
> +
> +  bool supports_exec_events () override;
> +
> +  bool supports_disable_randomization () override;
> +
> +  bool supports_qxfer_libraries_svr4 () override;
> +
> +  int qxfer_libraries_svr4 (const char*, unsigned char*, const unsigned char*,
> +			    CORE_ADDR, int) override;
> +
> +  bool supports_pid_to_exec_file () override;
> +
> +  char *pid_to_exec_file (int pid) override;
> +
> +  const char *thread_name (ptid_t thread) override;
> +
> +  bool supports_catch_syscall () override;
> +};
> +
> +/* The inferior's target description.  This is a global because the
> +   NetBSD ports support neither bi-arch nor multi-process.  */

The "nor multi-process" part of this comment is confusing.  What does the multi-process
feature have to do with the_low_target?  And netbsd_process_target was defined to
support multi-process above in the .cc file.

> +
> +extern struct netbsd_target_ops the_low_target;
> +
> +/* XXX: multilib */
> +extern const struct target_desc *netbsd_tdesc;
> +
> +#endif /* GDBSERVER_NETBSD_LOW_H */
> diff --git a/gdbserver/netbsd-x86_64-low.cc b/gdbserver/netbsd-x86_64-low.cc
> new file mode 100644
> index 00000000000..c8679d9f153
> --- /dev/null
> +++ b/gdbserver/netbsd-x86_64-low.cc
> @@ -0,0 +1,250 @@
> +/* Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/types.h>
> +#include <sys/ptrace.h>
> +#include <limits.h>
> +
> +#include "server.h"
> +#include "netbsd-low.h"
> +#include "gdbsupport/x86-xstate.h"
> +#include "arch/amd64.h"
> +#include "x86-tdesc.h"
> +#include "tdesc.h"
> +
> +static int use_xml;
> +
> +/* The index of various registers inside the regcache.  */
> +
> +enum netbsd_x86_64_gdb_regnum
> +{
> +  AMD64_RAX_REGNUM,	     /* %rax */
> +  AMD64_RBX_REGNUM,	     /* %rbx */
> +  AMD64_RCX_REGNUM,	     /* %rcx */
> +  AMD64_RDX_REGNUM,	     /* %rdx */
> +  AMD64_RSI_REGNUM,	     /* %rsi */
> +  AMD64_RDI_REGNUM,	     /* %rdi */
> +  AMD64_RBP_REGNUM,	     /* %rbp */
> +  AMD64_RSP_REGNUM,	     /* %rsp */
> +  AMD64_R8_REGNUM,	      /* %r8 */
> +  AMD64_R9_REGNUM,	      /* %r9 */
> +  AMD64_R10_REGNUM,	     /* %r10 */
> +  AMD64_R11_REGNUM,	     /* %r11 */
> +  AMD64_R12_REGNUM,	     /* %r12 */
> +  AMD64_R13_REGNUM,	     /* %r13 */
> +  AMD64_R14_REGNUM,	     /* %r14 */
> +  AMD64_R15_REGNUM,	     /* %r15 */
> +  AMD64_RIP_REGNUM,	     /* %rip */
> +  AMD64_EFLAGS_REGNUM,	  /* %eflags */
> +  AMD64_CS_REGNUM,	      /* %cs */
> +  AMD64_SS_REGNUM,	      /* %ss */
> +  AMD64_DS_REGNUM,	      /* %ds */
> +  AMD64_ES_REGNUM,	      /* %es */
> +  AMD64_FS_REGNUM,	      /* %fs */
> +  AMD64_GS_REGNUM,	      /* %gs */
> +  AMD64_ST0_REGNUM = 24,     /* %st0 */
> +  AMD64_ST1_REGNUM,	     /* %st1 */
> +  AMD64_FCTRL_REGNUM = AMD64_ST0_REGNUM + 8,
> +  AMD64_FSTAT_REGNUM = AMD64_ST0_REGNUM + 9,
> +  AMD64_FTAG_REGNUM = AMD64_ST0_REGNUM + 10,
> +  AMD64_XMM0_REGNUM = 40,   /* %xmm0 */
> +  AMD64_XMM1_REGNUM,	    /* %xmm1 */
> +  AMD64_MXCSR_REGNUM = AMD64_XMM0_REGNUM + 16,
> +  AMD64_YMM0H_REGNUM,	   /* %ymm0h */
> +  AMD64_YMM15H_REGNUM = AMD64_YMM0H_REGNUM + 15,
> +  AMD64_BND0R_REGNUM = AMD64_YMM15H_REGNUM + 1,
> +  AMD64_BND3R_REGNUM = AMD64_BND0R_REGNUM + 3,
> +  AMD64_BNDCFGU_REGNUM,
> +  AMD64_BNDSTATUS_REGNUM,
> +  AMD64_XMM16_REGNUM,
> +  AMD64_XMM31_REGNUM = AMD64_XMM16_REGNUM + 15,
> +  AMD64_YMM16H_REGNUM,
> +  AMD64_YMM31H_REGNUM = AMD64_YMM16H_REGNUM + 15,
> +  AMD64_K0_REGNUM,
> +  AMD64_K7_REGNUM = AMD64_K0_REGNUM + 7,
> +  AMD64_ZMM0H_REGNUM,
> +  AMD64_ZMM31H_REGNUM = AMD64_ZMM0H_REGNUM + 31,
> +  AMD64_PKRU_REGNUM,
> +  AMD64_FSBASE_REGNUM,
> +  AMD64_GSBASE_REGNUM
> +};
> +
> +/* The fill_function for the general-purpose register set.  */
> +
> +static void
> +netbsd_x86_64_fill_gregset (struct regcache *regcache, char *buf)
> +{
> +  struct reg *r = (struct reg *)buf;

Space after ')'.

> +
> +#define netbsd_x86_64_collect_gp(regnum, fld) do {		\
> +    collect_register (regcache, regnum, &r->regs[_REG_##fld]);	\
> +  } while (0)
> +
> +  netbsd_x86_64_collect_gp (AMD64_RAX_REGNUM, RAX);
> +  netbsd_x86_64_collect_gp (AMD64_RBX_REGNUM, RBX);
> +  netbsd_x86_64_collect_gp (AMD64_RCX_REGNUM, RCX);
> +  netbsd_x86_64_collect_gp (AMD64_RDX_REGNUM, RDX);
> +  netbsd_x86_64_collect_gp (AMD64_RSI_REGNUM, RSI);
> +  netbsd_x86_64_collect_gp (AMD64_RDI_REGNUM, RDI);
> +  netbsd_x86_64_collect_gp (AMD64_RBP_REGNUM, RBP);
> +  netbsd_x86_64_collect_gp (AMD64_RSP_REGNUM, RSP);
> +  netbsd_x86_64_collect_gp (AMD64_R8_REGNUM, R8);
> +  netbsd_x86_64_collect_gp (AMD64_R9_REGNUM, R9);
> +  netbsd_x86_64_collect_gp (AMD64_R10_REGNUM, R10);
> +  netbsd_x86_64_collect_gp (AMD64_R11_REGNUM, R11);
> +  netbsd_x86_64_collect_gp (AMD64_R12_REGNUM, R12);
> +  netbsd_x86_64_collect_gp (AMD64_R13_REGNUM, R13);
> +  netbsd_x86_64_collect_gp (AMD64_R14_REGNUM, R14);
> +  netbsd_x86_64_collect_gp (AMD64_R15_REGNUM, R15);
> +  netbsd_x86_64_collect_gp (AMD64_RIP_REGNUM, RIP);
> +  netbsd_x86_64_collect_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
> +  netbsd_x86_64_collect_gp (AMD64_CS_REGNUM, CS);
> +  netbsd_x86_64_collect_gp (AMD64_SS_REGNUM, SS);
> +  netbsd_x86_64_collect_gp (AMD64_DS_REGNUM, DS);
> +  netbsd_x86_64_collect_gp (AMD64_ES_REGNUM, ES);
> +  netbsd_x86_64_collect_gp (AMD64_FS_REGNUM, FS);
> +  netbsd_x86_64_collect_gp (AMD64_GS_REGNUM, GS);
> +}
> +
> +/* The store_function for the general-purpose register set.  */
> +
> +static void
> +netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
> +{
> +  struct reg *r = (struct reg *)buf;

Space after ')'.

> +
> +#define netbsd_x86_64_supply_gp(regnum, fld) do {		\
> +    supply_register (regcache, regnum, &r->regs[_REG_##fld]);	\
> +  } while(0)
> +
> +  netbsd_x86_64_supply_gp (AMD64_RAX_REGNUM, RAX);
> +  netbsd_x86_64_supply_gp (AMD64_RBX_REGNUM, RBX);
> +  netbsd_x86_64_supply_gp (AMD64_RCX_REGNUM, RCX);
> +  netbsd_x86_64_supply_gp (AMD64_RDX_REGNUM, RDX);
> +  netbsd_x86_64_supply_gp (AMD64_RSI_REGNUM, RSI);
> +  netbsd_x86_64_supply_gp (AMD64_RDI_REGNUM, RDI);
> +  netbsd_x86_64_supply_gp (AMD64_RBP_REGNUM, RBP);
> +  netbsd_x86_64_supply_gp (AMD64_RSP_REGNUM, RSP);
> +  netbsd_x86_64_supply_gp (AMD64_R8_REGNUM, R8);
> +  netbsd_x86_64_supply_gp (AMD64_R9_REGNUM, R9);
> +  netbsd_x86_64_supply_gp (AMD64_R10_REGNUM, R10);
> +  netbsd_x86_64_supply_gp (AMD64_R11_REGNUM, R11);
> +  netbsd_x86_64_supply_gp (AMD64_R12_REGNUM, R12);
> +  netbsd_x86_64_supply_gp (AMD64_R13_REGNUM, R13);
> +  netbsd_x86_64_supply_gp (AMD64_R14_REGNUM, R14);
> +  netbsd_x86_64_supply_gp (AMD64_R15_REGNUM, R15);
> +  netbsd_x86_64_supply_gp (AMD64_RIP_REGNUM, RIP);
> +  netbsd_x86_64_supply_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
> +  netbsd_x86_64_supply_gp (AMD64_CS_REGNUM, CS);
> +  netbsd_x86_64_supply_gp (AMD64_SS_REGNUM, SS);
> +  netbsd_x86_64_supply_gp (AMD64_DS_REGNUM, DS);
> +  netbsd_x86_64_supply_gp (AMD64_ES_REGNUM, ES);
> +  netbsd_x86_64_supply_gp (AMD64_FS_REGNUM, FS);
> +  netbsd_x86_64_supply_gp (AMD64_GS_REGNUM, GS);
> +}
> +
> +/* Implements the netbsd_target_ops.arch_setup routine.  */
> +
> +static void
> +netbsd_x86_64_arch_setup (void)
> +{
> +  struct target_desc *tdesc
> +    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
> +
> +  init_target_desc (tdesc, amd64_expedite_regs);
> +
> +  netbsd_tdesc = tdesc;
> +}
> +
> +/* Update all the target description of all processes; a new GDB
> +   connected, and it may or not support xml target descriptions.  */
> +
> +static void
> +x86_64_netbsd_update_xmltarget (void)
> +{
> +  struct thread_info *saved_thread = current_thread;
> +
> +  /* Before changing the register cache's internal layout, flush the
> +     contents of the current valid caches back to the threads, and
> +     release the current regcache objects.  */
> +  regcache_release ();
> +
> +  for_each_process ([] (process_info *proc) {
> +		      int pid = proc->pid;
> +
> +		      /* Look up any thread of this process.  */
> +		      current_thread = find_any_thread_of_pid (pid);
> +
> +		      the_low_target.arch_setup ();

I find this confusing because the target object is supposed to be a singleton.
Why is its arch_setup called for each process?

> +		    });
> +
> +  current_thread = saved_thread;
> +}
> +
> +/* Process qSupported query, "xmlRegisters=". */
> +
> +static void
> +netbsd_x86_64_process_qsupported (char **features, int count)
> +{
> +  /* Return if gdb doesn't support XML.  If gdb sends "xmlRegisters="
> +     with "i386" in qSupported query, it supports x86 XML target
> +     descriptions.  */
> +  use_xml = 0;
> +  for (int i = 0; i < count; i++)
> +    {
> +      const char *feature = features[i];
> +
> +      if (startswith (feature, "xmlRegisters="))
> +	{
> +	  char *copy = xstrdup (feature + 13);
> +	  char *last;
> +	  char *p;
> +
> +	  for (p = strtok_r (copy, ",", &last); p != NULL;
> +	       p = strtok_r (NULL, ",", &last))
> +	    {
> +	      if (strcmp (p, "i386") == 0)
> +		{
> +		  use_xml = 1;
> +		  break;
> +		}
> +	    }
> +
> +	  free (copy);
> +	}
> +    }
> +  x86_64_netbsd_update_xmltarget ();
> +}
> +
> +/* Description of all the x86-netbsd register sets.  */
> +
> +struct netbsd_regset_info netbsd_target_regsets[] =
> +{
> + /* General Purpose Registers.  */
> + {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
> +  netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
> + /* End of list marker.  */
> + {0, 0, -1, NULL, NULL }
> +};
> +
> +/* The netbsd_target_ops vector for x86-netbsd.  */
> +
> +struct netbsd_target_ops the_low_target =
> +{
> + netbsd_x86_64_arch_setup,
> + netbsd_x86_64_process_qsupported,
> +};
> --
> 2.28.0

Regards
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls
  2020-09-03 14:17   ` Tom Tromey
@ 2020-09-03 21:10     ` Kamil Rytarowski
  0 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-03 21:10 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches


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

On 03.09.2020 16:17, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
> 
> Kamil> +2020-03-17  Kamil Rytarowski  <n54@gmx.com>
> Kamil> +
> Kamil> +	* eintr.h: Add handle_eintr.
> 
> Kamil> +template <typename Fun, typename... Args>
> Kamil> +inline decltype (auto) handle_eintr (const Fun &F, const Args &... A)
> 
> I like this, but from what I read it seems that "decltype(auto)" is
> C++14 -- but gdb is still C++11.
> 

OK, I have downgraded this to C++11.

> 
> Kamil> +{
> Kamil> +  decltype (F (A...)) ret;
> Kamil> +  do
> Kamil> +    {
> Kamil> +      errno = 0;
> Kamil> +      ret = F (A...);
> Kamil> +    }
> Kamil> +  while (ret == -1 && errno == EINTR);
> 
> Also this seems to assume that "ret" is comparable to an int anyway.
> So maybe just assuming int everywhere would be ok.
> 

This assumption is not always valid, e.g. read(2) returns ssize_t.

I've added in the template flexibility of the returned type as it could
be e.g. nullptr for fopen(3).

> Tom
> 



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-03 17:42   ` Aktemur, Tankut Baris
@ 2020-09-04  0:13     ` Kamil Rytarowski
  2020-09-04  7:58       ` Aktemur, Tankut Baris
  0 siblings, 1 reply; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-04  0:13 UTC (permalink / raw)
  To: Aktemur, Tankut Baris, gdb-patches


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

On 03.09.2020 19:42, Aktemur, Tankut Baris wrote:
> On September 2, 2020 7:59 PM, Kamil Rytarowski wrote:
>> Implement the following functionality: create_inferior,
>> post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
>> resume, wait, fetch_registers, store_registers, read_memory, write_memory,
>> request_interrupt, supports_read_auxv, read_auxv,
>> supports_hardware_single_step, sw_breakpoint_from_kind,
>> supports_z_point_type, insert_point, remove_point,
>> stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
>> supports_stopped_by_sw_breakpoint, supports_non_stop,
>> supports_multi_process, supports_fork_events, supports_vfork_events,
>> supports_exec_events, supports_disable_randomization,
>> supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
>> supports_pid_to_exec_file, pid_to_exec_file, thread_name,
>> supports_catch_syscall.
>>
>> The only CPU architecture supported: x86_64.
>>
>> Implement only support for hardware assisted single step and
>> software breakpoint.
>>
>> Implement support only for regular X86 registers, thus no FPU.
>>
>> gdb/ChangeLog:
> 
> This should be gdbserver/ChangeLog.
> 

Fixed.

>>
>>        * netbsd-low.cc: Add.
>>        * netbsd-low.h: Likewise.
>>        * netbsd-x86_64-low.cc: Likewise.
>>        * Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
>>        "netbsd-x86_64-low.cc".
>>        * configure.srv: Add x86_64-*-netbsd*.
>> ---
>>  gdbserver/ChangeLog            |    9 +
>>  gdbserver/Makefile.in          |    3 +
>>  gdbserver/configure.srv        |    7 +
>>  gdbserver/netbsd-low.cc        | 1352 ++++++++++++++++++++++++++++++++
>>  gdbserver/netbsd-low.h         |  157 ++++
>>  gdbserver/netbsd-x86_64-low.cc |  250 ++++++
>>  6 files changed, 1778 insertions(+)
>>  create mode 100644 gdbserver/netbsd-low.cc
>>  create mode 100644 gdbserver/netbsd-low.h
>>  create mode 100644 gdbserver/netbsd-x86_64-low.cc
>>
>> diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
>> index e437493b56d..c0732d1a28a 100644
>> --- a/gdbserver/ChangeLog
>> +++ b/gdbserver/ChangeLog
>> @@ -1,3 +1,12 @@
>> +2020-08-13  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* netbsd-low.cc: Add.
>> +	* netbsd-low.h: Likewise.
>> +	* netbsd-x86_64-low.cc: Likewise.
>> +	* Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
>> +	"netbsd-x86_64-low.cc".
>> +	* configure.srv: Add x86_64-*-netbsd*.
>> +
>>  2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>
>>
>>  	* server.cc (captured_main): Accept multiple `--selftest=`
>> diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
>> index 9d7687be534..1d597b14523 100644
>> --- a/gdbserver/Makefile.in
>> +++ b/gdbserver/Makefile.in
>> @@ -193,6 +193,9 @@ SFILES = \
>>  	$(srcdir)/linux-x86-low.cc \
>>  	$(srcdir)/linux-xtensa-low.cc \
>>  	$(srcdir)/mem-break.cc \
>> +	$(srcdir)/netbsd-low.cc \
>> +	$(srcdir)/netbsd-low.h \
>> +	$(srcdir)/netbsd-x86_64-low.cc \
>>  	$(srcdir)/proc-service.cc \
>>  	$(srcdir)/proc-service.list \
>>  	$(srcdir)/regcache.cc \
>> diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
>> index 5e33bd9c54d..12f9d410b7c 100644
>> --- a/gdbserver/configure.srv
>> +++ b/gdbserver/configure.srv
>> @@ -349,6 +349,13 @@ case "${gdbserver_host}" in
>>  			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
>>  			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
>>  			;;
>> +  x86_64-*-netbsd*)	srv_regobj=""
>> +			srv_tgtobj="netbsd-low.o netbsd-x86_64-low.o fork-child.o"
>> +			srv_tgtobj="${srv_tgtobj} nat/fork-inferior.o"
>> +			srv_tgtobj="${srv_tgtobj} nat/netbsd-nat.o"
>> +			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
>> +			srv_netbsd=yes
> 
> Is `srv_netbsd` used anywhere?
> 

Not used. I have removed it.

Other targets use it in configure.ac and this is not needed for NetBDS.

>> +			;;
>>
>>    xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
>>  			srv_tgtobj="$srv_linux_obj linux-xtensa-low.o"
>> diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
>> new file mode 100644
>> index 00000000000..db9531c66d3
>> --- /dev/null
>> +++ b/gdbserver/netbsd-low.cc
>> @@ -0,0 +1,1352 @@
>> +/* Copyright (C) 2020 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include "server.h"
>> +#include "target.h"
>> +#include "netbsd-low.h"
>> +#include "nat/netbsd-nat.h"
>> +
>> +#include <sys/param.h>
>> +#include <sys/types.h>
>> +
>> +#include <sys/ptrace.h>
>> +#include <sys/sysctl.h>
>> +
>> +#include <limits.h>
>> +#include <unistd.h>
>> +#include <signal.h>
>> +
>> +#include <elf.h>
>> +
>> +#include <type_traits>
>> +
>> +#include "gdbsupport/eintr.h"
>> +#include "gdbsupport/gdb_wait.h"
>> +#include "gdbsupport/filestuff.h"
>> +#include "gdbsupport/common-inferior.h"
>> +#include "nat/fork-inferior.h"
>> +#include "hostio.h"
>> +
>> +int using_threads = 1;
>> +
>> +const struct target_desc *netbsd_tdesc;
>> +
>> +/* Call add_process with the given parameters, and initializes
> 
> To be consistent with the imperative "Call", "initializes" could read
> "initialize".
> 

Fixed.

>> +   the process' private data.  */
>> +
>> +static struct process_info *
>> +netbsd_add_process (int pid, int attached)
> 
> There are two uses of this function and in both the return value is ignored.
> The function could as well be void.
> 

Done.

>> +{
>> +  struct process_info *proc = add_process (pid, attached);
>> +  proc->tdesc = netbsd_tdesc;
>> +  proc->priv = nullptr;
>> +
>> +  return proc;
>> +}
>> +
>> +/* Callback used by fork_inferior to start tracing the inferior.  */
>> +
>> +static void
>> +netbsd_ptrace_fun ()
>> +{
>> +  /* Switch child to its own process group so that signals won't
>> +     directly affect GDBserver. */
>> +  if (setpgid (0, 0) < 0)
>> +    trace_start_error_with_name (("setpgid"));
>> +
>> +  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
>> +    trace_start_error_with_name (("ptrace"));
>> +
>> +  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
>> +     stdout to stderr so that inferior i/o doesn't corrupt the connection.
>> +     Also, redirect stdin to /dev/null.  */
>> +  if (remote_connection_is_stdio ())
>> +    {
>> +      if (close (0) < 0)
>> +	trace_start_error_with_name (("close"));
>> +      if (open ("/dev/null", O_RDONLY) < 0)
>> +	trace_start_error_with_name (("open"));
>> +      if (dup2 (2, 1) < 0)
>> +	trace_start_error_with_name (("dup2"));
>> +      if (write (2, "stdin/stdout redirected\n",
>> +		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
>> +	{
>> +	  /* Errors ignored.  */;
> 
> Any particular reason for the explicit ';'?
> 

Copied from Linux. I've removed the strat semicolon.

gdbserver/linux-low.cc:   /* Errors ignored.  */;

>> +	}
>> +    }
>> +}
>> +
>> +/* Register threads in the process specified by PID.  */
>> +
>> +static void
>> +netbsd_add_threads (pid_t pid)
>> +{
>> +  auto fn
>> +    = [] (ptid_t ptid)
>> +      {
>> +	add_thread (ptid, nullptr);
>> +      };
>> +
>> +  netbsd_nat::list_threads (pid, fn);
> 
> IMHO, the name "list_threads" gives the impression that it only lists the threads
> and is side-effect free.  Maybe use something like "for_each_thread", as in gdbthread.h?
> 

I've renamed it to for_each_thread.

>> +}
>> +
>> +/* Implement the create_inferior method of the target_ops vector.  */
>> +
>> +int
>> +netbsd_process_target::create_inferior (const char *program,
>> +					const std::vector<char *> &program_args)
>> +{
>> +  std::string str_program_args = construct_inferior_arguments (program_args);
>> +
>> +  pid_t pid = fork_inferior (program, str_program_args.c_str (),
>> +			     get_environ ()->envp (), netbsd_ptrace_fun,
>> +			     nullptr, nullptr, nullptr, nullptr);
>> +
>> +  netbsd_add_process (pid, 0);
>> +
>> +  post_fork_inferior (pid, program);
>> +
>> +  return pid;
>> +}
>> +
>> +/* Implement the post_create_inferior target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::post_create_inferior ()
>> +{
>> +  pid_t pid = current_process ()->pid;
>> +  netbsd_nat::enable_proc_events (pid);
>> +}
>> +
>> +/* Implement the attach target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::attach (unsigned long pid)
>> +{
>> +
>> +  if (ptrace (PT_ATTACH, pid, nullptr, 0) != 0)
>> +    error ("Cannot attach to process %lu: %s (%d)\n", pid,
>> +	    safe_strerror (errno), errno);
>> +
>> +  netbsd_add_process (pid, 1);
>> +  netbsd_nat::enable_proc_events (pid);
>> +  netbsd_add_threads (pid);
>> +
>> +  return 0;
>> +}
>> +
>> +/* Returns true if GDB is interested in any child syscalls.  */
>> +
>> +static bool
>> +gdb_catching_syscalls_p (pid_t pid)
>> +{
>> +  struct process_info *proc = find_process_pid (pid);
>> +  return !proc->syscalls_to_catch.empty ();
>> +}
>> +
>> +/* Implement the resume target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::resume (struct thread_resume *resume_info, size_t n)
>> +{
>> +  ptid_t resume_ptid = resume_info[0].thread;
>> +  const int signal = resume_info[0].sig;
>> +  const bool step = resume_info[0].kind == resume_step;
>> +
>> +  if (resume_ptid == minus_one_ptid)
>> +    resume_ptid = ptid_of (current_thread);
>> +
>> +  const pid_t pid = resume_ptid.pid ();
>> +  const lwpid_t lwp = resume_ptid.lwp ();
>> +  regcache_invalidate_pid (pid);
>> +
>> +  auto fn
>> +    = [&] (ptid_t ptid)
>> +      {
>> +	if (step)
>> +	  {
>> +	    if (ptid.lwp () == lwp || n != 1)
>> +	      {
>> +		if (ptrace (PT_SETSTEP, pid, NULL, ptid.lwp ()) == -1)
>> +		  perror_with_name (("ptrace"));
>> +		if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
>> +		  perror_with_name (("ptrace"));
>> +	      }
>> +	    else
>> +	      {
>> +		if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
>> +		  perror_with_name (("ptrace"));
>> +		if (ptrace (PT_SUSPEND, pid, NULL, ptid.lwp ()) == -1)
>> +		  perror_with_name (("ptrace"));
>> +	      }
>> +	  }
>> +	else
>> +	  {
>> +	    if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
>> +	      perror_with_name (("ptrace"));
>> +	    if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
>> +	      perror_with_name (("ptrace"));
>> +	  }
>> +      };
>> +
>> +  netbsd_nat::list_threads (pid, fn);
>> +
>> +  int request = gdb_catching_syscalls_p (pid) ? PT_CONTINUE : PT_SYSCALL;
>> +
>> +  errno = 0;
>> +  ptrace (request, pid, (void *)1, signal);
>> +  if (errno)
>> +    perror_with_name (("ptrace"));
>> +}
>> +
>> +/* Returns true if GDB is interested in the reported SYSNO syscall.  */
>> +
>> +static bool
>> +netbsd_catch_this_syscall (int sysno)
>> +{
>> +  struct process_info *proc = current_process ();
>> +
>> +  if (proc->syscalls_to_catch.empty ())
>> +    return false;
>> +
>> +  if (proc->syscalls_to_catch[0] == ANY_SYSCALL)
>> +    return true;
>> +
>> +  for (int iter : proc->syscalls_to_catch)
>> +    if (iter == sysno)
>> +      return true;
>> +
>> +  return false;
>> +}
>> +
>> +/* Helper function for child_wait and the derivatives of child_wait.
>> +   HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
>> +   translation of that in OURSTATUS.  */
> 
> A blank line here between the comment and the function header.
> 

Fixed.

>> +static void
>> +netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
>> +{
>> +  if (WIFEXITED (hoststatus))
>> +    {
>> +      ourstatus->kind = TARGET_WAITKIND_EXITED;
>> +      ourstatus->value.integer = WEXITSTATUS (hoststatus);
>> +    }
>> +  else if (!WIFSTOPPED (hoststatus))
>> +    {
>> +      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
>> +      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (hoststatus));
>> +    }
>> +  else
>> +    {
>> +      ourstatus->kind = TARGET_WAITKIND_STOPPED;
>> +      ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (hoststatus));
>> +    }
>> +}
>> +
>> +/* Implement a safe wrapper around waitpid().  */
>> +
>> +static pid_t
>> +netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
>> +{
>> +  int status;
>> +
>> +  pid_t pid = gdb::handle_eintr (::waitpid, ptid.pid (), &status, options);
>> +
>> +  if (pid == -1)
>> +    perror_with_name (_("Child process unexpectedly missing"));
>> +
>> +  netbsd_store_waitstatus (ourstatus, status);
>> +  return pid;
>> +}
>> +
>> +
>> +/* Implement the wait target_ops method.
>> +
>> +   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.  */
>> +
>> +static ptid_t
>> +netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>> +	     int target_options)
>> +{
>> +  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
>> +  ptid_t wptid = ptid_t (pid);
>> +
>> +  if (pid == 0)
>> +    {
>> +      gdb_assert (target_options & TARGET_WNOHANG);
>> +      ourstatus->kind = TARGET_WAITKIND_IGNORE;
>> +      return null_ptid;
>> +    }
>> +
>> +  gdb_assert (pid != -1);
>> +
>> +  /* 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;
>> +
>> +  lwpid_t 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..  */
> 
> Double periods.
> 

Fixed.

>> +  if (signo != SIGTRAP)
>> +    return wptid;
>> +
>> +  /* Stop examining non-debugger oriented SIGTRAP codes.  */
>> +  if (code <= SI_USER || code == SI_NOINFO)
>> +    return wptid;
>> +
>> +  /* Process state for threading events */
> 
> End with dot-space-space?
> 

Fixed.

>> +  ptrace_state_t pst = {};
>> +  if (code == TRAP_LWP)
>> +    {
>> +      if (ptrace (PT_GET_PROCESS_STATE, pid, &pst, sizeof (pst)) == -1)
>> +	perror_with_name (("ptrace"));
>> +    }
> 
> I think braces can be removed.
> 

Fixed.

>> +
>> +  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_EXIT)
>> +    {
>> +      /* If GDB attaches to a multi-threaded process, exiting
>> +	 threads might be skipped during post_attach that
>> +	 have not yet reported their PTRACE_LWP_EXIT event.
>> +	 Ignore exited events for an unknown LWP.  */
>> +      thread_info *thr = find_thread_ptid (wptid);
>> +      if (thr == nullptr)
>> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>> +      else
>> +	{
>> +	  ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
>> +	  /* NetBSD does not store an LWP exit status.  */
>> +	  ourstatus->value.integer = 0;
>> +
>> +	  remove_thread (thr);
>> +	}
>> +      return wptid;
>> +    }
>> +
>> +  if (find_thread_ptid (ptid_t (pid)))
>> +    current_thread = find_thread_ptid (wptid);
>> +
>> +  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_CREATE)
>> +    {
>> +      /* If GDB attaches to a multi-threaded process, newborn
>> +	 threads might be added by nbsd_add_threads that have
>> +	 not yet reported their PTRACE_LWP_CREATE event.  Ignore
>> +	 born events for an already-known LWP.  */
>> +      if (find_thread_ptid (wptid))
>> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>> +      else
>> +	{
>> +	  add_thread (wptid, NULL);
>> +	  ourstatus->kind = TARGET_WAITKIND_THREAD_CREATED;
>> +	}
>> +      return wptid;
>> +    }
>> +
>> +  if (code == TRAP_EXEC)
>> +    {
>> +      ourstatus->kind = TARGET_WAITKIND_EXECD;
>> +      ourstatus->value.execd_pathname
>> +	= xstrdup (netbsd_nat::pid_to_exec_file (pid));
>> +      return wptid;
>> +    }
>> +
>> +  if (code == TRAP_TRACE)
>> +    {
>> +      return wptid;
>> +    }
> 
> Braces can be removed.
> 

Fixed.

>> +
>> +  if (code == TRAP_SCE || code == TRAP_SCX)
>> +    {
>> +      int sysnum = si->si_sysnum;
>> +
>> +      if (!netbsd_catch_this_syscall(sysnum))
>> +	{
>> +	  /* If the core isn't interested in this event, ignore it.  */
>> +	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>> +	  return wptid;
>> +	}
>> +
>> +      ourstatus->kind =
> 
> The assignment operator should be on the next line.
> 

Fixed and added () for nicer alignment.

>> +	(code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
>> +	TARGET_WAITKIND_SYSCALL_RETURN;
>> +      ourstatus->value.syscall_number = sysnum;
>> +      return wptid;
>> +    }
>> +
>> +  if (code == TRAP_BRKPT)
>> +    {
>> +#ifdef PTRACE_BREAKPOINT_ADJ
>> +      CORE_ADDR pc;
>> +      struct reg r;
>> +      ptrace (PT_GETREGS, pid, &r, psi.psi_lwpid);
>> +      pc = PTRACE_REG_PC (&r);
>> +      PTRACE_REG_SET_PC (&r, pc - PTRACE_BREAKPOINT_ADJ);
>> +      ptrace (PT_SETREGS, pid, &r, psi.psi_lwpid);
>> +#endif
>> +      return wptid;
>> +    }
>> +
>> +  /* Unclassified SIGTRAP event.  */
>> +  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
>> +  return wptid;
>> +}
>> +
>> +/* Implement the wait target_ops method.  */
>> +
>> +ptid_t
>> +netbsd_process_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
>> +			     int target_options)
>> +{
>> +  while (true)
>> +    {
>> +      ptid_t wptid = netbsd_wait (ptid, ourstatus, target_options);
>> +
>> +      /* Register thread in the gdbcore if a thread was not reported earlier.
>> +	 This is required after ::create_inferior, when the gdbcore does not
>> +	 know about the first internal thread.
>> +	 This may also happen on attach, when an event is registered on a thread
>> +	 that was not fully initialized during the attach stage. */
> 
> Dot-space-space.
> 

Fixed.

>> +      if (wptid.lwp () != 0 && !find_thread_ptid (wptid)
>> +	  && ourstatus->kind != TARGET_WAITKIND_THREAD_EXITED)
>> +	add_thread (wptid, nullptr);
>> +
>> +      switch (ourstatus->kind)
>> +	{
>> +	case TARGET_WAITKIND_EXITED:
>> +	case TARGET_WAITKIND_STOPPED:
>> +	case TARGET_WAITKIND_SIGNALLED:
>> +	case TARGET_WAITKIND_FORKED:
>> +	case TARGET_WAITKIND_VFORKED:
>> +	case TARGET_WAITKIND_EXECD:
>> +	case TARGET_WAITKIND_VFORK_DONE:
>> +	case TARGET_WAITKIND_SYSCALL_ENTRY:
>> +	case TARGET_WAITKIND_SYSCALL_RETURN:
>> +	  /* Pass the result to the generic code.  */
>> +	  return wptid;
>> +	case TARGET_WAITKIND_THREAD_CREATED:
>> +	case TARGET_WAITKIND_THREAD_EXITED:
>> +	  /* The core needlessly stops on these events.  */
>> +	  /* FALLTHROUGH */
>> +	case TARGET_WAITKIND_SPURIOUS:
>> +	  /* Spurious events are unhandled by the gdbserver core.  */
>> +	  if (ptrace (PT_CONTINUE, current_process ()->pid, (void *) 1, 0)
>> +	      == -1)
>> +	    perror_with_name (("ptrace"));
>> +	  break;
>> +	default:
>> +	  error (("Unknwon stopped status"));
> 
> Typo "Unknwon".
> 

Fixed.

>> +	}
>> +    }
>> +}
>> +
>> +/* Implement the kill target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::kill (process_info *process)
>> +{
>> +  pid_t pid = process->pid;
>> +  ptrace (PT_KILL, pid, nullptr, 0);
>> +
>> +  int status;
>> +  gdb::handle_eintr (::waitpid, pid, &status, 0);
> 
> The "kill" target op is supposed to return -1 on failure.
> So, maybe not ignore the return value of handle_eintr here?
> 

Fixed. I've added an error check for ptrace(2) and waitpid(2).

>> +  mourn (process);
>> +  return 0;
>> +}
>> +
>> +/* Implement the detach target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::detach (process_info *process)
>> +{
>> +  pid_t pid = process->pid;
>> +
>> +  ptrace (PT_DETACH, pid, (void *) 1, 0);
>> +  mourn (process);
>> +  return 0;
>> +}
>> +
>> +/* Implement the mourn target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::mourn (struct process_info *proc)
>> +{
>> +  for_each_thread (proc->pid, remove_thread);
>> +
>> +  remove_process (proc);
>> +}
>> +
>> +/* Implement the join target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::join (int pid)
>> +{
>> +  /* The PT_DETACH is sufficient to detach from the process.
>> +     So no need to do anything extra.  */
>> +}
>> +
>> +/* Implement the thread_alive target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::thread_alive (ptid_t ptid)
>> +{
>> +  return netbsd_nat::thread_alive (ptid);
>> +}
>> +
>> +/* Implement the fetch_registers target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
>> +{
>> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
>> +  ptid_t inferior_ptid = ptid_of (current_thread);
>> +
>> +  while (regset->size >= 0)
>> +    {
>> +      char *buf = (char *) xmalloc (regset->size);
>> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +      if (res == -1)
>> +	perror_with_name (("ptrace"));
>> +      regset->store_function (regcache, buf);
>> +      free (buf);
>> +      regset++;
>> +    }
>> +}
>> +
>> +/* Implement the store_registers target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::store_registers (struct regcache *regcache, int regno)
>> +{
>> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
>> +  ptid_t inferior_ptid = ptid_of (current_thread);
>> +
>> +  while (regset->size >= 0)
>> +    {
>> +      char *buf = (char *)xmalloc (regset->size);
>> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +      if (res == -1)
>> +	perror_with_name (("ptrace"));
>> +      if (res == 0)
>> +	{
>> +	  /* Then overlay our cached registers on that.  */
>> +	  regset->fill_function (regcache, buf);
>> +	  /* Only now do we write the register set.  */
>> +	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +	}
>> +      free (buf);
>> +      regset++;
>> +    }
>> +}
>> +
>> +/* Implement the read_memory target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
>> +				    int size)
>> +{
>> +  struct ptrace_io_desc io;
>> +  io.piod_op = PIOD_READ_D;
>> +  io.piod_len = size;
>> +
>> +  pid_t pid = current_process ()->pid;
>> +
>> +  int bytes_read = 0;
>> +
>> +  if (size == 0)
>> +    {
>> +      /* Zero length write always succeeds.  */
>> +      return 0;
>> +    }
>> +  do
>> +    {
>> +      io.piod_offs = (void *)(memaddr + bytes_read);
>> +      io.piod_addr = myaddr + bytes_read;
>> +
>> +      int rv = ptrace (PT_IO, pid, &io, 0);
>> +      if (rv == -1)
>> +	return errno;
>> +      if (io.piod_len == 0)
>> +	return 0;
>> +
>> +      bytes_read += io.piod_len;
>> +      io.piod_len = size - bytes_read;
>> +    }
>> +  while (bytes_read < size);
>> +
>> +  return 0;
>> +}
>> +
>> +/* Implement the write_memory target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::write_memory (CORE_ADDR memaddr,
>> +				     const unsigned char *myaddr, int size)
>> +{
>> +  struct ptrace_io_desc io;
>> +  io.piod_op = PIOD_WRITE_D;
>> +  io.piod_len = size;
>> +
>> +  pid_t pid = current_process ()->pid;
>> +
>> +  int bytes_written = 0;
>> +
>> +  if (size == 0)
>> +    {
>> +      /* Zero length write always succeeds.  */
>> +      return 0;
>> +    }
>> +
>> +  do
>> +    {
>> +      io.piod_addr = (void *)(myaddr + bytes_written);
>> +      io.piod_offs = (void *)(memaddr + bytes_written);
>> +
>> +      int rv = ptrace (PT_IO, pid, &io, 0);
>> +      if (rv == -1)
>> +	return errno;
>> +      if (io.piod_len == 0)
>> +	return 0;
>> +
>> +      bytes_written += io.piod_len;
>> +      io.piod_len = size - bytes_written;
>> +    }
>> +  while (bytes_written < size);
>> +
>> +  return 0;
>> +}
>> +
>> +/* Implement the kill_request target_ops method.  */
> 
> "kill_request" -> "request_interrupt".
> 

Fixed.

>> +
>> +void
>> +netbsd_process_target::request_interrupt ()
>> +{
>> +  ptid_t inferior_ptid = ptid_of (get_first_thread ());
>> +
>> +  ::kill (inferior_ptid.pid(), SIGINT);
>> +}
>> +
> 
> I think a function comment is needed here.
> 

Fixed.

>> +static size_t
>> +netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
>> +{
>> +  struct ptrace_io_desc pio;
>> +
>> +  pio.piod_op = PIOD_READ_AUXV;
>> +  pio.piod_offs = offs;
>> +  pio.piod_addr = addr;
>> +  pio.piod_len = len;
>> +
>> +  if (ptrace (PT_IO, pid, &pio, 0) == -1)
>> +    perror_with_name (("ptrace"));
>> +
>> +  return pio.piod_len;
>> +}
>> +
>> +/* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
>> +   to debugger memory starting at MYADDR.  */
>> +
>> +int
>> +netbsd_process_target::read_auxv (CORE_ADDR offset,
>> +				  unsigned char *myaddr, unsigned int len)
>> +{
>> +  pid_t pid = pid_of (current_thread);
>> +
>> +  return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
>> +}
>> +
>> +bool
>> +netbsd_process_target::supports_z_point_type (char z_type)
>> +{
>> +  switch (z_type)
>> +    {
>> +    case Z_PACKET_SW_BP:
>> +      return true;
>> +    case Z_PACKET_HW_BP:
>> +    case Z_PACKET_WRITE_WP:
>> +    case Z_PACKET_READ_WP:
>> +    case Z_PACKET_ACCESS_WP:
>> +    default:
>> +      return false; /* Not supported.  */
>> +    }
>> +}
>> +
>> +/* Insert {break/watch}point at address ADDR.  SIZE is not used.  */
>> +
>> +int
>> +netbsd_process_target::insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
>> +		     int size, struct raw_breakpoint *bp)
>> +{
>> +  switch (type)
>> +    {
>> +    case raw_bkpt_type_sw:
>> +      return insert_memory_breakpoint (bp);
>> +    case raw_bkpt_type_hw:
>> +    case raw_bkpt_type_write_wp:
>> +    case raw_bkpt_type_read_wp:
>> +    case raw_bkpt_type_access_wp:
>> +    default:
>> +      return 1; /* Not supported.  */
>> +    }
>> +}
>> +
>> +/* Remove {break/watch}point at address ADDR.  SIZE is not used.  */
>> +
>> +int
>> +netbsd_process_target::remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
>> +				     int size, struct raw_breakpoint *bp)
>> +{
>> +  switch (type)
>> +    {
>> +    case raw_bkpt_type_sw:
>> +      return remove_memory_breakpoint (bp);
>> +    case raw_bkpt_type_hw:
>> +    case raw_bkpt_type_write_wp:
>> +    case raw_bkpt_type_read_wp:
>> +    case raw_bkpt_type_access_wp:
>> +    default:
>> +      return 1; /* Not supported.  */
>> +    }
>> +}
>> +
>> +/* Implement the to_stopped_by_sw_breakpoint target_ops
>> +   method.  */
> 
> "to_stopped_by_sw_breakpoint" -> "stopped_by_sw_breakpoint".
> 

Fixed.

>> +
>> +bool
>> +netbsd_process_target::stopped_by_sw_breakpoint ()
>> +{
>> +  ptrace_siginfo_t psi;
>> +  pid_t pid = current_process ()->pid;
>> +
>> +  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
>> +    perror_with_name (("ptrace"));
>> +
>> +  return psi.psi_siginfo.si_signo == SIGTRAP &&
>> +	 psi.psi_siginfo.si_code == TRAP_BRKPT;
>> +}
>> +
>> +/* Implement the to_supports_stopped_by_sw_breakpoint target_ops
>> +   method.  */
> 
> The "to_" prefix shall be removed here, too.
> 

Fixed.

>> +
>> +bool
>> +netbsd_process_target::supports_stopped_by_sw_breakpoint ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implement the supports_qxfer_siginfo target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_qxfer_siginfo ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implement the qxfer_siginfo target_ops method.  */
>> +
>> +int
>> +netbsd_process_target::qxfer_siginfo (const char *annex, unsigned char *readbuf,
>> +				      unsigned const char *writebuf,
>> +				      CORE_ADDR offset, int len)
>> +{
>> +  if (current_thread == nullptr)
>> +    return -1;
>> +
>> +  pid_t pid = current_process ()->pid;
>> +
>> +  return netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset, len);
>> +}
>> +
>> +/* Implement the supports_non_stop target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_non_stop ()
>> +{
>> +  return false;
>> +}
>> +
>> +/* Implement the supports_multi_process target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_multi_process ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Check if fork events are supported.  */
>> +
>> +bool
>> +netbsd_process_target::supports_fork_events ()
>> +{
>> +  return false;
>> +}
>> +
>> +/* Check if vfork events are supported.  */
>> +
>> +bool
>> +netbsd_process_target::supports_vfork_events ()
>> +{
>> +  return false;
>> +}
>> +
>> +/* Check if exec events are supported.  */
>> +
>> +bool
>> +netbsd_process_target::supports_exec_events ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implement the supports_disable_randomization target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_disable_randomization ()
>> +{
>> +  return false;
>> +}
> 
> Some of the target ops above do not change the base class behavior.  The override
> can be omitted for code simplicity.  I think it's also OK to retain them if you want
> to make the definitions explicit.
> 

Most of the can be supported in future, except
supports_disable_randomization and supports_non_stop.

I will leave them around for the time being.

>> +
>> +/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
>> +
>> +template <typename T>
>> +int get_phdr_phnum_from_proc_auxv (const pid_t pid,
>> +				   CORE_ADDR *phdr_memaddr, int *num_phdr)
>> +{
>> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
>> +				    Aux64Info, Aux32Info>::type auxv_type;
>> +  const size_t auxv_size = sizeof (auxv_type);
>> +  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
>> +
>> +  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
>> +
>> +  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
>> +
>> +  *phdr_memaddr = 0;
>> +  *num_phdr = 0;
>> +
>> +  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
>> +    {
>> +      auxv_type *const aux = (auxv_type *) buf;
>> +
>> +      switch (aux->a_type)
>> +	{
>> +	case AT_PHDR:
>> +	  *phdr_memaddr = aux->a_v;
>> +	  break;
>> +	case AT_PHNUM:
>> +	  *num_phdr = aux->a_v;
>> +	  break;
>> +	}
>> +
>> +      if (*phdr_memaddr != 0 && *num_phdr != 0)
>> +	break;
>> +    }
>> +
>> +  xfree (auxv_buf);
>> +
>> +  if (*phdr_memaddr == 0 || *num_phdr == 0)
>> +    {
>> +      warning ("Unexpected missing AT_PHDR and/or AT_PHNUM: "
>> +	       "phdr_memaddr = %ld, phdr_num = %d",
>> +	       (long) *phdr_memaddr, *num_phdr);
> 
> It might be better to use `core_addr_to_string` or `core_addr_to_string_nz`
> to print phdr_memaddr.
> 

Fixed. I've picked core_addr_to_string ().

>> +      return 2;
>> +    }
>> +
>> +  return 0;
>> +}
>> +
>> +/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
>> +
>> +template <typename T>
>> +static CORE_ADDR
>> +get_dynamic (netbsd_process_target *target, const pid_t pid)
>> +{
>> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
>> +				    Elf64_Phdr, Elf32_Phdr>::type phdr_type;
>> +  const int phdr_size = sizeof (phdr_type);
>> +
>> +  CORE_ADDR phdr_memaddr;
>> +  int num_phdr;
>> +  if (get_phdr_phnum_from_proc_auxv<T> (pid, &phdr_memaddr, &num_phdr))
>> +    return 0;
>> +
>> +  std::vector<unsigned char> phdr_buf;
>> +  phdr_buf.resize (num_phdr * phdr_size);
>> +
>> +  if ((*target).read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
> 
> Why not simply `target->read_memory`?

Fixed.

> Also, it might be better to explicitly check != 0.
> 

I've avoided it as the line is too long and would need to be wrapped.

>> +    return 0;
>> +
>> +  /* Compute relocation: it is expected to be 0 for "regular" executables,
>> +     non-zero for PIE ones.  */
>> +  CORE_ADDR relocation = -1;
>> +  for (int i = 0; relocation == -1 && i < num_phdr; i++)
>> +    {
>> +      phdr_type *const p = (phdr_type *) (phdr_buf.data() + i * phdr_size);
>> +
>> +      if (p->p_type == PT_PHDR)
>> +	relocation = phdr_memaddr - p->p_vaddr;
>> +    }
>> +
>> +  if (relocation == -1)
>> +    {
>> +      /* PT_PHDR is optional, but necessary for PIE in general.  Fortunately
>> +	 any real world executables, including PIE executables, have always
>> +	 PT_PHDR present.  PT_PHDR is not present in some shared libraries or
>> +	 in fpc (Free Pascal 2.4) binaries but neither of those have a need for
>> +	 or present DT_DEBUG anyway (fpc binaries are statically linked).
>> +
>> +	 Therefore if there exists DT_DEBUG there is always also PT_PHDR.
>> +
>> +	 GDB could find RELOCATION also from AT_ENTRY - e_entry.  */
>> +
>> +      return 0;
>> +    }
>> +
>> +  for (int i = 0; i < num_phdr; i++)
>> +    {
>> +      phdr_type *const p = (phdr_type *) (phdr_buf.data () + i * phdr_size);
>> +
>> +      if (p->p_type == PT_DYNAMIC)
>> +	return p->p_vaddr + relocation;
>> +    }
>> +
>> +  return 0;
>> +}
>> +
>> +/* Return &_r_debug in the inferior, or -1 if not present.  Return value
>> +   can be 0 if the inferior does not yet have the library list initialized.
>> +   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
>> +   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
>> +
>> +template <typename T>
>> +static CORE_ADDR
>> +get_r_debug (netbsd_process_target *target, const int pid)
>> +{
>> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
>> +				    Elf64_Dyn, Elf32_Dyn>::type dyn_type;
>> +  const int dyn_size = sizeof (dyn_type);
>> +  unsigned char buf[sizeof (dyn_type)];  /* The larger of the two.  */
>> +  CORE_ADDR map = -1;
>> +
>> +  CORE_ADDR dynamic_memaddr = get_dynamic<T> (target, pid);
>> +  if (dynamic_memaddr == 0)
>> +    return map;
>> +
>> +  while ((*target).read_memory (dynamic_memaddr, buf, dyn_size) == 0)
> 
> The `target->read_memory` comment here, too.
> 

Fixed.

>> +    {
>> +      dyn_type *const dyn = (dyn_type *) buf;
>> +#if defined DT_MIPS_RLD_MAP
>> +      union
>> +      {
>> +	T map;
>> +	unsigned char buf[sizeof (T)];
>> +      }
>> +      rld_map;
>> +
>> +      if (dyn->d_tag == DT_MIPS_RLD_MAP)
>> +	{
>> +	  if (read_memory (dyn->d_un.d_val,
>> +			   rld_map.buf, sizeof (rld_map.buf)) == 0)
>> +	    return rld_map.map;
>> +	  else
>> +	    break;
>> +	}
>> +#endif  /* DT_MIPS_RLD_MAP */
>> +
>> +      if (dyn->d_tag == DT_DEBUG && map == -1)
>> +	map = dyn->d_un.d_val;
>> +
>> +      if (dyn->d_tag == DT_NULL)
>> +	break;
>> +
>> +      dynamic_memaddr += dyn_size;
>> +    }
>> +
>> +  return map;
>> +}
>> +
>> +/* Read one pointer from MEMADDR in the inferior.  */
>> +
>> +static int
>> +read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
>> +	      int ptr_size)
>> +{
>> +  /* Go through a union so this works on either big or little endian
>> +     hosts, when the inferior's pointer size is smaller than the size
>> +     of CORE_ADDR.  It is assumed the inferior's endianness is the
>> +     same of the superior's.  */
>> +
>> +  union
>> +  {
>> +    CORE_ADDR core_addr;
>> +    unsigned int ui;
>> +    unsigned char uc;
>> +  } addr;
>> +
>> +  int ret = (*target).read_memory (memaddr, &addr.uc, ptr_size);
> 
> Here, too.
> 

Fixed.

>> +  if (ret == 0)
>> +    {
>> +      if (ptr_size == sizeof (CORE_ADDR))
>> +	*ptr = addr.core_addr;
>> +      else if (ptr_size == sizeof (unsigned int))
>> +	*ptr = addr.ui;
>> +      else
>> +	gdb_assert_not_reached ("unhandled pointer size");
>> +    }
>> +  return ret;
>> +}
>> +
>> +/* Construct qXfer:libraries-svr4:read reply.  */
>> +
>> +template <typename T>
>> +int
>> +netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
>> +			     const pid_t pid, const char *annex,
>> +			     unsigned char *readbuf,
>> +			     unsigned const char *writebuf,
>> +			     CORE_ADDR offset, int len)
>> +{
>> +  struct link_map_offsets
>> +  {
>> +    /* Offset and size of r_debug.r_version.  */
>> +    int r_version_offset;
>> +
>> +    /* Offset and size of r_debug.r_map.  */
>> +    int r_map_offset;
>> +
>> +    /* Offset to l_addr field in struct link_map.  */
>> +    int l_addr_offset;
>> +
>> +    /* Offset to l_name field in struct link_map.  */
>> +    int l_name_offset;
>> +
>> +    /* Offset to l_ld field in struct link_map.  */
>> +    int l_ld_offset;
>> +
>> +    /* Offset to l_next field in struct link_map.  */
>> +    int l_next_offset;
>> +
>> +    /* Offset to l_prev field in struct link_map.  */
>> +    int l_prev_offset;
>> +  };
>> +
>> +  static const struct link_map_offsets lmo_32bit_offsets =
>> +    {
>> +      0,     /* r_version offset. */
>> +      4,     /* r_debug.r_map offset.  */
>> +      0,     /* l_addr offset in link_map.  */
>> +      4,     /* l_name offset in link_map.  */
>> +      8,     /* l_ld offset in link_map.  */
>> +      12,    /* l_next offset in link_map.  */
>> +      16     /* l_prev offset in link_map.  */
>> +    };
>> +
>> +  static const struct link_map_offsets lmo_64bit_offsets =
>> +    {
>> +      0,     /* r_version offset. */
>> +      8,     /* r_debug.r_map offset.  */
>> +      0,     /* l_addr offset in link_map.  */
>> +      8,     /* l_name offset in link_map.  */
>> +      16,    /* l_ld offset in link_map.  */
>> +      24,    /* l_next offset in link_map.  */
>> +      32     /* l_prev offset in link_map.  */
>> +    };
>> +
>> +  CORE_ADDR lm_addr = 0, lm_prev = 0;
>> +  CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
>> +  int header_done = 0;
>> +
>> +  const struct link_map_offsets *lmo
>> +    = (sizeof (T) == sizeof (int64_t))
>> +    ? &lmo_64bit_offsets : &lmo_32bit_offsets;
> 
> An extra pair of parens can be used to better align '?'.  Like this:
> 
>   const struct link_map_offsets *lmo
>     = ((sizeof (T) == sizeof (int64_t))
>        ? &lmo_64bit_offsets : &lmo_32bit_offsets);
> 

Fixed.

>> +  int ptr_size = sizeof (T);
>> +
>> +  while (annex[0] != '\0')
>> +    {
>> +      const char *sep = strchr (annex, '=');
>> +      if (sep == nullptr)
>> +	break;
>> +
>> +      int name_len = sep - annex;
>> +      CORE_ADDR *addrp;
>> +      if (name_len == 5 && startswith (annex, "start"))
>> +	addrp = &lm_addr;
>> +      else if (name_len == 4 && startswith (annex, "prev"))
>> +	addrp = &lm_prev;
>> +      else
>> +	{
>> +	  annex = strchr (sep, ';');
>> +	  if (annex == nullptr)
>> +	    break;
>> +	  annex++;
>> +	  continue;
>> +	}
>> +
>> +      annex = decode_address_to_semicolon (addrp, sep + 1);
>> +    }
>> +
>> +  if (lm_addr == 0)
>> +    {
>> +      CORE_ADDR r_debug = get_r_debug<T> (target, pid);
>> +
>> +      /* We failed to find DT_DEBUG.  Such situation will not change
>> +	 for this inferior - do not retry it.  Report it to GDB as
>> +	 E01, see for the reasons at the GDB solib-svr4.c side.  */
>> +      if (r_debug == (CORE_ADDR) -1)
>> +	return -1;
>> +
>> +      if (r_debug != 0)
>> +	{
>> +	  if (read_one_ptr (target, r_debug + lmo->r_map_offset,
>> +			    &lm_addr, ptr_size) != 0)
>> +	    {
>> +	      warning ("unable to read r_map from 0x%lx",
>> +		       (long) r_debug + lmo->r_map_offset);
> 
> Again, `core_addr_to_string` could be used.
> 

Fixed and I've refactored this code into:
+         CORE_ADDR map_offset = r_debug + lmo->r_map_offset;
+         if (read_one_ptr (target, map_offset, &lm_addr, ptr_size) != 0)
+           warning ("unable to read r_map from %s",
+                    core_addr_to_string (map_offset));

>> +	    }
>> +	}
> 
> Braces can be removed in both conditional statements above.  And, if you prefer,
> the conditions can be merged via &&.
> 

Fixed.

>> +    }
>> +
>> +  std::string document = "<library-list-svr4 version=\"1.0\"";
>> +
>> +  while (lm_addr
>> +	 && read_one_ptr (target, lm_addr + lmo->l_name_offset,
>> +			  &l_name, ptr_size) == 0
>> +	 && read_one_ptr (target, lm_addr + lmo->l_addr_offset,
>> +			  &l_addr, ptr_size) == 0
>> +	 && read_one_ptr (target, lm_addr + lmo->l_ld_offset,
>> +			  &l_ld, ptr_size) == 0
>> +	 && read_one_ptr (target, lm_addr + lmo->l_prev_offset,
>> +			  &l_prev, ptr_size) == 0
>> +	 && read_one_ptr (target, lm_addr + lmo->l_next_offset,
>> +			  &l_next, ptr_size) == 0)
>> +    {
>> +      if (lm_prev != l_prev)
>> +	{
>> +	  warning ("Corrupted shared library list: 0x%lx != 0x%lx",
>> +		   (long) lm_prev, (long) l_prev);
>> +	  break;
>> +	}
>> +
>> +      /* Ignore the first entry even if it has valid name as the first entry
>> +	 corresponds to the main executable.  The first entry should not be
>> +	 skipped if the dynamic loader was loaded late by a static executable
>> +	 (see solib-svr4.c parameter ignore_first).  But in such case the main
>> +	 executable does not have PT_DYNAMIC present and this function already
>> +	 exited above due to failed get_r_debug.  */
>> +      if (lm_prev == 0)
>> +	string_appendf (document, " main-lm=\"0x%lx\"",
>> +			(unsigned long) lm_addr);
>> +      else
>> +	{
>> +	  unsigned char libname[PATH_MAX];
>> +
>> +	  /* Not checking for error because reading may stop before
>> +	     we've got PATH_MAX worth of characters.  */
>> +	  libname[0] = '\0';
>> +	  (*target).read_memory (l_name, libname, sizeof (libname) - 1);
>> +	  libname[sizeof (libname) - 1] = '\0';
>> +	  if (libname[0] != '\0')
>> +	    {
>> +	      if (!header_done)
>> +		{
>> +		  /* Terminate `<library-list-svr4'.  */
>> +		  document += '>';
>> +		  header_done = 1;
>> +		}
>> +
>> +	      string_appendf (document, "<library name=\"");
>> +	      xml_escape_text_append (&document, (char *) libname);
>> +	      string_appendf (document, "\" lm=\"0x%lx\" "
>> +			      "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
>> +			      (unsigned long) lm_addr, (unsigned long) l_addr,
>> +			      (unsigned long) l_ld);
>> +	    }
>> +	}
>> +
>> +      lm_prev = lm_addr;
>> +      lm_addr = l_next;
>> +    }
>> +
>> +  if (!header_done)
>> +    {
>> +      /* Empty list; terminate `<library-list-svr4'.  */
>> +      document += "/>";
>> +    }
>> +  else
>> +    document += "</library-list-svr4>";
>> +
>> +  int document_len = document.length ();
>> +  if (offset < document_len)
>> +    document_len -= offset;
>> +  else
>> +    document_len = 0;
>> +  if (len > document_len)
>> +    len = document_len;
>> +
>> +  memcpy (readbuf, document.data () + offset, len);
>> +
>> +  return len;
>> +}
>> +
>> +/* Return true if FILE is a 64-bit ELF file,
>> +   false if the file is not a 64-bit ELF file,
>> +   and error if the file is not accessible or doesn't exist.  */
>> +
>> +static bool
>> +elf_64_file_p (const char *file)
>> +{
>> +  int fd = gdb::handle_eintr (::open, file, O_RDONLY);
>> +  if (fd < 0)
>> +    perror_with_name (("open"));
>> +
>> +  Elf64_Ehdr header;
>> +  ssize_t ret = gdb::handle_eintr (::read, fd, &header, sizeof (header));
>> +  if (ret == -1)
>> +    perror_with_name (("read"));
>> +  gdb::handle_eintr (::close, fd);
>> +  if (ret != sizeof (header))
>> +    error ("Cannot read ELF file header: %s", file);
>> +
>> +  if (header.e_ident[EI_MAG0] != ELFMAG0
>> +      || header.e_ident[EI_MAG1] != ELFMAG1
>> +      || header.e_ident[EI_MAG2] != ELFMAG2
>> +      || header.e_ident[EI_MAG3] != ELFMAG3)
>> +    error ("Unrecognized ELF file header: %s", file);
>> +
>> +  return header.e_ident[EI_CLASS] == ELFCLASS64;
>> +}
>> +
>> +/* Construct qXfer:libraries-svr4:read reply.  */
>> +
>> +int
>> +netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
>> +					     unsigned char *readbuf,
>> +					     unsigned const char *writebuf,
>> +					     CORE_ADDR offset, int len)
>> +{
>> +  if (writebuf != nullptr)
>> +    return -2;
>> +  if (readbuf == nullptr)
>> +    return -1;
>> +
>> +  struct process_info *proc = current_process ();
>> +  pid_t pid = proc->pid;
>> +  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
>> +
>> +  if (is_elf64)
>> +    return netbsd_qxfer_libraries_svr4<int64_t> (this, pid, annex, readbuf,
>> +						 writebuf, offset, len);
>> +  else
>> +    return netbsd_qxfer_libraries_svr4<int32_t> (this, pid, annex, readbuf,
>> +						 writebuf, offset, len);
>> +}
>> +
>> +/* Implement the supports_qxfer_libraries_svr4 target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_qxfer_libraries_svr4 ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Return the name of a file that can be opened to get the symbols for
>> +   the child process identified by PID.  */
>> +
>> +char *
>> +netbsd_process_target::pid_to_exec_file (pid_t pid)
>> +{
>> +  return netbsd_nat::pid_to_exec_file (pid);
>> +}
>> +
>> +/* Implementation of the target_ops method "supports_pid_to_exec_file".  */
>> +
>> +bool
>> +netbsd_process_target::supports_pid_to_exec_file ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implementation of the target_ops method "supports_hardware_single_step".  */
>> +bool
>> +netbsd_process_target::supports_hardware_single_step ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implementation of the target_ops method "sw_breakpoint_from_kind".  */
>> +
>> +const gdb_byte *
>> +netbsd_process_target::sw_breakpoint_from_kind (int kind, int *size)
>> +{
>> +  static gdb_byte brkpt[PTRACE_BREAKPOINT_SIZE];
>> +
>> +  *size = PTRACE_BREAKPOINT_SIZE;
>> +
>> +  memcpy (brkpt, PTRACE_BREAKPOINT, PTRACE_BREAKPOINT_SIZE);
> 
> Is there a need to memcpy each time the function is called?
> 

I've optimized it to remove memcpy(3).

>> +
>> +  return brkpt;
>> +}
>> +
>> +/* Implement the thread_name target_ops method.  */
>> +
>> +const char *
>> +netbsd_process_target::thread_name (ptid_t ptid)
>> +{
>> +  return netbsd_nat::thread_name (ptid);
>> +}
>> +
>> +/* Implement the supports_catch_syscall target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_catch_syscall ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* Implement the supports_read_auxv target_ops method.  */
>> +
>> +bool
>> +netbsd_process_target::supports_read_auxv ()
>> +{
>> +  return true;
>> +}
>> +
>> +/* The NetBSD target ops object.  */
>> +
>> +static netbsd_process_target the_netbsd_target;
>> +
>> +void
>> +initialize_low ()
>> +{
>> +  set_target_ops (&the_netbsd_target);
>> +  the_low_target.arch_setup ();
>> +}
>> diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
>> new file mode 100644
>> index 00000000000..2bd57c51726
>> --- /dev/null
>> +++ b/gdbserver/netbsd-low.h
>> @@ -0,0 +1,157 @@
>> +/* Copyright (C) 2020 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#ifndef GDBSERVER_NETBSD_LOW_H
>> +#define GDBSERVER_NETBSD_LOW_H
>> +
>> +struct regcache;
>> +struct target_desc;
>> +
>> +/*  Some information relative to a given register set.   */
>> +
>> +struct netbsd_regset_info
>> +{
>> +  /* The ptrace request needed to get/set registers of this set.  */
>> +  int get_request, set_request;
>> +  /* The size of the register set.  */
>> +  int size;
>> +  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
>> +  void (*fill_function) (struct regcache *regcache, char *buf);
>> +  /* Store the register value in BUF in the given REGCACHE.  */
>> +  void (*store_function) (struct regcache *regcache, const char *buf);
> 
> Leaving one blank line between each field may improve readability.
> 

Fixed.

>> +};
>> +
>> +/* A list of regsets for the target being debugged, terminated by an entry
>> +   where the size is negative.
>> +
>> +   This list should be created by the target-specific code.  */
>> +
>> +extern struct netbsd_regset_info netbsd_target_regsets[];
>> +
>> +/* The target-specific operations for NetBSD support.  */
>> +
>> +struct netbsd_target_ops
>> +{
>> +  /* Architecture-specific setup.  */
>> +  void (*arch_setup) ();
>> +
>> +  /* Hook to support target specific qSupported.  */
>> +  void (*process_qsupported) (char **, int count);
> 
> Is the `process_qsupported` function called anywhere?
> 

Hmm... it was originally copied from Linux (before C++ification). It
looks like unused now, at least I'm not triggering a call for it.

Should I drop it?

>> +};
> 
> No strong opinion, but the low target could be a derived class of
> netbsd_process_target, like the linux low targets.
> 

This is a simplified x86_64 support and for the time being I will leave
it as it is. During addition of i386, I will switch it to more
linux-like approach.

>> +
>> +/* Target ops definitions for a NetBSD target.  */
>> +
>> +class netbsd_process_target : public process_stratum_target
>> +{
>> +public:
>> +
>> +  int create_inferior (const char *program,
>> +		       const std::vector<char *> &program_args) override;
>> +
>> +  void post_create_inferior () override;
>> +
>> +  int attach (unsigned long pid) override;
>> +
>> +  int kill (process_info *proc) override;
>> +
>> +  int detach (process_info *proc) override;
>> +
>> +  void mourn (process_info *proc) override;
>> +
>> +  void join (int pid) override;
>> +
>> +  bool thread_alive (ptid_t pid) override;
>> +
>> +  void resume (thread_resume *resume_info, size_t n) override;
>> +
>> +  ptid_t wait (ptid_t ptid, target_waitstatus *status,
>> +	       int options) override;
>> +
>> +  void fetch_registers (regcache *regcache, int regno) override;
>> +
>> +  void store_registers (regcache *regcache, int regno) override;
>> +
>> +  int read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
>> +		   int len) override;
>> +
>> +  int write_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
>> +		    int len) override;
>> +
>> +  void request_interrupt () override;
>> +
>> +  bool supports_read_auxv () override;
>> +
>> +  int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
>> +		 unsigned int len) override;
>> +
>> +  bool supports_hardware_single_step () override;
>> +
>> +  const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
>> +
>> +  bool supports_z_point_type (char z_type) override;
>> +
>> +  int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
>> +		    int size, struct raw_breakpoint *bp) override;
>> +
>> +  int remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
>> +		    int size, struct raw_breakpoint *bp) override;
>> +
>> +  bool stopped_by_sw_breakpoint () override;
>> +
>> +  bool supports_qxfer_siginfo () override;
>> +
>> +  int qxfer_siginfo (const char *annex, unsigned char *readbuf,
>> +		     unsigned const char *writebuf, CORE_ADDR offset,
>> +		     int len) override;
>> +
>> +  bool supports_stopped_by_sw_breakpoint () override;
>> +
>> +  bool supports_non_stop () override;
>> +
>> +  bool supports_multi_process () override;
>> +
>> +  bool supports_fork_events () override;
>> +
>> +  bool supports_vfork_events () override;
>> +
>> +  bool supports_exec_events () override;
>> +
>> +  bool supports_disable_randomization () override;
>> +
>> +  bool supports_qxfer_libraries_svr4 () override;
>> +
>> +  int qxfer_libraries_svr4 (const char*, unsigned char*, const unsigned char*,
>> +			    CORE_ADDR, int) override;
>> +
>> +  bool supports_pid_to_exec_file () override;
>> +
>> +  char *pid_to_exec_file (int pid) override;
>> +
>> +  const char *thread_name (ptid_t thread) override;
>> +
>> +  bool supports_catch_syscall () override;
>> +};
>> +
>> +/* The inferior's target description.  This is a global because the
>> +   NetBSD ports support neither bi-arch nor multi-process.  */
> 
> The "nor multi-process" part of this comment is confusing.  What does the multi-process
> feature have to do with the_low_target?  And netbsd_process_target was defined to
> support multi-process above in the .cc file.
> 

It was an old comment and I have updated it.

>> +
>> +extern struct netbsd_target_ops the_low_target;
>> +
>> +/* XXX: multilib */
>> +extern const struct target_desc *netbsd_tdesc;
>> +
>> +#endif /* GDBSERVER_NETBSD_LOW_H */
>> diff --git a/gdbserver/netbsd-x86_64-low.cc b/gdbserver/netbsd-x86_64-low.cc
>> new file mode 100644
>> index 00000000000..c8679d9f153
>> --- /dev/null
>> +++ b/gdbserver/netbsd-x86_64-low.cc
>> @@ -0,0 +1,250 @@
>> +/* Copyright (C) 2020 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include <sys/types.h>
>> +#include <sys/ptrace.h>
>> +#include <limits.h>
>> +
>> +#include "server.h"
>> +#include "netbsd-low.h"
>> +#include "gdbsupport/x86-xstate.h"
>> +#include "arch/amd64.h"
>> +#include "x86-tdesc.h"
>> +#include "tdesc.h"
>> +
>> +static int use_xml;
>> +
>> +/* The index of various registers inside the regcache.  */
>> +
>> +enum netbsd_x86_64_gdb_regnum
>> +{
>> +  AMD64_RAX_REGNUM,	     /* %rax */
>> +  AMD64_RBX_REGNUM,	     /* %rbx */
>> +  AMD64_RCX_REGNUM,	     /* %rcx */
>> +  AMD64_RDX_REGNUM,	     /* %rdx */
>> +  AMD64_RSI_REGNUM,	     /* %rsi */
>> +  AMD64_RDI_REGNUM,	     /* %rdi */
>> +  AMD64_RBP_REGNUM,	     /* %rbp */
>> +  AMD64_RSP_REGNUM,	     /* %rsp */
>> +  AMD64_R8_REGNUM,	      /* %r8 */
>> +  AMD64_R9_REGNUM,	      /* %r9 */
>> +  AMD64_R10_REGNUM,	     /* %r10 */
>> +  AMD64_R11_REGNUM,	     /* %r11 */
>> +  AMD64_R12_REGNUM,	     /* %r12 */
>> +  AMD64_R13_REGNUM,	     /* %r13 */
>> +  AMD64_R14_REGNUM,	     /* %r14 */
>> +  AMD64_R15_REGNUM,	     /* %r15 */
>> +  AMD64_RIP_REGNUM,	     /* %rip */
>> +  AMD64_EFLAGS_REGNUM,	  /* %eflags */
>> +  AMD64_CS_REGNUM,	      /* %cs */
>> +  AMD64_SS_REGNUM,	      /* %ss */
>> +  AMD64_DS_REGNUM,	      /* %ds */
>> +  AMD64_ES_REGNUM,	      /* %es */
>> +  AMD64_FS_REGNUM,	      /* %fs */
>> +  AMD64_GS_REGNUM,	      /* %gs */
>> +  AMD64_ST0_REGNUM = 24,     /* %st0 */
>> +  AMD64_ST1_REGNUM,	     /* %st1 */
>> +  AMD64_FCTRL_REGNUM = AMD64_ST0_REGNUM + 8,
>> +  AMD64_FSTAT_REGNUM = AMD64_ST0_REGNUM + 9,
>> +  AMD64_FTAG_REGNUM = AMD64_ST0_REGNUM + 10,
>> +  AMD64_XMM0_REGNUM = 40,   /* %xmm0 */
>> +  AMD64_XMM1_REGNUM,	    /* %xmm1 */
>> +  AMD64_MXCSR_REGNUM = AMD64_XMM0_REGNUM + 16,
>> +  AMD64_YMM0H_REGNUM,	   /* %ymm0h */
>> +  AMD64_YMM15H_REGNUM = AMD64_YMM0H_REGNUM + 15,
>> +  AMD64_BND0R_REGNUM = AMD64_YMM15H_REGNUM + 1,
>> +  AMD64_BND3R_REGNUM = AMD64_BND0R_REGNUM + 3,
>> +  AMD64_BNDCFGU_REGNUM,
>> +  AMD64_BNDSTATUS_REGNUM,
>> +  AMD64_XMM16_REGNUM,
>> +  AMD64_XMM31_REGNUM = AMD64_XMM16_REGNUM + 15,
>> +  AMD64_YMM16H_REGNUM,
>> +  AMD64_YMM31H_REGNUM = AMD64_YMM16H_REGNUM + 15,
>> +  AMD64_K0_REGNUM,
>> +  AMD64_K7_REGNUM = AMD64_K0_REGNUM + 7,
>> +  AMD64_ZMM0H_REGNUM,
>> +  AMD64_ZMM31H_REGNUM = AMD64_ZMM0H_REGNUM + 31,
>> +  AMD64_PKRU_REGNUM,
>> +  AMD64_FSBASE_REGNUM,
>> +  AMD64_GSBASE_REGNUM
>> +};
>> +
>> +/* The fill_function for the general-purpose register set.  */
>> +
>> +static void
>> +netbsd_x86_64_fill_gregset (struct regcache *regcache, char *buf)
>> +{
>> +  struct reg *r = (struct reg *)buf;
> 
> Space after ')'.
> 

Fixed.

>> +
>> +#define netbsd_x86_64_collect_gp(regnum, fld) do {		\
>> +    collect_register (regcache, regnum, &r->regs[_REG_##fld]);	\
>> +  } while (0)
>> +
>> +  netbsd_x86_64_collect_gp (AMD64_RAX_REGNUM, RAX);
>> +  netbsd_x86_64_collect_gp (AMD64_RBX_REGNUM, RBX);
>> +  netbsd_x86_64_collect_gp (AMD64_RCX_REGNUM, RCX);
>> +  netbsd_x86_64_collect_gp (AMD64_RDX_REGNUM, RDX);
>> +  netbsd_x86_64_collect_gp (AMD64_RSI_REGNUM, RSI);
>> +  netbsd_x86_64_collect_gp (AMD64_RDI_REGNUM, RDI);
>> +  netbsd_x86_64_collect_gp (AMD64_RBP_REGNUM, RBP);
>> +  netbsd_x86_64_collect_gp (AMD64_RSP_REGNUM, RSP);
>> +  netbsd_x86_64_collect_gp (AMD64_R8_REGNUM, R8);
>> +  netbsd_x86_64_collect_gp (AMD64_R9_REGNUM, R9);
>> +  netbsd_x86_64_collect_gp (AMD64_R10_REGNUM, R10);
>> +  netbsd_x86_64_collect_gp (AMD64_R11_REGNUM, R11);
>> +  netbsd_x86_64_collect_gp (AMD64_R12_REGNUM, R12);
>> +  netbsd_x86_64_collect_gp (AMD64_R13_REGNUM, R13);
>> +  netbsd_x86_64_collect_gp (AMD64_R14_REGNUM, R14);
>> +  netbsd_x86_64_collect_gp (AMD64_R15_REGNUM, R15);
>> +  netbsd_x86_64_collect_gp (AMD64_RIP_REGNUM, RIP);
>> +  netbsd_x86_64_collect_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
>> +  netbsd_x86_64_collect_gp (AMD64_CS_REGNUM, CS);
>> +  netbsd_x86_64_collect_gp (AMD64_SS_REGNUM, SS);
>> +  netbsd_x86_64_collect_gp (AMD64_DS_REGNUM, DS);
>> +  netbsd_x86_64_collect_gp (AMD64_ES_REGNUM, ES);
>> +  netbsd_x86_64_collect_gp (AMD64_FS_REGNUM, FS);
>> +  netbsd_x86_64_collect_gp (AMD64_GS_REGNUM, GS);
>> +}
>> +
>> +/* The store_function for the general-purpose register set.  */
>> +
>> +static void
>> +netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
>> +{
>> +  struct reg *r = (struct reg *)buf;
> 
> Space after ')'.
> 

Fixed.

>> +
>> +#define netbsd_x86_64_supply_gp(regnum, fld) do {		\
>> +    supply_register (regcache, regnum, &r->regs[_REG_##fld]);	\
>> +  } while(0)
>> +
>> +  netbsd_x86_64_supply_gp (AMD64_RAX_REGNUM, RAX);
>> +  netbsd_x86_64_supply_gp (AMD64_RBX_REGNUM, RBX);
>> +  netbsd_x86_64_supply_gp (AMD64_RCX_REGNUM, RCX);
>> +  netbsd_x86_64_supply_gp (AMD64_RDX_REGNUM, RDX);
>> +  netbsd_x86_64_supply_gp (AMD64_RSI_REGNUM, RSI);
>> +  netbsd_x86_64_supply_gp (AMD64_RDI_REGNUM, RDI);
>> +  netbsd_x86_64_supply_gp (AMD64_RBP_REGNUM, RBP);
>> +  netbsd_x86_64_supply_gp (AMD64_RSP_REGNUM, RSP);
>> +  netbsd_x86_64_supply_gp (AMD64_R8_REGNUM, R8);
>> +  netbsd_x86_64_supply_gp (AMD64_R9_REGNUM, R9);
>> +  netbsd_x86_64_supply_gp (AMD64_R10_REGNUM, R10);
>> +  netbsd_x86_64_supply_gp (AMD64_R11_REGNUM, R11);
>> +  netbsd_x86_64_supply_gp (AMD64_R12_REGNUM, R12);
>> +  netbsd_x86_64_supply_gp (AMD64_R13_REGNUM, R13);
>> +  netbsd_x86_64_supply_gp (AMD64_R14_REGNUM, R14);
>> +  netbsd_x86_64_supply_gp (AMD64_R15_REGNUM, R15);
>> +  netbsd_x86_64_supply_gp (AMD64_RIP_REGNUM, RIP);
>> +  netbsd_x86_64_supply_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
>> +  netbsd_x86_64_supply_gp (AMD64_CS_REGNUM, CS);
>> +  netbsd_x86_64_supply_gp (AMD64_SS_REGNUM, SS);
>> +  netbsd_x86_64_supply_gp (AMD64_DS_REGNUM, DS);
>> +  netbsd_x86_64_supply_gp (AMD64_ES_REGNUM, ES);
>> +  netbsd_x86_64_supply_gp (AMD64_FS_REGNUM, FS);
>> +  netbsd_x86_64_supply_gp (AMD64_GS_REGNUM, GS);
>> +}
>> +
>> +/* Implements the netbsd_target_ops.arch_setup routine.  */
>> +
>> +static void
>> +netbsd_x86_64_arch_setup (void)
>> +{
>> +  struct target_desc *tdesc
>> +    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
>> +
>> +  init_target_desc (tdesc, amd64_expedite_regs);
>> +
>> +  netbsd_tdesc = tdesc;
>> +}
>> +
>> +/* Update all the target description of all processes; a new GDB
>> +   connected, and it may or not support xml target descriptions.  */
>> +
>> +static void
>> +x86_64_netbsd_update_xmltarget (void)
>> +{
>> +  struct thread_info *saved_thread = current_thread;
>> +
>> +  /* Before changing the register cache's internal layout, flush the
>> +     contents of the current valid caches back to the threads, and
>> +     release the current regcache objects.  */
>> +  regcache_release ();
>> +
>> +  for_each_process ([] (process_info *proc) {
>> +		      int pid = proc->pid;
>> +
>> +		      /* Look up any thread of this process.  */
>> +		      current_thread = find_any_thread_of_pid (pid);
>> +
>> +		      the_low_target.arch_setup ();
> 
> I find this confusing because the target object is supposed to be a singleton.
> Why is its arch_setup called for each process?
> 

This logic was copied from Linux, should I drop it? However.. after
removal of process_qsupported this is no longer in use and I have
removed the x86_64_netbsd_update_xmltarget function entirely.

>> +		    });
>> +
>> +  current_thread = saved_thread;
>> +}
>> +
>> +/* Process qSupported query, "xmlRegisters=". */
>> +
>> +static void
>> +netbsd_x86_64_process_qsupported (char **features, int count)
>> +{
>> +  /* Return if gdb doesn't support XML.  If gdb sends "xmlRegisters="
>> +     with "i386" in qSupported query, it supports x86 XML target
>> +     descriptions.  */
>> +  use_xml = 0;
>> +  for (int i = 0; i < count; i++)
>> +    {
>> +      const char *feature = features[i];
>> +
>> +      if (startswith (feature, "xmlRegisters="))
>> +	{
>> +	  char *copy = xstrdup (feature + 13);
>> +	  char *last;
>> +	  char *p;
>> +
>> +	  for (p = strtok_r (copy, ",", &last); p != NULL;
>> +	       p = strtok_r (NULL, ",", &last))
>> +	    {
>> +	      if (strcmp (p, "i386") == 0)
>> +		{
>> +		  use_xml = 1;
>> +		  break;
>> +		}
>> +	    }
>> +
>> +	  free (copy);
>> +	}
>> +    }
>> +  x86_64_netbsd_update_xmltarget ();
>> +}
>> +
>> +/* Description of all the x86-netbsd register sets.  */
>> +
>> +struct netbsd_regset_info netbsd_target_regsets[] =
>> +{
>> + /* General Purpose Registers.  */
>> + {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
>> +  netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
>> + /* End of list marker.  */
>> + {0, 0, -1, NULL, NULL }
>> +};
>> +
>> +/* The netbsd_target_ops vector for x86-netbsd.  */
>> +
>> +struct netbsd_target_ops the_low_target =
>> +{
>> + netbsd_x86_64_arch_setup,
>> + netbsd_x86_64_process_qsupported,
>> +};
>> --
>> 2.28.0
> 
> Regards
> -Baris
> 
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Christin Eisenschmid, Gary Kershaw
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
> 



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-04  0:13     ` Kamil Rytarowski
@ 2020-09-04  7:58       ` Aktemur, Tankut Baris
  2020-09-04 12:35         ` Kamil Rytarowski
  0 siblings, 1 reply; 19+ messages in thread
From: Aktemur, Tankut Baris @ 2020-09-04  7:58 UTC (permalink / raw)
  To: Kamil Rytarowski, gdb-patches

On Friday, September 4, 2020 2:13 AM, Kamil Rytarowski wrote:
> On 03.09.2020 19:42, Aktemur, Tankut Baris wrote:
> > On September 2, 2020 7:59 PM, Kamil Rytarowski wrote:
> >> +/* Callback used by fork_inferior to start tracing the inferior.  */
> >> +
> >> +static void
> >> +netbsd_ptrace_fun ()
> >> +{
> >> +  /* Switch child to its own process group so that signals won't
> >> +     directly affect GDBserver. */
> >> +  if (setpgid (0, 0) < 0)
> >> +    trace_start_error_with_name (("setpgid"));
> >> +
> >> +  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
> >> +    trace_start_error_with_name (("ptrace"));
> >> +
> >> +  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
> >> +     stdout to stderr so that inferior i/o doesn't corrupt the connection.
> >> +     Also, redirect stdin to /dev/null.  */
> >> +  if (remote_connection_is_stdio ())
> >> +    {
> >> +      if (close (0) < 0)
> >> +	trace_start_error_with_name (("close"));
> >> +      if (open ("/dev/null", O_RDONLY) < 0)
> >> +	trace_start_error_with_name (("open"));
> >> +      if (dup2 (2, 1) < 0)
> >> +	trace_start_error_with_name (("dup2"));
> >> +      if (write (2, "stdin/stdout redirected\n",
> >> +		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
> >> +	{
> >> +	  /* Errors ignored.  */;
> >
> > Any particular reason for the explicit ';'?
> >
> 
> Copied from Linux. I've removed the strat semicolon.
> 
> gdbserver/linux-low.cc:   /* Errors ignored.  */;
> 

It seems originally the code did not have braces, so the semicolon was
in fact needed.  Braces were added later by commit 8c29b58e98b4, but the
semicolon was not removed.

-           /* Errors ignored.  */;
+           {
+             /* Errors ignored.  */;
+           }

> >> +  if ((*target).read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
> >
> > Why not simply `target->read_memory`?
> 
> Fixed.
> 
> > Also, it might be better to explicitly check != 0.
> >
> 
> I've avoided it as the line is too long and would need to be wrapped.

I believe the explicit check is required per coding rules despite forcing a wrap
(please note that I'm not a maintainer with approval/waiver authority).

> >> +extern struct netbsd_regset_info netbsd_target_regsets[];
> >> +
> >> +/* The target-specific operations for NetBSD support.  */
> >> +
> >> +struct netbsd_target_ops
> >> +{
> >> +  /* Architecture-specific setup.  */
> >> +  void (*arch_setup) ();
> >> +
> >> +  /* Hook to support target specific qSupported.  */
> >> +  void (*process_qsupported) (char **, int count);
> >
> > Is the `process_qsupported` function called anywhere?
> >
> 
> Hmm... it was originally copied from Linux (before C++ification). It
> looks like unused now, at least I'm not triggering a call for it.
> 
> Should I drop it?
> 

I see.  Before C++ification, the linux target directly forwarded the request to
the low target.  After C++ification, the linux target simply inherits the default
implementation from its parent, process_stratum_target, where it's just an empty-bodied
method.  The low targets are free to override, and that's what 
x86_target::process_qsupported does in linux-x86-low.cc.  I think you can either
override `process_qsupported` in netbsd_process_target to simply call the low
target's `process_qsupported`, or ...

> >> +};
> >
> > No strong opinion, but the low target could be a derived class of
> > netbsd_process_target, like the linux low targets.
> >
> 
> This is a simplified x86_64 support and for the time being I will leave
> it as it is. During addition of i386, I will switch it to more
> linux-like approach.

... make the low target derive from netbsd_process_target and override the 
method there :).

> >> +/* Update all the target description of all processes; a new GDB
> >> +   connected, and it may or not support xml target descriptions.  */
> >> +
> >> +static void
> >> +x86_64_netbsd_update_xmltarget (void)
> >> +{
> >> +  struct thread_info *saved_thread = current_thread;
> >> +
> >> +  /* Before changing the register cache's internal layout, flush the
> >> +     contents of the current valid caches back to the threads, and
> >> +     release the current regcache objects.  */
> >> +  regcache_release ();
> >> +
> >> +  for_each_process ([] (process_info *proc) {
> >> +		      int pid = proc->pid;
> >> +
> >> +		      /* Look up any thread of this process.  */
> >> +		      current_thread = find_any_thread_of_pid (pid);
> >> +
> >> +		      the_low_target.arch_setup ();
> >
> > I find this confusing because the target object is supposed to be a singleton.
> > Why is its arch_setup called for each process?
> >
> 
> This logic was copied from Linux, should I drop it? However.. after
> removal of process_qsupported this is no longer in use and I have
> removed the x86_64_netbsd_update_xmltarget function entirely.

Hmm, the x86 low target updates the tdesc of the current process.
The lambda above changes the current thread before calling arch_setup.

	/* Initialize the target description for the architecture of the
	   inferior.  */

	void
	x86_target::low_arch_setup ()
	{
	  current_process ()->tdesc = x86_linux_read_description ();
	}

So, it seems fine from this perspective.  Sorry for misleading.

A final note: I'm not fully knowledgeable in the semantics of all the target ops
or in NetBSD.  Please consider my comments as a general check against the code style
and smells.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-04  7:58       ` Aktemur, Tankut Baris
@ 2020-09-04 12:35         ` Kamil Rytarowski
  0 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-04 12:35 UTC (permalink / raw)
  To: Aktemur, Tankut Baris, Kamil Rytarowski, gdb-patches


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

On 04.09.2020 09:58, Aktemur, Tankut Baris via Gdb-patches wrote:
> A final note: I'm not fully knowledgeable in the semantics of all the target ops
> or in NetBSD.  Please consider my comments as a general check against the code style
> and smells.

Thanks for the review. I have applied your changes in v2 with a few
exceptions as noted earlier.

This is a simplified code (no FPU support etc) and x86_64 support only.

I'm looking forward to more review notes, especially from people with
approval authority.


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-02 17:59 ` [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver Kamil Rytarowski
  2020-09-03 17:42   ` Aktemur, Tankut Baris
@ 2020-09-16 16:08   ` Tom Tromey
  2020-09-18 17:41     ` Kamil Rytarowski
  1 sibling, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2020-09-16 16:08 UTC (permalink / raw)
  To: Kamil Rytarowski; +Cc: gdb-patches

>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:

While switching the target wait flags to be a flag enum type, I found a
small problem in the new NetBSD gdbserver work.

Kamil> +/* Implement a safe wrapper around waitpid().  */
Kamil> +
Kamil> +static pid_t
Kamil> +netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)

Here options is either 0 or:

#define TARGET_WNOHANG 1

Kamil> +{
Kamil> +  int status;
Kamil> +
Kamil> +  pid_t pid = gdb::handle_eintr (::waitpid, ptid.pid (), &status, options);

... but it is passed directly to ::waitpid.  This is fine for now if the
system WNOHANG happens to be 1.  However, I think it would be better not
to rely on this.  For one thing, there's no guarantee that the value of
TARGET_WNOHANG will never change.

Kamil> +  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);

Here's a spot making the call; this shows that the target option is
being passed untranslated.

thanks,
Tom

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver
  2020-09-16 16:08   ` Tom Tromey
@ 2020-09-18 17:41     ` Kamil Rytarowski
  0 siblings, 0 replies; 19+ messages in thread
From: Kamil Rytarowski @ 2020-09-18 17:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches


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

On 16.09.2020 18:08, Tom Tromey wrote:
>>>>>> "Kamil" == Kamil Rytarowski <n54@gmx.com> writes:
> 
> While switching the target wait flags to be a flag enum type, I found a
> small problem in the new NetBSD gdbserver work.
> 
> Kamil> +/* Implement a safe wrapper around waitpid().  */
> Kamil> +
> Kamil> +static pid_t
> Kamil> +netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
> 
> Here options is either 0 or:
> 
> #define TARGET_WNOHANG 1
> 
> Kamil> +{
> Kamil> +  int status;
> Kamil> +
> Kamil> +  pid_t pid = gdb::handle_eintr (::waitpid, ptid.pid (), &status, options);
> 
> ... but it is passed directly to ::waitpid.  This is fine for now if the
> system WNOHANG happens to be 1.  However, I think it would be better not
> to rely on this.  For one thing, there's no guarantee that the value of
> TARGET_WNOHANG will never change.
> 
> Kamil> +  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
> 
> Here's a spot making the call; this shows that the target option is
> being passed untranslated.
> 

I see. It happens that WNOHANG is equal to TARGET_WNOHANG on NetBSD and
it went unnoticed.

> thanks,
> Tom
> 



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-09-18 17:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 17:59 [PATCH 00/10] Add minimal NetBSD/amd64 gdbserver support Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 01/10] Add handle_eintr to wrap EINTR handling in syscalls Kamil Rytarowski
2020-09-03 14:17   ` Tom Tromey
2020-09-03 21:10     ` Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 02/10] Register a placeholder for NetBSD shared functions in gdb/nat Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 03/10] Build nat/netbsd-nat.o for the NetBSD native target Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 04/10] Add netbsd_nat::pid_to_exec_file Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 05/10] Add gdb/nat common functions for listing threads Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 06/10] Add netbsd_nat::enable_proc_events in gdb/nat Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 07/10] Add a common utility function to read and write siginfo_t in inferior Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 08/10] Avoid double free in startup_inferior Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 09/10] Switch local native code to gdb/nat shared functions Kamil Rytarowski
2020-09-02 17:59 ` [PATCH 10/10] Add minimal and functional NetBSD/amd64 gdbserver Kamil Rytarowski
2020-09-03 17:42   ` Aktemur, Tankut Baris
2020-09-04  0:13     ` Kamil Rytarowski
2020-09-04  7:58       ` Aktemur, Tankut Baris
2020-09-04 12:35         ` Kamil Rytarowski
2020-09-16 16:08   ` Tom Tromey
2020-09-18 17:41     ` Kamil Rytarowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).