[PATCH v2 0/5] Refine the NetBSD support

[PATCH v2 0/5] Refine the NetBSD support

[Kamil Rytarowski]

Fix the NetBSD/i386 build of the native GDB code.

Refactor the gdbserver and amd64 target to be agnostic to more
targets. Add i386 gdbserver support.

Fix portability issue in agent.cc.

Changes in v2:
 * Include x86-bsd-nat.h in i386-bsd-nat.h, rather than i386-nbsd-nat.c.
 * Make the message description clearer for the sockaddr_un change.
 * Code style fixes.

Kamil Rytarowski (5):
  Include the x86-bsd-nat.h header and fix the NetBSD/i386 build
  Preinitialize the sockaddr_un variable to zero
  Refactor the NetBSD amd64 gdbserver support
  Add NetBSD/i386 gdbserver support
  Remove the old sanity check of sigcontext offsets for NetBSD/i386

 gdb/ChangeLog                 |   8 ++
 gdb/i386-bsd-nat.c            |   6 +-
 gdb/i386-bsd-nat.h            |   2 +
 gdbserver/ChangeLog           |  23 +++++
 gdbserver/Makefile.in         |   1 +
 gdbserver/configure.srv       |   6 ++
 gdbserver/netbsd-amd64-low.cc |  53 ++++++++----
 gdbserver/netbsd-i386-low.cc  | 157 ++++++++++++++++++++++++++++++++++
 gdbserver/netbsd-low.cc       |  13 ++-
 gdbserver/netbsd-low.h        |  30 ++-----
 gdbsupport/ChangeLog          |   5 +-
 gdbsupport/agent.cc           |   2 +-
 12 files changed, 253 insertions(+), 53 deletions(-)
 create mode 100644 gdbserver/netbsd-i386-low.cc

--
2.28.0

[PATCH v2 1/5] Include the x86-bsd-nat.h header and fix the NetBSD/i386 build

[Kamil Rytarowski]

In file included from ../../gdb/i386-nbsd-nat.c:26:
../../gdb/i386-bsd-nat.h:34:53: error: expected template-name before '<' token
   34 | class i386_bsd_nat_target : public x86bsd_nat_target<BaseTarget>
      |                                                     ^
../../gdb/i386-bsd-nat.h:34:53: error: expected '{' before '<' token
../../gdb/i386-nbsd-nat.c:74:45: error: aggregate 'i386_bsd_nat_target<nbsd_nat_target> the_i386_nbsd_nat_target' has incomplete type and cannot be defined
   74 | static i386_bsd_nat_target<nbsd_nat_target> the_i386_nbsd_nat_target;
      |                                             ^~~~~~~~~~~~~~~~~~~~~~~~

gdb/ChangeLog:

	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
---
 gdb/ChangeLog      | 4 ++++
 gdb/i386-bsd-nat.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9c802c57ef4..46c1c06dee3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-30  Kamil Rytarowski  <n54@gmx.com>
+
+	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
+
 2020-09-29  Tom Tromey  <tom@tromey.com>

 	* dwarf2/read.c (lookup_dwo_id, get_type_unit_group)
diff --git a/gdb/i386-bsd-nat.h b/gdb/i386-bsd-nat.h
index f1c4fc6dfa0..be53ea05626 100644
--- a/gdb/i386-bsd-nat.h
+++ b/gdb/i386-bsd-nat.h
@@ -20,6 +20,8 @@
 #ifndef I386_BSD_NAT_H
 #define I386_BSD_NAT_H

+#include "x86-bsd-nat.h"
+
 /* Helper functions.  See definitions.  */
 extern void i386bsd_fetch_inferior_registers (struct regcache *regcache,
 					      int regnum);
--
2.28.0

[PATCH v2 2/5] Preinitialize the sockaddr_un variable to zero

[Kamil Rytarowski]

Don't pass random sun_len for the BSD's,
zero the whole structure as recommended for portability.

Reported by Coverity.

gdbsupport/ChangeLog:

	* agent.cc (gdb_connect_sync_socket): Preinitialize addr with zeros.
