[PATCH 0/5] Add helper functions for FreeBSD native targets

[PATCH 0/5] Add helper functions for FreeBSD native targets

[John Baldwin]

I have another series I'm working on that will end up using these
helpers for the x86 native targets as well.  This mostly just serves
to reduce some code duplication.

John Baldwin (5):
  Add regcache_map_supplies helper routine.
  fbsd-nat: Add helper functions to fetch and store register sets.
  riscv-fbsd-nat: Use fetch_register_set and store_register_set.
  aarch64-fbsd-nat: Use fetch_register_set and store_register_set.
  arm-fbsd-nat: Use fetch_register_set and store_register_set.

 gdb/ChangeLog          | 30 ++++++++++++++++
 gdb/aarch64-fbsd-nat.c | 77 +++++----------------------------------
 gdb/arm-fbsd-nat.c     | 81 +++++-------------------------------------
 gdb/fbsd-nat.h         | 44 +++++++++++++++++++++++
 gdb/regcache.c         | 27 ++++++++++++++
 gdb/regcache.h         |  7 ++++
 gdb/riscv-fbsd-nat.c   | 78 +++++-----------------------------------
 7 files changed, 132 insertions(+), 212 deletions(-)

-- 
2.31.1

[PATCH 1/5] Add regcache_map_supplies helper routine.

[John Baldwin]

This helper can be used in the fetch_registers and store_registers
target methods to determine if a register set includes a specific
register.

gdb/ChangeLog:

	* regcache.c (regcache_map_supplies): New.
	* regcache.h: Likewise.
---
 gdb/ChangeLog  |  5 +++++
 gdb/regcache.c | 27 +++++++++++++++++++++++++++
 gdb/regcache.h |  7 +++++++
 3 files changed, 39 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 03910c0634c..2e61aead178 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2021-05-28  John Baldwin <jhb@FreeBSD.org>
+
+	* regcache.c (regcache_map_supplies): New.
+	* regcache.h: Likewise.
+
 2021-05-27  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* Fix tab after space indentation issues throughout.
diff --git a/gdb/regcache.c b/gdb/regcache.c
index fde0c612975..fe61512e50d 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1264,6 +1264,33 @@ regcache::collect_regset (const struct regset *regset,
   transfer_regset (regset, nullptr, regnum, nullptr, (gdb_byte *) buf, size);
 }
 
+/* See regcache.h  */
+
+bool
+regcache_map_supplies (const struct regcache_map_entry *map, int regnum,
+		       struct gdbarch *gdbarch, size_t size)
+{
+  int offs = 0, count;
+
+  for (; (count = map->count) != 0; map++)
+    {
+      int regno = map->regno;
+      int slot_size = map->size;
+
+      if (slot_size == 0 && regno != REGCACHE_MAP_SKIP)
+	slot_size = register_size (gdbarch, regno);
+
+      if (regno != REGCACHE_MAP_SKIP && regnum >= regno
+	  && regnum < regno + count)
+	return offs + (regnum - regno + 1) * slot_size <= size;
+
+      offs += count * slot_size;
+      if (offs >= size)
+	return false;
+    }
+  return false;
+}
+
 /* See gdbsupport/common-regcache.h.  */
 
 bool
diff --git a/gdb/regcache.h b/gdb/regcache.h
index ee254f381f4..a1f63d40253 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -150,6 +150,13 @@ extern void regcache_collect_regset (const struct regset *regset,
 				     int regnum, void *buf, size_t size);
 
 
+/* Return true if a set of registers (whose layout is described by
+   MAP) contains the value of the register numbered REGNUM.  */
+
+extern bool regcache_map_supplies (const struct regcache_map_entry *map,
+				   int regnum, struct gdbarch *gdbarch,
+				   size_t size);
+
 /* The type of a register.  This function is slightly more efficient
    then its gdbarch vector counterpart since it returns a precomputed
    value stored in a table.  */
-- 
2.31.1

[PATCH 2/5] fbsd-nat: Add helper functions to fetch and store register sets.

[John Baldwin]

