public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers
@ 2021-07-14 14:07 John Baldwin
  2021-07-14 14:07 ` [PATCH 1/8] Remove vestigal FreeBSD/i386 3.x support John Baldwin
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

The x86 architectures in GDB provide existing helpers for parsing
general-purpose register sets.  However, these helpers have some
limitations, such as assuming that registers are always full size.  On
FreeBSD/amd64 in particular, segment registers are stored as 16-bit
quantities that in some cases are packed together.  GDB for historical
reasons treats these 16-bit registers as 32 bits in size.  Using the
more generic regcache_map_entry to describe the GP register sets
permits supporting these registers as 16-bit values.  In addition, the
FreeBSD x86 signal frames have included the base address of the FS and
GS segments (equivalent to the fs_base and gs_base registers), but the
existing signal context helpers were written before those registers
were added to GDB.

Longer term my goal is to use regcache_map_entry-based register sets
in FreeBSD gdbserver support to simplify the implementation.

Note that patch 4 fixes an issue in regcache_collect_regset where it
didn't quite do what I thought it did.  I believe the change is ok,
but it definitely warrants review.

I have tested this on both FreeBSD/amd64 (32-bit and 64-bit processes)
and FreeBSD/i386.

John Baldwin (8):
  Remove vestigal FreeBSD/i386 3.x support.
  Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
  FreeBSD x86: Remove fallback for detecting signal trampolines by
    address.
  regcache: Zero-extend small registers described by a register map.
  Use register maps for gp regsets on FreeBSD/x86 core dumps.
  FreeBSD x86: Use tramp-frame for signal frames.
  fbsd-nat: Return a bool from fetch_register_set and
    store_register_set.
  FreeBSD x86 nat: Use register maps for GP register sets.

 gdb/amd64-bsd-nat.c   |  96 ---------
 gdb/amd64-fbsd-nat.c  | 346 ++++++++++++++++++-------------
 gdb/amd64-fbsd-tdep.c | 279 ++++++++++++++-----------
 gdb/amd64-fbsd-tdep.h |  27 +++
 gdb/amd64-tdep.h      |   5 -
 gdb/configure.nat     |   4 +-
 gdb/fbsd-nat.c        |   8 +-
 gdb/fbsd-nat.h        |  21 +-
 gdb/i386-bsd-nat.c    |  98 +--------
 gdb/i386-fbsd-nat.c   | 255 +++++++++++++++++++----
 gdb/i386-fbsd-tdep.c  | 461 ++++++++++++++++++------------------------
 gdb/i386-fbsd-tdep.h  |   4 +
 gdb/i386-tdep.h       |   4 -
 gdb/regcache.c        |   7 +-
 gdb/x86-bsd-nat.c     |   4 -
 gdb/x86-bsd-nat.h     |   3 -
 16 files changed, 844 insertions(+), 778 deletions(-)
 create mode 100644 gdb/amd64-fbsd-tdep.h

-- 
2.31.1


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

* [PATCH 1/8] Remove vestigal FreeBSD/i386 3.x support.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 2/8] Remove support for pre-5.0 FreeBSD/i386 signal trampolines John Baldwin
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

This was orphaned when a.out support was removed as the FreeBSD/i386
ELF support always used the register layouts from 4.0+.
---
 gdb/i386-bsd-nat.c   |   4 +-
 gdb/i386-fbsd-tdep.c | 117 ++++++++++++-------------------------------
 gdb/i386-tdep.h      |   1 -
 3 files changed, 32 insertions(+), 90 deletions(-)

diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
index a7c55284e0..1fad6c5d48 100644
--- a/gdb/i386-bsd-nat.c
+++ b/gdb/i386-bsd-nat.c
@@ -350,9 +350,7 @@ _initialize_i386bsd_nat ()
      system header files and sysctl(3) to get at the relevant
      information.  */
 
-#if defined (__FreeBSD_version) && __FreeBSD_version >= 400011
-#define SC_REG_OFFSET i386fbsd4_sc_reg_offset
-#elif defined (__FreeBSD_version) && __FreeBSD_version >= 300005
+#if defined (__FreeBSD_version)
 #define SC_REG_OFFSET i386fbsd_sc_reg_offset
 #elif defined (OpenBSD)
 #define SC_REG_OFFSET i386obsd_sc_reg_offset
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 103972490c..3e2b8a322b 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -186,17 +186,15 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame)
   return 1;
 }
 
-/* FreeBSD 3.0-RELEASE or later.  */
-
 /* From <machine/reg.h>.  */
 static int i386fbsd_r_reg_offset[] =
 {
-  9 * 4, 8 * 4, 7 * 4, 6 * 4,	/* %eax, %ecx, %edx, %ebx */
-  15 * 4, 4 * 4,		/* %esp, %ebp */
-  3 * 4, 2 * 4,			/* %esi, %edi */
-  12 * 4, 14 * 4,		/* %eip, %eflags */
-  13 * 4, 16 * 4,		/* %cs, %ss */
-  1 * 4, 0 * 4, -1, -1		/* %ds, %es, %fs, %gs */
+  10 * 4, 9 * 4, 8 * 4, 7 * 4,	/* %eax, %ecx, %edx, %ebx */
+  16 * 4, 5 * 4,		/* %esp, %ebp */
+  4 * 4, 3 * 4,			/* %esi, %edi */
+  13 * 4, 15 * 4,		/* %eip, %eflags */
+  14 * 4, 17 * 4,		/* %cs, %ss */
+  2 * 4, 1 * 4, 0 * 4, 18 * 4	/* %ds, %es, %fs, %gs */
 };
 
 /* Sigtramp routine location.  */
@@ -206,22 +204,22 @@ CORE_ADDR i386fbsd_sigtramp_end_addr;
 /* From <machine/signal.h>.  */
 int i386fbsd_sc_reg_offset[] =
 {
-  8 + 14 * 4,			/* %eax */
-  8 + 13 * 4,			/* %ecx */
-  8 + 12 * 4,			/* %edx */
-  8 + 11 * 4,			/* %ebx */
-  8 + 0 * 4,                    /* %esp */
-  8 + 1 * 4,                    /* %ebp */
-  8 + 10 * 4,                   /* %esi */
-  8 + 9 * 4,                    /* %edi */
-  8 + 3 * 4,                    /* %eip */
-  8 + 4 * 4,                    /* %eflags */
-  8 + 7 * 4,                    /* %cs */
-  8 + 8 * 4,                    /* %ss */
-  8 + 6 * 4,                    /* %ds */
-  8 + 5 * 4,                    /* %es */
-  8 + 15 * 4,			/* %fs */
-  8 + 16 * 4			/* %gs */
+  20 + 11 * 4,			/* %eax */
+  20 + 10 * 4,			/* %ecx */
+  20 + 9 * 4,			/* %edx */
+  20 + 8 * 4,			/* %ebx */
+  20 + 17 * 4,			/* %esp */
+  20 + 6 * 4,			/* %ebp */
+  20 + 5 * 4,			/* %esi */
+  20 + 4 * 4,			/* %edi */
+  20 + 14 * 4,			/* %eip */
+  20 + 16 * 4,			/* %eflags */
+  20 + 15 * 4,			/* %cs */
+  20 + 18 * 4,			/* %ss */
+  20 + 3 * 4,			/* %ds */
+  20 + 2 * 4,			/* %es */
+  20 + 1 * 4,			/* %fs */
+  20 + 0 * 4			/* %gs */
 };
 
 /* Get XSAVE extended state xcr0 from core dump.  */
@@ -351,6 +349,9 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
+  /* Generic FreeBSD support. */
+  fbsd_init_abi (info, gdbarch);
+
   /* Obviously FreeBSD is BSD-based.  */
   i386bsd_init_abi (info, gdbarch);
 
@@ -358,7 +359,7 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
      its FPU emulator in `struct fpreg'.  */
   tdep->gregset_reg_offset = i386fbsd_r_reg_offset;
   tdep->gregset_num_regs = ARRAY_SIZE (i386fbsd_r_reg_offset);
-  tdep->sizeof_gregset = 18 * 4;
+  tdep->sizeof_gregset = 19 * 4;
   tdep->sizeof_fpregset = 176;
 
   /* FreeBSD uses -freg-struct-return by default.  */
@@ -376,66 +377,6 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   i386_elf_init_abi (info, gdbarch);
 
-  /* FreeBSD uses SVR4-style shared libraries.  */
-  set_solib_svr4_fetch_link_map_offsets
-    (gdbarch, svr4_ilp32_fetch_link_map_offsets);
-}
-
-/* FreeBSD 4.0-RELEASE or later.  */
-
-/* From <machine/reg.h>.  */
-static int i386fbsd4_r_reg_offset[] =
-{
-  10 * 4, 9 * 4, 8 * 4, 7 * 4,	/* %eax, %ecx, %edx, %ebx */
-  16 * 4, 5 * 4,		/* %esp, %ebp */
-  4 * 4, 3 * 4,			/* %esi, %edi */
-  13 * 4, 15 * 4,		/* %eip, %eflags */
-  14 * 4, 17 * 4,		/* %cs, %ss */
-  2 * 4, 1 * 4, 0 * 4, 18 * 4	/* %ds, %es, %fs, %gs */
-};
-
-/* From <machine/signal.h>.  */
-int i386fbsd4_sc_reg_offset[] =
-{
-  20 + 11 * 4,			/* %eax */
-  20 + 10 * 4,			/* %ecx */
-  20 + 9 * 4,			/* %edx */
-  20 + 8 * 4,			/* %ebx */
-  20 + 17 * 4,			/* %esp */
-  20 + 6 * 4,			/* %ebp */
-  20 + 5 * 4,			/* %esi */
-  20 + 4 * 4,			/* %edi */
-  20 + 14 * 4,			/* %eip */
-  20 + 16 * 4,			/* %eflags */
-  20 + 15 * 4,			/* %cs */
-  20 + 18 * 4,			/* %ss */
-  20 + 3 * 4,			/* %ds */
-  20 + 2 * 4,			/* %es */
-  20 + 1 * 4,			/* %fs */
-  20 + 0 * 4			/* %gs */
-};
-
-static void
-i386fbsd4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
-{
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-
-  /* Generic FreeBSD support. */
-  fbsd_init_abi (info, gdbarch);
-
-  /* Inherit stuff from older releases.  We assume that FreeBSD
-     4.0-RELEASE always uses ELF.  */
-  i386fbsd_init_abi (info, gdbarch);
-
-  /* FreeBSD 4.0 introduced a new `struct reg'.  */
-  tdep->gregset_reg_offset = i386fbsd4_r_reg_offset;
-  tdep->gregset_num_regs = ARRAY_SIZE (i386fbsd4_r_reg_offset);
-  tdep->sizeof_gregset = 19 * 4;
-
-  /* FreeBSD 4.0 introduced a new `struct sigcontext'.  */
-  tdep->sc_reg_offset = i386fbsd4_sc_reg_offset;
-  tdep->sc_num_regs = ARRAY_SIZE (i386fbsd4_sc_reg_offset);
-
   tdep->xsave_xcr0_offset = I386_FBSD_XSAVE_XCR0_OFFSET;
 
   /* Iterate over core file register note sections.  */
@@ -445,6 +386,10 @@ i386fbsd4_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_core_read_description (gdbarch,
 				     i386fbsd_core_read_description);
 
+  /* FreeBSD uses SVR4-style shared libraries.  */
+  set_solib_svr4_fetch_link_map_offsets
+    (gdbarch, svr4_ilp32_fetch_link_map_offsets);
+
   set_gdbarch_fetch_tls_load_module_address (gdbarch,
 					     svr4_fetch_objfile_link_map);
   set_gdbarch_get_thread_local_address (gdbarch,
@@ -456,5 +401,5 @@ void
 _initialize_i386fbsd_tdep ()
 {
   gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_FREEBSD,
-			  i386fbsd4_init_abi);
+			  i386fbsd_init_abi);
 }
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index ee7655e986..4c45a9a75d 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -476,7 +476,6 @@ extern CORE_ADDR i386fbsd_sigtramp_start_addr;
 extern CORE_ADDR i386fbsd_sigtramp_end_addr;
 extern CORE_ADDR i386obsd_sigtramp_start_addr;
 extern CORE_ADDR i386obsd_sigtramp_end_addr;
-extern int i386fbsd4_sc_reg_offset[];
 extern int i386fbsd_sc_reg_offset[];
 extern int i386obsd_sc_reg_offset[];
 extern int i386bsd_sc_reg_offset[];
-- 
2.31.1


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

