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

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

[John Baldwin]

Changes since the first version:

- Dropped Changelog changes
- Added a better comment in regcache.h.
- Refactored the helper methods in patch 2 so that most of the logic is in
  two private methods wrapped by templated methods.
- Various style fixes.

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/aarch64-fbsd-nat.c | 77 +++++----------------------------------
 gdb/arm-fbsd-nat.c     | 81 +++++-------------------------------------
 gdb/fbsd-nat.c         | 46 ++++++++++++++++++++++++
 gdb/fbsd-nat.h         | 43 ++++++++++++++++++++++
 gdb/regcache.c         | 27 ++++++++++++++
 gdb/regcache.h         |  9 +++++
 gdb/riscv-fbsd-nat.c   | 78 +++++-----------------------------------
 7 files changed, 149 insertions(+), 212 deletions(-)

-- 
2.31.1

[PATCH v2 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/regcache.c | 27 +++++++++++++++++++++++++++
 gdb/regcache.h |  9 +++++++++
 2 files changed, 36 insertions(+)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index fde0c61297..fe61512e50 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 ee254f381f..cd2e441fed 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -150,6 +150,15 @@ extern void regcache_collect_regset (const struct regset *regset,
 				     int regnum, void *buf, size_t size);
 
 
+/* 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.  */
+
+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 v2 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/fbsd-nat.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-nat.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 234e74fcfd..33eddb5f22 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1601,6 +1601,52 @@ fbsd_nat_target::supports_disable_randomization ()
 #endif
 }
 
+/* See fbsd-nat.h.  */
+
+void
+fbsd_nat_target::fetch_register_set (struct regcache *regcache, int regnum,
+				     int fetch_op, const struct regset *regset,
+				     void *regs, size_t size)
+{
+  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(),
+					     size))
+    {
+      if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      regcache->supply_regset (regset, regnum, regs, size);
+    }
+}
+
+/* See fbsd-nat.h.  */
+
+void
+fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
+				     int fetch_op, int store_op,
+				     const struct regset *regset, void *regs,
+				     size_t size)
+{
+  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(),
+					     size))
+    {
+      if (ptrace (fetch_op, pid, (PTRACE_TYPE_ARG3) regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      regcache->collect_regset (regset, regnum, regs, size);
+
+      if (ptrace (store_op, pid, (PTRACE_TYPE_ARG3) regs, 0) == -1)
+	perror_with_name (_("Couldn't write registers"));
+    }
+}
+
 void _initialize_fbsd_nat ();
 void
 _initialize_fbsd_nat ()
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index 1fdb939935..a59065415b 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>
 
@@ -103,6 +105,47 @@ class fbsd_nat_target : public inf_ptrace_target
   bool supports_multi_process () override;
 
   bool supports_disable_randomization () override;
+
+private:
+  /* Helper routines for use in fetch_registers and store_registers in
+     subclasses.  These routines fetch and store a single set of
+     registers described by REGSET.  The REGSET's 'regmap' field must
+     point to an array of 'struct regcache_map_entry'.
+
+     FETCH_OP is a ptrace operation to fetch the set of registers from
+     a native thread.  STORE_OP is a ptrace operation to store the set
+     of registers to a native thread.
+
+     The caller must provide storage for the set of registers in REGS,
+     and SIZE is the size of the storage.  */
+
+  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   const struct regset *regset, void *regs, size_t size);
+
+  void store_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   int store_op, const struct regset *regset,
+			   void *regs, size_t size);
+protected:
+  /* Wrapper versions of the above helpers which accept a register set
+     type such as 'struct reg' or 'struct fpreg'.  */
+
+  template <class Regset>
+  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   const struct regset *regset)
+  {
+    Regset regs;
+    fetch_register_set (regcache, regnum, fetch_op, regset, &regs,
+			sizeof (regs));
+  }
+
+  template <class Regset>
+  void store_register_set (struct regcache *regcache, int regnum, int fetch_op,
+			   int store_op, const struct regset *regset)
+  {
+    Regset regs;
+    store_register_set (regcache, regnum, fetch_op, store_op, regset, &regs,
+			sizeof (regs));
+  }
 };
 
 #endif /* fbsd-nat.h */
-- 
2.31.1

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

[John Baldwin]

---
 gdb/riscv-fbsd-nat.c | 78 +++++---------------------------------------
 1 file changed, 8 insertions(+), 70 deletions(-)

diff --git a/gdb/riscv-fbsd-nat.c b/gdb/riscv-fbsd-nat.c
index b7b3b7c4c8..30f55fa12a 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 v2 4/5] aarch64-fbsd-nat: Use fetch_register_set and store_register_set.

[John Baldwin]

---
 gdb/aarch64-fbsd-nat.c | 77 +++++-------------------------------------
 1 file changed, 8 insertions(+), 69 deletions(-)

diff --git a/gdb/aarch64-fbsd-nat.c b/gdb/aarch64-fbsd-nat.c
index 275f9b6a32..4442c9fa62 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 v2 5/5] arm-fbsd-nat: Use fetch_register_set and store_register_set.

[John Baldwin]

---
 gdb/arm-fbsd-nat.c | 81 +++++-----------------------------------------
 1 file changed, 8 insertions(+), 73 deletions(-)

diff --git a/gdb/arm-fbsd-nat.c b/gdb/arm-fbsd-nat.c
index 3862a09bf0..f9fb8e1999 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 v2 0/5] Add helper functions for FreeBSD native targets

[Pedro Alves]

On 2021-07-12 4:53 p.m., John Baldwin wrote:
> Changes since the first version:
> 
> - Dropped Changelog changes
> - Added a better comment in regcache.h.
> - Refactored the helper methods in patch 2 so that most of the logic is in
>   two private methods wrapped by templated methods.
> - Various style fixes.
> 
> 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.

Thanks, this version LGTM.