In particular, this supports register sets described by a regcache_map
which are fetched and stored with dedicated ptrace operations.  These
functions are intended to be used in architecture-specific
fetch_registers and store_registers target methods.

gdb/ChangeLog:

	* fbsd-nat.h (fetch_register_set, store_register_set): New.
---
 gdb/ChangeLog  |  4 ++++
 gdb/fbsd-nat.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e61aead178..cb48a7f242d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2021-05-28  John Baldwin <jhb@FreeBSD.org>
+
+	* fbsd-nat.h (fetch_register_set, store_register_set): New.
+
 2021-05-28  John Baldwin <jhb@FreeBSD.org>
 
 	* regcache.c (regcache_map_supplies): New.
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 772655d320e..3cca361df4a 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -21,6 +21,8 @@
 #define FBSD_NAT_H
 
 #include "inf-ptrace.h"
+#include "regcache.h"
+#include "regset.h"
 #include <osreldate.h>
 #include <sys/proc.h>
 
@@ -98,6 +100,48 @@ class fbsd_nat_target : public inf_ptrace_target
 #endif
 
   bool supports_multi_process () override;
+
+protected:
+  template <class R>
+  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   const struct regset *regset)
+  {
+    const struct regcache_map_entry *map =
+      (const struct regcache_map_entry *) regset->regmap;
+    pid_t pid = get_ptrace_pid (regcache->ptid ());
+    R regs;
+
+    if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
+					       sizeof(regs)))
+      {
+	if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	  perror_with_name (_("Couldn't get registers"));
+
+	regcache->supply_regset (regset, regnum, &regs, sizeof (regs));
+      }
+  }
+
+  template <class R>
+  void store_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   int store_op, const struct regset *regset)
+  {
+    const struct regcache_map_entry *map =
+      (const struct regcache_map_entry *) regset->regmap;
+    pid_t pid = get_ptrace_pid (regcache->ptid ());
+    R regs;
+
+    if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
+					       sizeof(regs)))
+      {
+	if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	  perror_with_name (_("Couldn't get registers"));
+
+	regcache->collect_regset (regset, regnum, &regs, sizeof (regs));
+
+	if (ptrace (store_op, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	  perror_with_name (_("Couldn't write registers"));
+      }
+  }
 };
 
 #endif /* fbsd-nat.h */
-- 
2.31.1

[PATCH 3/5] riscv-fbsd-nat: Use fetch_register_set and store_register_set.

[John Baldwin]

gdb/ChangeLog:

	* riscv-fbsd-nat.c (getregs_supplies, getfpregs_supplies): Remove.
	(riscv_fbsd_nat_target::fetch_registers): Use fetch_register_set.
	(riscv_fbsd_nat_target::store_registers): Use store_register_set.
---
 gdb/ChangeLog        |  6 ++++
 gdb/riscv-fbsd-nat.c | 78 +++++---------------------------------------
 2 files changed, 14 insertions(+), 70 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cb48a7f242d..efee8edbbf4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2021-05-28  John Baldwin <jhb@FreeBSD.org>
+
+	* riscv-fbsd-nat.c (getregs_supplies, getfpregs_supplies): Remove.
+	(riscv_fbsd_nat_target::fetch_registers): Use fetch_register_set.
+	(riscv_fbsd_nat_target::store_registers): Use store_register_set.
+
 2021-05-28  John Baldwin <jhb@FreeBSD.org>
 
 	* fbsd-nat.h (fetch_register_set, store_register_set): New.
diff --git a/gdb/riscv-fbsd-nat.c b/gdb/riscv-fbsd-nat.c
index b7b3b7c4c89..30f55fa12ab 100644
--- a/gdb/riscv-fbsd-nat.c
+++ b/gdb/riscv-fbsd-nat.c
@@ -38,24 +38,6 @@ struct riscv_fbsd_nat_target final : public fbsd_nat_target
 
 static riscv_fbsd_nat_target the_riscv_fbsd_nat_target;
 
