[PATCH 0/5] Refine the NetBSD support

[PATCH 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.

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-nbsd-nat.c           |   1 +
 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          |   4 +
 gdbsupport/agent.cc           |   2 +-
 12 files changed, 252 insertions(+), 52 deletions(-)
 create mode 100644 gdbserver/netbsd-i386-low.cc

--
2.28.0

[PATCH 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-nbsd-nat.c: Include "x86-bsd-nat.h".
---
 gdb/ChangeLog       | 4 ++++
 gdb/i386-nbsd-nat.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ce9cef5f516..c95e681b4a8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-23  Kamil Rytarowski  <n54@gmx.com>
+
+	* i386-nbsd-nat.c: Include "x86-bsd-nat.h".
+
 2020-09-21  Tom Tromey  <tromey@adacore.com>

 	* sparc-tdep.c (sparc32_skip_prologue): Use
diff --git a/gdb/i386-nbsd-nat.c b/gdb/i386-nbsd-nat.c
index ac775c1ff84..d535408f091 100644
--- a/gdb/i386-nbsd-nat.c
+++ b/gdb/i386-nbsd-nat.c
@@ -23,6 +23,7 @@
 #include "target.h"

 #include "i386-tdep.h"
+#include "x86-bsd-nat.h"
 #include "i386-bsd-nat.h"

 /* Support for debugging kernel virtual memory images.  */
--
2.28.0

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

[Kamil Rytarowski]

Fix portability issue as passing random sun_len is not portable.

gdbsupport/ChangeLog:

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

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index d46f8c77bb0..36961c90cfd 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-23  Kamil Rytarowski  <n54@gmx.com>
+
+	* agent.cc (gdb_connect_sync_socket): Preinitialize addr with zeros.
+
 2020-09-16  John Baldwin  <jhb@FreeBSD.org>

 	* common.m4 (GDB_AC_COMMON): Refactor checks for kinfo_getfile().
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 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 d47fcfd300a..5eb0eef82b2 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,20 @@
+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.
+	(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-18  Tom Tromey  <tromey@adacore.com>

 	* netbsd-low.h (class netbsd_process_target) <wait>: Update.
diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
index 9b8ea9b8aa6..2324bdbbfea 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[] =
+const static 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..58bfcad4fd9 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 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 5eb0eef82b2..3a6fdb7ad78 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-09-22  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.
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 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 c95e681b4a8..716df36fdd2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-23  Kamil Rytarowski  <n54@gmx.com>
+
+	* i386-bsd-nat.c (_initialize_i386bsd_nat): Update.
+
 2020-09-23  Kamil Rytarowski  <n54@gmx.com>

 	* i386-nbsd-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 1/5] Include the x86-bsd-nat.h header and fix the NetBSD/i386 build

[Christian Biesinger]

On Wed, Sep 23, 2020 at 6:26 AM Kamil Rytarowski <n54@gmx.com> 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>
>       |                                                     ^

Seems like x86-bsd-nat.h should be included by i386-bsd-nat.h instead?

Christian

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

[Kamil Rytarowski]

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

On 23.09.2020 15:15, Christian Biesinger wrote:
> On Wed, Sep 23, 2020 at 6:26 AM Kamil Rytarowski <n54@gmx.com> 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>
>>       |                                                     ^
> 
> Seems like x86-bsd-nat.h should be included by i386-bsd-nat.h instead?
> 

I have got no opinion. This patch replicates what FreeBSD does.

> Christian
> 



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

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

[John Baldwin]

On 9/23/20 6:15 AM, Christian Biesinger via Gdb-patches wrote:
> On Wed, Sep 23, 2020 at 6:26 AM Kamil Rytarowski <n54@gmx.com> 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>
>>       |                                                     ^
> 
> Seems like x86-bsd-nat.h should be included by i386-bsd-nat.h instead?

That might be a better fix yes.  amd64-bsd-nat.h already includes x86-bsd-nat.h.

-- 
John Baldwin

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

[Simon Marchi]

On 2020-09-23 9:29 a.m., Kamil Rytarowski wrote:
> On 23.09.2020 15:15, Christian Biesinger wrote:
>> On Wed, Sep 23, 2020 at 6:26 AM Kamil Rytarowski <n54@gmx.com> 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>
>>>       |                                                     ^
>>
>> Seems like x86-bsd-nat.h should be included by i386-bsd-nat.h instead?
>>
>
> I have got no opinion. This patch replicates what FreeBSD does.

Indeed, i386-bsd-nat.h uses x86bsd_nat_target, so it needs to include
x86-bsd-nat.h.  The patch is ok with that fixed.

Simon

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

[Simon Marchi]

On 2020-09-23 12:25 a.m., Kamil Rytarowski wrote:
> Fix portability issue as passing random sun_len is not portable.

I don't understand the problem you are fixing, can you expand (and make
the commit message clearer)?

Thanks,

Simon

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

[Simon Marchi]

On 2020-09-23 12:25 a.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.

I just spotted some nits, but otherwise the patch is OK.

> diff --git a/gdbserver/netbsd-amd64-low.cc b/gdbserver/netbsd-amd64-low.cc
> index 9b8ea9b8aa6..2324bdbbfea 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[] =
> +const static struct netbsd_regset_info netbsd_target_regsets[] =

"const static" -> "static const", just for consistency.

> @@ -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();

Space before parenthesis.

>    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();

Space before parenthesis.

Simon