* [PATCH 2/8] Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
  2021-07-14 14:07 ` [PATCH 1/8] Remove vestigal FreeBSD/i386 3.x support John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 3/8] FreeBSD x86: Remove fallback for detecting signal trampolines by address John Baldwin
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

The last relevant release (FreeBSD 4.11) was released in January of
2005.
---
 gdb/i386-fbsd-tdep.c | 93 +++++---------------------------------------
 1 file changed, 9 insertions(+), 84 deletions(-)

diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 3e2b8a322b..6248eaf1c4 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -37,12 +37,10 @@
 /* Return whether THIS_FRAME corresponds to a FreeBSD sigtramp
    routine.  */
 
-/* FreeBSD/i386 supports three different signal trampolines, one for
-   versions before 4.0, a second for 4.x, and a third for 5.0 and
-   later.  To complicate matters, FreeBSD/i386 binaries running under
-   an amd64 kernel use a different set of trampolines.  These
-   trampolines differ from the i386 kernel trampolines in that they
-   omit a middle section that conditionally restores %gs.  */
+/* FreeBSD/i386 binaries running under an amd64 kernel use a different
+   trampoline This trampoline differs from the i386 kernel trampoline
+   in that it omits a middle section that conditionally restores
+   %gs.  */
 
 static const gdb_byte i386fbsd_sigtramp_start[] =
 {
@@ -65,62 +63,6 @@ static const gdb_byte i386fbsd_sigtramp_end[] =
   0xcd, 0x80			/* int	   $0x80 */
 };
 
-static const gdb_byte i386fbsd_freebsd4_sigtramp_start[] =
-{
-  0x8d, 0x44, 0x24, 0x14,	/* lea	   SIGF_UC4(%esp),%eax */
-  0x50				/* pushl   %eax */
-};
-
-static const gdb_byte i386fbsd_freebsd4_sigtramp_middle[] =
-{
-  0xf7, 0x40, 0x54, 0x00, 0x00, 0x02, 0x00,
-				/* testl   $PSL_VM,UC4_EFLAGS(%eax) */
-  0x75, 0x03,			/* jne	   +3 */
-  0x8e, 0x68, 0x14		/* mov	   UC4_GS(%eax),%gs */
-};
-
-static const gdb_byte i386fbsd_freebsd4_sigtramp_end[] =
-{
-  0xb8, 0x58, 0x01, 0x00, 0x00, /* movl    $344,%eax */
-  0x50,			/* pushl   %eax */
-  0xcd, 0x80			/* int	   $0x80 */
-};
-
-static const gdb_byte i386fbsd_osigtramp_start[] =
-{
-  0x8d, 0x44, 0x24, 0x14,	/* lea	   SIGF_SC(%esp),%eax */
-  0x50				/* pushl   %eax */
-};
-
-static const gdb_byte i386fbsd_osigtramp_middle[] =
-{
-  0xf7, 0x40, 0x18, 0x00, 0x00, 0x02, 0x00,
-				/* testl   $PSL_VM,SC_PS(%eax) */
-  0x75, 0x03,			/* jne	   +3 */
-  0x8e, 0x68, 0x44		/* mov	   SC_GS(%eax),%gs */
-};
-
-static const gdb_byte i386fbsd_osigtramp_end[] =
-{
-  0xb8, 0x67, 0x00, 0x00, 0x00, /* movl    $103,%eax */
-  0x50,			/* pushl   %eax */
-  0xcd, 0x80			/* int	   $0x80 */
-};
-
-/* The three different trampolines are all the same size.  */
-gdb_static_assert (sizeof i386fbsd_sigtramp_start
-		   == sizeof i386fbsd_freebsd4_sigtramp_start);
-gdb_static_assert (sizeof i386fbsd_sigtramp_start
-		   == sizeof i386fbsd_osigtramp_start);
-gdb_static_assert (sizeof i386fbsd_sigtramp_middle
-		   == sizeof i386fbsd_freebsd4_sigtramp_middle);
-gdb_static_assert (sizeof i386fbsd_sigtramp_middle
-		   == sizeof i386fbsd_osigtramp_middle);
-gdb_static_assert (sizeof i386fbsd_sigtramp_end
-		   == sizeof i386fbsd_freebsd4_sigtramp_end);
-gdb_static_assert (sizeof i386fbsd_sigtramp_end
-		   == sizeof i386fbsd_osigtramp_end);
-
 /* We assume that the middle is the largest chunk below.  */
 gdb_static_assert (sizeof i386fbsd_sigtramp_middle
 		   > sizeof i386fbsd_sigtramp_start);
@@ -132,31 +74,13 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame)
 {
   CORE_ADDR pc = get_frame_pc (this_frame);
   gdb_byte buf[sizeof i386fbsd_sigtramp_middle];
-  const gdb_byte *middle, *end;
 
   /* Look for a matching start.  */
   if (!safe_frame_unwind_memory (this_frame, pc,
 				 {buf, sizeof i386fbsd_sigtramp_start}))
     return 0;
   if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start)
-      == 0)
-    {
-      middle = i386fbsd_sigtramp_middle;
-      end = i386fbsd_sigtramp_end;
-    }
-  else if (memcmp (buf, i386fbsd_freebsd4_sigtramp_start,
-		   sizeof i386fbsd_freebsd4_sigtramp_start) == 0)
-    {
-      middle = i386fbsd_freebsd4_sigtramp_middle;
-      end = i386fbsd_freebsd4_sigtramp_end;
-    }
-  else if (memcmp (buf, i386fbsd_osigtramp_start,
-		   sizeof i386fbsd_osigtramp_start) == 0)
-    {
-      middle = i386fbsd_osigtramp_middle;
-      end = i386fbsd_osigtramp_end;
-    }
-  else
+      != 0)
     return 0;
 
   /* Since the end is shorter than the middle, check for a matching end
@@ -165,14 +89,15 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame)
   if (!safe_frame_unwind_memory (this_frame, pc,
 				 {buf, sizeof i386fbsd_sigtramp_end}))
     return 0;
-  if (memcmp (buf, end, sizeof i386fbsd_sigtramp_end) == 0)
+  if (memcmp (buf, i386fbsd_sigtramp_end, sizeof i386fbsd_sigtramp_end) == 0)
     return 1;
 
   /* If the end didn't match, check for a matching middle.  */
   if (!safe_frame_unwind_memory (this_frame, pc,
 				 {buf, sizeof i386fbsd_sigtramp_middle}))
     return 0;
-  if (memcmp (buf, middle, sizeof i386fbsd_sigtramp_middle) != 0)
+  if (memcmp (buf, i386fbsd_sigtramp_middle, sizeof i386fbsd_sigtramp_middle)
+      != 0)
     return 0;
 
   /* The middle matched, check for a matching end.  */
@@ -180,7 +105,7 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame)
   if (!safe_frame_unwind_memory (this_frame, pc,
 				 {buf, sizeof i386fbsd_sigtramp_end}))
     return 0;
-  if (memcmp (buf, end, sizeof i386fbsd_sigtramp_end) != 0)
+  if (memcmp (buf, i386fbsd_sigtramp_end, sizeof i386fbsd_sigtramp_end) != 0)
     return 0;
 
   return 1;
-- 
2.31.1


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

* [PATCH 3/8] FreeBSD x86: Remove fallback for detecting signal trampolines by address.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
  2021-07-14 14:07 ` [PATCH 1/8] Remove vestigal FreeBSD/i386 3.x support John Baldwin
  2021-07-14 14:07 ` [PATCH 2/8] Remove support for pre-5.0 FreeBSD/i386 signal trampolines John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 4/8] regcache: Zero-extend small registers described by a register map John Baldwin
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

A few FreeBSD releases did not include the page holding the signal
code in core dumps.  As a workaround, a sysctl was used to fetch the
default location of the signal code instead.  The youngest affected
FreeBSD release is 10.1 released in November 2014 and EOLed in
December 2016.  The fallback only works for native processes and would
require a separate unwinder once the FreeBSD arches are converted to
use tramp_frame for signal frames.
---
 gdb/amd64-fbsd-nat.c  | 28 ----------------------------
 gdb/amd64-fbsd-tdep.c |  6 ------
 gdb/amd64-tdep.h      |  2 --
 gdb/i386-fbsd-nat.c   | 26 --------------------------
 gdb/i386-fbsd-tdep.c  |  8 --------
 gdb/i386-tdep.h       |  2 --
 6 files changed, 72 deletions(-)

diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index bd3687b69c..44dfb5a18f 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -278,32 +278,4 @@ Please report this to <bug-gdb@gnu.org>."),
     }
 
   SC_RBP_OFFSET = offset;
-
-#ifdef KERN_PROC_SIGTRAMP
-  /* Normally signal frames are detected via amd64fbsd_sigtramp_p.
-     However, FreeBSD 9.2 through 10.1 do not include the page holding
-     the signal code in core dumps.  These releases do provide a
-     kern.proc.sigtramp.<pid> sysctl that returns the location of the
-     signal trampoline for a running process.  We fetch the location
-     of the current (gdb) process and use this to identify signal
-     frames in core dumps from these releases.  Note that this only
-     works for core dumps of 64-bit (FreeBSD/amd64) processes and does
-     not handle core dumps of 32-bit (FreeBSD/i386) processes.  */
-  {
-    int mib[4];
-    struct kinfo_sigtramp kst;
-    size_t len;
-
-    mib[0] = CTL_KERN;
-    mib[1] = KERN_PROC;
-    mib[2] = KERN_PROC_SIGTRAMP;
-    mib[3] = getpid ();
-    len = sizeof (kst);
-    if (sysctl (mib, 4, &kst, &len, NULL, 0) == 0)
-      {
-	amd64fbsd_sigtramp_start_addr = (uintptr_t) kst.ksigtramp_start;
-	amd64fbsd_sigtramp_end_addr = (uintptr_t) kst.ksigtramp_end;
-      }
-  }
-#endif
 }
diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index 289431306a..e8ad246f61 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -117,10 +117,6 @@ static int amd64fbsd_r_reg_offset[] =
   -1				/* %gs */
 };
 
-/* Location of the signal trampoline.  */
-CORE_ADDR amd64fbsd_sigtramp_start_addr;
-CORE_ADDR amd64fbsd_sigtramp_end_addr;
-
 /* From <machine/signal.h>.  */
 int amd64fbsd_sc_reg_offset[] =
 {
@@ -245,8 +241,6 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
 
   tdep->sigtramp_p = amd64fbsd_sigtramp_p;
-  tdep->sigtramp_start = amd64fbsd_sigtramp_start_addr;
-  tdep->sigtramp_end = amd64fbsd_sigtramp_end_addr;
   tdep->sigcontext_addr = amd64fbsd_sigcontext_addr;
   tdep->sc_reg_offset = amd64fbsd_sc_reg_offset;
   tdep->sc_num_regs = ARRAY_SIZE (amd64fbsd_sc_reg_offset);
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index 6faa399ceb..b2a29aa5f8 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -145,8 +145,6 @@ extern int amd64nbsd_r_reg_offset[];
 extern int amd64obsd_r_reg_offset[];
 
 /* Variables exported from amd64-fbsd-tdep.c.  */
-extern CORE_ADDR amd64fbsd_sigtramp_start_addr;
-extern CORE_ADDR amd64fbsd_sigtramp_end_addr;
 extern int amd64fbsd_sc_reg_offset[];
 
 #endif /* amd64-tdep.h */
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index f15fd625b1..9b5913d88e 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -182,30 +182,4 @@ _initialize_i386fbsd_nat ()
 
   /* Support debugging kernel virtual memory images.  */
   bsd_kvm_add_target (i386fbsd_supply_pcb);
-
-#ifdef KERN_PROC_SIGTRAMP
-  /* Normally signal frames are detected via i386fbsd_sigtramp_p.
-     However, FreeBSD 9.2 through 10.1 do not include the page holding
-     the signal code in core dumps.  These releases do provide a
-     kern.proc.sigtramp.<pid> sysctl that returns the location of the
-     signal trampoline for a running process.  We fetch the location
-     of the current (gdb) process and use this to identify signal
-     frames in core dumps from these releases.  */
-  {
-    int mib[4];
-    struct kinfo_sigtramp kst;
-    size_t len;
-
-    mib[0] = CTL_KERN;
-    mib[1] = KERN_PROC;
-    mib[2] = KERN_PROC_SIGTRAMP;
-    mib[3] = getpid ();
-    len = sizeof (kst);
-    if (sysctl (mib, 4, &kst, &len, NULL, 0) == 0)
-      {
-	i386fbsd_sigtramp_start_addr = (uintptr_t) kst.ksigtramp_start;
-	i386fbsd_sigtramp_end_addr = (uintptr_t) kst.ksigtramp_end;
-      }
-  }
-#endif
 }
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 6248eaf1c4..4d235541ea 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -122,10 +122,6 @@ static int i386fbsd_r_reg_offset[] =
   2 * 4, 1 * 4, 0 * 4, 18 * 4	/* %ds, %es, %fs, %gs */
 };
 
-/* Sigtramp routine location.  */
-CORE_ADDR i386fbsd_sigtramp_start_addr;
-CORE_ADDR i386fbsd_sigtramp_end_addr;
-
 /* From <machine/signal.h>.  */
 int i386fbsd_sc_reg_offset[] =
 {
@@ -292,10 +288,6 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 
   tdep->sigtramp_p = i386fbsd_sigtramp_p;
 
-  /* FreeBSD uses a different memory layout.  */
-  tdep->sigtramp_start = i386fbsd_sigtramp_start_addr;
-  tdep->sigtramp_end = i386fbsd_sigtramp_end_addr;
-
   /* FreeBSD has a more complete `struct sigcontext'.  */
   tdep->sc_reg_offset = i386fbsd_sc_reg_offset;
   tdep->sc_num_regs = ARRAY_SIZE (i386fbsd_sc_reg_offset);
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 4c45a9a75d..92f0257916 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -472,8 +472,6 @@ extern int i386_mpx_enabled (void);
 /* Functions and variables exported from i386-bsd-tdep.c.  */
 
 extern void i386bsd_init_abi (struct gdbarch_info, struct gdbarch *);
-extern CORE_ADDR i386fbsd_sigtramp_start_addr;
-extern CORE_ADDR i386fbsd_sigtramp_end_addr;
 extern CORE_ADDR i386obsd_sigtramp_start_addr;
 extern CORE_ADDR i386obsd_sigtramp_end_addr;
 extern int i386fbsd_sc_reg_offset[];
-- 
2.31.1


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

* [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (2 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 3/8] FreeBSD x86: Remove fallback for detecting signal trampolines by address John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-10-19  8:36   ` Andrew Burgess
  2021-07-14 14:07 ` [PATCH 5/8] Use register maps for gp regsets on FreeBSD/x86 core dumps John Baldwin
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

When registers are supplied via regcache_supply_register from a register
block described by a register map, registers may be stored in slots smaller
than GDB's native register size (e.g. x86 segment registers are 16 bits,
but the GDB registers for those are 32-bits).  regcache_collect_regset
is careful to zero-extend slots larger than a register size, but
regcache_supply_regset just used regcache::raw_supply_part and did not
initialize the upper bytes of a register value.

trad_frame_set_reg_regmap assumes these semantics (zero-extending
short registers) as I had misread the implementation of
regcache::transfer_regset and assumed it zero-extended short
registers.  In my specific use case (x86 segment registers stored as
16-bit values), I need the semantics of zero-extending a register
value in a smaller slot.
---
 gdb/regcache.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/regcache.c b/gdb/regcache.c
index 672da0556f..396923c8a5 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1169,7 +1169,12 @@ regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
 	memset (out_buf + offs + reg_size, 0, slot_size - reg_size);
     }
   else if (in_buf != nullptr)
-    out_regcache->raw_supply_part (regnum, 0, reg_size, in_buf + offs);
+    {
+      /* Zero-extend the register value if the slot is smaller than the register.  */
+      if (slot_size < register_size (gdbarch, regnum))
+	out_regcache->raw_supply_zeroed (regnum);
+      out_regcache->raw_supply_part (regnum, 0, reg_size, in_buf + offs);
+    }
   else
     {
       /* Invalidate the register.  */
-- 
2.31.1


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

* [PATCH 5/8] Use register maps for gp regsets on FreeBSD/x86 core dumps.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (3 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 4/8] regcache: Zero-extend small registers described by a register map John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 6/8] FreeBSD x86: Use tramp-frame for signal frames John Baldwin
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

In particular, this permits reporting the value of the $ds, $es, $fs,
and $gs segment registers from amd64 core dumps since they are stored
as 16-bit values rather than the 32-bit size assumed by i386_gregset.
---
 gdb/amd64-fbsd-tdep.c | 89 +++++++++++++++++++++++--------------------
 gdb/i386-fbsd-tdep.c  | 58 +++++++++++++++++++---------
 2 files changed, 87 insertions(+), 60 deletions(-)

diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index e8ad246f61..b52fc6d431 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -32,6 +32,51 @@
 #include "solib-svr4.h"
 #include "inferior.h"
 