-/* Determine if PT_GETREGS fetches REGNUM.  */
-
-static bool
-getregs_supplies (int regnum)
-{
-  return ((regnum >= RISCV_RA_REGNUM && regnum <= RISCV_PC_REGNUM)
-	  || regnum == RISCV_CSR_SSTATUS_REGNUM);
-}
-
-/* Determine if PT_GETFPREGS fetches REGNUM.  */
-
-static bool
-getfpregs_supplies (int regnum)
-{
-  return ((regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
-	  || regnum == RISCV_CSR_FCSR_REGNUM);
-}
-
 /* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
    for all registers.  */
 
@@ -63,31 +45,12 @@ void
 riscv_fbsd_nat_target::fetch_registers (struct regcache *regcache,
 					int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
   if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
     regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->supply_regset (&riscv_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-    }
-
-  if (regnum == -1 || getfpregs_supplies (regnum))
-    {
-      struct fpreg fpregs;
-
-      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->supply_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
-			       sizeof (fpregs));
-    }
+  fetch_register_set<struct reg> (regcache, regnum, PT_GETREGS,
+				  &riscv_fbsd_gregset);
+  fetch_register_set<struct fpreg> (regcache, regnum, PT_GETFPREGS,
+				    &riscv_fbsd_fpregset);
 }
 
 /* Store register REGNUM back into the inferior.  If REGNUM is -1, do
@@ -97,35 +60,10 @@ void
 riscv_fbsd_nat_target::store_registers (struct regcache *regcache,
 					int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->collect_regset (&riscv_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-
-      if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't write registers"));
-    }
-
-  if (regnum == -1 || getfpregs_supplies (regnum))
-    {
-      struct fpreg fpregs;
-
-      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->collect_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
-				sizeof (fpregs));
-
-      if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't write floating point status"));
-    }
+  store_register_set<struct reg> (regcache, regnum, PT_GETREGS, PT_SETREGS,
+				  &riscv_fbsd_gregset);
+  store_register_set<struct fpreg> (regcache, regnum, PT_GETFPREGS,
+				    PT_SETFPREGS, &riscv_fbsd_fpregset);
 }
 
 void _initialize_riscv_fbsd_nat ();
-- 
2.31.1

[PATCH 4/5] aarch64-fbsd-nat: Use fetch_register_set and store_register_set.

[John Baldwin]

gdb/ChangeLog:

	* aarch64-fbsd-nat.c (getregs_supplies, getfpregs_supplies):
	Remove.
	(aarch64_fbsd_nat_target::fetch_registers): Use
	fetch_register_set.
	(aarch64_fbsd_nat_target::store_registers): Use
	store_register_set.
---
 gdb/ChangeLog          |  9 +++++
 gdb/aarch64-fbsd-nat.c | 77 +++++-------------------------------------
 2 files changed, 17 insertions(+), 69 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index efee8edbbf4..5ff827aa07e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2021-05-28  John Baldwin <jhb@FreeBSD.org>
+
+	* aarch64-fbsd-nat.c (getregs_supplies, getfpregs_supplies):
+	Remove.
+	(aarch64_fbsd_nat_target::fetch_registers): Use
+	fetch_register_set.
+	(aarch64_fbsd_nat_target::store_registers): Use
+	store_register_set.
+
 2021-05-28  John Baldwin <jhb@FreeBSD.org>
 
 	* riscv-fbsd-nat.c (getregs_supplies, getfpregs_supplies): Remove.
diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c
index 275f9b6a32f..4442c9fa624 100644
--- a/gdb/aarch64-fbsd-nat.c
+++ b/gdb/aarch64-fbsd-nat.c
@@ -26,7 +26,6 @@
 #include <machine/reg.h>
 
 #include "fbsd-nat.h"
-#include "aarch64-tdep.h"
 #include "aarch64-fbsd-tdep.h"
 #include "inf-ptrace.h"
 
@@ -38,22 +37,6 @@ struct aarch64_fbsd_nat_target final : public fbsd_nat_target
 
 static aarch64_fbsd_nat_target the_aarch64_fbsd_nat_target;
 
-/* Determine if PT_GETREGS fetches REGNUM.  */
-
-static bool
-getregs_supplies (int regnum)
-{
-  return (regnum >= AARCH64_X0_REGNUM && regnum <= AARCH64_CPSR_REGNUM);
-}
-
-/* Determine if PT_GETFPREGS fetches REGNUM.  */
-
-static bool
-getfpregs_supplies (int regnum)
-{
-  return (regnum >= AARCH64_V0_REGNUM && regnum <= AARCH64_FPCR_REGNUM);
-}
-
 /* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
    for all registers.  */
 