---
 gdbsupport/ChangeLog | 5 ++++-
 gdbsupport/agent.cc  | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index 91435ff1e8e..651299fe0c1 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-30  Kamil Rytarowski  <n54@gmx.com>
+
+	* agent.cc (gdb_connect_sync_socket): Preinitialize addr with zeros.
+
 2020-09-29  Pedro Alves  <pedro@palves.net>

 	* valid-expr.h (CHECK_VALID_EXPR_INT): Make archetype a template
@@ -327,4 +331,3 @@
 	* acinclude.m4, aclocal.m4, config.in, configure, configure.ac,
 	Makefile.am, Makefile.in, README: New files.
 	* Moved from ../gdb/gdbsupport/
-
diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc
index b4127ff6baf..9588e1ff309 100644
--- a/gdbsupport/agent.cc
+++ b/gdbsupport/agent.cc
@@ -138,7 +138,7 @@ static int
 gdb_connect_sync_socket (int pid)
 {
 #ifdef HAVE_SYS_UN_H
-  struct sockaddr_un addr;
+  struct sockaddr_un addr = {};
   int res, fd;
   char path[UNIX_PATH_MAX];

--
2.28.0

[PATCH v2 3/5] Refactor the NetBSD amd64 gdbserver support

[Kamil Rytarowski]

Replace the pre-C++ construct of netbsd_target_ops, netbsd_regset_info
and netbsd_tdesc with C++ inheritance approach found in the Linux
gdbserver code. Add netbsd_amd64_target, that inherits from the
netbsd_process_target class and add proper singleton object for
the_netbsd_target, initialized from netbsd_amd64_target.

Call low_arch_setup () on post process creation, which sets machine
specific properties of the traced process.

Remove global singleton the_netbsd_target object from the generic
gdbserver code.

This refactoring introduces no functional change from the end-user
point of view.

gdbserver/ChangeLog:

	* netbsd-amd64-low.cc (netbsd_x86_64_arch_setup): Remove.
	(netbsd_target_regsets): Now const.
	(the_low_target): Remove.
	(class netbsd_amd64_target, the_netbsd_amd64_target)
	(the_netbsd_target): Add.
	* netbsd-low.cc (netbsd_process_target::post_create_inferior): Call
	low_arch_setup ().
	(netbsd_process_target::fetch_registers)
	(netbsd_process_target::store_registers, initialize_low): Update.
	(the_netbsd_target): Remove.
	* netbsd-low.h (netbsd_target_regsets, netbsd_target_ops)
	(the_low_target, netbsd_tdesc): Remove.
	(netbsd_process_target::get_regs_info)
	(netbsd_process_target::low_arch_setup): Add.
---
 gdbserver/ChangeLog           | 17 +++++++++++
 gdbserver/netbsd-amd64-low.cc | 53 ++++++++++++++++++++++++-----------
 gdbserver/netbsd-low.cc       | 13 ++++-----
 gdbserver/netbsd-low.h        | 30 ++++++--------------
 4 files changed, 67 insertions(+), 46 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index c97d4b1d284..2dfd951ece6 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,20 @@
+2020-09-30  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-amd64-low.cc (netbsd_x86_64_arch_setup): Remove.
+	(netbsd_target_regsets): Now const.
+	(the_low_target): Remove.
+	(class netbsd_amd64_target, the_netbsd_amd64_target)
+	(the_netbsd_target): Add.
+	* netbsd-low.cc (netbsd_process_target::post_create_inferior): Call
+	low_arch_setup ().
+	(netbsd_process_target::fetch_registers)
+	(netbsd_process_target::store_registers, initialize_low): Update.
+	(the_netbsd_target): Remove.
+	* netbsd-low.h (netbsd_target_regsets, netbsd_target_ops)
+	(the_low_target, netbsd_tdesc): Remove.
+	(netbsd_process_target::get_regs_info)
+	(netbsd_process_target::low_arch_setup): Add.
+
 2020-09-23  Hannes Domani  <ssbssa@yahoo.de>

 	* win32-low.cc: Remove local wow64_process variable.
diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
index 9b8ea9b8aa6..c59ebc8f2cf 100644
--- a/gdbserver/netbsd-amd64-low.cc
+++ b/gdbserver/netbsd-amd64-low.cc
@@ -155,22 +155,9 @@ netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
   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;
-}
-
 /* Description of all the x86-netbsd register sets.  */