+/* The general-purpose regset consists of 22 64-bit slots, most of
+   which contain individual registers, but a few contain multiple
+   16-bit segment registers.  */
+#define AMD64_FBSD_SIZEOF_GREGSET	(22 * 8)
+
+/* Register maps.  */
+
+static const struct regcache_map_entry amd64_fbsd_gregmap[] =
+{
+  { 1, AMD64_R15_REGNUM, 0 },
+  { 1, AMD64_R14_REGNUM, 0 },
+  { 1, AMD64_R13_REGNUM, 0 },
+  { 1, AMD64_R12_REGNUM, 0 },
+  { 1, AMD64_R11_REGNUM, 0 },
+  { 1, AMD64_R10_REGNUM, 0 },
+  { 1, AMD64_R9_REGNUM, 0 },
+  { 1, AMD64_R8_REGNUM, 0 },
+  { 1, AMD64_RDI_REGNUM, 0 },
+  { 1, AMD64_RSI_REGNUM, 0 },
+  { 1, AMD64_RBP_REGNUM, 0 },
+  { 1, AMD64_RBX_REGNUM, 0 },
+  { 1, AMD64_RDX_REGNUM, 0 },
+  { 1, AMD64_RCX_REGNUM, 0 },
+  { 1, AMD64_RAX_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* trapno */
+  { 1, AMD64_FS_REGNUM, 2 },
+  { 1, AMD64_GS_REGNUM, 2 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* err */
+  { 1, AMD64_ES_REGNUM, 2 },
+  { 1, AMD64_DS_REGNUM, 2 },
+  { 1, AMD64_RIP_REGNUM, 0 },
+  { 1, AMD64_CS_REGNUM, 8 },
+  { 1, AMD64_EFLAGS_REGNUM, 8 },
+  { 1, AMD64_RSP_REGNUM, 0 },
+  { 1, AMD64_SS_REGNUM, 8 },
+  { 0 }
+};
+
+/* Register set definitions.  */
+
+const struct regset amd64_fbsd_gregset =
+{
+  amd64_fbsd_gregmap, regcache_supply_regset, regcache_collect_regset
+};
+
 /* Support for signal handlers.  */
 
 /* Return whether THIS_FRAME corresponds to a FreeBSD sigtramp
@@ -80,42 +125,6 @@ amd64fbsd_sigcontext_addr (struct frame_info *this_frame)
   return sp + 16;
 }
 \f
-/* FreeBSD 5.1-RELEASE or later.  */
-
-/* Mapping between the general-purpose registers in `struct reg'
-   format and GDB's register cache layout.
-
-   Note that some registers are 32-bit, but since we're little-endian
-   we get away with that.  */
-
-/* From <machine/reg.h>.  */
-static int amd64fbsd_r_reg_offset[] =
-{
-  14 * 8,			/* %rax */
-  11 * 8,			/* %rbx */
-  13 * 8,			/* %rcx */
-  12 * 8,			/* %rdx */
-  9 * 8,			/* %rsi */
-  8 * 8,			/* %rdi */
-  10 * 8,			/* %rbp */
-  20 * 8,			/* %rsp */
-  7 * 8,			/* %r8 ...  */
-  6 * 8,
-  5 * 8,
-  4 * 8,
-  3 * 8,
-  2 * 8,
-  1 * 8,
-  0 * 8,			/* ... %r15 */
-  17 * 8,			/* %rip */
-  19 * 8,			/* %eflags */
-  18 * 8,			/* %cs */
-  21 * 8,			/* %ss */
-  -1,				/* %ds */
-  -1,				/* %es */
-  -1,				/* %fs */
-  -1				/* %gs */
-};
 
 /* From <machine/signal.h>.  */
 int amd64fbsd_sc_reg_offset[] =
@@ -193,8 +202,8 @@ amd64fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  cb (".reg", tdep->sizeof_gregset, tdep->sizeof_gregset, &i386_gregset, NULL,
-      cb_data);
+  cb (".reg", AMD64_FBSD_SIZEOF_GREGSET, AMD64_FBSD_SIZEOF_GREGSET,
+      &amd64_fbsd_gregset, NULL, cb_data);
   cb (".reg2", tdep->sizeof_fpregset, tdep->sizeof_fpregset, &amd64_fpregset,
       NULL, cb_data);
   cb (".reg-xstate", X86_XSTATE_SIZE (tdep->xcr0), X86_XSTATE_SIZE (tdep->xcr0),
@@ -233,10 +242,6 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously FreeBSD is BSD-based.  */
   i386bsd_init_abi (info, gdbarch);
 
-  tdep->gregset_reg_offset = amd64fbsd_r_reg_offset;
-  tdep->gregset_num_regs = ARRAY_SIZE (amd64fbsd_r_reg_offset);
-  tdep->sizeof_gregset = 22 * 8;
-
   amd64_init_abi (info, gdbarch,
 		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
 
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 4d235541ea..2aeb51dbec 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -32,6 +32,42 @@
 #include "solib-svr4.h"
 #include "inferior.h"
 
+/* The general-purpose regset consists of 19 32-bit slots.  */
+#define I386_FBSD_SIZEOF_GREGSET	(19 * 4)
+
+/* Register maps.  */
+
+static const struct regcache_map_entry i386_fbsd_gregmap[] =
+{
+  { 1, I386_FS_REGNUM, 4 },
+  { 1, I386_ES_REGNUM, 4 },
+  { 1, I386_DS_REGNUM, 4 },
+  { 1, I386_EDI_REGNUM, 0 },
+  { 1, I386_ESI_REGNUM, 0 },
+  { 1, I386_EBP_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* isp */
+  { 1, I386_EBX_REGNUM, 0 },
+  { 1, I386_EDX_REGNUM, 0 },
+  { 1, I386_ECX_REGNUM, 0 },
+  { 1, I386_EAX_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* trapno */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* err */
+  { 1, I386_EIP_REGNUM, 0 },
+  { 1, I386_CS_REGNUM, 4 },
+  { 1, I386_EFLAGS_REGNUM, 0 },
+  { 1, I386_ESP_REGNUM, 0 },
+  { 1, I386_SS_REGNUM, 4 },
+  { 1, I386_GS_REGNUM, 4 },
+  { 0 }
+};
+
+/* Register set definitions.  */
+
+const struct regset i386_fbsd_gregset =
+{
+  i386_fbsd_gregmap, regcache_supply_regset, regcache_collect_regset
+};
+
 /* Support for signal handlers.  */
 
 /* Return whether THIS_FRAME corresponds to a FreeBSD sigtramp
@@ -111,17 +147,6 @@ i386fbsd_sigtramp_p (struct frame_info *this_frame)
   return 1;
 }
 
-/* From <machine/reg.h>.  */
-static int i386fbsd_r_reg_offset[] =
-{
-  10 * 4, 9 * 4, 8 * 4, 7 * 4,	/* %eax, %ecx, %edx, %ebx */
-  16 * 4, 5 * 4,		/* %esp, %ebp */
-  4 * 4, 3 * 4,			/* %esi, %edi */
-  13 * 4, 15 * 4,		/* %eip, %eflags */
-  14 * 4, 17 * 4,		/* %cs, %ss */
-  2 * 4, 1 * 4, 0 * 4, 18 * 4	/* %ds, %es, %fs, %gs */
-};
-
 /* From <machine/signal.h>.  */
 int i386fbsd_sc_reg_offset[] =
 {
@@ -229,8 +254,8 @@ i386fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
-  cb (".reg", tdep->sizeof_gregset, tdep->sizeof_gregset, &i386_gregset, NULL,
-      cb_data);
+  cb (".reg", I386_FBSD_SIZEOF_GREGSET, I386_FBSD_SIZEOF_GREGSET,
+      &i386_fbsd_gregset, NULL, cb_data);
   cb (".reg2", tdep->sizeof_fpregset, tdep->sizeof_fpregset, &i386_fpregset,
       NULL, cb_data);
 
@@ -276,11 +301,8 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* Obviously FreeBSD is BSD-based.  */
   i386bsd_init_abi (info, gdbarch);
 
-  /* FreeBSD has a different `struct reg', and reserves some space for
-     its FPU emulator in `struct fpreg'.  */
-  tdep->gregset_reg_offset = i386fbsd_r_reg_offset;
-  tdep->gregset_num_regs = ARRAY_SIZE (i386fbsd_r_reg_offset);
-  tdep->sizeof_gregset = 19 * 4;
+  /* FreeBSD reserves some space for its FPU emulator in
+     `struct fpreg'.  */
   tdep->sizeof_fpregset = 176;
 
   /* FreeBSD uses -freg-struct-return by default.  */
-- 
2.31.1


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

* [PATCH 6/8] FreeBSD x86: Use tramp-frame for signal frames.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (4 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 5/8] Use register maps for gp regsets on FreeBSD/x86 core dumps John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 7/8] fbsd-nat: Return a bool from fetch_register_set and store_register_set John Baldwin
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

Use a register map to describe the registers in mcontext_t as part of
the signal frame as is done on several other FreeBSD arches.  This
permits fetching the fsbase and gsbase register values from the signal
frame for both amd64 and i386 and permits fetching additional segment
registers stored as 16-bit values on amd64.

While signal frames on FreeBSD do contain floating point/XSAVE state,
these unwinders do not attempt to supply those registers.  The
existing x86 signal frame uwinders do not support these registers, and
the only existing functions which handle FSAVE/FXSAVE/XSAVE state all
work with regcaches.  In the future these unwinders could create a
tempory regcache, collect floating point registers, and then supply
values out of the regcache into the trad-frame.
---
 gdb/amd64-fbsd-nat.c  |  58 ----------
 gdb/amd64-fbsd-tdep.c | 183 ++++++++++++++++++------------
 gdb/amd64-tdep.h      |   3 -
 gdb/i386-bsd-nat.c    |   4 +-
 gdb/i386-fbsd-tdep.c  | 251 ++++++++++++++++++++++++++----------------
 gdb/i386-tdep.h       |   1 -
 6 files changed, 269 insertions(+), 231 deletions(-)

diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 44dfb5a18f..91fc477d4d 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -213,8 +213,6 @@ void _initialize_amd64fbsd_nat ();
 void
 _initialize_amd64fbsd_nat ()
 {
-  int offset;
-
   amd64_native_gregset32_reg_offset = amd64fbsd32_r_reg_offset;
   amd64_native_gregset64_reg_offset = amd64fbsd64_r_reg_offset;
 
@@ -222,60 +220,4 @@ _initialize_amd64fbsd_nat ()
 
   /* Support debugging kernel virtual memory images.  */
   bsd_kvm_add_target (amd64fbsd_supply_pcb);
-
-  /* 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
-     system header files and sysctl(3) to get at the relevant
-     information.  */
-
-#define SC_REG_OFFSET amd64fbsd_sc_reg_offset
-
-  /* We only check the program counter, stack pointer and frame
-     pointer since these members of `struct sigcontext' are essential
-     for providing backtraces.  */
-
-#define SC_RIP_OFFSET SC_REG_OFFSET[AMD64_RIP_REGNUM]
-#define SC_RSP_OFFSET SC_REG_OFFSET[AMD64_RSP_REGNUM]
-#define SC_RBP_OFFSET SC_REG_OFFSET[AMD64_RBP_REGNUM]
-
-  /* Override the default value for the offset of the program counter
-     in the sigcontext structure.  */
-  offset = offsetof (struct sigcontext, sc_rip);
-
-  if (SC_RIP_OFFSET != offset)
-    {
-      warning (_("\
-offsetof (struct sigcontext, sc_rip) yields %d instead of %d.\n\
-Please report this to <bug-gdb@gnu.org>."),
-	       offset, SC_RIP_OFFSET);
-    }
-
-  SC_RIP_OFFSET = offset;
-
-  /* Likewise for the stack pointer.  */
-  offset = offsetof (struct sigcontext, sc_rsp);
-
-  if (SC_RSP_OFFSET != offset)
-    {
-      warning (_("\
-offsetof (struct sigcontext, sc_rsp) yields %d instead of %d.\n\
-Please report this to <bug-gdb@gnu.org>."),
-	       offset, SC_RSP_OFFSET);
-    }
-
-  SC_RSP_OFFSET = offset;
-
-  /* And the frame pointer.  */
-  offset = offsetof (struct sigcontext, sc_rbp);
-
-  if (SC_RBP_OFFSET != offset)
-    {
-      warning (_("\
-offsetof (struct sigcontext, sc_rbp) yields %d instead of %d.\n\
-Please report this to <bug-gdb@gnu.org>."),
-	       offset, SC_RBP_OFFSET);
-    }
-
-  SC_RBP_OFFSET = offset;
 }
diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index b52fc6d431..648f929546 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -18,12 +18,11 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "arch-utils.h"
-#include "frame.h"
-#include "gdbcore.h"
-#include "regcache.h"
 #include "osabi.h"
 #include "regset.h"
+#include "target.h"
+#include "trad-frame.h"
+#include "tramp-frame.h"
 #include "i386-fbsd-tdep.h"
 #include "gdbsupport/x86-xstate.h"
 
@@ -70,6 +69,49 @@ static const struct regcache_map_entry amd64_fbsd_gregmap[] =
   { 0 }
 };
 
+/* This layout including fsbase and gsbase was adopted in FreeBSD
+   8.0.  */
+
+static const struct regcache_map_entry amd64_fbsd_mcregmap[] =
+{
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_onstack */
+  { 1, AMD64_RDI_REGNUM, 0 },
+  { 1, AMD64_RSI_REGNUM, 0 },
+  { 1, AMD64_RDX_REGNUM, 0 },
+  { 1, AMD64_RCX_REGNUM, 0 },
+  { 1, AMD64_R8_REGNUM, 0 },
+  { 1, AMD64_R9_REGNUM, 0 },
+  { 1, AMD64_RAX_REGNUM, 0 },
+  { 1, AMD64_RBX_REGNUM, 0 },
+  { 1, AMD64_RBP_REGNUM, 0 },
+  { 1, AMD64_R10_REGNUM, 0 },
+  { 1, AMD64_R11_REGNUM, 0 },
+  { 1, AMD64_R12_REGNUM, 0 },
+  { 1, AMD64_R13_REGNUM, 0 },
+  { 1, AMD64_R14_REGNUM, 0 },
+  { 1, AMD64_R15_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_trapno */
+  { 1, AMD64_FS_REGNUM, 2 },
+  { 1, AMD64_GS_REGNUM, 2 },
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_addr */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_flags */
+  { 1, AMD64_ES_REGNUM, 2 },
+  { 1, AMD64_DS_REGNUM, 2 },
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_err */
+  { 1, AMD64_RIP_REGNUM, 0 },
+  { 1, AMD64_CS_REGNUM, 8 },
+  { 1, AMD64_EFLAGS_REGNUM, 8 },
+  { 1, AMD64_RSP_REGNUM, 0 },
+  { 1, AMD64_SS_REGNUM, 8 },
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_len */
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_fpformat */
+  { 1, REGCACHE_MAP_SKIP, 8 },	/* mc_ownedfp */
+  { 64, REGCACHE_MAP_SKIP, 8 },	/* mc_fpstate */
+  { 1, AMD64_FSBASE_REGNUM, 0 },
+  { 1, AMD64_GSBASE_REGNUM, 0 },
+  { 0 }
+};
+
 /* Register set definitions.  */
 
 const struct regset amd64_fbsd_gregset =
@@ -79,80 +121,86 @@ const struct regset amd64_fbsd_gregset =
 
 /* Support for signal handlers.  */
 
-/* Return whether THIS_FRAME corresponds to a FreeBSD sigtramp
-   routine.  */
+/* In a signal frame, rsp points to a 'struct sigframe' which is
+   defined as:
 
-static const gdb_byte amd64fbsd_sigtramp_code[] =
-{
-  0x48, 0x8d, 0x7c, 0x24, 0x10, /* lea     SIGF_UC(%rsp),%rdi */
-  0x6a, 0x00,			/* pushq   $0 */
-  0x48, 0xc7, 0xc0, 0xa1, 0x01, 0x00, 0x00,
-				/* movq    $SYS_sigreturn,%rax */
-  0x0f, 0x05                    /* syscall */
-};
+   struct sigframe {
+	union {
+		__siginfohandler_t	*sf_action;
+		__sighandler_t		*sf_handler;
+	} sf_ahu;
+	ucontext_t	sf_uc;
+        ...
+   }
 
-static int
-amd64fbsd_sigtramp_p (struct frame_info *this_frame)
-{
-  CORE_ADDR pc = get_frame_pc (this_frame);
-  gdb_byte buf[sizeof amd64fbsd_sigtramp_code];
+   ucontext_t is defined as:
 
-  if (!safe_frame_unwind_memory (this_frame, pc, buf))
-    return 0;
-  if (memcmp (buf, amd64fbsd_sigtramp_code, sizeof amd64fbsd_sigtramp_code)
-      != 0)
-    return 0;
+   struct __ucontext {
+	   sigset_t	uc_sigmask;
+	   mcontext_t	uc_mcontext;
+	   ...
+   };
 
-  return 1;
-}
+   The mcontext_t contains the general purpose register set as well
+   as the floating point or XSAVE state.  */
 
-/* Assuming THIS_FRAME is for a BSD sigtramp routine, return the
-   address of the associated sigcontext structure.  */
+/* NB: There is an 8 byte padding hole between sf_ahu and sf_uc. */
+#define AMD64_SIGFRAME_UCONTEXT_OFFSET 		16
+#define AMD64_UCONTEXT_MCONTEXT_OFFSET		16
+#define AMD64_SIZEOF_MCONTEXT_T			800
 
-static CORE_ADDR
-amd64fbsd_sigcontext_addr (struct frame_info *this_frame)
+/* Implement the "init" method of struct tramp_frame.  */
+
+static void
+amd64_fbsd_sigframe_init (const struct tramp_frame *self,
+			  struct frame_info *this_frame,
+			  struct trad_frame_cache *this_cache,
+			  CORE_ADDR func)
 {
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  CORE_ADDR sp;
-  gdb_byte buf[8];
+  CORE_ADDR sp = get_frame_register_unsigned (this_frame, AMD64_RSP_REGNUM);
+  CORE_ADDR mcontext_addr
+    = (sp
+       + AMD64_SIGFRAME_UCONTEXT_OFFSET
+       + AMD64_UCONTEXT_MCONTEXT_OFFSET);
 
-  /* The `struct sigcontext' (which really is an `ucontext_t' on
-     FreeBSD/amd64) lives at a fixed offset in the signal frame.  See
-     <machine/sigframe.h>.  */
-  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-  sp = extract_unsigned_integer (buf, 8, byte_order);
-  return sp + 16;
+  trad_frame_set_reg_regmap (this_cache, amd64_fbsd_mcregmap, mcontext_addr,
+			     AMD64_SIZEOF_MCONTEXT_T);
+
+  /* Don't bother with floating point or XSAVE state for now.  The
+     current helper routines for parsing FXSAVE and XSAVE state only
+     work with regcaches.  This could perhaps create a temporary
+     regcache, collect the register values from mc_fpstate and
+     mc_xfpustate, and then set register values in the trad_frame.  */
+
+  trad_frame_set_id (this_cache, frame_id_build (sp, func));
 }
-\f
 
-/* From <machine/signal.h>.  */
-int amd64fbsd_sc_reg_offset[] =
+static const struct tramp_frame amd64_fbsd_sigframe =
 {
-  24 + 6 * 8,			/* %rax */
-  24 + 7 * 8,			/* %rbx */
-  24 + 3 * 8,			/* %rcx */
-  24 + 2 * 8,			/* %rdx */
-  24 + 1 * 8,			/* %rsi */
-  24 + 0 * 8,			/* %rdi */
-  24 + 8 * 8,			/* %rbp */
-  24 + 22 * 8,			/* %rsp */
-  24 + 4 * 8,			/* %r8 ...  */
-  24 + 5 * 8,
-  24 + 9 * 8,
-  24 + 10 * 8,
-  24 + 11 * 8,
-  24 + 12 * 8,
-  24 + 13 * 8,
-  24 + 14 * 8,			/* ... %r15 */
-  24 + 19 * 8,			/* %rip */
-  24 + 21 * 8,			/* %eflags */
-  24 + 20 * 8,			/* %cs */
-  24 + 23 * 8,			/* %ss */
-  -1,				/* %ds */
-  -1,				/* %es */
-  -1,				/* %fs */
-  -1				/* %gs */
+  SIGTRAMP_FRAME,
+  1,
+  {
+    {0x48, ULONGEST_MAX},		/* lea	   SIGF_UC(%rsp),%rdi */
+    {0x8d, ULONGEST_MAX},
+    {0x7c, ULONGEST_MAX},
+    {0x24, ULONGEST_MAX},
+    {0x10, ULONGEST_MAX},
+    {0x6a, ULONGEST_MAX},		/* pushq   $0 */
+    {0x00, ULONGEST_MAX},
+    {0x48, ULONGEST_MAX},		/* movq	   $SYS_sigreturn,%rax */
+    {0xc7, ULONGEST_MAX},
+    {0xc0, ULONGEST_MAX},
+    {0xa1, ULONGEST_MAX},
+    {0x01, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x0f, ULONGEST_MAX},		/* syscall */
+    {0x05, ULONGEST_MAX},
+    {TRAMP_SENTINEL_INSN, ULONGEST_MAX}
+  },
+  amd64_fbsd_sigframe_init
 };
 
 /* Implement the core_read_description gdbarch method.  */
@@ -245,10 +293,7 @@ amd64fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   amd64_init_abi (info, gdbarch,
 		  amd64_target_description (X86_XSTATE_SSE_MASK, true));
 
-  tdep->sigtramp_p = amd64fbsd_sigtramp_p;
-  tdep->sigcontext_addr = amd64fbsd_sigcontext_addr;
-  tdep->sc_reg_offset = amd64fbsd_sc_reg_offset;
-  tdep->sc_num_regs = ARRAY_SIZE (amd64fbsd_sc_reg_offset);
+  tramp_frame_prepend_unwinder (gdbarch, &amd64_fbsd_sigframe);
 
   tdep->xsave_xcr0_offset = I386_FBSD_XSAVE_XCR0_OFFSET;
 
diff --git a/gdb/amd64-tdep.h b/gdb/amd64-tdep.h
index b2a29aa5f8..302ce6f2dd 100644
--- a/gdb/amd64-tdep.h
+++ b/gdb/amd64-tdep.h
@@ -144,7 +144,4 @@ extern int amd64nbsd_r_reg_offset[];
 /* Variables exported from amd64-obsd-tdep.c.  */
 extern int amd64obsd_r_reg_offset[];
 
-/* Variables exported from amd64-fbsd-tdep.c.  */
-extern int amd64fbsd_sc_reg_offset[];
-
 #endif /* amd64-tdep.h */
diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
index 1fad6c5d48..7312c4bc12 100644
--- a/gdb/i386-bsd-nat.c
+++ b/gdb/i386-bsd-nat.c
@@ -350,9 +350,7 @@ _initialize_i386bsd_nat ()
      system header files and sysctl(3) to get at the relevant
      information.  */
 
-#if defined (__FreeBSD_version)
-#define SC_REG_OFFSET i386fbsd_sc_reg_offset
-#elif defined (OpenBSD)
+#if defined (OpenBSD)
 #define SC_REG_OFFSET i386obsd_sc_reg_offset
 #endif
 
diff --git a/gdb/i386-fbsd-tdep.c b/gdb/i386-fbsd-tdep.c
index 2aeb51dbec..6aa65e2aef 100644
--- a/gdb/i386-fbsd-tdep.c
+++ b/gdb/i386-fbsd-tdep.c
@@ -18,11 +18,11 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#include "arch-utils.h"
-#include "gdbcore.h"
 #include "osabi.h"
 #include "regcache.h"
 #include "regset.h"
+#include "trad-frame.h"
+#include "tramp-frame.h"
 #include "i386-fbsd-tdep.h"
 #include "gdbsupport/x86-xstate.h"
 
@@ -61,6 +61,41 @@ static const struct regcache_map_entry i386_fbsd_gregmap[] =
   { 0 }
 };
 
+/* This layout including fsbase and gsbase was adopted in FreeBSD
+   8.0.  */
+
+static const struct regcache_map_entry i386_fbsd_mcregmap[] =
+{
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_onstack */
+  { 1, I386_GS_REGNUM, 4 },
+  { 1, I386_FS_REGNUM, 4 },
+  { 1, I386_ES_REGNUM, 4 },
+  { 1, I386_DS_REGNUM, 4 },
+  { 1, I386_EDI_REGNUM, 0 },
+  { 1, I386_ESI_REGNUM, 0 },
+  { 1, I386_EBP_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* isp */
+  { 1, I386_EBX_REGNUM, 0 },
+  { 1, I386_EDX_REGNUM, 0 },
+  { 1, I386_ECX_REGNUM, 0 },
+  { 1, I386_EAX_REGNUM, 0 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_trapno */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_err */
+  { 1, I386_EIP_REGNUM, 0 },
+  { 1, I386_CS_REGNUM, 4 },
+  { 1, I386_EFLAGS_REGNUM, 0 },
+  { 1, I386_ESP_REGNUM, 0 },
+  { 1, I386_SS_REGNUM, 4 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_len */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_fpformat */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_ownedfp */
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* mc_flags */
+  { 128, REGCACHE_MAP_SKIP, 4 },/* mc_fpstate */
+  { 1, I386_FSBASE_REGNUM, 0 },
+  { 1, I386_GSBASE_REGNUM, 0 },
+  { 0 }
+};
+
 /* Register set definitions.  */
 
 const struct regset i386_fbsd_gregset =
@@ -70,102 +105,127 @@ const struct regset i386_fbsd_gregset =
 
 /* Support for signal handlers.  */
 
-/* Return whether THIS_FRAME corresponds to a FreeBSD sigtramp
-   routine.  */
+/* In a signal frame, esp points to a 'struct sigframe' which is
+   defined as:
+
+   struct sigframe {
+	register_t	sf_signum;
+	register_t	sf_siginfo;
+	register_t	sf_ucontext;
+	register_t	sf_addr;
+	union {
+		__siginfohandler_t	*sf_action;
+		__sighandler_t		*sf_handler;
+	} sf_ahu;
+	ucontext_t	sf_uc;
+        ...
+   }
+
+   ucontext_t is defined as:
+
+   struct __ucontext {
+	   sigset_t	uc_sigmask;
+	   mcontext_t	uc_mcontext;
+	   ...
+   };
+
+   The mcontext_t contains the general purpose register set as well
+   as the floating point or XSAVE state.  */
+
+/* NB: There is a 12 byte padding hole between sf_ahu and sf_uc. */
+#define I386_SIGFRAME_UCONTEXT_OFFSET 		32
+#define I386_UCONTEXT_MCONTEXT_OFFSET		16
+#define I386_SIZEOF_MCONTEXT_T			640
+
+/* Implement the "init" method of struct tramp_frame.  */
+
+static void
+i386_fbsd_sigframe_init (const struct tramp_frame *self,
+			 struct frame_info *this_frame,
+			 struct trad_frame_cache *this_cache,
+			 CORE_ADDR func)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_frame);
+  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+  CORE_ADDR sp = get_frame_register_unsigned (this_frame, I386_ESP_REGNUM);
+  CORE_ADDR mcontext_addr
+    = (sp
+       + I386_SIGFRAME_UCONTEXT_OFFSET
+       + I386_UCONTEXT_MCONTEXT_OFFSET);
+
+  trad_frame_set_reg_regmap (this_cache, i386_fbsd_mcregmap, mcontext_addr,
+			     I386_SIZEOF_MCONTEXT_T);
+
+  /* Don't bother with floating point or XSAVE state for now.  The
+     current helper routines for parsing FXSAVE and XSAVE state only
+     work with regcaches.  This could perhaps create a temporary
+     regcache, collect the register values from mc_fpstate and
+     mc_xfpustate, and then set register values in the trad_frame.  */
+
+  trad_frame_set_id (this_cache, frame_id_build (sp, func));
+}
+
+static const struct tramp_frame i386_fbsd_sigframe =
+{
+  SIGTRAMP_FRAME,
+  1,
+  {
+    {0x8d, ULONGEST_MAX},		/* lea     SIGF_UC(%esp),%eax */
+    {0x44, ULONGEST_MAX},
+    {0x24, ULONGEST_MAX},
+    {0x20, ULONGEST_MAX},
+    {0x50, ULONGEST_MAX},		/* pushl   %eax */
+    {0xf7, ULONGEST_MAX},		/* testl   $PSL_VM,UC_EFLAGS(%eax) */
+    {0x40, ULONGEST_MAX},
+    {0x54, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x02, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x75, ULONGEST_MAX},		/* jne	   +3 */
+    {0x03, ULONGEST_MAX},
+    {0x8e, ULONGEST_MAX},		/* mov	   UC_GS(%eax),%gs */
+    {0x68, ULONGEST_MAX},
+    {0x14, ULONGEST_MAX},
+    {0xb8, ULONGEST_MAX},		/* movl   $SYS_sigreturn,%eax */
+    {0xa1, ULONGEST_MAX},
+    {0x01, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x50, ULONGEST_MAX},		/* pushl   %eax */
+    {0xcd, ULONGEST_MAX},		/* int	   $0x80 */
+    {0x80, ULONGEST_MAX},
+    {TRAMP_SENTINEL_INSN, ULONGEST_MAX}
+  },
+  i386_fbsd_sigframe_init
+};
 
 /* FreeBSD/i386 binaries running under an amd64 kernel use a different
-   trampoline This trampoline differs from the i386 kernel trampoline
+   trampoline.  This trampoline differs from the i386 kernel trampoline
    in that it omits a middle section that conditionally restores
    %gs.  */
 
-static const gdb_byte i386fbsd_sigtramp_start[] =
+static const struct tramp_frame i386_fbsd64_sigframe =
 {
-  0x8d, 0x44, 0x24, 0x20,       /* lea     SIGF_UC(%esp),%eax */
-  0x50				/* pushl   %eax */
-};
-
-static const gdb_byte i386fbsd_sigtramp_middle[] =
-{
-  0xf7, 0x40, 0x54, 0x00, 0x00, 0x02, 0x00,
-				/* testl   $PSL_VM,UC_EFLAGS(%eax) */
-  0x75, 0x03,			/* jne	   +3 */
-  0x8e, 0x68, 0x14		/* mov	   UC_GS(%eax),%gs */
-};
-
-static const gdb_byte i386fbsd_sigtramp_end[] =
-{
-  0xb8, 0xa1, 0x01, 0x00, 0x00, /* movl   $SYS_sigreturn,%eax */
-  0x50,			/* pushl   %eax */
-  0xcd, 0x80			/* int	   $0x80 */
-};
-
-/* We assume that the middle is the largest chunk below.  */
-gdb_static_assert (sizeof i386fbsd_sigtramp_middle
-		   > sizeof i386fbsd_sigtramp_start);
-gdb_static_assert (sizeof i386fbsd_sigtramp_middle
-		   > sizeof i386fbsd_sigtramp_end);
-
-static int
-i386fbsd_sigtramp_p (struct frame_info *this_frame)
-{
-  CORE_ADDR pc = get_frame_pc (this_frame);
-  gdb_byte buf[sizeof i386fbsd_sigtramp_middle];
-
-  /* Look for a matching start.  */
-  if (!safe_frame_unwind_memory (this_frame, pc,
-				 {buf, sizeof i386fbsd_sigtramp_start}))
-    return 0;
-  if (memcmp (buf, i386fbsd_sigtramp_start, sizeof i386fbsd_sigtramp_start)
-      != 0)
-    return 0;
-
-  /* Since the end is shorter than the middle, check for a matching end
-     next.  */
-  pc += sizeof i386fbsd_sigtramp_start;
-  if (!safe_frame_unwind_memory (this_frame, pc,
-				 {buf, sizeof i386fbsd_sigtramp_end}))
-    return 0;
-  if (memcmp (buf, i386fbsd_sigtramp_end, sizeof i386fbsd_sigtramp_end) == 0)
-    return 1;
-
-  /* If the end didn't match, check for a matching middle.  */
-  if (!safe_frame_unwind_memory (this_frame, pc,
-				 {buf, sizeof i386fbsd_sigtramp_middle}))
-    return 0;
-  if (memcmp (buf, i386fbsd_sigtramp_middle, sizeof i386fbsd_sigtramp_middle)
-      != 0)
-    return 0;
-
-  /* The middle matched, check for a matching end.  */
-  pc += sizeof i386fbsd_sigtramp_middle;
-  if (!safe_frame_unwind_memory (this_frame, pc,
-				 {buf, sizeof i386fbsd_sigtramp_end}))
-    return 0;
-  if (memcmp (buf, i386fbsd_sigtramp_end, sizeof i386fbsd_sigtramp_end) != 0)
-    return 0;
-
-  return 1;
-}
-
-/* From <machine/signal.h>.  */
-int i386fbsd_sc_reg_offset[] =
-{
-  20 + 11 * 4,			/* %eax */
-  20 + 10 * 4,			/* %ecx */
-  20 + 9 * 4,			/* %edx */
-  20 + 8 * 4,			/* %ebx */
-  20 + 17 * 4,			/* %esp */
-  20 + 6 * 4,			/* %ebp */
-  20 + 5 * 4,			/* %esi */
-  20 + 4 * 4,			/* %edi */
-  20 + 14 * 4,			/* %eip */
-  20 + 16 * 4,			/* %eflags */
-  20 + 15 * 4,			/* %cs */
-  20 + 18 * 4,			/* %ss */
-  20 + 3 * 4,			/* %ds */
-  20 + 2 * 4,			/* %es */
-  20 + 1 * 4,			/* %fs */
-  20 + 0 * 4			/* %gs */
+  SIGTRAMP_FRAME,
+  1,
+  {
+    {0x8d, ULONGEST_MAX},		/* lea     SIGF_UC(%esp),%eax */
+    {0x44, ULONGEST_MAX},
+    {0x24, ULONGEST_MAX},
+    {0x20, ULONGEST_MAX},
+    {0x50, ULONGEST_MAX},		/* pushl   %eax */
+    {0xb8, ULONGEST_MAX},		/* movl   $SYS_sigreturn,%eax */
+    {0xa1, ULONGEST_MAX},
+    {0x01, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x00, ULONGEST_MAX},
+    {0x50, ULONGEST_MAX},		/* pushl   %eax */
+    {0xcd, ULONGEST_MAX},		/* int	   $0x80 */
+    {0x80, ULONGEST_MAX},
+    {TRAMP_SENTINEL_INSN, ULONGEST_MAX}
+  },
+  i386_fbsd_sigframe_init
 };
 
 /* Get XSAVE extended state xcr0 from core dump.  */
@@ -308,11 +368,8 @@ i386fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   /* FreeBSD uses -freg-struct-return by default.  */
   tdep->struct_return = reg_struct_return;
 
-  tdep->sigtramp_p = i386fbsd_sigtramp_p;
-
-  /* FreeBSD has a more complete `struct sigcontext'.  */
-  tdep->sc_reg_offset = i386fbsd_sc_reg_offset;
-  tdep->sc_num_regs = ARRAY_SIZE (i386fbsd_sc_reg_offset);
+  tramp_frame_prepend_unwinder (gdbarch, &i386_fbsd_sigframe);
+  tramp_frame_prepend_unwinder (gdbarch, &i386_fbsd64_sigframe);
 
   i386_elf_init_abi (info, gdbarch);
 
diff --git a/gdb/i386-tdep.h b/gdb/i386-tdep.h
index 92f0257916..a3025f456d 100644
--- a/gdb/i386-tdep.h
+++ b/gdb/i386-tdep.h
@@ -474,7 +474,6 @@ extern int i386_mpx_enabled (void);
 extern void i386bsd_init_abi (struct gdbarch_info, struct gdbarch *);
 extern CORE_ADDR i386obsd_sigtramp_start_addr;
 extern CORE_ADDR i386obsd_sigtramp_end_addr;
-extern int i386fbsd_sc_reg_offset[];
 extern int i386obsd_sc_reg_offset[];
 extern int i386bsd_sc_reg_offset[];
 
-- 
2.31.1


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

* [PATCH 7/8] fbsd-nat: Return a bool from fetch_register_set and store_register_set.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (5 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 6/8] FreeBSD x86: Use tramp-frame for signal frames John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-07-14 14:07 ` [PATCH 8/8] FreeBSD x86 nat: Use register maps for GP register sets John Baldwin
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

Change these helper functions to return true if they did any work.
---
 gdb/fbsd-nat.c |  8 ++++++--
 gdb/fbsd-nat.h | 21 ++++++++++++---------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 33eddb5f22..aa6abecec1 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1603,7 +1603,7 @@ fbsd_nat_target::supports_disable_randomization ()
 
 /* See fbsd-nat.h.  */
 
-void
+bool
 fbsd_nat_target::fetch_register_set (struct regcache *regcache, int regnum,
 				     int fetch_op, const struct regset *regset,
 				     void *regs, size_t size)
@@ -1619,12 +1619,14 @@ fbsd_nat_target::fetch_register_set (struct regcache *regcache, int regnum,
 	perror_with_name (_("Couldn't get registers"));
 
       regcache->supply_regset (regset, regnum, regs, size);
+      return true;
     }
+  return false;
 }
 
 /* See fbsd-nat.h.  */
 
-void
+bool
 fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
 				     int fetch_op, int store_op,
 				     const struct regset *regset, void *regs,
@@ -1644,7 +1646,9 @@ fbsd_nat_target::store_register_set (struct regcache *regcache, int regnum,
 
       if (ptrace (store_op, pid, (PTRACE_TYPE_ARG3) regs, 0) == -1)
 	perror_with_name (_("Couldn't write registers"));
+      return true;
     }
+  return false;
 }
 
 void _initialize_fbsd_nat ();
diff --git a/gdb/fbsd-nat.h b/gdb/fbsd-nat.h
index a59065415b..292542bf8e 100644
--- a/gdb/fbsd-nat.h
+++ b/gdb/fbsd-nat.h
@@ -117,12 +117,15 @@ class fbsd_nat_target : public inf_ptrace_target
      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.  */
+     and SIZE is the size of the storage.
 
-  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
+     Returns true if the register set was transferred due to a
+     matching REGNUM.*/
+
+  bool 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,
+  bool store_register_set (struct regcache *regcache, int regnum, int fetch_op,
 			   int store_op, const struct regset *regset,
 			   void *regs, size_t size);
 protected:
@@ -130,21 +133,21 @@ class fbsd_nat_target : public inf_ptrace_target
      type such as 'struct reg' or 'struct fpreg'.  */
 
   template <class Regset>
-  void fetch_register_set (struct regcache *regcache, int regnum, int fetch_op,
+  bool 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));
+    return 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,
+  bool 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));
+    return store_register_set (regcache, regnum, fetch_op, store_op, regset,
+			       &regs, sizeof (regs));
   }
 };
 
-- 
2.31.1


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

* [PATCH 8/8] FreeBSD x86 nat: Use register maps for GP register sets.
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (6 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 7/8] fbsd-nat: Return a bool from fetch_register_set and store_register_set John Baldwin
@ 2021-07-14 14:07 ` John Baldwin
  2021-08-10 15:56 ` [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
  2022-01-28 15:44 ` Andrew Burgess
  9 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-07-14 14:07 UTC (permalink / raw)
  To: gdb-patches

Rather than using the x86-specific register offset tables, use
register maps to describe the layout of the general purpose registers
fetched via PT_GETREGS.  The sole user-visible difference is that
FreeBSD/amd64 will now report additional segment registers ($ds, $es,
$fs, and $gs) for both 32-bit and 64-bit processes.

As part of these changes, the FreeBSD x86 native targets no longer use
amd64-bsd-nat.c or i386-bsd-nat.c.  Remove FreeBSD-specific register
handling (for $fs_base, $gs_base, and XSAVE state) from these files.
Similarly, remove the global x86bsd_xsave_len from x86-bsd-nat.c.  The
FreeBSD x86 native targets use a static xsave_len instead.

While here, rework the probing of PT_GETXMMREGS on FreeBSD/i386.
Probe the ptrace op once in the target read_description method and
cache the result for the future similar to the way the status of XSAVE
support is probed in the read_description method.  In addition, return
the proper xcr0 mask (X87-only) for old kernels or systems without
either XSAVE or XMM support.
---
 gdb/amd64-bsd-nat.c   |  96 ----------------
 gdb/amd64-fbsd-nat.c  | 260 +++++++++++++++++++++++++++++++++---------
 gdb/amd64-fbsd-tdep.c |   1 +
 gdb/amd64-fbsd-tdep.h |  27 +++++
 gdb/configure.nat     |   4 +-
 gdb/i386-bsd-nat.c    |  92 ---------------
 gdb/i386-fbsd-nat.c   | 229 +++++++++++++++++++++++++++++++++++--
 gdb/i386-fbsd-tdep.h  |   4 +
 gdb/x86-bsd-nat.c     |   4 -
 gdb/x86-bsd-nat.h     |   3 -
 10 files changed, 457 insertions(+), 263 deletions(-)
 create mode 100644 gdb/amd64-fbsd-tdep.h

diff --git a/gdb/amd64-bsd-nat.c b/gdb/amd64-bsd-nat.c
index 3e0c29a22f..e994bb71b6 100644
--- a/gdb/amd64-bsd-nat.c
+++ b/gdb/amd64-bsd-nat.c
@@ -59,9 +59,6 @@ amd64bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   ptid_t ptid = regcache->ptid ();
-#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
-  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-#endif
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
@@ -75,50 +72,9 @@ amd64bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
-#ifdef PT_GETFSBASE
-  if (regnum == -1 || regnum == tdep->fsbase_regnum)
-    {
-      register_t base;
-
-      if (gdb_ptrace (PT_GETFSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't get segment register fs_base"));
-
-      regcache->raw_supply (tdep->fsbase_regnum, &base);
-      if (regnum != -1)
-	return;
-    }
-#endif
-#ifdef PT_GETGSBASE
-  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
-    {
-      register_t base;
-
-      if (gdb_ptrace (PT_GETGSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't get segment register gs_base"));
-
-      regcache->raw_supply (tdep->fsbase_regnum + 1, &base);
-      if (regnum != -1)
-	return;
-    }
-#endif
-
   if (regnum == -1 || !amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
       struct fpreg fpregs;
-#ifdef PT_GETXSTATE_INFO
-      void *xstateregs;
-
-      if (x86bsd_xsave_len != 0)
-	{
-	  xstateregs = alloca (x86bsd_xsave_len);
-	  if (gdb_ptrace (PT_GETXSTATE, ptid, (PTRACE_TYPE_ARG3) xstateregs, 0)
-	      == -1)
-	    perror_with_name (_("Couldn't get extended state status"));
-
-	  amd64_supply_xsave (regcache, -1, xstateregs);
-	  return;
-	}
-#endif
 
       if (gdb_ptrace (PT_GETFPREGS, ptid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
 	perror_with_name (_("Couldn't get floating point status"));
@@ -135,9 +91,6 @@ amd64bsd_store_inferior_registers (struct regcache *regcache, int regnum)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   ptid_t ptid = regcache->ptid ();
-#if defined(PT_SETFSBASE) || defined(PT_SETGSBASE)
-  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-#endif
 
   if (regnum == -1 || amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
@@ -155,58 +108,9 @@ amd64bsd_store_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
-#ifdef PT_SETFSBASE
-  if (regnum == -1 || regnum == tdep->fsbase_regnum)
-    {
-      register_t base;
-
-      /* Clear the full base value to support 32-bit targets.  */
-      base = 0;
-      regcache->raw_collect (tdep->fsbase_regnum, &base);
-
-      if (gdb_ptrace (PT_SETFSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't write segment register fs_base"));
-      if (regnum != -1)
-	return;
-    }
-#endif
-#ifdef PT_SETGSBASE
-  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
-    {
-      register_t base;
-
-      /* Clear the full base value to support 32-bit targets.  */
-      base = 0;
-      regcache->raw_collect (tdep->fsbase_regnum + 1, &base);
-
-      if (gdb_ptrace (PT_SETGSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't write segment register gs_base"));
-      if (regnum != -1)
-	return;
-    }
-#endif
-
   if (regnum == -1 || !amd64_native_gregset_supplies_p (gdbarch, regnum))
     {
       struct fpreg fpregs;
-#ifdef PT_GETXSTATE_INFO
-      void *xstateregs;
-
-      if (x86bsd_xsave_len != 0)
-	{
-	  xstateregs = alloca (x86bsd_xsave_len);
-	  if (gdb_ptrace (PT_GETXSTATE, ptid, (PTRACE_TYPE_ARG3) xstateregs, 0)
-	      == -1)
-	    perror_with_name (_("Couldn't get extended state status"));
-
-	  amd64_collect_xsave (regcache, regnum, xstateregs, 0);
-
-	  if (gdb_ptrace (PT_SETXSTATE, ptid, (PTRACE_TYPE_ARG3) xstateregs,
-			  x86bsd_xsave_len) == -1)
-	    perror_with_name (_("Couldn't write extended state status"));
-	  return;
-	}
-#endif
 
       if (gdb_ptrace (PT_GETFPREGS, ptid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
 	perror_with_name (_("Couldn't get floating point status"));
diff --git a/gdb/amd64-fbsd-nat.c b/gdb/amd64-fbsd-nat.c
index 91fc477d4d..a20d3c04b8 100644
--- a/gdb/amd64-fbsd-nat.c
+++ b/gdb/amd64-fbsd-nat.c
@@ -31,17 +31,19 @@
 
 #include "fbsd-nat.h"
 #include "amd64-tdep.h"
+#include "amd64-fbsd-tdep.h"
 #include "amd64-nat.h"
-#include "amd64-bsd-nat.h"
 #include "x86-nat.h"
 #include "gdbsupport/x86-xstate.h"
-\f
+#include "x86-bsd-nat.h"
 
 class amd64_fbsd_nat_target final
-  : public amd64_bsd_nat_target<fbsd_nat_target>
+  : public x86bsd_nat_target<fbsd_nat_target>
 {
 public:
-  /* Add some extra features to the common *BSD/amd64 target.  */
+  void fetch_registers (struct regcache *, int) override;
+  void store_registers (struct regcache *, int) override;
+
   const struct target_desc *read_description () override;
 
 #if defined(HAVE_PT_GETDBREGS) && defined(USE_SIGTRAP_SIGINFO)
@@ -51,61 +53,208 @@ class amd64_fbsd_nat_target final
 
 static amd64_fbsd_nat_target the_amd64_fbsd_nat_target;
 
-/* Offset in `struct reg' where MEMBER is stored.  */
-#define REG_OFFSET(member) offsetof (struct reg, member)
+#ifdef PT_GETXSTATE_INFO
+static size_t xsave_len;
+#endif
 
-/* At amd64fbsd64_r_reg_offset[REGNUM] you'll find the offset in
-   `struct reg' location where the GDB register REGNUM is stored.
-   Unsupported registers are marked with `-1'.  */
-static int amd64fbsd64_r_reg_offset[] =
+/* This is a layout of the amd64 'struct reg' but with i386
+   registers.  */
+
+static const struct regcache_map_entry amd64_fbsd32_gregmap[] =
 {
-  REG_OFFSET (r_rax),
-  REG_OFFSET (r_rbx),
-  REG_OFFSET (r_rcx),
-  REG_OFFSET (r_rdx),
-  REG_OFFSET (r_rsi),
-  REG_OFFSET (r_rdi),
-  REG_OFFSET (r_rbp),
-  REG_OFFSET (r_rsp),
-  REG_OFFSET (r_r8),
-  REG_OFFSET (r_r9),
-  REG_OFFSET (r_r10),
-  REG_OFFSET (r_r11),
-  REG_OFFSET (r_r12),
-  REG_OFFSET (r_r13),
-  REG_OFFSET (r_r14),
-  REG_OFFSET (r_r15),
-  REG_OFFSET (r_rip),
-  REG_OFFSET (r_rflags),
-  REG_OFFSET (r_cs),
-  REG_OFFSET (r_ss),
-  -1,
-  -1,
-  -1,
-  -1
+  { 8, REGCACHE_MAP_SKIP, 8 },
+  { 1, I386_EDI_REGNUM, 8 },
+  { 1, I386_ESI_REGNUM, 8 },
+  { 1, I386_EBP_REGNUM, 8 },
+  { 1, I386_EBX_REGNUM, 8 },
+  { 1, I386_EDX_REGNUM, 8 },
+  { 1, I386_ECX_REGNUM, 8 },
+  { 1, I386_EAX_REGNUM, 8 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* trapno */
+  { 1, I386_FS_REGNUM, 2 },
+  { 1, I386_GS_REGNUM, 2 },
+  { 1, REGCACHE_MAP_SKIP, 4 },	/* err */
+  { 1, I386_ES_REGNUM, 2 },
+  { 1, I386_DS_REGNUM, 2 },
+  { 1, I386_EIP_REGNUM, 8 },
+  { 1, I386_CS_REGNUM, 8 },
+  { 1, I386_EFLAGS_REGNUM, 8 },
+  { 1, I386_ESP_REGNUM, 0 },
+  { 1, I386_SS_REGNUM, 8 },
+  { 0 }
 };
-\f
 
-/* Mapping between the general-purpose registers in FreeBSD/amd64
-   `struct reg' format and GDB's register cache layout for
-   FreeBSD/i386.
+static const struct regset amd64_fbsd32_gregset =
+{
+  amd64_fbsd32_gregmap, regcache_supply_regset, regcache_collect_regset
+};
 
-   Note that most FreeBSD/amd64 registers are 64-bit, while the
-   FreeBSD/i386 registers are all 32-bit, but since we're
-   little-endian we get away with that.  */
+/* Return the regset to use for 'struct reg' for the GDBARCH.  */
 
-/* From <machine/reg.h>.  */
-static int amd64fbsd32_r_reg_offset[I386_NUM_GREGS] =
+static const struct regset *
+find_gregset (struct gdbarch *gdbarch)
 {
-  14 * 8, 13 * 8,		/* %eax, %ecx */
-  12 * 8, 11 * 8,		/* %edx, %ebx */
-  20 * 8, 10 * 8,		/* %esp, %ebp */
-  9 * 8, 8 * 8,			/* %esi, %edi */
-  17 * 8, 19 * 8,		/* %eip, %eflags */
-  18 * 8, 21 * 8,		/* %cs, %ss */
-  -1, -1, -1, -1		/* %ds, %es, %fs, %gs */
-};
-\f
+  if (gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 32)
+    return &amd64_fbsd32_gregset;
+  else
+    return &amd64_fbsd_gregset;
+}
+
+/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
+   for all registers.  */
+
+void
+amd64_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+  const struct regset *gregset = find_gregset (gdbarch);
+
+  if (fetch_register_set<struct reg> (regcache, regnum, PT_GETREGS, gregset))
+    {
+      if (regnum != -1)
+	return;
+    }
+
+#ifdef PT_GETFSBASE
+  if (regnum == -1 || regnum == tdep->fsbase_regnum)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register fs_base"));
+
+      regcache->raw_supply (tdep->fsbase_regnum, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_GETGSBASE
+  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register gs_base"));
+
+      regcache->raw_supply (tdep->fsbase_regnum + 1, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+
+  /* There is no amd64_fxsave_supplies or amd64_xsave_supplies.
+     Instead, the earlier register sets return early if the request
+     was for a specific register that was already satisified to avoid
+     fetching the FPU/XSAVE state unnecessarily.  */
+
+#ifdef PT_GETXSTATE_INFO
+  if (xsave_len != 0)
+    {
+      void *xstateregs = alloca (xsave_len);
+
+      if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
+	perror_with_name (_("Couldn't get extended state status"));
+
+      amd64_supply_xsave (regcache, regnum, xstateregs);
+      return;
+    }
+#endif
+
+  struct fpreg fpregs;
+
+  if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't get floating point status"));
+
+  amd64_supply_fxsave (regcache, regnum, &fpregs);
+}
+
+/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
+   this for all registers.  */
+
+void
+amd64_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+  const struct regset *gregset = find_gregset (gdbarch);
+
+  if (store_register_set<struct reg> (regcache, regnum, PT_GETREGS, PT_SETREGS,
+				      gregset))
+    {
+      if (regnum != -1)
+	return;
+    }
+
+#ifdef PT_SETFSBASE
+  if (regnum == -1 || regnum == tdep->fsbase_regnum)
+    {
+      register_t base;
+
+      /* Clear the full base value to support 32-bit targets.  */
+      base = 0;
+      regcache->raw_collect (tdep->fsbase_regnum, &base);
+
+      if (ptrace (PT_SETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register fs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_SETGSBASE
+  if (regnum == -1 || regnum == tdep->fsbase_regnum + 1)
+    {
+      register_t base;
+
+      /* Clear the full base value to support 32-bit targets.  */
+      base = 0;
+      regcache->raw_collect (tdep->fsbase_regnum + 1, &base);
+
+      if (ptrace (PT_SETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register gs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+
+  /* There is no amd64_fxsave_supplies or amd64_xsave_supplies.
+     Instead, the earlier register sets return early if the request
+     was for a specific register that was already satisified to avoid
+     fetching the FPU/XSAVE state unnecessarily.  */
+
+#ifdef PT_GETXSTATE_INFO
+  if (xsave_len != 0)
+    {
+      void *xstateregs = alloca (xsave_len);
+
+      if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
+	perror_with_name (_("Couldn't get extended state status"));
+
+      amd64_collect_xsave (regcache, regnum, xstateregs, 0);
+
+      if (ptrace (PT_SETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs,
+		  xsave_len) == -1)
+	perror_with_name (_("Couldn't write extended state status"));
+      return;
+    }
+#endif
+
+  struct fpreg fpregs;
+
+  if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't get floating point status"));
+
+  amd64_collect_fxsave (regcache, regnum, &fpregs);
+
+  if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't write floating point status"));
+}
 
 /* Support for debugging kernel virtual memory images.  */
 
@@ -179,13 +328,13 @@ amd64_fbsd_nat_target::read_description ()
       if (ptrace (PT_GETXSTATE_INFO, inferior_ptid.pid (),
 		  (PTRACE_TYPE_ARG3) &info, sizeof (info)) == 0)
 	{
-	  x86bsd_xsave_len = info.xsave_len;
+	  xsave_len = info.xsave_len;
 	  xcr0 = info.xsave_mask;
 	}
       xsave_probed = 1;
     }
 
-  if (x86bsd_xsave_len != 0)
+  if (xsave_len != 0)
     {
       if (is64)
 	return amd64_target_description (xcr0, true);
@@ -213,9 +362,6 @@ void _initialize_amd64fbsd_nat ();
 void
 _initialize_amd64fbsd_nat ()
 {
-  amd64_native_gregset32_reg_offset = amd64fbsd32_r_reg_offset;
-  amd64_native_gregset64_reg_offset = amd64fbsd64_r_reg_offset;
-
   add_inf_child_target (&the_amd64_fbsd_nat_target);
 
   /* Support debugging kernel virtual memory images.  */
diff --git a/gdb/amd64-fbsd-tdep.c b/gdb/amd64-fbsd-tdep.c
index 648f929546..859a2bff7a 100644
--- a/gdb/amd64-fbsd-tdep.c
+++ b/gdb/amd64-fbsd-tdep.c
@@ -27,6 +27,7 @@
 #include "gdbsupport/x86-xstate.h"
 
 #include "amd64-tdep.h"
+#include "amd64-fbsd-tdep.h"
 #include "fbsd-tdep.h"
 #include "solib-svr4.h"
 #include "inferior.h"
diff --git a/gdb/amd64-fbsd-tdep.h b/gdb/amd64-fbsd-tdep.h
new file mode 100644
index 0000000000..0a18dbcbfd
--- /dev/null
+++ b/gdb/amd64-fbsd-tdep.h
@@ -0,0 +1,27 @@
+/* FreeBSD/amd64 target support, prototypes.
+
+   Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef AMD64_FBSD_TDEP_H
+#define AMD64_FBSD_TDEP_H
+
+#include "regset.h"
+
+extern const struct regset amd64_fbsd_gregset;
+
+#endif /* AMD64_FBSD_TDEP_H */
diff --git a/gdb/configure.nat b/gdb/configure.nat
index e34cccffd9..26f214cd87 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -165,7 +165,7 @@ case ${gdb_host} in
 	    i386)
 		# Host: FreeBSD/i386
 		NATDEPFILES="${NATDEPFILES} x86-nat.o nat/x86-dregs.o \
-		x86-bsd-nat.o i386-bsd-nat.o i386-fbsd-nat.o bsd-kvm.o"
+		x86-bsd-nat.o i386-fbsd-nat.o bsd-kvm.o"
 		;;
 	    mips)
 		# Host: FreeBSD/mips
@@ -192,7 +192,7 @@ case ${gdb_host} in
 	case ${gdb_host_cpu} in
 	    i386)
 		# Host: FreeBSD/amd64
-		NATDEPFILES="${NATDEPFILES} amd64-nat.o amd64-bsd-nat.o \
+		NATDEPFILES="${NATDEPFILES} amd64-nat.o \
 		amd64-fbsd-nat.o bsd-kvm.o x86-nat.o nat/x86-dregs.o \
 		x86-bsd-nat.o"
 		;;
diff --git a/gdb/i386-bsd-nat.c b/gdb/i386-bsd-nat.c
index 7312c4bc12..6006dadf7e 100644
--- a/gdb/i386-bsd-nat.c
+++ b/gdb/i386-bsd-nat.c
@@ -159,56 +159,12 @@ i386bsd_fetch_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
-#ifdef PT_GETFSBASE
-  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
-    {
-      register_t base;
-
-      if (gdb_ptrace (PT_GETFSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't get segment register fs_base"));
-
-      regcache->raw_supply (I386_FSBASE_REGNUM, &base);
-      if (regnum != -1)
-	return;
-    }
-#endif
-#ifdef PT_GETGSBASE
-  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
-    {
-      register_t base;
-
-      if (gdb_ptrace (PT_GETGSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't get segment register gs_base"));
-
-      regcache->raw_supply (I386_GSBASE_REGNUM, &base);
-      if (regnum != -1)
-	return;
-    }
-#endif
-
   if (regnum == -1 || regnum >= I386_ST0_REGNUM)
     {
       struct fpreg fpregs;
 #ifdef HAVE_PT_GETXMMREGS
       char xmmregs[512];
-#endif
 
-#ifdef PT_GETXSTATE_INFO
-      if (x86bsd_xsave_len != 0)
-	{
-	  void *xstateregs;
-
-	  xstateregs = alloca (x86bsd_xsave_len);
-	  if (gdb_ptrace (PT_GETXSTATE, ptid,
-			  (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
-	    perror_with_name (_("Couldn't get extended state status"));
-
-	  i387_supply_xsave (regcache, -1, xstateregs);
-	  return;
-	}
-#endif
-      
-#ifdef HAVE_PT_GETXMMREGS
       if (have_ptrace_xmmregs != 0
 	  && gdb_ptrace(PT_GETXMMREGS, ptid,
 			(PTRACE_TYPE_ARG3) xmmregs, 0) == 0)
@@ -255,60 +211,12 @@ i386bsd_store_inferior_registers (struct regcache *regcache, int regnum)
 	return;
     }
 
-#ifdef PT_SETFSBASE
-  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
-    {
-      register_t base;
-
-      regcache->raw_collect (I386_FSBASE_REGNUM, &base);
-
-      if (gdb_ptrace (PT_SETFSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't write segment register fs_base"));
-      if (regnum != -1)
-	return;
-    }
-#endif
-#ifdef PT_SETGSBASE
-  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
-    {
-      register_t base;
-
-      regcache->raw_collect (I386_GSBASE_REGNUM, &base);
-
-      if (gdb_ptrace (PT_SETGSBASE, ptid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
-	perror_with_name (_("Couldn't write segment register gs_base"));
-      if (regnum != -1)
-	return;
-    }
-#endif
-
   if (regnum == -1 || regnum >= I386_ST0_REGNUM)
     {
       struct fpreg fpregs;
 #ifdef HAVE_PT_GETXMMREGS
       char xmmregs[512];
-#endif
 
-#ifdef PT_GETXSTATE_INFO
-      if (x86bsd_xsave_len != 0)
-	{
-	  void *xstateregs;
-
-	  xstateregs = alloca (x86bsd_xsave_len);
-	  if (gdb_ptrace (PT_GETXSTATE, ptid,
-			  (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
-	    perror_with_name (_("Couldn't get extended state status"));
-
-	  i387_collect_xsave (regcache, -1, xstateregs, 0);
-
-	  if (gdb_ptrace (PT_SETXSTATE, ptid, (PTRACE_TYPE_ARG3) xstateregs,
-			  x86bsd_xsave_len) == -1)
-	    perror_with_name (_("Couldn't write extended state status"));
-	  return;
-	}
-#endif
-
-#ifdef HAVE_PT_GETXMMREGS
       if (have_ptrace_xmmregs != 0
 	  && gdb_ptrace(PT_GETXMMREGS, ptid,
 			(PTRACE_TYPE_ARG3) xmmregs, 0) == 0)
diff --git a/gdb/i386-fbsd-nat.c b/gdb/i386-fbsd-nat.c
index 9b5913d88e..a67a4267ac 100644
--- a/gdb/i386-fbsd-nat.c
+++ b/gdb/i386-fbsd-nat.c
@@ -29,17 +29,20 @@
 
 #include "fbsd-nat.h"
 #include "i386-tdep.h"
+#include "i386-fbsd-tdep.h"
+#include "i387-tdep.h"
 #include "x86-nat.h"
 #include "gdbsupport/x86-xstate.h"
 #include "x86-bsd-nat.h"
-#include "i386-bsd-nat.h"
 
 class i386_fbsd_nat_target final
-  : public i386_bsd_nat_target<fbsd_nat_target>
+  : public x86bsd_nat_target<fbsd_nat_target>
 {
 public:
-  /* Add some extra features to the common *BSD/i386 target.  */
-#ifdef PT_GETXSTATE_INFO
+  void fetch_registers (struct regcache *, int) override;
+  void store_registers (struct regcache *, int) override;
+
+#if defined(PT_GETXMMREGS) || defined(PT_GETXSTATE_INFO)
   const struct target_desc *read_description () override;
 #endif
 
@@ -52,6 +55,192 @@ class i386_fbsd_nat_target final
 
 static i386_fbsd_nat_target the_i386_fbsd_nat_target;
 
+#ifdef PT_GETXSTATE_INFO
+static size_t xsave_len;
+#endif
+
+#ifdef HAVE_PT_GETXMMREGS
+static int have_ptrace_xmmregs;
+#endif
+
+/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
+   for all registers.  */
+
+void
+i386_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+  if (fetch_register_set<struct reg> (regcache, regnum, PT_GETREGS,
+				      &i386_fbsd_gregset))
+    {
+      if (regnum != -1)
+	return;
+    }
+
+#ifdef PT_GETFSBASE
+  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register fs_base"));
+
+      regcache->raw_supply (I386_FSBASE_REGNUM, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_GETGSBASE
+  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
+    {
+      register_t base;
+
+      if (ptrace (PT_GETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't get segment register gs_base"));
+
+      regcache->raw_supply (I386_GSBASE_REGNUM, &base);
+      if (regnum != -1)
+	return;
+    }
+#endif
+
+  /* There is no i386_fxsave_supplies or i386_xsave_supplies.
+     Instead, the earlier register sets return early if the request
+     was for a specific register that was already satisified to avoid
+     fetching the FPU/XSAVE state unnecessarily.  */
+
+#ifdef PT_GETXSTATE_INFO
+  if (xsave_len != 0)
+    {
+      void *xstateregs = alloca (xsave_len);
+
+      if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
+	perror_with_name (_("Couldn't get extended state status"));
+
+      i387_supply_xsave (regcache, regnum, xstateregs);
+      return;
+    }
+#endif
+#ifdef HAVE_PT_GETXMMREGS
+  if (have_ptrace_xmmregs != 0)
+    {
+      char xmmregs[I387_SIZEOF_FXSAVE];
+
+      if (ptrace(PT_GETXMMREGS, pid, (PTRACE_TYPE_ARG3) xmmregs, 0) == -1)
+	perror_with_name (_("Couldn't get XMM registers"));
+
+      i387_supply_fxsave (regcache, regnum, xmmregs);
+      return;
+    }
+#endif
+
+  struct fpreg fpregs;
+
+  if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't get floating point status"));
+
+  i387_supply_fsave (regcache, regnum, &fpregs);
+}
+
+/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
+   this for all registers.  */
+
+void
+i386_fbsd_nat_target::store_registers (struct regcache *regcache, int regnum)
+{
+  struct gdbarch *gdbarch = regcache->arch ();
+#if defined(PT_GETFSBASE) || defined(PT_GETGSBASE)
+  const struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+#endif
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+  if (store_register_set<struct reg> (regcache, regnum, PT_GETREGS, PT_SETREGS,
+				      &i386_fbsd_gregset))
+    {
+      if (regnum != -1)
+	return;
+    }
+
+#ifdef PT_SETFSBASE
+  if (regnum == -1 || regnum == I386_FSBASE_REGNUM)
+    {
+      register_t base;
+
+      regcache->raw_collect (I386_FSBASE_REGNUM, &base);
+
+      if (ptrace (PT_SETFSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register fs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+#ifdef PT_SETGSBASE
+  if (regnum == -1 || regnum == I386_GSBASE_REGNUM)
+    {
+      register_t base;
+
+      regcache->raw_collect (I386_GSBASE_REGNUM, &base);
+
+      if (ptrace (PT_SETGSBASE, pid, (PTRACE_TYPE_ARG3) &base, 0) == -1)
+	perror_with_name (_("Couldn't write segment register gs_base"));
+      if (regnum != -1)
+	return;
+    }
+#endif
+
+  /* There is no i386_fxsave_supplies or i386_xsave_supplies.
+     Instead, the earlier register sets return early if the request
+     was for a specific register that was already satisified to avoid
+     fetching the FPU/XSAVE state unnecessarily.  */
+
+#ifdef PT_GETXSTATE_INFO
+  if (xsave_len != 0)
+    {
+      void *xstateregs = alloca (xsave_len);
+
+      if (ptrace (PT_GETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, 0) == -1)
+	perror_with_name (_("Couldn't get extended state status"));
+
+      i387_collect_xsave (regcache, regnum, xstateregs, 0);
+
+      if (ptrace (PT_SETXSTATE, pid, (PTRACE_TYPE_ARG3) xstateregs, xsave_len)
+	  == -1)
+	perror_with_name (_("Couldn't write extended state status"));
+      return;
+    }
+#endif
+#ifdef HAVE_PT_GETXMMREGS
+  if (have_ptrace_xmmregs != 0)
+    {
+      char xmmregs[I387_SIZEOF_FXSAVE];
+
+      if (ptrace(PT_GETXMMREGS, pid, (PTRACE_TYPE_ARG3) xmmregs, 0) == -1)
+	perror_with_name (_("Couldn't get XMM registers"));
+
+      i387_collect_fxsave (regcache, regnum, xmmregs);
+
+      if (ptrace (PT_SETXMMREGS, pid, (PTRACE_TYPE_ARG3) xmmregs, 0) == -1)
+	perror_with_name (_("Couldn't write XMM registers"));
+      return;
+    }
+#endif
+
+  struct fpreg fpregs;
+
+  if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't get floating point status"));
+
+  i387_collect_fsave (regcache, regnum, &fpregs);
+
+  if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+    perror_with_name (_("Couldn't write floating point status"));
+}
+
 /* Resume execution of the inferior process.  If STEP is nonzero,
    single-step it.  If SIGNAL is nonzero, give it that signal.  */
 
@@ -135,15 +324,21 @@ i386fbsd_supply_pcb (struct regcache *regcache, struct pcb *pcb)
 }
 \f
 
-#ifdef PT_GETXSTATE_INFO
+#if defined(PT_GETXMMREGS) || defined(PT_GETXSTATE_INFO)
 /* Implement the read_description method.  */
 
 const struct target_desc *
 i386_fbsd_nat_target::read_description ()
 {
+#ifdef PT_GETXSTATE_INFO
   static int xsave_probed;
   static uint64_t xcr0;
+#endif
+#ifdef PT_GETXMMREGS
+  static int xmm_probed;
+#endif
 
+#ifdef PT_GETXSTATE_INFO
   if (!xsave_probed)
     {
       struct ptrace_xstate_info info;
@@ -151,16 +346,32 @@ i386_fbsd_nat_target::read_description ()
       if (ptrace (PT_GETXSTATE_INFO, inferior_ptid.pid (),
 		  (PTRACE_TYPE_ARG3) &info, sizeof (info)) == 0)
 	{
-	  x86bsd_xsave_len = info.xsave_len;
+	  xsave_len = info.xsave_len;
 	  xcr0 = info.xsave_mask;
 	}
       xsave_probed = 1;
     }
 
-  if (x86bsd_xsave_len == 0)
-    xcr0 = X86_XSTATE_SSE_MASK;
+  if (xsave_len != 0)
+    return i386_target_description (xcr0, true);
+#endif
 
-  return i386_target_description (xcr0, true);
+#ifdef PT_GETXMMREGS
+  if (!xmm_probed)
+    {
+      char xmmregs[I387_SIZEOF_FXSAVE];
+
+      if (ptrace (PT_GETXMMREGS, inferior_ptid.pid (),
+		  (PTRACE_TYPE_ARG3) xmmregs, 0) == 0)
+	have_ptrace_xmmregs = 1;
+      xmm_probed = 1;
+    }
+
+  if (have_ptrace_xmmregs)
+    return i386_target_description (X86_XSTATE_SSE_MASK, true);
+#endif
+
+  return i386_target_description (X86_XSTATE_X87_MASK, true);
 }
 #endif
 
diff --git a/gdb/i386-fbsd-tdep.h b/gdb/i386-fbsd-tdep.h
index c2bbda60e8..6cb05073e7 100644
--- a/gdb/i386-fbsd-tdep.h
+++ b/gdb/i386-fbsd-tdep.h
@@ -20,6 +20,8 @@
 #ifndef I386_FBSD_TDEP_H
 #define I386_FBSD_TDEP_H
 
+#include "regset.h"
+
 /* Get XSAVE extended state xcr0 from core dump.  */
 extern uint64_t i386fbsd_core_read_xcr0 (bfd *abfd);
 
@@ -28,4 +30,6 @@ extern uint64_t i386fbsd_core_read_xcr0 (bfd *abfd);
    matches the layout on Linux.  */
 #define I386_FBSD_XSAVE_XCR0_OFFSET 464
 
+extern const struct regset i386_fbsd_gregset;
+
 #endif /* i386-fbsd-tdep.h */
diff --git a/gdb/x86-bsd-nat.c b/gdb/x86-bsd-nat.c
index 453fc44116..7e432b7e91 100644
--- a/gdb/x86-bsd-nat.c
+++ b/gdb/x86-bsd-nat.c
@@ -46,10 +46,6 @@ gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr)
 #endif
 }
 
-#ifdef PT_GETXSTATE_INFO
-size_t x86bsd_xsave_len;
-#endif
-
 /* Support for debug registers.  */
 
 #ifdef HAVE_PT_GETDBREGS
diff --git a/gdb/x86-bsd-nat.h b/gdb/x86-bsd-nat.h
index 02d61c20b0..d4b929ba8a 100644
--- a/gdb/x86-bsd-nat.h
+++ b/gdb/x86-bsd-nat.h
@@ -22,9 +22,6 @@
 
 #include "x86-nat.h"
 
-/* Low level x86 XSAVE info.  */
-extern size_t x86bsd_xsave_len;
-
 /* A prototype *BSD/x86 target.  */
 
 template<typename BaseTarget>
-- 
2.31.1


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

* Re: [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (7 preceding siblings ...)
  2021-07-14 14:07 ` [PATCH 8/8] FreeBSD x86 nat: Use register maps for GP register sets John Baldwin
@ 2021-08-10 15:56 ` John Baldwin
  2021-09-14 15:43   ` [PING] " John Baldwin
  2022-01-28 15:44 ` Andrew Burgess
  9 siblings, 1 reply; 19+ messages in thread
From: John Baldwin @ 2021-08-10 15:56 UTC (permalink / raw)
  To: gdb-patches

On 7/14/21 7:07 AM, John Baldwin wrote:
> The x86 architectures in GDB provide existing helpers for parsing
> general-purpose register sets.  However, these helpers have some
> limitations, such as assuming that registers are always full size.  On
> FreeBSD/amd64 in particular, segment registers are stored as 16-bit
> quantities that in some cases are packed together.  GDB for historical
> reasons treats these 16-bit registers as 32 bits in size.  Using the
> more generic regcache_map_entry to describe the GP register sets
> permits supporting these registers as 16-bit values.  In addition, the
> FreeBSD x86 signal frames have included the base address of the FS and
> GS segments (equivalent to the fs_base and gs_base registers), but the
> existing signal context helpers were written before those registers
> were added to GDB.
> 
> Longer term my goal is to use regcache_map_entry-based register sets
> in FreeBSD gdbserver support to simplify the implementation.
> 
> Note that patch 4 fixes an issue in regcache_collect_regset where it
> didn't quite do what I thought it did.  I believe the change is ok,
> but it definitely warrants review.

Ping.  Patch 4 is the only one that isn't FreeBSD-specific.

> I have tested this on both FreeBSD/amd64 (32-bit and 64-bit processes)
> and FreeBSD/i386.
> 
> John Baldwin (8):
>    Remove vestigal FreeBSD/i386 3.x support.
>    Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
>    FreeBSD x86: Remove fallback for detecting signal trampolines by
>      address.
>    regcache: Zero-extend small registers described by a register map.
>    Use register maps for gp regsets on FreeBSD/x86 core dumps.
>    FreeBSD x86: Use tramp-frame for signal frames.
>    fbsd-nat: Return a bool from fetch_register_set and
>      store_register_set.
>    FreeBSD x86 nat: Use register maps for GP register sets.
> 
>   gdb/amd64-bsd-nat.c   |  96 ---------
>   gdb/amd64-fbsd-nat.c  | 346 ++++++++++++++++++-------------
>   gdb/amd64-fbsd-tdep.c | 279 ++++++++++++++-----------
>   gdb/amd64-fbsd-tdep.h |  27 +++
>   gdb/amd64-tdep.h      |   5 -
>   gdb/configure.nat     |   4 +-
>   gdb/fbsd-nat.c        |   8 +-
>   gdb/fbsd-nat.h        |  21 +-
>   gdb/i386-bsd-nat.c    |  98 +--------
>   gdb/i386-fbsd-nat.c   | 255 +++++++++++++++++++----
>   gdb/i386-fbsd-tdep.c  | 461 ++++++++++++++++++------------------------
>   gdb/i386-fbsd-tdep.h  |   4 +
>   gdb/i386-tdep.h       |   4 -
>   gdb/regcache.c        |   7 +-
>   gdb/x86-bsd-nat.c     |   4 -
>   gdb/x86-bsd-nat.h     |   3 -
>   16 files changed, 844 insertions(+), 778 deletions(-)
>   create mode 100644 gdb/amd64-fbsd-tdep.h
> 


-- 
John Baldwin

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

* [PING] [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers
  2021-08-10 15:56 ` [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
@ 2021-09-14 15:43   ` John Baldwin
  2021-10-18 20:46     ` John Baldwin
  0 siblings, 1 reply; 19+ messages in thread
From: John Baldwin @ 2021-09-14 15:43 UTC (permalink / raw)
  To: gdb-patches

On 8/10/21 8:56 AM, John Baldwin wrote:
> On 7/14/21 7:07 AM, John Baldwin wrote:
>> The x86 architectures in GDB provide existing helpers for parsing
>> general-purpose register sets.  However, these helpers have some
>> limitations, such as assuming that registers are always full size.  On
>> FreeBSD/amd64 in particular, segment registers are stored as 16-bit
>> quantities that in some cases are packed together.  GDB for historical
>> reasons treats these 16-bit registers as 32 bits in size.  Using the
>> more generic regcache_map_entry to describe the GP register sets
>> permits supporting these registers as 16-bit values.  In addition, the
>> FreeBSD x86 signal frames have included the base address of the FS and
>> GS segments (equivalent to the fs_base and gs_base registers), but the
>> existing signal context helpers were written before those registers
>> were added to GDB.
>>
>> Longer term my goal is to use regcache_map_entry-based register sets
>> in FreeBSD gdbserver support to simplify the implementation.
>>
>> Note that patch 4 fixes an issue in regcache_collect_regset where it
>> didn't quite do what I thought it did.  I believe the change is ok,
>> but it definitely warrants review.
> 
> Ping.  Patch 4 is the only one that isn't FreeBSD-specific.

Pinging again.  I could perhaps push the first 3 patches as they are
FreeBSD specific.  Patch 4 is a prereq for the following patches however.

>> I have tested this on both FreeBSD/amd64 (32-bit and 64-bit processes)
>> and FreeBSD/i386.
>>
>> John Baldwin (8):
>>     Remove vestigal FreeBSD/i386 3.x support.
>>     Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
>>     FreeBSD x86: Remove fallback for detecting signal trampolines by
>>       address.
>>     regcache: Zero-extend small registers described by a register map.
>>     Use register maps for gp regsets on FreeBSD/x86 core dumps.
>>     FreeBSD x86: Use tramp-frame for signal frames.
>>     fbsd-nat: Return a bool from fetch_register_set and
>>       store_register_set.
>>     FreeBSD x86 nat: Use register maps for GP register sets.
>>
>>    gdb/amd64-bsd-nat.c   |  96 ---------
>>    gdb/amd64-fbsd-nat.c  | 346 ++++++++++++++++++-------------
>>    gdb/amd64-fbsd-tdep.c | 279 ++++++++++++++-----------
>>    gdb/amd64-fbsd-tdep.h |  27 +++
>>    gdb/amd64-tdep.h      |   5 -
>>    gdb/configure.nat     |   4 +-
>>    gdb/fbsd-nat.c        |   8 +-
>>    gdb/fbsd-nat.h        |  21 +-
>>    gdb/i386-bsd-nat.c    |  98 +--------
>>    gdb/i386-fbsd-nat.c   | 255 +++++++++++++++++++----
>>    gdb/i386-fbsd-tdep.c  | 461 ++++++++++++++++++------------------------
>>    gdb/i386-fbsd-tdep.h  |   4 +
>>    gdb/i386-tdep.h       |   4 -
>>    gdb/regcache.c        |   7 +-
>>    gdb/x86-bsd-nat.c     |   4 -
>>    gdb/x86-bsd-nat.h     |   3 -
>>    16 files changed, 844 insertions(+), 778 deletions(-)
>>    create mode 100644 gdb/amd64-fbsd-tdep.h
>>
> 
> 


-- 
John Baldwin

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

* Re: [PING] [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers
  2021-09-14 15:43   ` [PING] " John Baldwin
@ 2021-10-18 20:46     ` John Baldwin
  0 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2021-10-18 20:46 UTC (permalink / raw)
  To: gdb-patches

On 9/14/21 8:43 AM, John Baldwin wrote:
> On 8/10/21 8:56 AM, John Baldwin wrote:
>> On 7/14/21 7:07 AM, John Baldwin wrote:
>>> The x86 architectures in GDB provide existing helpers for parsing
>>> general-purpose register sets.  However, these helpers have some
>>> limitations, such as assuming that registers are always full size.  On
>>> FreeBSD/amd64 in particular, segment registers are stored as 16-bit
>>> quantities that in some cases are packed together.  GDB for historical
>>> reasons treats these 16-bit registers as 32 bits in size.  Using the
>>> more generic regcache_map_entry to describe the GP register sets
>>> permits supporting these registers as 16-bit values.  In addition, the
>>> FreeBSD x86 signal frames have included the base address of the FS and
>>> GS segments (equivalent to the fs_base and gs_base registers), but the
>>> existing signal context helpers were written before those registers
>>> were added to GDB.
>>>
>>> Longer term my goal is to use regcache_map_entry-based register sets
>>> in FreeBSD gdbserver support to simplify the implementation.
>>>
>>> Note that patch 4 fixes an issue in regcache_collect_regset where it
>>> didn't quite do what I thought it did.  I believe the change is ok,
>>> but it definitely warrants review.
>>
>> Ping.  Patch 4 is the only one that isn't FreeBSD-specific.
> 
> Pinging again.  I could perhaps push the first 3 patches as they are
> FreeBSD specific.  Patch 4 is a prereq for the following patches however.

Pinging again.

>>> I have tested this on both FreeBSD/amd64 (32-bit and 64-bit processes)
>>> and FreeBSD/i386.
>>>
>>> John Baldwin (8):
>>>      Remove vestigal FreeBSD/i386 3.x support.
>>>      Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
>>>      FreeBSD x86: Remove fallback for detecting signal trampolines by
>>>        address.
>>>      regcache: Zero-extend small registers described by a register map.
>>>      Use register maps for gp regsets on FreeBSD/x86 core dumps.
>>>      FreeBSD x86: Use tramp-frame for signal frames.
>>>      fbsd-nat: Return a bool from fetch_register_set and
>>>        store_register_set.
>>>      FreeBSD x86 nat: Use register maps for GP register sets.
>>>
>>>     gdb/amd64-bsd-nat.c   |  96 ---------
>>>     gdb/amd64-fbsd-nat.c  | 346 ++++++++++++++++++-------------
>>>     gdb/amd64-fbsd-tdep.c | 279 ++++++++++++++-----------
>>>     gdb/amd64-fbsd-tdep.h |  27 +++
>>>     gdb/amd64-tdep.h      |   5 -
>>>     gdb/configure.nat     |   4 +-
>>>     gdb/fbsd-nat.c        |   8 +-
>>>     gdb/fbsd-nat.h        |  21 +-
>>>     gdb/i386-bsd-nat.c    |  98 +--------
>>>     gdb/i386-fbsd-nat.c   | 255 +++++++++++++++++++----
>>>     gdb/i386-fbsd-tdep.c  | 461 ++++++++++++++++++------------------------
>>>     gdb/i386-fbsd-tdep.h  |   4 +
>>>     gdb/i386-tdep.h       |   4 -
>>>     gdb/regcache.c        |   7 +-
>>>     gdb/x86-bsd-nat.c     |   4 -
>>>     gdb/x86-bsd-nat.h     |   3 -
>>>     16 files changed, 844 insertions(+), 778 deletions(-)
>>>     create mode 100644 gdb/amd64-fbsd-tdep.h
>>>
>>
>>
> 
> 


-- 
John Baldwin

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-07-14 14:07 ` [PATCH 4/8] regcache: Zero-extend small registers described by a register map John Baldwin
@ 2021-10-19  8:36   ` Andrew Burgess
  2021-10-19 16:31     ` John Baldwin
  0 siblings, 1 reply; 19+ messages in thread
From: Andrew Burgess @ 2021-10-19  8:36 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

* John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:

> When registers are supplied via regcache_supply_register from a register
> block described by a register map, registers may be stored in slots smaller
> than GDB's native register size (e.g. x86 segment registers are 16 bits,
> but the GDB registers for those are 32-bits).  regcache_collect_regset
> is careful to zero-extend slots larger than a register size, but
> regcache_supply_regset just used regcache::raw_supply_part and did not
> initialize the upper bytes of a register value.
> 
> trad_frame_set_reg_regmap assumes these semantics (zero-extending
> short registers) as I had misread the implementation of
> regcache::transfer_regset and assumed it zero-extended short
> registers.  In my specific use case (x86 segment registers stored as
> 16-bit values), I need the semantics of zero-extending a register
> value in a smaller slot.

I don't claim to know if anyone relies on the old behaviour of
transfer_regset_register, but the change you propose seems reasonable.

However, the second paragraph of your commit message really confuses
me.

It seems to say that a mistake was made in trad_frame_set_reg_regmap,
and so transfer_regset_register should change, then you just jump to
saying you need the zero extend.  I don't really understand the
connection between all these ideas.

Also, when you say "In my specific use case ....  I need the semantics
of zero-extending ...", but you don't say what your use case is, so if
I wanted to look at which piece of code is calling this for your
specific use case, I can't...

Thanks,
Andrew

> ---
>  gdb/regcache.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 672da0556f..396923c8a5 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1169,7 +1169,12 @@ regcache::transfer_regset_register (struct regcache *out_regcache, int regnum,
>  	memset (out_buf + offs + reg_size, 0, slot_size - reg_size);
>      }
>    else if (in_buf != nullptr)
> -    out_regcache->raw_supply_part (regnum, 0, reg_size, in_buf + offs);
> +    {
> +      /* Zero-extend the register value if the slot is smaller than the register.  */
> +      if (slot_size < register_size (gdbarch, regnum))
> +	out_regcache->raw_supply_zeroed (regnum);
> +      out_regcache->raw_supply_part (regnum, 0, reg_size, in_buf + offs);
> +    }
>    else
>      {
>        /* Invalidate the register.  */
> -- 
> 2.31.1
> 

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-10-19  8:36   ` Andrew Burgess
@ 2021-10-19 16:31     ` John Baldwin
  2021-10-26 21:17       ` John Baldwin
  0 siblings, 1 reply; 19+ messages in thread
From: John Baldwin @ 2021-10-19 16:31 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 10/19/21 1:36 AM, Andrew Burgess wrote:
> * John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:
> 
>> When registers are supplied via regcache_supply_register from a register
>> block described by a register map, registers may be stored in slots smaller
>> than GDB's native register size (e.g. x86 segment registers are 16 bits,
>> but the GDB registers for those are 32-bits).  regcache_collect_regset
>> is careful to zero-extend slots larger than a register size, but
>> regcache_supply_regset just used regcache::raw_supply_part and did not
>> initialize the upper bytes of a register value.
>>
>> trad_frame_set_reg_regmap assumes these semantics (zero-extending
>> short registers) as I had misread the implementation of
>> regcache::transfer_regset and assumed it zero-extended short
>> registers.  In my specific use case (x86 segment registers stored as
>> 16-bit values), I need the semantics of zero-extending a register
>> value in a smaller slot.
> 
> I don't claim to know if anyone relies on the old behaviour of
> transfer_regset_register, but the change you propose seems reasonable.
> 
> However, the second paragraph of your commit message really confuses
> me.
> 
> It seems to say that a mistake was made in trad_frame_set_reg_regmap,
> and so transfer_regset_register should change, then you just jump to
> saying you need the zero extend.  I don't really understand the
> connection between all these ideas.

Sorry.  I added trad_frame_set_reg_regmap in commit
498f740792fe0edd2955c5cee6bb864f60a5b173 so that custom unwinders can use
a regcache map to describe the layout of a block of registers (e.g. to
describe the register block in a signal context for a signal frame
unwinder).  In the commit log I describe the semantics for when the
register size != the slot size as:

     If a register map entry's size does not match the native size of a
     register, try to match the semantics used by
     regcache::transfer_regset.  If a register slot is too large, assume
     that the register's value is stored in the first N bytes and ignore
     the remaning bytes.  If the register slot is smaller than the
     register, assume the slot holds the low N bytes of the register's
     value.  Read these low N bytes from the target and zero-extend them to
     generate a register value.

I do need the zero-extend semantics for my use case of x86 segment
registers (more below).  I was trying to say "oops, I goofed up in my
previous commit by thinking this behavior that I need was already there
and had used it to justify the semantics in a new function".  Maybe I
should just simplify it a bit to:

   trad_frame_set_reg_regmap assumes these semantics (zero-extending
   short registers).  In my specific use case (x86 segment registers
   stored as 16-bit values), I need the semantics of zero-extending a
   register value extracted from a smaller slot.

> Also, when you say "In my specific use case ....  I need the semantics
> of zero-extending ...", but you don't say what your use case is, so if
> I wanted to look at which piece of code is calling this for your
> specific use case, I can't...

I can perhaps expand more in the description.  My use case is the
x86 segment registers stored in register blocks.  Architecturally, these
are 16-bit registers, but GDB treats them as a 32-bit register.  We can't
really fix that part of GDB now (it's baked into the 'g' packet format
for example).  FreeBSD sometimes stores segment registers as zero-extended
values in 32-bit slots, but in some other cases stores them as 16-bit
values.  Today FreeBSD/amd64 support in GDB generally doesn't report
segment register values because they are stored as 16-bit values with
other data in the adjacent 16 bits.  This change in particular permits
reporting the segment register values from FreeBSD/amd64 core dumps and
via the native target (ptrace()).  The trad_frame_set_regmap change I
had made earlier permits reporting segment registers when unwinding signal
frames on FreeBSD/amd64.

-- 
John Baldwin

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-10-19 16:31     ` John Baldwin
@ 2021-10-26 21:17       ` John Baldwin
  2021-12-22 21:20         ` John Baldwin
  2022-01-28 15:43         ` Andrew Burgess
  0 siblings, 2 replies; 19+ messages in thread
From: John Baldwin @ 2021-10-26 21:17 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 10/19/21 9:31 AM, John Baldwin wrote:
> On 10/19/21 1:36 AM, Andrew Burgess wrote:
>> * John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:
>>
>>> When registers are supplied via regcache_supply_register from a register
>>> block described by a register map, registers may be stored in slots smaller
>>> than GDB's native register size (e.g. x86 segment registers are 16 bits,
>>> but the GDB registers for those are 32-bits).  regcache_collect_regset
>>> is careful to zero-extend slots larger than a register size, but
>>> regcache_supply_regset just used regcache::raw_supply_part and did not
>>> initialize the upper bytes of a register value.
>>>
>>> trad_frame_set_reg_regmap assumes these semantics (zero-extending
>>> short registers) as I had misread the implementation of
>>> regcache::transfer_regset and assumed it zero-extended short
>>> registers.  In my specific use case (x86 segment registers stored as
>>> 16-bit values), I need the semantics of zero-extending a register
>>> value in a smaller slot.
>>
>> I don't claim to know if anyone relies on the old behaviour of
>> transfer_regset_register, but the change you propose seems reasonable.
>>
>> However, the second paragraph of your commit message really confuses
>> me.
>>
>> It seems to say that a mistake was made in trad_frame_set_reg_regmap,
>> and so transfer_regset_register should change, then you just jump to
>> saying you need the zero extend.  I don't really understand the
>> connection between all these ideas.

Here's an updated log message that hopefully is clearer:

     regcache: Zero-extend small registers described by a register map.
     
     When registers are supplied via regcache_supply_register from a
     register block described by a register map, registers may be stored in
     slots smaller than GDB's native register size (e.g. x86 segment
     registers are 16 bits, but the GDB registers for those are 32-bits).
     regcache_collect_regset is careful to zero-extend slots larger than a
     register size, but regcache_supply_regset just used
     regcache::raw_supply_part and did not initialize the upper bytes of a
     register value.
     
     trad_frame_set_reg_regmap assumes these semantics (zero-extending
     short registers).  Upcoming patches also require these semantics for
     handling x86 segment register values stored in 16-bit slots on
     FreeBSD.  Note that architecturally x86 segment registers are 16 bits,
     but the x86 gdb architectures treat these registers as 32 bits.

-- 
John Baldwin

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-10-26 21:17       ` John Baldwin
@ 2021-12-22 21:20         ` John Baldwin
  2022-01-26 18:43           ` John Baldwin
  2022-01-28 15:43         ` Andrew Burgess
  1 sibling, 1 reply; 19+ messages in thread
From: John Baldwin @ 2021-12-22 21:20 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 10/26/21 2:17 PM, John Baldwin wrote:
> On 10/19/21 9:31 AM, John Baldwin wrote:
>> On 10/19/21 1:36 AM, Andrew Burgess wrote:
>>> * John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:
>>>
>>>> When registers are supplied via regcache_supply_register from a register
>>>> block described by a register map, registers may be stored in slots smaller
>>>> than GDB's native register size (e.g. x86 segment registers are 16 bits,
>>>> but the GDB registers for those are 32-bits).  regcache_collect_regset
>>>> is careful to zero-extend slots larger than a register size, but
>>>> regcache_supply_regset just used regcache::raw_supply_part and did not
>>>> initialize the upper bytes of a register value.
>>>>
>>>> trad_frame_set_reg_regmap assumes these semantics (zero-extending
>>>> short registers) as I had misread the implementation of
>>>> regcache::transfer_regset and assumed it zero-extended short
>>>> registers.  In my specific use case (x86 segment registers stored as
>>>> 16-bit values), I need the semantics of zero-extending a register
>>>> value in a smaller slot.
>>>
>>> I don't claim to know if anyone relies on the old behaviour of
>>> transfer_regset_register, but the change you propose seems reasonable.
>>>
>>> However, the second paragraph of your commit message really confuses
>>> me.
>>>
>>> It seems to say that a mistake was made in trad_frame_set_reg_regmap,
>>> and so transfer_regset_register should change, then you just jump to
>>> saying you need the zero extend.  I don't really understand the
>>> connection between all these ideas.
> 
> Here's an updated log message that hopefully is clearer:
> 
>       regcache: Zero-extend small registers described by a register map.
>       
>       When registers are supplied via regcache_supply_register from a
>       register block described by a register map, registers may be stored in
>       slots smaller than GDB's native register size (e.g. x86 segment
>       registers are 16 bits, but the GDB registers for those are 32-bits).
>       regcache_collect_regset is careful to zero-extend slots larger than a
>       register size, but regcache_supply_regset just used
>       regcache::raw_supply_part and did not initialize the upper bytes of a
>       register value.
>       
>       trad_frame_set_reg_regmap assumes these semantics (zero-extending
>       short registers).  Upcoming patches also require these semantics for
>       handling x86 segment register values stored in 16-bit slots on
>       FreeBSD.  Note that architecturally x86 segment registers are 16 bits,
>       but the x86 gdb architectures treat these registers as 32 bits.

Ping on the updated commit log message.

-- 
John Baldwin

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-12-22 21:20         ` John Baldwin
@ 2022-01-26 18:43           ` John Baldwin
  0 siblings, 0 replies; 19+ messages in thread
From: John Baldwin @ 2022-01-26 18:43 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

On 12/22/21 1:20 PM, John Baldwin wrote:
> On 10/26/21 2:17 PM, John Baldwin wrote:
>> On 10/19/21 9:31 AM, John Baldwin wrote:
>>> On 10/19/21 1:36 AM, Andrew Burgess wrote:
>>>> * John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:
>>>>
>>>>> When registers are supplied via regcache_supply_register from a register
>>>>> block described by a register map, registers may be stored in slots smaller
>>>>> than GDB's native register size (e.g. x86 segment registers are 16 bits,
>>>>> but the GDB registers for those are 32-bits).  regcache_collect_regset
>>>>> is careful to zero-extend slots larger than a register size, but
>>>>> regcache_supply_regset just used regcache::raw_supply_part and did not
>>>>> initialize the upper bytes of a register value.
>>>>>
>>>>> trad_frame_set_reg_regmap assumes these semantics (zero-extending
>>>>> short registers) as I had misread the implementation of
>>>>> regcache::transfer_regset and assumed it zero-extended short
>>>>> registers.  In my specific use case (x86 segment registers stored as
>>>>> 16-bit values), I need the semantics of zero-extending a register
>>>>> value in a smaller slot.
>>>>
>>>> I don't claim to know if anyone relies on the old behaviour of
>>>> transfer_regset_register, but the change you propose seems reasonable.
>>>>
>>>> However, the second paragraph of your commit message really confuses
>>>> me.
>>>>
>>>> It seems to say that a mistake was made in trad_frame_set_reg_regmap,
>>>> and so transfer_regset_register should change, then you just jump to
>>>> saying you need the zero extend.  I don't really understand the
>>>> connection between all these ideas.
>>
>> Here's an updated log message that hopefully is clearer:
>>
>>        regcache: Zero-extend small registers described by a register map.
>>        
>>        When registers are supplied via regcache_supply_register from a
>>        register block described by a register map, registers may be stored in
>>        slots smaller than GDB's native register size (e.g. x86 segment
>>        registers are 16 bits, but the GDB registers for those are 32-bits).
>>        regcache_collect_regset is careful to zero-extend slots larger than a
>>        register size, but regcache_supply_regset just used
>>        regcache::raw_supply_part and did not initialize the upper bytes of a
>>        register value.
>>        
>>        trad_frame_set_reg_regmap assumes these semantics (zero-extending
>>        short registers).  Upcoming patches also require these semantics for
>>        handling x86 segment register values stored in 16-bit slots on
>>        FreeBSD.  Note that architecturally x86 segment registers are 16 bits,
>>        but the x86 gdb architectures treat these registers as 32 bits.
> 
> Ping on the updated commit log message.

Ping.

-- 
John Baldwin

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

* Re: [PATCH 4/8] regcache: Zero-extend small registers described by a register map.
  2021-10-26 21:17       ` John Baldwin
  2021-12-22 21:20         ` John Baldwin
@ 2022-01-28 15:43         ` Andrew Burgess
  1 sibling, 0 replies; 19+ messages in thread
From: Andrew Burgess @ 2022-01-28 15:43 UTC (permalink / raw)
  To: John Baldwin; +Cc: Andrew Burgess, gdb-patches

* John Baldwin <jhb@FreeBSD.org> [2021-10-26 14:17:36 -0700]:

> On 10/19/21 9:31 AM, John Baldwin wrote:
> > On 10/19/21 1:36 AM, Andrew Burgess wrote:
> > > * John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:37 -0700]:
> > > 
> > > > When registers are supplied via regcache_supply_register from a register
> > > > block described by a register map, registers may be stored in slots smaller
> > > > than GDB's native register size (e.g. x86 segment registers are 16 bits,
> > > > but the GDB registers for those are 32-bits).  regcache_collect_regset
> > > > is careful to zero-extend slots larger than a register size, but
> > > > regcache_supply_regset just used regcache::raw_supply_part and did not
> > > > initialize the upper bytes of a register value.
> > > > 
> > > > trad_frame_set_reg_regmap assumes these semantics (zero-extending
> > > > short registers) as I had misread the implementation of
> > > > regcache::transfer_regset and assumed it zero-extended short
> > > > registers.  In my specific use case (x86 segment registers stored as
> > > > 16-bit values), I need the semantics of zero-extending a register
> > > > value in a smaller slot.
> > > 
> > > I don't claim to know if anyone relies on the old behaviour of
> > > transfer_regset_register, but the change you propose seems reasonable.
> > > 
> > > However, the second paragraph of your commit message really confuses
> > > me.
> > > 
> > > It seems to say that a mistake was made in trad_frame_set_reg_regmap,
> > > and so transfer_regset_register should change, then you just jump to
> > > saying you need the zero extend.  I don't really understand the
> > > connection between all these ideas.
> 
> Here's an updated log message that hopefully is clearer:
> 
>     regcache: Zero-extend small registers described by a register map.
>
>     When registers are supplied via regcache_supply_register from a
>     register block described by a register map, registers may be stored in
>     slots smaller than GDB's native register size (e.g. x86 segment
>     registers are 16 bits, but the GDB registers for those are 32-bits).
>     regcache_collect_regset is careful to zero-extend slots larger than a
>     register size, but regcache_supply_regset just used
>     regcache::raw_supply_part and did not initialize the upper bytes of a
>     register value.
>
>     trad_frame_set_reg_regmap assumes these semantics (zero-extending
>     short registers).  Upcoming patches also require these semantics for
>     handling x86 segment register values stored in 16-bit slots on
>     FreeBSD.  Note that architecturally x86 segment registers are 16 bits,
>     but the x86 gdb architectures treat these registers as 32 bits.

Sorry for the delay, I lost track of this series.

Thanks for the updated message, I think this is clearer.

I still wonder if there could ever a case where, if the type of a
register was signed, we should be sign extending, rather than zero
extending...

...but given that, such a situation must already be broken, I'm
assuming that isn't something that exists today.

As such, and with lack of comment from anyone else, I think this is
good to go.

Thanks,
Andrew


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

* Re: [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers
  2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
                   ` (8 preceding siblings ...)
  2021-08-10 15:56 ` [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
@ 2022-01-28 15:44 ` Andrew Burgess
  9 siblings, 0 replies; 19+ messages in thread
From: Andrew Burgess @ 2022-01-28 15:44 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches

John,

I've read through the whole series.  I know next to nothing about
FreeBSD, but I looked through all the changes, and they all looked
sensible.  So I think you can go ahead and merge this series.

Thanks,
Andrew


* John Baldwin <jhb@FreeBSD.org> [2021-07-14 07:07:33 -0700]:

> The x86 architectures in GDB provide existing helpers for parsing
> general-purpose register sets.  However, these helpers have some
> limitations, such as assuming that registers are always full size.  On
> FreeBSD/amd64 in particular, segment registers are stored as 16-bit
> quantities that in some cases are packed together.  GDB for historical
> reasons treats these 16-bit registers as 32 bits in size.  Using the
> more generic regcache_map_entry to describe the GP register sets
> permits supporting these registers as 16-bit values.  In addition, the
> FreeBSD x86 signal frames have included the base address of the FS and
> GS segments (equivalent to the fs_base and gs_base registers), but the
> existing signal context helpers were written before those registers
> were added to GDB.
> 
> Longer term my goal is to use regcache_map_entry-based register sets
> in FreeBSD gdbserver support to simplify the implementation.
> 
> Note that patch 4 fixes an issue in regcache_collect_regset where it
> didn't quite do what I thought it did.  I believe the change is ok,
> but it definitely warrants review.
> 
> I have tested this on both FreeBSD/amd64 (32-bit and 64-bit processes)
> and FreeBSD/i386.
> 
> John Baldwin (8):
>   Remove vestigal FreeBSD/i386 3.x support.
>   Remove support for pre-5.0 FreeBSD/i386 signal trampolines.
>   FreeBSD x86: Remove fallback for detecting signal trampolines by
>     address.
>   regcache: Zero-extend small registers described by a register map.
>   Use register maps for gp regsets on FreeBSD/x86 core dumps.
>   FreeBSD x86: Use tramp-frame for signal frames.
>   fbsd-nat: Return a bool from fetch_register_set and
>     store_register_set.
>   FreeBSD x86 nat: Use register maps for GP register sets.
> 
>  gdb/amd64-bsd-nat.c   |  96 ---------
>  gdb/amd64-fbsd-nat.c  | 346 ++++++++++++++++++-------------
>  gdb/amd64-fbsd-tdep.c | 279 ++++++++++++++-----------
>  gdb/amd64-fbsd-tdep.h |  27 +++
>  gdb/amd64-tdep.h      |   5 -
>  gdb/configure.nat     |   4 +-
>  gdb/fbsd-nat.c        |   8 +-
>  gdb/fbsd-nat.h        |  21 +-
>  gdb/i386-bsd-nat.c    |  98 +--------
>  gdb/i386-fbsd-nat.c   | 255 +++++++++++++++++++----
>  gdb/i386-fbsd-tdep.c  | 461 ++++++++++++++++++------------------------
>  gdb/i386-fbsd-tdep.h  |   4 +
>  gdb/i386-tdep.h       |   4 -
>  gdb/regcache.c        |   7 +-
>  gdb/x86-bsd-nat.c     |   4 -
>  gdb/x86-bsd-nat.h     |   3 -
>  16 files changed, 844 insertions(+), 778 deletions(-)
>  create mode 100644 gdb/amd64-fbsd-tdep.h
> 
> -- 
> 2.31.1
> 


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

end of thread, other threads:[~2022-01-28 15:44 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14 14:07 [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
2021-07-14 14:07 ` [PATCH 1/8] Remove vestigal FreeBSD/i386 3.x support John Baldwin
2021-07-14 14:07 ` [PATCH 2/8] Remove support for pre-5.0 FreeBSD/i386 signal trampolines John Baldwin
2021-07-14 14:07 ` [PATCH 3/8] FreeBSD x86: Remove fallback for detecting signal trampolines by address John Baldwin
2021-07-14 14:07 ` [PATCH 4/8] regcache: Zero-extend small registers described by a register map John Baldwin
2021-10-19  8:36   ` Andrew Burgess
2021-10-19 16:31     ` John Baldwin
2021-10-26 21:17       ` John Baldwin
2021-12-22 21:20         ` John Baldwin
2022-01-26 18:43           ` John Baldwin
2022-01-28 15:43         ` Andrew Burgess
2021-07-14 14:07 ` [PATCH 5/8] Use register maps for gp regsets on FreeBSD/x86 core dumps John Baldwin
2021-07-14 14:07 ` [PATCH 6/8] FreeBSD x86: Use tramp-frame for signal frames John Baldwin
2021-07-14 14:07 ` [PATCH 7/8] fbsd-nat: Return a bool from fetch_register_set and store_register_set John Baldwin
2021-07-14 14:07 ` [PATCH 8/8] FreeBSD x86 nat: Use register maps for GP register sets John Baldwin
2021-08-10 15:56 ` [PATCH 0/8] Switch FreeBSD x86 to using register maps for GP registers John Baldwin
2021-09-14 15:43   ` [PING] " John Baldwin
2021-10-18 20:46     ` John Baldwin
2022-01-28 15:44 ` Andrew Burgess

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