@@ -61,29 +44,10 @@ void
 aarch64_fbsd_nat_target::fetch_registers (struct regcache *regcache,
 					  int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->supply_regset (&aarch64_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-    }
-
-  if (regnum == -1 || getfpregs_supplies (regnum))
-    {
-      struct fpreg fpregs;
-
-      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->supply_regset (&aarch64_fbsd_fpregset, regnum, &fpregs,
-			       sizeof (fpregs));
-    }
+  fetch_register_set<struct reg> (regcache, regnum, PT_GETREGS,
+				  &aarch64_fbsd_gregset);
+  fetch_register_set<struct fpreg> (regcache, regnum, PT_GETFPREGS,
+				    &aarch64_fbsd_fpregset);
 }
 
 /* Store register REGNUM back into the inferior.  If REGNUM is -1, do
@@ -93,35 +57,10 @@ void
 aarch64_fbsd_nat_target::store_registers (struct regcache *regcache,
 					  int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->collect_regset (&aarch64_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-
-      if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't write registers"));
-    }
-
-  if (regnum == -1 || getfpregs_supplies (regnum))
-    {
-      struct fpreg fpregs;
-
-      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->collect_regset (&aarch64_fbsd_fpregset, regnum, &fpregs,
-				sizeof (fpregs));
-
-      if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
-	perror_with_name (_("Couldn't write floating point status"));
-    }
+  store_register_set<struct reg> (regcache, regnum, PT_GETREGS, PT_SETREGS,
+				  &aarch64_fbsd_gregset);
+  store_register_set<struct fpreg> (regcache, regnum, PT_GETFPREGS,
+				    PT_SETFPREGS, &aarch64_fbsd_fpregset);
 }
 
 void _initialize_aarch64_fbsd_nat ();
-- 
2.31.1

[PATCH 5/5] arm-fbsd-nat: Use fetch_register_set and store_register_set.

[John Baldwin]

gdb/ChangeLog:

	* arm-fbsd-nat.c (getregs_supplies, getvfpregs_supplies): Remove.
	(arm_fbsd_nat_target::fetch_registers): Use fetch_register_set.
	(arm_fbsd_nat_target::store_registers): Use store_register_set.
---
 gdb/ChangeLog      |  6 ++++
 gdb/arm-fbsd-nat.c | 81 +++++-----------------------------------------
 2 files changed, 14 insertions(+), 73 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5ff827aa07e..d46c525ea2a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2021-05-28  John Baldwin <jhb@FreeBSD.org>
+
+	* arm-fbsd-nat.c (getregs_supplies, getvfpregs_supplies): Remove.
+	(arm_fbsd_nat_target::fetch_registers): Use fetch_register_set.
+	(arm_fbsd_nat_target::store_registers): Use store_register_set.
+
 2021-05-28  John Baldwin <jhb@FreeBSD.org>
 
 	* aarch64-fbsd-nat.c (getregs_supplies, getfpregs_supplies):
diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
index 3862a09bf0f..f9fb8e1999a 100644
--- a/gdb/arm-fbsd-nat.c
+++ b/gdb/arm-fbsd-nat.c
@@ -25,7 +25,6 @@
 #include <machine/reg.h>
 
 #include "fbsd-nat.h"
-#include "arm-tdep.h"
 #include "arm-fbsd-tdep.h"
 #include "inf-ptrace.h"
 
@@ -38,56 +37,17 @@ struct arm_fbsd_nat_target : public fbsd_nat_target
 
 static arm_fbsd_nat_target the_arm_fbsd_nat_target;
 
-/* Determine if PT_GETREGS fetches REGNUM.  */
-
-static bool
-getregs_supplies (int regnum)
-{
-  return ((regnum >= ARM_A1_REGNUM && regnum <= ARM_PC_REGNUM)
-	  || regnum == ARM_PS_REGNUM);
-}
-
-#ifdef PT_GETVFPREGS
-/* Determine if PT_GETVFPREGS fetches REGNUM.  */
-
-static bool
-getvfpregs_supplies (int regnum)
-{
-  return ((regnum >= ARM_D0_REGNUM && regnum <= ARM_D31_REGNUM)
-	  || regnum == ARM_FPSCR_REGNUM);
-}
-#endif
-
 /* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
    for all registers.  */
 
 void
 arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->supply_regset (&arm_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-    }