-struct netbsd_regset_info netbsd_target_regsets[] =
+static const struct netbsd_regset_info netbsd_target_regsets[] =
 {
  /* General Purpose Registers.  */
  {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
@@ -179,9 +166,41 @@ struct netbsd_regset_info netbsd_target_regsets[] =
  {0, 0, -1, NULL, NULL }
 };

-/* The netbsd_target_ops vector for x86-netbsd.  */
+/* NetBSD target op definitions for the amd64 architecture.  */

-struct netbsd_target_ops the_low_target =
+class netbsd_amd64_target : public netbsd_process_target
 {
- netbsd_x86_64_arch_setup,
+protected:
+  const netbsd_regset_info *get_regs_info () override;
+
+  void low_arch_setup () override;
 };
+
+/* Return the information to access registers.  */
+
+const netbsd_regset_info *
+netbsd_amd64_target::get_regs_info ()
+{
+  return netbsd_target_regsets;
+}
+
+/* Architecture-specific setup for the current process.  */
+
+void
+netbsd_amd64_target::low_arch_setup ()
+{
+  target_desc *tdesc
+    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
+
+  init_target_desc (tdesc, amd64_expedite_regs);
+
+  current_process ()->tdesc = tdesc;
+}
+
+/* The singleton target ops object.  */
+
+static netbsd_amd64_target the_netbsd_amd64_target;
+
+/* The NetBSD target ops object.  */
+
+netbsd_process_target *the_netbsd_target = &the_netbsd_amd64_target;
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 3eb2c0f25f6..7bec55a56ac 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -114,6 +114,8 @@ netbsd_process_target::post_create_inferior ()
 {
   pid_t pid = current_process ()->pid;
   netbsd_nat::enable_proc_events (pid);
+
+  low_arch_setup ();
 }

 /* Implement the attach target_ops method.  */
@@ -504,7 +506,7 @@ netbsd_process_target::thread_alive (ptid_t ptid)
 void
 netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
 {
-  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  const netbsd_regset_info *regset = get_regs_info ();
   ptid_t inferior_ptid = ptid_of (current_thread);

   while (regset->size >= 0)
@@ -525,7 +527,7 @@ netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
 void
 netbsd_process_target::store_registers (struct regcache *regcache, int regno)
 {
-  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  const netbsd_regset_info *regset = get_regs_info ();
   ptid_t inferior_ptid = ptid_of (current_thread);

   while (regset->size >= 0)
@@ -1317,13 +1319,8 @@ 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 ();
+  set_target_ops (the_netbsd_target);
 }
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
index 96ad6d92474..c229a0f9f61 100644
--- a/gdbserver/netbsd-low.h
+++ b/gdbserver/netbsd-low.h
@@ -35,21 +35,6 @@ struct netbsd_regset_info
   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) ();
-};
-
 /* Target ops definitions for a NetBSD target.  */

 class netbsd_process_target : public process_stratum_target
@@ -141,14 +126,17 @@ class netbsd_process_target : public process_stratum_target
   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.  */
+protected:
+  /* The architecture-specific "low" methods are listed below.  */

-extern struct netbsd_target_ops the_low_target;
+  /* Return the information to access registers.  */
+  virtual const netbsd_regset_info *get_regs_info () = 0;
+
+  /* Architecture-specific setup for the current process.  */
+  virtual void low_arch_setup () = 0;
+};

-/* XXX: multilib */
-extern const struct target_desc *netbsd_tdesc;
+extern netbsd_process_target *the_netbsd_target;

 #endif /* GDBSERVER_NETBSD_LOW_H */
--
2.28.0

[PATCH v2 4/5] Add NetBSD/i386 gdbserver support

[Kamil Rytarowski]