-
+  fetch_register_set<struct reg> (regcache, regnum, PT_GETREGS,
+				  &arm_fbsd_gregset);
 #ifdef PT_GETVFPREGS
-  if (regnum == -1 || getvfpregs_supplies (regnum))
-    {
-      struct vfpreg vfpregs;
-
-      if (ptrace (PT_GETVFPREGS, pid, (PTRACE_TYPE_ARG3) &vfpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->supply_regset (&arm_fbsd_vfpregset, regnum, &vfpregs,
-			       sizeof (vfpregs));
-    }
+  fetch_register_set<struct vfpreg> (regcache, regnum, PT_GETVFPREGS,
+				     &arm_fbsd_vfpregset);
 #endif
 }
 
@@ -97,36 +57,11 @@ arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
 void
 arm_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
 {
-  pid_t pid = get_ptrace_pid (regcache->ptid ());
-
-  if (regnum == -1 || getregs_supplies (regnum))
-    {
-      struct reg regs;
-
-      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't get registers"));
-
-      regcache->collect_regset (&arm_fbsd_gregset, regnum, &regs,
-			       sizeof (regs));
-
-      if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
-	perror_with_name (_("Couldn't write registers"));
-    }
-
+  store_register_set<struct reg> (regcache, regnum, PT_GETREGS, PT_SETREGS,
+				  &arm_fbsd_gregset);
 #ifdef PT_GETVFPREGS
-  if (regnum == -1 || getvfpregs_supplies (regnum))
-    {
-      struct vfpreg vfpregs;
-
-      if (ptrace (PT_GETVFPREGS, pid, (PTRACE_TYPE_ARG3) &vfpregs, 0) == -1)
-	perror_with_name (_("Couldn't get floating point status"));
-
-      regcache->collect_regset (&arm_fbsd_vfpregset, regnum, &vfpregs,
-				sizeof (vfpregs));
-
-      if (ptrace (PT_SETVFPREGS, pid, (PTRACE_TYPE_ARG3) &vfpregs, 0) == -1)
-	perror_with_name (_("Couldn't write floating point status"));
-    }
+  store_register_set<struct vfpreg> (regcache, regnum, PT_GETVFPREGS,
+				     PT_SETVFPREGS, &arm_fbsd_vfpregset);
 #endif
 }
 
-- 
2.31.1

Re: [PATCH 0/5] Add helper functions for FreeBSD native targets

[John Baldwin]

On 5/28/21 1:26 PM, John Baldwin wrote:
> I have another series I'm working on that will end up using these
> helpers for the x86 native targets as well.  This mostly just serves
> to reduce some code duplication.
> 
> John Baldwin (5):
>    Add regcache_map_supplies helper routine.
>    fbsd-nat: Add helper functions to fetch and store register sets.
>    riscv-fbsd-nat: Use fetch_register_set and store_register_set.
>    aarch64-fbsd-nat: Use fetch_register_set and store_register_set.
>    arm-fbsd-nat: Use fetch_register_set and store_register_set.
> 
>   gdb/ChangeLog          | 30 ++++++++++++++++
>   gdb/aarch64-fbsd-nat.c | 77 +++++----------------------------------
>   gdb/arm-fbsd-nat.c     | 81 +++++-------------------------------------
>   gdb/fbsd-nat.h         | 44 +++++++++++++++++++++++
>   gdb/regcache.c         | 27 ++++++++++++++
>   gdb/regcache.h         |  7 ++++
>   gdb/riscv-fbsd-nat.c   | 78 +++++-----------------------------------
>   7 files changed, 132 insertions(+), 212 deletions(-)

Ping.  The first patch adds a new regcache_map API.  The rest of the
series is specific to the FreeBSD native target and can probably go
in without further review if the first patch is ok.

-- 
John Baldwin

[PING] [PATCH 0/5] Add helper functions for FreeBSD native targets

[John Baldwin]

On 6/12/21 4:17 PM, John Baldwin wrote:
> On 5/28/21 1:26 PM, John Baldwin wrote:
>> I have another series I'm working on that will end up using these
>> helpers for the x86 native targets as well.  This mostly just serves
>> to reduce some code duplication.
>>
>> John Baldwin (5):
>>     Add regcache_map_supplies helper routine.
>>     fbsd-nat: Add helper functions to fetch and store register sets.
>>     riscv-fbsd-nat: Use fetch_register_set and store_register_set.
>>     aarch64-fbsd-nat: Use fetch_register_set and store_register_set.
>>     arm-fbsd-nat: Use fetch_register_set and store_register_set.
>>
>>    gdb/ChangeLog          | 30 ++++++++++++++++
>>    gdb/aarch64-fbsd-nat.c | 77 +++++----------------------------------
>>    gdb/arm-fbsd-nat.c     | 81 +++++-------------------------------------
>>    gdb/fbsd-nat.h         | 44 +++++++++++++++++++++++
>>    gdb/regcache.c         | 27 ++++++++++++++
>>    gdb/regcache.h         |  7 ++++
>>    gdb/riscv-fbsd-nat.c   | 78 +++++-----------------------------------
>>    7 files changed, 132 insertions(+), 212 deletions(-)
> 
> Ping.  The first patch adds a new regcache_map API.  The rest of the
> series is specific to the FreeBSD native target and can probably go
> in without further review if the first patch is ok.

I have some follow-on patches for the FreeBSD/x86 targets that will
make use of this as well, and so this is a dependency of those.

-- 
John Baldwin

Re: [PATCH 1/5] Add regcache_map_supplies helper routine.

[Pedro Alves]

On 2021-05-28 9:26 p.m., John Baldwin wrote:

> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index ee254f381f4..a1f63d40253 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -150,6 +150,13 @@ extern void regcache_collect_regset (const struct regset *regset,
>  				     int regnum, void *buf, size_t size);
>  
>  
> +/* Return true if a set of registers (whose layout is described by
> +   MAP) contains the value of the register numbered REGNUM.  */

I assume SIZE is the register's size?  From the description of regcache_map, some of the
map entries have sizes smaller than the corresponding register's size.  How do
they interact?  IOW, please document SIZE.  :-)

> +
> +extern bool regcache_map_supplies (const struct regcache_map_entry *map,
> +				   int regnum, struct gdbarch *gdbarch,
> +				   size_t size);

Re: [PATCH 2/5] fbsd-nat: Add helper functions to fetch and store register sets.

[Pedro Alves]

On 2021-05-28 9:26 p.m., John Baldwin wrote:
> In particular, this supports register sets described by a regcache_map
> which are fetched and stored with dedicated ptrace operations.  These
> functions are intended to be used in architecture-specific
> fetch_registers and store_registers target methods.
> 
> gdb/ChangeLog:
> 
> 	* fbsd-nat.h (fetch_register_set, store_register_set): New.
> ---
>  gdb/ChangeLog  |  4 ++++
>  gdb/fbsd-nat.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2e61aead178..cb48a7f242d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2021-05-28  John Baldwin <jhb@FreeBSD.org>
> +
> +	* fbsd-nat.h (fetch_register_set, store_register_set): New.
> +
>  2021-05-28  John Baldwin <jhb@FreeBSD.org>
>  
>  	* regcache.c (regcache_map_supplies): New.
> diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
> index 772655d320e..3cca361df4a 100644
> --- a/gdb/fbsd-nat.h
> +++ b/gdb/fbsd-nat.h
> @@ -21,6 +21,8 @@
>  #define FBSD_NAT_H
>  
>  #include "inf-ptrace.h"
> +#include "regcache.h"
> +#include "regset.h"
>  #include <osreldate.h>
>  #include <sys/proc.h>
>  
> @@ -98,6 +100,48 @@ class fbsd_nat_target : public inf_ptrace_target
>  #endif
>  
>    bool supports_multi_process () override;
> +
> +protected:
> +  template <class R>
> +  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
> +			   const struct regset *regset)
> +  {
> +    const struct regcache_map_entry *map =
> +      (const struct regcache_map_entry *) regset->regmap;
> +    pid_t pid = get_ptrace_pid (regcache->ptid ());
> +    R regs;
> +
> +    if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
> +					       sizeof(regs)))
> +      {
> +	if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
> +	  perror_with_name (_("Couldn't get registers"));
> +
> +	regcache->supply_regset (regset, regnum, &regs, sizeof (regs));
> +      }
> +  }