The support is on part with NetBSD/amd64, thus GPR works,
single step and software breakpoint are operational, and the
SVR4 r_debug integration is functional.

gdbserver/ChangeLog:

	* netbsd-i386-low.cc: Add.
	* Makefile.in (SFILES): Register "netbsd-i386-low.c".
	* configure.srv: Add i[34567]86-*-netbsd*.
---
 gdbserver/ChangeLog          |   6 ++
 gdbserver/Makefile.in        |   1 +
 gdbserver/configure.srv      |   6 ++
 gdbserver/netbsd-i386-low.cc | 157 +++++++++++++++++++++++++++++++++++
 4 files changed, 170 insertions(+)
 create mode 100644 gdbserver/netbsd-i386-low.cc

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index 2dfd951ece6..6f68a91eb07 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,5 +1,11 @@
 2020-09-30  Kamil Rytarowski  <n54@gmx.com>

+	* netbsd-i386-low.cc: Add.
+	* Makefile.in (SFILES): Register "netbsd-i386-low.c".
+	* configure.srv: Add i[34567]86-*-netbsd*.
+
+2020-09-22  Kamil Rytarowski  <n54@gmx.com>
+
 	* netbsd-amd64-low.cc (netbsd_x86_64_arch_setup): Remove.
 	(netbsd_target_regsets): Now const.
 	(the_low_target): Remove.
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index b0bad0cdb09..8b80acebdf1 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -194,6 +194,7 @@ SFILES = \
 	$(srcdir)/linux-xtensa-low.cc \
 	$(srcdir)/mem-break.cc \
 	$(srcdir)/netbsd-amd64-low.cc \
+	$(srcdir)/netbsd-i386-low.cc \
 	$(srcdir)/netbsd-low.cc \
 	$(srcdir)/netbsd-low.h \
 	$(srcdir)/proc-service.cc \
diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index b9a3703d754..c4fe4ca81ee 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -101,6 +101,12 @@ case "${gdbserver_host}" in
 			srv_tgtobj="${srv_tgtobj} arch/i386.o"
 			srv_mingw=yes
 			;;
+  i[34567]86-*-netbsd*)	srv_regobj=""
+			srv_tgtobj="netbsd-low.o netbsd-i386-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/i386.o"
+			;;
   ia64-*-linux*)	srv_regobj=reg-ia64.o
 			srv_tgtobj="$srv_linux_obj linux-ia64-low.o"
 			srv_linux_usrregs=yes