BTW, AFAICT, you could de-duplicate most of this template function by splitting
it in two, with the main worker part not needing doesn't need to care about R's type.
Something like:

private:
  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
			   const struct regset *regset, 
			   void *regs, size_t sizeof_regs)
  {
    const struct regcache_map_entry *map =
      (const struct regcache_map_entry *) regset->regmap;
    pid_t pid = get_ptrace_pid (regcache->ptid ());

    if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
					       sizeof_regs))
      {
	if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) regs, 0) == -1)
	  perror_with_name (_("Couldn't get registers"));

	regcache->supply_regset (regset, regnum, regs, sizeof (regs));
      }
  }

protected:
  template <class R>
  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
			   const struct regset *regset)
  {
    R regs;
    fetch_register_set (regcache, regnum, fetch_op, regset, 
			&regs, sizeof (regs));
  }

Re: [PATCH 1/5] Add regcache_map_supplies helper routine.

[John Baldwin]

On 7/10/21 12:25 PM, Pedro Alves wrote:
> On 2021-05-28 9:26 p.m., John Baldwin wrote:
> 
>> diff --git a/gdb/regcache.h b/gdb/regcache.h
>> index ee254f381f4..a1f63d40253 100644
>> --- a/gdb/regcache.h
>> +++ b/gdb/regcache.h
>> @@ -150,6 +150,13 @@ extern void regcache_collect_regset (const struct regset *regset,
>>   				     int regnum, void *buf, size_t size);
>>   
>>   
>> +/* Return true if a set of registers (whose layout is described by
>> +   MAP) contains the value of the register numbered REGNUM.  */
> 
> I assume SIZE is the register's size?  From the description of regcache_map, some of the
> map entries have sizes smaller than the corresponding register's size.  How do
> they interact?  IOW, please document SIZE.  :-)

Ah, will do.  In this case SIZE is the size of the entire register set, the same as
the size passed to regcache::collect_regset and regcache::supply_regset that this
is intended to be paired with, e.g. for a native target fetch_registers:

   struct reg regs;

   if (regnum == -1
      || regcache_map_supplies (map, regnum, gdbarch, sizeof (regs))
     {
       /* populate 'regs' */

       regcache->supply_regset (regset, regnum, &regs, sizeof (regs));
     }

I've modified the comment to:

/* Return true if a set of registers contains the value of the
    register numbered REGNUM.  The size of the set of registers is
    given in SIZE, and the layout of the set of registers is described
    by MAP.  */

-- 
John Baldwin

Re: [PATCH 1/5] Add regcache_map_supplies helper routine.

[Pedro Alves]

On 2021-07-12 2:13 p.m., John Baldwin wrote:
> On 7/10/21 12:25 PM, Pedro Alves wrote:
>> On 2021-05-28 9:26 p.m., John Baldwin wrote:
>>
>>> diff --git a/gdb/regcache.h b/gdb/regcache.h
>>> index ee254f381f4..a1f63d40253 100644
>>> --- a/gdb/regcache.h
>>> +++ b/gdb/regcache.h
>>> @@ -150,6 +150,13 @@ extern void regcache_collect_regset (const struct regset *regset,
>>>                        int regnum, void *buf, size_t size);
>>>     +/* Return true if a set of registers (whose layout is described by
>>> +   MAP) contains the value of the register numbered REGNUM.  */
>>
>> I assume SIZE is the register's size?  From the description of regcache_map, some of the
>> map entries have sizes smaller than the corresponding register's size.  How do
>> they interact?  IOW, please document SIZE.  :-)
> 
> Ah, will do.  In this case SIZE is the size of the entire register set, the same as
> the size passed to regcache::collect_regset and regcache::supply_regset that this
> is intended to be paired with, e.g. for a native target fetch_registers:
> 
>   struct reg regs;
> 
>   if (regnum == -1
>      || regcache_map_supplies (map, regnum, gdbarch, sizeof (regs))
>     {
>       /* populate 'regs' */
> 
>       regcache->supply_regset (regset, regnum, &regs, sizeof (regs));
>     }
> 
> I've modified the comment to:
> 
> /* Return true if a set of registers contains the value of the
>    register numbered REGNUM.  The size of the set of registers is
>    given in SIZE, and the layout of the set of registers is described
>    by MAP.  */
> 

Works for me, thanks!

Re: [PATCH 2/5] fbsd-nat: Add helper functions to fetch and store register sets.

[John Baldwin]

On 7/10/21 12:42 PM, Pedro Alves wrote:
> On 2021-05-28 9:26 p.m., John Baldwin wrote:
>> In particular, this supports register sets described by a regcache_map
>> which are fetched and stored with dedicated ptrace operations.  These
>> functions are intended to be used in architecture-specific
>> fetch_registers and store_registers target methods.
>>
>> gdb/ChangeLog:
>>
>> 	* fbsd-nat.h (fetch_register_set, store_register_set): New.
>> ---
>>   gdb/ChangeLog  |  4 ++++
>>   gdb/fbsd-nat.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 2e61aead178..cb48a7f242d 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2021-05-28  John Baldwin <jhb@FreeBSD.org>
>> +
>> +	* fbsd-nat.h (fetch_register_set, store_register_set): New.
>> +
>>   2021-05-28  John Baldwin <jhb@FreeBSD.org>
>>   
>>   	* regcache.c (regcache_map_supplies): New.
>> diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
>> index 772655d320e..3cca361df4a 100644
>> --- a/gdb/fbsd-nat.h
>> +++ b/gdb/fbsd-nat.h
>> @@ -21,6 +21,8 @@
>>   #define FBSD_NAT_H
>>   
>>   #include "inf-ptrace.h"
>> +#include "regcache.h"
>> +#include "regset.h"
>>   #include <osreldate.h>
>>   #include <sys/proc.h>
>>   
>> @@ -98,6 +100,48 @@ class fbsd_nat_target : public inf_ptrace_target
>>   #endif
>>   
>>     bool supports_multi_process () override;
>> +
>> +protected:
>> +  template <class R>
>> +  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
>> +			   const struct regset *regset)
>> +  {
>> +    const struct regcache_map_entry *map =
>> +      (const struct regcache_map_entry *) regset->regmap;
>> +    pid_t pid = get_ptrace_pid (regcache->ptid ());
>> +    R regs;
>> +
>> +    if (regnum == -1 || regcache_map_supplies (map, regnum, regcache->arch(),
>> +					       sizeof(regs)))
>> +      {
>> +	if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
>> +	  perror_with_name (_("Couldn't get registers"));
>> +
>> +	regcache->supply_regset (regset, regnum, &regs, sizeof (regs));
>> +      }
>> +  }
> 
> BTW, AFAICT, you could de-duplicate most of this template function by splitting
> it in two, with the main worker part not needing doesn't need to care about R's type.
> Something like:

I've done that (with the change of putting the private methods in the .c file
itself) along with fixing the style things.  I've just mailed it out as a V2 of
the series (easier to see the result).  Thanks!

-- 
John Baldwin