diff --git a/gdbserver/netbsd-i386-low.cc b/gdbserver/netbsd-i386-low.cc
new file mode 100644
index 00000000000..768cc2516f6
--- /dev/null
+++ b/gdbserver/netbsd-i386-low.cc
@@ -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/>.  */
+
+#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/i386.h"
+#include "x86-tdesc.h"
+#include "tdesc.h"
+
+/* The index of various registers inside the regcache.  */
+
+enum netbsd_i386_gdb_regnum
+{
+  I386_EAX_REGNUM,              /* %eax */
+  I386_ECX_REGNUM,              /* %ecx */
+  I386_EDX_REGNUM,              /* %edx */
+  I386_EBX_REGNUM,              /* %ebx */
+  I386_ESP_REGNUM,              /* %esp */
+  I386_EBP_REGNUM,              /* %ebp */
+  I386_ESI_REGNUM,              /* %esi */
+  I386_EDI_REGNUM,              /* %edi */
+  I386_EIP_REGNUM,              /* %eip */
+  I386_EFLAGS_REGNUM,           /* %eflags */
+  I386_CS_REGNUM,               /* %cs */
+  I386_SS_REGNUM,               /* %ss */
+  I386_DS_REGNUM,               /* %ds */
+  I386_ES_REGNUM,               /* %es */
+  I386_FS_REGNUM,               /* %fs */
+  I386_GS_REGNUM,               /* %gs */
+  I386_ST0_REGNUM               /* %st(0) */
+};
+
+/* The fill_function for the general-purpose register set.  */
+
+static void
+netbsd_i386_fill_gregset (struct regcache *regcache, char *buf)
+{
+  struct reg *r = (struct reg *) buf;
+
+#define netbsd_i386_collect_gp(regnum, fld) do {		\
+    collect_register (regcache, regnum, &r->r_##fld);		\
+  } while (0)
+
+  netbsd_i386_collect_gp (I386_EAX_REGNUM, eax);
+  netbsd_i386_collect_gp (I386_EBX_REGNUM, ebx);
+  netbsd_i386_collect_gp (I386_ECX_REGNUM, ecx);
+  netbsd_i386_collect_gp (I386_EDX_REGNUM, edx);
+  netbsd_i386_collect_gp (I386_ESP_REGNUM, esp);
+  netbsd_i386_collect_gp (I386_EBP_REGNUM, ebp);
+  netbsd_i386_collect_gp (I386_ESI_REGNUM, esi);
+  netbsd_i386_collect_gp (I386_EDI_REGNUM, edi);
+  netbsd_i386_collect_gp (I386_EIP_REGNUM, eip);
+  netbsd_i386_collect_gp (I386_EFLAGS_REGNUM, eflags);
+  netbsd_i386_collect_gp (I386_CS_REGNUM, cs);
+  netbsd_i386_collect_gp (I386_SS_REGNUM, ss);
+  netbsd_i386_collect_gp (I386_DS_REGNUM, ds);
+  netbsd_i386_collect_gp (I386_ES_REGNUM, es);
+  netbsd_i386_collect_gp (I386_FS_REGNUM, fs);
+  netbsd_i386_collect_gp (I386_GS_REGNUM, gs);
+}
+
+/* The store_function for the general-purpose register set.  */
+
+static void
+netbsd_i386_store_gregset (struct regcache *regcache, const char *buf)
+{
+  struct reg *r = (struct reg *) buf;
+
+#define netbsd_i386_supply_gp(regnum, fld) do {		\
+    supply_register (regcache, regnum, &r->r_##fld);	\
+  } while(0)
+
+  netbsd_i386_supply_gp (I386_EAX_REGNUM, eax);
+  netbsd_i386_supply_gp (I386_EBX_REGNUM, ebx);
+  netbsd_i386_supply_gp (I386_ECX_REGNUM, ecx);
+  netbsd_i386_supply_gp (I386_EDX_REGNUM, edx);
+  netbsd_i386_supply_gp (I386_ESP_REGNUM, esp);
+  netbsd_i386_supply_gp (I386_EBP_REGNUM, ebp);
+  netbsd_i386_supply_gp (I386_ESI_REGNUM, esi);
+  netbsd_i386_supply_gp (I386_EDI_REGNUM, edi);
+  netbsd_i386_supply_gp (I386_EIP_REGNUM, eip);
+  netbsd_i386_supply_gp (I386_EFLAGS_REGNUM, eflags);
+  netbsd_i386_supply_gp (I386_CS_REGNUM, cs);
+  netbsd_i386_supply_gp (I386_SS_REGNUM, ss);
+  netbsd_i386_supply_gp (I386_DS_REGNUM, ds);
+  netbsd_i386_supply_gp (I386_ES_REGNUM, es);
+  netbsd_i386_supply_gp (I386_FS_REGNUM, fs);
+  netbsd_i386_supply_gp (I386_GS_REGNUM, gs);
+}
+
+/* Description of all the x86-netbsd register sets.  */
+
+const static struct netbsd_regset_info netbsd_target_regsets[] =
+{
+ /* General Purpose Registers.  */
+ {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
+  netbsd_i386_fill_gregset, netbsd_i386_store_gregset},
+ /* End of list marker.  */
+ {0, 0, -1, NULL, NULL }
+};
+
+/* NetBSD target op definitions for the amd64 architecture.  */
+
+class netbsd_i386_target : public netbsd_process_target
+{
+public:
+  const netbsd_regset_info *get_regs_info () override;
+
+  void low_arch_setup () override;
+};
+
+const netbsd_regset_info *
+netbsd_i386_target::get_regs_info ()
+{
+  return netbsd_target_regsets;
+}
+
+/* Initialize the target description for the architecture of the
+   inferior.  */
+
+void
+netbsd_i386_target::low_arch_setup ()
+{
+  target_desc *tdesc
+    = i386_create_target_description (X86_XSTATE_SSE_MASK, false, false);
+
+  init_target_desc (tdesc, i386_expedite_regs);
+
+  current_process ()->tdesc = tdesc;
+}
+
+/* The singleton target ops object.  */
+
+static netbsd_i386_target the_netbsd_i386_target;
+
+/* The NetBSD target ops object.  */
+
+netbsd_process_target *the_netbsd_target = &the_netbsd_i386_target;
--
2.28.0

[PATCH v2 5/5] Remove the old sanity check of sigcontext offsets for NetBSD/i386

[Kamil Rytarowski]

NetBSD switched to ucontext, back in 2003 and the sigcontext code
is no longer available for users, except for legacy compat layers.

This code was not available anyway as the pre-processor check
was probably never operational and buildable on NetBSD. The code
inside it does not compile.

Meanwhile, move the offset variable into the ifdef goards and avoid
the error about unused variable.

../../gdb/i386-bsd-nat.c: In function 'void _initialize_i386bsd_nat()':
../../gdb/i386-bsd-nat.c:347:7: error: unused variable 'offset' [-Werror=unused-variable]
  347 |   int offset;
      |       ^~~~~~
cc1plus: all warnings being treated as errors

gdb/ChangeLog:

	* i386-bsd-nat.c (_initialize_i386bsd_nat): Update.
---
 gdb/ChangeLog      | 4 ++++
 gdb/i386-bsd-nat.c | 6 +-----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 46c1c06dee3..fb6d46480a4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-30  Kamil Rytarowski  <n54@gmx.com>
+
+	* i386-bsd-nat.c (_initialize_i386bsd_nat): Update.
+
 2020-09-30  Kamil Rytarowski  <n54@gmx.com>

 	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
index 4e8693246c8..12b45efb465 100644
--- a/gdb/i386-bsd-nat.c
+++ b/gdb/i386-bsd-nat.c
@@ -344,8 +344,6 @@ void _initialize_i386bsd_nat ();
 void
 _initialize_i386bsd_nat ()
 {
-  int offset;
-
   /* To support the recognition of signal handlers, i386-bsd-tdep.c
      hardcodes some constants.  Inclusion of this file means that we
      are compiling a native debugger, which means that we can use the
@@ -356,8 +354,6 @@ _initialize_i386bsd_nat ()
 #define SC_REG_OFFSET i386fbsd4_sc_reg_offset
 #elif defined (__FreeBSD_version) && __FreeBSD_version >= 300005
 #define SC_REG_OFFSET i386fbsd_sc_reg_offset
-#elif defined (NetBSD) || defined (__NetBSD_Version__)
-#define SC_REG_OFFSET i386nbsd_sc_reg_offset
 #elif defined (OpenBSD)
 #define SC_REG_OFFSET i386obsd_sc_reg_offset
 #endif
@@ -376,7 +372,7 @@ _initialize_i386bsd_nat ()

   /* Override the default value for the offset of the program counter
      in the sigcontext structure.  */
-  offset = offsetof (struct sigcontext, sc_pc);
+  int offset = offsetof (struct sigcontext, sc_pc);

   if (SC_PC_OFFSET != offset)
     {
--
2.28.0

Re: [PATCH v2 1/5] Include the x86-bsd-nat.h header and fix the NetBSD/i386 build

[Simon Marchi]

On 2020-10-01 3:22 p.m., Kamil Rytarowski wrote:
> In file included from ../../gdb/i386-nbsd-nat.c:26:
> ../../gdb/i386-bsd-nat.h:34:53: error: expected template-name before '<' token
>    34 | class i386_bsd_nat_target : public x86bsd_nat_target<BaseTarget>
>       |                                                     ^
> ../../gdb/i386-bsd-nat.h:34:53: error: expected '{' before '<' token
> ../../gdb/i386-nbsd-nat.c:74:45: error: aggregate 'i386_bsd_nat_target<nbsd_nat_target> the_i386_nbsd_nat_target' has incomplete type and cannot be defined
>    74 | static i386_bsd_nat_target<nbsd_nat_target> the_i386_nbsd_nat_target;
>       |                                             ^~~~~~~~~~~~~~~~~~~~~~~~
> 
> gdb/ChangeLog:
> 
> 	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
> ---
>  gdb/ChangeLog      | 4 ++++
>  gdb/i386-bsd-nat.h | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9c802c57ef4..46c1c06dee3 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-09-30  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
> +
>  2020-09-29  Tom Tromey  <tom@tromey.com>
> 
>  	* dwarf2/read.c (lookup_dwo_id, get_type_unit_group)
> diff --git a/gdb/i386-bsd-nat.h b/gdb/i386-bsd-nat.h
> index f1c4fc6dfa0..be53ea05626 100644
> --- a/gdb/i386-bsd-nat.h
> +++ b/gdb/i386-bsd-nat.h
> @@ -20,6 +20,8 @@
>  #ifndef I386_BSD_NAT_H
>  #define I386_BSD_NAT_H
> 
> +#include "x86-bsd-nat.h"
> +
>  /* Helper functions.  See definitions.  */
>  extern void i386bsd_fetch_inferior_registers (struct regcache *regcache,
>  					      int regnum);
> --
> 2.28.0
> 

This is ok, please merge.

Simon

Re: [PATCH v2 2/5] Preinitialize the sockaddr_un variable to zero

[Simon Marchi]

On 2020-10-01 3:22 p.m., Kamil Rytarowski wrote:
> Don't pass random sun_len for the BSD's,
> zero the whole structure as recommended for portability.
> 
> Reported by Coverity.
> 
> gdbsupport/ChangeLog:
> 
> 	* agent.cc (gdb_connect_sync_socket): Preinitialize addr with zeros.
> ---
>  gdbsupport/ChangeLog | 5 ++++-
>  gdbsupport/agent.cc  | 2 +-
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
> index 91435ff1e8e..651299fe0c1 100644
> --- a/gdbsupport/ChangeLog
> +++ b/gdbsupport/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-09-30  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* agent.cc (gdb_connect_sync_socket): Preinitialize addr with zeros.
> +
>  2020-09-29  Pedro Alves  <pedro@palves.net>
> 
>  	* valid-expr.h (CHECK_VALID_EXPR_INT): Make archetype a template
> @@ -327,4 +331,3 @@
>  	* acinclude.m4, aclocal.m4, config.in, configure, configure.ac,
>  	Makefile.am, Makefile.in, README: New files.
>  	* Moved from ../gdb/gdbsupport/
> -
> diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc
> index b4127ff6baf..9588e1ff309 100644
> --- a/gdbsupport/agent.cc
> +++ b/gdbsupport/agent.cc
> @@ -138,7 +138,7 @@ static int
>  gdb_connect_sync_socket (int pid)
>  {
>  #ifdef HAVE_SYS_UN_H
> -  struct sockaddr_un addr;
> +  struct sockaddr_un addr = {};
>    int res, fd;
>    char path[UNIX_PATH_MAX];
> 
> --
> 2.28.0
> 

Thanks, this is ok, please merge.

Simon

Re: [PATCH v2 3/5] Refactor the NetBSD amd64 gdbserver support

[Simon Marchi]

On 2020-10-01 3:22 p.m., Kamil Rytarowski wrote:
> Replace the pre-C++ construct of netbsd_target_ops, netbsd_regset_info
> and netbsd_tdesc with C++ inheritance approach found in the Linux
> gdbserver code. Add netbsd_amd64_target, that inherits from the
> netbsd_process_target class and add proper singleton object for
> the_netbsd_target, initialized from netbsd_amd64_target.
> 
> Call low_arch_setup () on post process creation, which sets machine
> specific properties of the traced process.
> 
> Remove global singleton the_netbsd_target object from the generic
> gdbserver code.
> 
> This refactoring introduces no functional change from the end-user
> point of view.

Thanks, this was ok with the small nits fixed, so please merge.

Simon

Re: [PATCH v2 4/5] Add NetBSD/i386 gdbserver support

[Simon Marchi]

On 2020-10-01 3:22 p.m., Kamil Rytarowski wrote:
> +/* Description of all the x86-netbsd register sets.  */
> +
> +const static struct netbsd_regset_info netbsd_target_regsets[] =
> +{
> + /* General Purpose Registers.  */
> + {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
> +  netbsd_i386_fill_gregset, netbsd_i386_store_gregset},
> + /* End of list marker.  */
> + {0, 0, -1, NULL, NULL }

Two spaces of indentation here.

Otherwise, the patch LGTM, please merge.

Simon

Re: [PATCH v2 5/5] Remove the old sanity check of sigcontext offsets for NetBSD/i386

[Simon Marchi]

On 2020-10-01 3:22 p.m., Kamil Rytarowski wrote:
> NetBSD switched to ucontext, back in 2003 and the sigcontext code
> is no longer available for users, except for legacy compat layers.
>
> This code was not available anyway as the pre-processor check
> was probably never operational and buildable on NetBSD. The code
> inside it does not compile.
>
> Meanwhile, move the offset variable into the ifdef goards and avoid
> the error about unused variable.
>
> ../../gdb/i386-bsd-nat.c: In function 'void _initialize_i386bsd_nat()':
> ../../gdb/i386-bsd-nat.c:347:7: error: unused variable 'offset' [-Werror=unused-variable]
>   347 |   int offset;
>       |       ^~~~~~
> cc1plus: all warnings being treated as errors
>
> gdb/ChangeLog:
>
> 	* i386-bsd-nat.c (_initialize_i386bsd_nat): Update.
> ---
>  gdb/ChangeLog      | 4 ++++
>  gdb/i386-bsd-nat.c | 6 +-----
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 46c1c06dee3..fb6d46480a4 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-09-30  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* i386-bsd-nat.c (_initialize_i386bsd_nat): Update.
> +
>  2020-09-30  Kamil Rytarowski  <n54@gmx.com>
>
>  	* i386-bsd-nat.c: Include "x86-bsd-nat.h".
> diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
> index 4e8693246c8..12b45efb465 100644
> --- a/gdb/i386-bsd-nat.c
> +++ b/gdb/i386-bsd-nat.c
> @@ -344,8 +344,6 @@ void _initialize_i386bsd_nat ();
>  void
>  _initialize_i386bsd_nat ()
>  {
> -  int offset;
> -
>    /* To support the recognition of signal handlers, i386-bsd-tdep.c
>       hardcodes some constants.  Inclusion of this file means that we
>       are compiling a native debugger, which means that we can use the
> @@ -356,8 +354,6 @@ _initialize_i386bsd_nat ()
>  #define SC_REG_OFFSET i386fbsd4_sc_reg_offset
>  #elif defined (__FreeBSD_version) && __FreeBSD_version >= 300005
>  #define SC_REG_OFFSET i386fbsd_sc_reg_offset
> -#elif defined (NetBSD) || defined (__NetBSD_Version__)
> -#define SC_REG_OFFSET i386nbsd_sc_reg_offset
>  #elif defined (OpenBSD)
>  #define SC_REG_OFFSET i386obsd_sc_reg_offset
>  #endif
> @@ -376,7 +372,7 @@ _initialize_i386bsd_nat ()
>
>    /* Override the default value for the offset of the program counter
>       in the sigcontext structure.  */
> -  offset = offsetof (struct sigcontext, sc_pc);
> +  int offset = offsetof (struct sigcontext, sc_pc);
>
>    if (SC_PC_OFFSET != offset)
>      {
> --
> 2.28.0
>

i386nbsd_sc_reg_offset is now only used in gdb/i386-nbsd-tdep.c, so make
it static to that file.  Otherwise, the patch LGTM (I trust you on the
fact that this is not useful anymore), please merge.

John, do you know if there would be a similar cleanup possible for
FreeBSD here?

Simon