[PATCH v2 0/4] Initial support for FreeBSD/riscv

[PATCH v2 0/4] Initial support for FreeBSD/riscv

[John Baldwin]

Relative to the first version, this series drops patch to return a
default value for MISA and replaces it with a partly-reviewed patch to
rework riscv breakpoints to not require a valid MISA.

Patch 1 still needs review.  I've included my comments from the first
cover letter about patch 1 below:

Patch 1 tries to make it easier to write handlers for signal frame by
allowing the register map structures used with register caches to be
used with trad-frame to supply a block of registers at a starting
address.  It gets somewhat squishy when thinking about how to handle
registers whose size doesn't match the "slot" size in a register map.
I've attempted to make the trad-frame handling match the semantics
that regcache uses.  However, these semantics aren't documented
anywhere and we should perhaps document them.  Also, in this patch I
used 'void *' for the map only because it matches what the regcache
functions do.  I'm happy to make the map argument type-safe instead if
others prefer that.  Also, the comments for regcache_map_entry should
perhaps be made more generic to say it isn't specific to regcache but
is used to describe the layout of a register block.  Arguably the type
should even be renamed to something less regcache-specific
(register_map_entry?).  If we do adopt this patch I will probably make
use of it in some other FreeBSD architectures (aarch64 and arm at
least, possibly x86).



John Baldwin (4):
  Add helper functions to trad_frame to support register cache maps.
  Use the existing instruction to determine the RISC-V breakpoint kind.
  Add FreeBSD/riscv architecture.
  Add native target for FreeBSD/riscv.

 gdb/ChangeLog          |  38 ++++++++
 gdb/Makefile.in        |   4 +
 gdb/NEWS               |   2 +
 gdb/configure.host     |   1 +
 gdb/configure.nat      |   4 +
 gdb/configure.tgt      |   5 +
 gdb/disasm-selftests.c |   7 +-
 gdb/doc/ChangeLog      |   5 +
 gdb/doc/gdb.texinfo    |   6 ++
 gdb/riscv-fbsd-nat.c   | 135 +++++++++++++++++++++++++++
 gdb/riscv-fbsd-tdep.c  | 206 +++++++++++++++++++++++++++++++++++++++++
 gdb/riscv-fbsd-tdep.h  |  33 +++++++
 gdb/riscv-tdep.c       |  50 ++++++----
 gdb/trad-frame.c       |  69 ++++++++++++++
 gdb/trad-frame.h       |   8 ++
 15 files changed, 554 insertions(+), 19 deletions(-)
 create mode 100644 gdb/riscv-fbsd-nat.c
 create mode 100644 gdb/riscv-fbsd-tdep.c
 create mode 100644 gdb/riscv-fbsd-tdep.h

-- 
2.18.0

[PATCH v2 3/4] Add FreeBSD/riscv architecture.

[John Baldwin]

Support for collecting and supplying general purpose and floating
point register sets is provided along with signal frame unwinding.

FreeBSD only supports RV64 currently, so while some provision is made
for RV32 in the general-purpose register set, the changes have only
been tested on RV64.

gdb/ChangeLog:

	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
	(HFILES_NO_SRCDIR): Add riscv-fbsd-tdep.h.
	(ALLDEPFILES): Add riscv-fbsd-tdep.c.
	* NEWS: Mention new FreeBSD/riscv target.
	* configure.tgt: Add riscv*-*-freebsd*.
	* riscv-fbsd-tdep.c: New file.
	* riscv-fbsd-tdep.h: New file.
---
 gdb/ChangeLog         |  10 ++
 gdb/Makefile.in       |   3 +
 gdb/NEWS              |   1 +
 gdb/configure.tgt     |   5 +
 gdb/riscv-fbsd-tdep.c | 206 ++++++++++++++++++++++++++++++++++++++++++
 gdb/riscv-fbsd-tdep.h |  33 +++++++
 6 files changed, 258 insertions(+)
 create mode 100644 gdb/riscv-fbsd-tdep.c
 create mode 100644 gdb/riscv-fbsd-tdep.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f56b0487cc..1367f37db7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2018-09-24  John Baldwin  <jhb@FreeBSD.org>
+
+	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
+	(HFILES_NO_SRCDIR): Add riscv-fbsd-tdep.h.
+	(ALLDEPFILES): Add riscv-fbsd-tdep.c.
+	* NEWS: Mention new FreeBSD/riscv target.
+	* configure.tgt: Add riscv*-*-freebsd*.
+	* riscv-fbsd-tdep.c: New file.
+	* riscv-fbsd-tdep.h: New file.
+
 2018-09-24  John Baldwin  <jhb@FreeBSD.org>
 
 	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3b158fa1db..2e03bb956c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -745,6 +745,7 @@ ALL_TARGET_OBS = \
 	ppc-sysv-tdep.o \
 	ppc64-tdep.o \
 	ravenscar-thread.o \
+	riscv-fbsd-tdep.o \
 	riscv-linux-tdep.o \
 	riscv-tdep.o \
 	rl78-tdep.o \
@@ -1339,6 +1340,7 @@ HFILES_NO_SRCDIR = \
 	remote.h \
 	remote-fileio.h \
 	remote-notif.h \
+	riscv-fbsd-tdep.h \
 	riscv-tdep.h \
 	rs6000-aix-tdep.h \
 	rs6000-tdep.h \
@@ -2306,6 +2308,7 @@ ALLDEPFILES = \
 	procfs.c \
 	ravenscar-thread.c \
 	remote-sim.c \
+	riscv-fbsd-tdep.c \
 	riscv-linux-nat.c \
 	riscv-linux-tdep.c \
 	riscv-tdep.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index a1936ca1cc..a191ae2714 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -86,6 +86,7 @@ GNU/Linux/RISC-V		riscv*-*-linux*
 GNU/Linux/RISC-V		riscv*-*-linux*
 CSKY ELF			csky*-*-elf
 CSKY GNU/LINUX			csky*-*-linux
+FreeBSD/riscv			riscv*-*-freebsd*
 
 * Python API
 
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 6d1a4df84a..3b94942732 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -528,6 +528,11 @@ s390*-*-linux*)
 	build_gdbserver=yes
 	;;
 
+riscv*-*-freebsd*)
+	# Target: FreeBSD/riscv
+	gdb_target_obs="riscv-fbsd-tdep.o riscv-tdep.o"
+	;;
+
 riscv*-*-linux*)
 	# Target: Linux/RISC-V
 	gdb_target_obs="riscv-linux-tdep.o riscv-tdep.o glibc-tdep.o \
diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
new file mode 100644
index 0000000000..c09ec650ca
--- /dev/null
+++ b/gdb/riscv-fbsd-tdep.c
@@ -0,0 +1,206 @@
+/* Target-dependent code for FreeBSD on RISC-V processors.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "fbsd-tdep.h"
+#include "osabi.h"
+#include "riscv-tdep.h"
+#include "riscv-fbsd-tdep.h"
+#include "solib-svr4.h"
+#include "target.h"
+#include "trad-frame.h"
+#include "tramp-frame.h"
+
+/* Register maps.  */
+
+static const struct regcache_map_entry riscv_fbsd_gregmap[] =
+  {
+    { 1, RISCV_RA_REGNUM, 0 },
+    { 1, RISCV_SP_REGNUM, 0 },
+    { 1, RISCV_GP_REGNUM, 0 },
+    { 1, RISCV_TP_REGNUM, 0 },
+    { 3, 5, 0 },		/* t0 - t2 */
+    { 4, 28, 0 },		/* t3 - t6 */
+    { 2, RISCV_FP_REGNUM, 0 },	/* s0 - s1 */
+    { 10, 18, 0 },		/* s2 - s11 */
+    { 8, RISCV_A0_REGNUM, 0 },	/* a0 - a7 */
+    { 1, RISCV_PC_REGNUM, 0 },
+    { 1, RISCV_CSR_SSTATUS_REGNUM, 0 },
+    { 0 }
+  };
+
+static const struct regcache_map_entry riscv_fbsd_fpregmap[] =
+  {
+    { 32, RISCV_FIRST_FP_REGNUM, 16 },
+    { 1, RISCV_CSR_FCSR_REGNUM, 8 },
+    { 0 }
+  };
+
+/* Supply the general-purpose registers stored in GREGS to REGCACHE.
+   This function only exists to supply the always-zero x0 in addition
+   to the registers in GREGS.  */
+
+static void
+riscv_fbsd_supply_gregset (const struct regset *regset,
+			   struct regcache *regcache, int regnum,
+			   const void *gregs, size_t len)
+{
+  regcache->supply_regset (&riscv_fbsd_gregset, regnum, gregs, len);
+  if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
+    regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
+}
+
+/* Register set definitions.  */
+
+const struct regset riscv_fbsd_gregset =
+  {
+    riscv_fbsd_gregmap,
+    riscv_fbsd_supply_gregset, regcache_collect_regset
+  };
+
+const struct regset riscv_fbsd_fpregset =
+  {
+    riscv_fbsd_fpregmap,
+    regcache_supply_regset, regcache_collect_regset
+  };
+
+/* Implement the "regset_from_core_section" gdbarch method.  */
+
+static void
+riscv_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
+					 iterate_over_regset_sections_cb *cb,
+					 void *cb_data,
+					 const struct regcache *regcache)
+{
+  cb (".reg", RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch),
+      RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch),
+      &riscv_fbsd_gregset, NULL, cb_data);
+  cb (".reg2", RISCV_FBSD_SIZEOF_FPREGSET, RISCV_FBSD_SIZEOF_FPREGSET,
+      &riscv_fbsd_fpregset, NULL, cb_data);
+}
+
+/* In a signal frame, sp points to a 'struct sigframe' which is
+   defined as:
+
+   struct sigframe {
+	   siginfo_t	sf_si;
+	   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 followed
+   by the floating point register set.  The floating point register
+   set is only valid if the _MC_FP_VALID flag is set in mc_flags.  */
+
+#define RISCV_SIGFRAME_UCONTEXT_OFFSET 		80
+#define RISCV_UCONTEXT_MCONTEXT_OFFSET		16
+#define RISCV_MCONTEXT_FLAG_FP_VALID		0x1
+
+/* Implement the "init" method of struct tramp_frame.  */
+
+static void
+riscv_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, RISCV_SP_REGNUM);
+  CORE_ADDR mcontext_addr =
+    sp
+    + RISCV_SIGFRAME_UCONTEXT_OFFSET
+    + RISCV_UCONTEXT_MCONTEXT_OFFSET;
+  gdb_byte buf[4];
+  int i;
+
+  trad_frame_set_reg_regmap (this_cache, riscv_fbsd_gregmap, mcontext_addr,
+			     RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch));
+
+  CORE_ADDR fpregs_addr
+    = mcontext_addr + RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch);
+  CORE_ADDR fp_flags_addr
+    = fpregs_addr + RISCV_FBSD_SIZEOF_FPREGSET;
+  if (target_read_memory (fp_flags_addr, buf, 4) == 0
+      && (extract_unsigned_integer (buf, 4, byte_order)
+	  & RISCV_MCONTEXT_FLAG_FP_VALID))
+    trad_frame_set_reg_regmap (this_cache, riscv_fbsd_fpregmap, fpregs_addr,
+			       RISCV_FBSD_SIZEOF_FPREGSET);
+
+  trad_frame_set_id (this_cache, frame_id_build (sp, func));
+}
+
+/* RISC-V supports 16-bit instructions ("C") as well as 32-bit
+   instructions.  The signal trampoline on FreeBSD uses a mix of
+   these, but tramp_frame assumes a fixed instruction size.  To cope,
+   claim that all instructions are 16 bits and use two "slots" for
+   32-bit instructions.  */
+
+static const struct tramp_frame riscv_fbsd_sigframe =
+{
+  SIGTRAMP_FRAME,
+  2,
+  {
+    {0x850a, ULONGEST_MAX},		/* mov  a0, sp  */
+    {0x0513, ULONGEST_MAX},		/* addi a0, a0, #SF_UC  */
+    {0x0505, ULONGEST_MAX},
+    {0x0293, ULONGEST_MAX},		/* li   t0, #SYS_sigreturn  */
+    {0x1a10, ULONGEST_MAX},
+    {0x0073, ULONGEST_MAX},		/* ecall  */
+    {0x0000, ULONGEST_MAX},
+    {TRAMP_SENTINEL_INSN, ULONGEST_MAX}
+  },
+  riscv_fbsd_sigframe_init
+};
+
+/* Implement the 'init_osabi' method of struct gdb_osabi_handler.  */
+
+static void
+riscv_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+  /* Generic FreeBSD support.  */
+  fbsd_init_abi (info, gdbarch);
+
+  set_gdbarch_software_single_step (gdbarch, riscv_software_single_step);
+
+  set_solib_svr4_fetch_link_map_offsets (gdbarch,
+					 (riscv_isa_xlen (gdbarch) == 4
+					  ? svr4_ilp32_fetch_link_map_offsets
+					  : svr4_lp64_fetch_link_map_offsets));
+
+  tramp_frame_prepend_unwinder (gdbarch, &riscv_fbsd_sigframe);
+
+  set_gdbarch_iterate_over_regset_sections
+    (gdbarch, riscv_fbsd_iterate_over_regset_sections);
+}
+
+void
+_initialize_riscv_fbsd_tdep (void)
+{
+  gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_FREEBSD,
+			  riscv_fbsd_init_abi);
+}
diff --git a/gdb/riscv-fbsd-tdep.h b/gdb/riscv-fbsd-tdep.h
new file mode 100644
index 0000000000..8b6abd565b
--- /dev/null
+++ b/gdb/riscv-fbsd-tdep.h
@@ -0,0 +1,33 @@
+/* FreeBSD/riscv target support, prototypes.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "regset.h"
+
+/* The general-purpose regset consists of 31 X registers, EPC, and
+   SSTATUS.  */
+#define RISCV_FBSD_NUM_GREGS		33
+
+/* The fp regset always consists of 32 128-bit registers, plus a
+   64-bit CSR_FCSR.  If 'Q' is not supported, only the low 64-bits of
+   each floating point register are valid.  If 'D' is not supported,
+   only the low 32-bits of each floating point register are valid.  */
+#define RISCV_FBSD_SIZEOF_FPREGSET (32 * 16 + 8)
+
+extern const struct regset riscv_fbsd_gregset;
+extern const struct regset riscv_fbsd_fpregset;
-- 
2.18.0

[PATCH v2 2/4] Use the existing instruction to determine the RISC-V breakpoint kind.

[John Baldwin]

RISC-V supports instructions of varying lengths.  Standard existing
instructions in the base ISA are 4 bytes in length, but the 'C'
extension adds support for compressed, 2 byte instructions.  RISC-V
supports two different breakpoint instructions: EBREAK is a 4 byte
instruction in the base ISA, and C.EBREAK is a 2 byte instruction only
available on processors implementing the 'C' extension.  Using EBREAK
to set breakpoints on compressed instructions causes problems as the
second half of EBREAK will overwrite the first 2 bytes of the
following instruction breaking other threads in the process if their
PC is the following instruction.  Thus, breakpoints on compressed
instructions need to use C.EBREAK instead of EBREAK.

Previously, the riscv architecture checked the MISA register to
determine if the 'C' extension was available.  If so, it used C.EBREAK
for all breakpoints.  However, the MISA register is not necessarily
available to supervisor mode operating systems.  While native targets
could provide a fake MISA register value, this patch instead examines
the existing instruction at a breakpoint target to determine which
breakpoint instruction to use.  If the existing instruction is a
compressed instruction, C.EBREAK is used, otherwise EBREAK is used.

gdb/ChangeLog:

	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
	case with explicit breakpoint kind.
	* riscv-tdep.c (show_use_compressed_breakpoints): Remove
	'additional_info' and related logic.
	(riscv_debug_breakpoints): New variable.
	(riscv_breakpoint_kind_from_pc): Use the length of the existing
	instruction to determine the breakpoint kind.
	(_initialize_riscv_tdep): Add 'set/show debug riscv breakpoints'
	flag.  Update description of 'set/show riscv
	use-compressed-breakpoints' flag.
---
 gdb/ChangeLog          | 13 +++++++++++
 gdb/disasm-selftests.c |  7 +++---
 gdb/riscv-tdep.c       | 50 ++++++++++++++++++++++++++++--------------
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2624975146..f56b0487cc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2018-09-24  John Baldwin  <jhb@FreeBSD.org>
+
+	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
+	case with explicit breakpoint kind.
+	* riscv-tdep.c (show_use_compressed_breakpoints): Remove
+	'additional_info' and related logic.
+	(riscv_debug_breakpoints): New variable.
+	(riscv_breakpoint_kind_from_pc): Use the length of the existing
+	instruction to determine the breakpoint kind.
+	(_initialize_riscv_tdep): Add 'set/show debug riscv breakpoints'
+	flag.  Update description of 'set/show riscv
+	use-compressed-breakpoints' flag.
+
 2018-09-24  John Baldwin  <jhb@FreeBSD.org>
 
 	* trad-frame.c (trad_frame_set_regmap, trad_frame_set_reg_regmap):
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index f7d0ecca0e..8cc267631e 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -77,9 +77,10 @@ print_one_insn_test (struct gdbarch *gdbarch)
       /* fall through */
     case bfd_arch_nios2:
     case bfd_arch_score:
-      /* nios2 and score need to know the current instruction to select
-	 breakpoint instruction.  Give the breakpoint instruction kind
-	 explicitly.  */
+    case bfd_arch_riscv:
+      /* nios2, riscv, and score need to know the current instruction
+	 to select breakpoint instruction.  Give the breakpoint
+	 instruction kind explicitly.  */
       int bplen;
       insn = gdbarch_sw_breakpoint_from_kind (gdbarch, 4, &bplen);
       len = bplen;
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 254914c9c7..7faa7d01f0 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -210,20 +210,9 @@ show_use_compressed_breakpoints (struct ui_file *file, int from_tty,
 				 struct cmd_list_element *c,
 				 const char *value)
 {
-  const char *additional_info;
-  struct gdbarch *gdbarch = target_gdbarch ();
-
-  if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO)
-    if (riscv_has_feature (gdbarch, 'C'))
-      additional_info = _(" (currently on)");
-    else
-      additional_info = _(" (currently off)");
-  else
-    additional_info = "";
-
   fprintf_filtered (file,
 		    _("Debugger's use of compressed breakpoints is set "
-		      "to %s%s.\n"), value, additional_info);
+		      "to %s.\n"), value);
 }
 
 /* The set and show lists for 'set riscv' and 'show riscv' prefixes.  */
@@ -284,6 +273,11 @@ show_riscv_debug_variable (struct ui_file *file, int from_tty,
 		    c->name, value);
 }
 
+/* When this is set to non-zero debugging information about breakpoint
+   kinds will be printed.  */
+
+static unsigned int riscv_debug_breakpoints = 0;
+
 /* When this is set to non-zero debugging information about inferior calls
    will be printed.  */
 
@@ -417,7 +411,21 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
 {
   if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO)
     {
-      if (riscv_has_feature (gdbarch, 'C'))
+      gdb_byte buf[1];
+      int status;
+
+      /* Read the opcode byte to determine the instruction length.  */
+      status = target_read_code (*pcptr, buf, 1);
+      if (status)
+	memory_error (TARGET_XFER_E_IO, *pcptr);
+
+      if (riscv_debug_breakpoints)
+	fprintf_unfiltered
+	  (gdb_stdlog,
+	   "Using %s for breakpoint at %s (instruction length %d)\n",
+	   riscv_insn_length (buf[0]) == 2 ? "C.EBREAK" : "EBREAK",
+	   paddress (gdbarch, *pcptr), riscv_insn_length (buf[0]));
+      if (riscv_insn_length (buf[0]) == 2)
 	return 2;
       else
 	return 4;
@@ -2936,6 +2944,16 @@ _initialize_riscv_tdep (void)
 		  &showdebugriscvcmdlist, "show debug riscv ", 0,
 		  &showdebuglist);
 
+  add_setshow_zuinteger_cmd ("breakpoints", class_maintenance,
+			     &riscv_debug_breakpoints,  _("\
+Set riscv breakpoint debugging."), _("\
+Show riscv breakpoint debugging."), _("\
+When non-zero, print debugging information for the riscv specific parts\n\
+of the breakpoint mechanism."),
+			     NULL,
+			     show_riscv_debug_variable,
+			     &setdebugriscvcmdlist, &showdebugriscvcmdlist);
+
   add_setshow_zuinteger_cmd ("infcall", class_maintenance,
 			     &riscv_debug_infcall,  _("\
 Set riscv inferior call debugging."), _("\
@@ -2973,9 +2991,9 @@ of the stack unwinding mechanism."),
 Set debugger's use of compressed breakpoints."), _("	\
 Show debugger's use of compressed breakpoints."), _("\
 Debugging compressed code requires compressed breakpoints to be used. If\n \
-left to 'auto' then gdb will use them if $misa indicates the C extension\n \
-is supported. If that doesn't give the correct behavior, then this option\n\
-can be used."),
+left to 'auto' then gdb will use them if the existing instruction is a\n \
+compressed instruction. If that doesn't give the correct behavior, then\n \
+this option can be used."),
 				NULL,
 				show_use_compressed_breakpoints,
 				&setriscvcmdlist,
-- 
2.18.0

[PATCH v2 1/4] Add helper functions to trad_frame to support register cache maps.

[John Baldwin]

Currently, signal frame handlers require explicitly coded calls to
trad_frame_set_reg_addr() to describe the location of saved registers
within a signal frame.  This change permits the regcache_map_entry
arrays used with regcache::supply_regset and regcache::collect_regset
to be used to describe a block of saved registers given an initial
address for the register block.

Some systems use the same layout for registers in core dump notes,
native register sets with ptrace(), and the register contexts saved in
signal frames.  On these systems, a single register map can now be
used to describe the layout of registers in all three places.

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.

gdb/ChangeLog:

	* trad-frame.c (trad_frame_set_regmap, trad_frame_set_reg_regmap):
	New.
	* trad-frame.h (trad_frame_set_regmap, trad_frame_set_reg_regmap):
	New.
---
 gdb/ChangeLog    |  7 +++++
 gdb/trad-frame.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/trad-frame.h |  8 ++++++
 3 files changed, 84 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 13fac9256f..2624975146 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-24  John Baldwin  <jhb@FreeBSD.org>
+
+	* trad-frame.c (trad_frame_set_regmap, trad_frame_set_reg_regmap):
+	New.
+	* trad-frame.h (trad_frame_set_regmap, trad_frame_set_reg_regmap):
+	New.
+
 2018-09-24  Tom Tromey  <tom@tromey.com>
 
 	* common/pathstuff.c (get_standard_cache_dir): Make
diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c
index fd9f24084d..e09a333499 100644
--- a/gdb/trad-frame.c
+++ b/gdb/trad-frame.c
@@ -22,6 +22,7 @@
 #include "trad-frame.h"
 #include "regcache.h"
 #include "frame-unwind.h"
+#include "target.h"
 #include "value.h"
 
 struct trad_frame_cache
@@ -125,6 +126,64 @@ trad_frame_set_addr (struct trad_frame_saved_reg this_saved_regs[],
   this_saved_regs[regnum].addr = addr;
 }
 
+void
+trad_frame_set_regmap (struct trad_frame_saved_reg this_saved_regs[],
+		       struct gdbarch *gdbarch, const void *regmap,
+		       CORE_ADDR addr, size_t size)
+{
+  const struct regcache_map_entry *map;
+  int offs = 0, count;
+
+  for (map = (const struct regcache_map_entry *) regmap;
+       (count = map->count) != 0;
+       map++)
+    {
+      int regno = map->regno;
+      int slot_size = map->size;
+
+      if (slot_size == 0 && regno != REGCACHE_MAP_SKIP)
+	slot_size = register_size (gdbarch, regno);
+
+      if (offs + slot_size > size)
+	break;
+
+      if (regno == REGCACHE_MAP_SKIP)
+	offs += count * slot_size;
+      else
+	for (; count--; regno++, offs += slot_size)
+	  {
+	    /* Mimic the semantics of regcache::transfer_regset if a
+	       register slot's size does not match the size of a
+	       register.
+
+	       If a register slot is larger than a register, assume
+	       the register's value is stored in the first N bytes of
+	       the slot and ignore the remaining bytes.
+
+	       If the register slot is smaller than the register,
+	       assume that the slot contains the low N bytes of the
+	       register's value.  Since trad_frame assumes that
+	       registers stored by address are sized according to the
+	       register, read the low N bytes and zero-extend them to
+	       generate a register value.  */
+	    if (slot_size >= register_size (gdbarch, regno))
+	      trad_frame_set_addr (this_saved_regs, regno, addr + offs);
+	    else
+	      {
+		enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+		gdb_byte buf[slot_size];
+
+		if (target_read_memory (addr + offs, buf, sizeof buf) == 0)
+		  {
+		    LONGEST val
+		      = extract_unsigned_integer (buf, sizeof buf, byte_order);
+		    trad_frame_set_value (this_saved_regs, regno, val);
+		  }
+	      }
+	  }
+    }
+}
+
 void
 trad_frame_set_reg_value (struct trad_frame_cache *this_trad_cache,
 			  int regnum, LONGEST val)
@@ -148,6 +207,16 @@ trad_frame_set_reg_addr (struct trad_frame_cache *this_trad_cache,
   trad_frame_set_addr (this_trad_cache->prev_regs, regnum, addr);
 }
 
+void
+trad_frame_set_reg_regmap (struct trad_frame_cache *this_trad_cache,
+			   const void *regmap, CORE_ADDR addr, size_t size)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_trad_cache->this_frame);
+
+  trad_frame_set_regmap (this_trad_cache->prev_regs, gdbarch, regmap, addr,
+			 size);
+}
+
 void
 trad_frame_set_unknown (struct trad_frame_saved_reg this_saved_regs[],
 			int regnum)
diff --git a/gdb/trad-frame.h b/gdb/trad-frame.h
index a11a2c29c9..1fc1859996 100644
--- a/gdb/trad-frame.h
+++ b/gdb/trad-frame.h
@@ -45,6 +45,9 @@ void trad_frame_set_reg_realreg (struct trad_frame_cache *this_trad_cache,
 				 int regnum, int realreg);
 void trad_frame_set_reg_addr (struct trad_frame_cache *this_trad_cache,
 			      int regnum, CORE_ADDR addr);
+void trad_frame_set_reg_regmap (struct trad_frame_cache *this_trad_cache,
+				const void *regmap, CORE_ADDR addr,
+				size_t size);
 void trad_frame_set_reg_value (struct trad_frame_cache *this_cache,
 			       int regnum, LONGEST val);
 
@@ -96,6 +99,11 @@ void trad_frame_set_realreg (struct trad_frame_saved_reg this_saved_regs[],
 void trad_frame_set_addr (struct trad_frame_saved_reg this_trad_cache[],
 			  int regnum, CORE_ADDR addr);
 
+/* Encode registers in REGMAP are at address ADDR in the trad-frame.  */
+void trad_frame_set_regmap (struct trad_frame_saved_reg this_saved_regs[],
+			    struct gdbarch *gdbarch, const void *regmap,
+			    CORE_ADDR addr, size_t size);
+
 /* Mark REGNUM as unknown.  */
 void trad_frame_set_unknown (struct trad_frame_saved_reg this_saved_regs[],
 			     int regnum);
-- 
2.18.0

[PATCH v2 4/4] Add native target for FreeBSD/riscv.

[John Baldwin]

gdb/ChangeLog:

	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
	* NEWS: Mention new FreeBSD/riscv native configuration.
	* configure.host: Add riscv*-*-freebsd*.
	* configure.nat: Likewise.
	* riscv-fbsd-nat.c: New file.

gdb/doc/ChangeLog:

	* gdb.texinfo (Contributors): Add SRI International and University
	of Cambridge for FreeBSD/riscv.
---
 gdb/ChangeLog        |   8 +++
 gdb/Makefile.in      |   1 +
 gdb/NEWS             |   1 +
 gdb/configure.host   |   1 +
 gdb/configure.nat    |   4 ++
 gdb/doc/ChangeLog    |   5 ++
 gdb/doc/gdb.texinfo  |   6 ++
 gdb/riscv-fbsd-nat.c | 135 +++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 161 insertions(+)
 create mode 100644 gdb/riscv-fbsd-nat.c

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1367f37db7..19c63706e9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2018-09-24  John Baldwin  <jhb@FreeBSD.org>
+
+	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
+	* NEWS: Mention new FreeBSD/riscv native configuration.
+	* configure.host: Add riscv*-*-freebsd*.
+	* configure.nat: Likewise.
+	* riscv-fbsd-nat.c: New file.
+
 2018-09-24  John Baldwin  <jhb@FreeBSD.org>
 
 	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 2e03bb956c..13a8e16eec 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -2308,6 +2308,7 @@ ALLDEPFILES = \
 	procfs.c \
 	ravenscar-thread.c \
 	remote-sim.c \
+	riscv-fbsd-nat.c \
 	riscv-fbsd-tdep.c \
 	riscv-linux-nat.c \
 	riscv-linux-tdep.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index a191ae2714..4ae3adedc0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -80,6 +80,7 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
 * New native configurations
 
 GNU/Linux/RISC-V		riscv*-*-linux*
+FreeBSD/riscv			riscv*-*-freebsd*
 
 * New targets
 
diff --git a/gdb/configure.host b/gdb/configure.host
index 23a2f16399..c87f997abc 100644
--- a/gdb/configure.host
+++ b/gdb/configure.host
@@ -149,6 +149,7 @@ powerpc64*-*-linux*)	gdb_host=ppc64-linux
 			;;
 powerpc*-*-linux*)	gdb_host=linux ;;
 
+riscv*-*-freebsd*)	gdb_host=fbsd ;;
 riscv*-*-linux*)	gdb_host=linux ;;
 
 s390*-*-linux*)		gdb_host=linux ;;
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 10bf65fec3..200b716924 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -177,6 +177,10 @@ case ${gdb_host} in
 		# systems running FreeBSD.
 		NATDEPFILES="${NATDEPFILES} ppc-fbsd-nat.o bsd-kvm.o"
 		;;
+	    riscv*)
+		# Host: FreeBSD/riscv
+		NATDEPFILES="${NATDEPFILES} riscv-fbsd-nat.o"
+		;;
 	    sparc)
 		# Host: FreeBSD/sparc64
 		NATDEPFILES="${NATDEPFILES} sparc-nat.o sparc64-nat.o \
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 4285a4d8bb..0197650c6a 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-24  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.texinfo (Contributors): Add SRI International and University
+	of Cambridge for FreeBSD/riscv.
+
 2018-09-23  Tom Tromey  <tom@tromey.com>
 
 	PR python/18852:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b5b6089153..b8af74e949 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -546,6 +546,12 @@ was developed by SRI International and the University of Cambridge
 Computer Laboratory under DARPA/AFRL contract FA8750-10-C-0237
 ("CTSRD"), as part of the DARPA CRASH research programme.
 
+Initial support for the FreeBSD/riscv target and native configuration
+was developed by SRI International and the University of Cambridge
+Computer Laboratory (Department of Computer Science and Technology)
+under DARPA contract HR0011-18-C-0016 ("ECATS"), as part of the DARPA
+SSITH research programme.
+
 The original port to the OpenRISC 1000 is believed to be due to
 Alessandro Forin and Per Bothner.  More recent ports have been the work
 of Jeremy Bennett, Franck Jullien, Stefan Wallentowitz and
diff --git a/gdb/riscv-fbsd-nat.c b/gdb/riscv-fbsd-nat.c
new file mode 100644
index 0000000000..ad4ea1e0ad
--- /dev/null
+++ b/gdb/riscv-fbsd-nat.c
@@ -0,0 +1,135 @@
+/* Native-dependent code for FreeBSD/riscv.
+
+   Copyright (C) 2018 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "target.h"
+
+#include <sys/types.h>
+#include <sys/ptrace.h>
+#include <machine/reg.h>
+
+#include "fbsd-nat.h"
+#include "riscv-tdep.h"
+#include "riscv-fbsd-tdep.h"
+#include "inf-ptrace.h"
+
+struct riscv_fbsd_nat_target final : public fbsd_nat_target
+{
+  void fetch_registers (struct regcache *, int) override;
+  void store_registers (struct regcache *, int) override;
+};
+
+static riscv_fbsd_nat_target the_riscv_fbsd_nat_target;
+
+/* Determine if PT_GETREGS fetches REGNUM.  */
+
+static bool
+getregs_supplies (struct gdbarch *gdbarch, int regnum)
+{
+  return (regnum >= RISCV_RA_REGNUM && regnum <= RISCV_PC_REGNUM);
+}
+
+/* Determine if PT_GETFPREGS fetches REGNUM.  */
+
+static bool
+getfpregs_supplies (struct gdbarch *gdbarch, int regnum)
+{
+  return ((regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
+	  || regnum == RISCV_CSR_FCSR_REGNUM);
+}
+
+/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
+   for all registers.  */
+
+void
+riscv_fbsd_nat_target::fetch_registers (struct regcache *regcache,
+					int regnum)
+{
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+  struct gdbarch *gdbarch = regcache->arch ();
+  if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
+    regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
+  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
+    {
+      struct reg regs;
+
+      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      regcache->supply_regset (&riscv_fbsd_gregset, regnum, &regs,
+			       sizeof (regs));
+    }
+
+  if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
+    {
+      struct fpreg fpregs;
+
+      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't get floating point status"));
+
+      regcache->supply_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
+			       sizeof (fpregs));
+    }
+}
+
+/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
+   this for all registers.  */
+
+void
+riscv_fbsd_nat_target::store_registers (struct regcache *regcache,
+					int regnum)
+{
+  pid_t pid = get_ptrace_pid (regcache->ptid ());
+
+  struct gdbarch *gdbarch = regcache->arch ();
+  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
+    {
+      struct reg regs;
+
+      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't get registers"));
+
+      regcache->collect_regset (&riscv_fbsd_gregset, regnum, &regs,
+			       sizeof (regs));
+
+      if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
+	perror_with_name (_("Couldn't write registers"));
+    }
+
+  if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
+    {
+      struct fpreg fpregs;
+
+      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't get floating point status"));
+
+      regcache->collect_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
+				sizeof (fpregs));
+
+      if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
+	perror_with_name (_("Couldn't write floating point status"));
+    }
+}
+
+void
+_initialize_riscv_fbsd_nat (void)
+{
+  add_inf_child_target (&the_riscv_fbsd_nat_target);
+}
-- 
2.18.0

Re: [PATCH v2 3/4] Add FreeBSD/riscv architecture.

[Eli Zaretskii]

> From: John Baldwin <jhb@FreeBSD.org>
> Cc: andrew.burgess@embecosm.com,	jimw@sifive.com
> Date: Mon, 24 Sep 2018 13:51:50 -0700
> 
> gdb/ChangeLog:
> 
> 	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
> 	(HFILES_NO_SRCDIR): Add riscv-fbsd-tdep.h.
> 	(ALLDEPFILES): Add riscv-fbsd-tdep.c.
> 	* NEWS: Mention new FreeBSD/riscv target.
> 	* configure.tgt: Add riscv*-*-freebsd*.
> 	* riscv-fbsd-tdep.c: New file.
> 	* riscv-fbsd-tdep.h: New file.

OK for the NEWS part.

Thanks.

Re: [PATCH v2 4/4] Add native target for FreeBSD/riscv.

[Eli Zaretskii]

> From: John Baldwin <jhb@FreeBSD.org>
> Cc: andrew.burgess@embecosm.com,	jimw@sifive.com
> Date: Mon, 24 Sep 2018 13:51:51 -0700
> 
> gdb/ChangeLog:
> 
> 	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
> 	* NEWS: Mention new FreeBSD/riscv native configuration.
> 	* configure.host: Add riscv*-*-freebsd*.
> 	* configure.nat: Likewise.
> 	* riscv-fbsd-nat.c: New file.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Contributors): Add SRI International and University
> 	of Cambridge for FreeBSD/riscv.

OK for the documentation parts.

Thanks.

Re: [PATCH v2 1/4] Add helper functions to trad_frame to support register cache maps.

[Simon Marchi]

On 2018-09-24 04:51 PM, John Baldwin wrote:
> Currently, signal frame handlers require explicitly coded calls to
> trad_frame_set_reg_addr() to describe the location of saved registers
> within a signal frame.  This change permits the regcache_map_entry
> arrays used with regcache::supply_regset and regcache::collect_regset
> to be used to describe a block of saved registers given an initial
> address for the register block.
> 
> Some systems use the same layout for registers in core dump notes,
> native register sets with ptrace(), and the register contexts saved in
> signal frames.  On these systems, a single register map can now be
> used to describe the layout of registers in all three places.
> 
> 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.

LGTM.  It sounds very good to de-duplicate the knowledge of the register
layout in those structures.

Simon

Re: [PATCH v2 2/4] Use the existing instruction to determine the RISC-V breakpoint kind.

[Simon Marchi]

On 2018-09-24 04:51 PM, John Baldwin wrote:
> @@ -417,7 +411,21 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
>  {
>    if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO)
>      {
> -      if (riscv_has_feature (gdbarch, 'C'))
> +      gdb_byte buf[1];
> +      int status;
> +
> +      /* Read the opcode byte to determine the instruction length.  */
> +      status = target_read_code (*pcptr, buf, 1);
> +      if (status)
> +	memory_error (TARGET_XFER_E_IO, *pcptr);

Here you could use read_code if you want, which takes care of throwing an exception
on failure.

Based on the discussion you guys had about v1, this looks good to me, but the RISC-V
experts should take a look.

Simon

Re: [PATCH v2 3/4] Add FreeBSD/riscv architecture.

[Simon Marchi]

On 2018-09-24 04:51 PM, John Baldwin wrote:
> +/* Implement the "init" method of struct tramp_frame.  */
> +
> +static void
> +riscv_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, RISCV_SP_REGNUM);
> +  CORE_ADDR mcontext_addr =
> +    sp
> +    + RISCV_SIGFRAME_UCONTEXT_OFFSET
> +    + RISCV_UCONTEXT_MCONTEXT_OFFSET;

Just a formatting nit, to match the GNU style it should be something like

  CORE_ADDR mcontext_addr
    = (sp
       + RISCV_SIGFRAME_UCONTEXT_OFFSET
       + RISCV_UCONTEXT_MCONTEXT_OFFSET);

The break should always be before a binary operator, and the parentheses help (I've
been told) with auto-formatting in Emacs.

Otherwise everything looks good to me, but again I think the RISC-V wizards should take a look.

Simon

Re: [PATCH v2 4/4] Add native target for FreeBSD/riscv.

[Simon Marchi]

On 2018-09-24 04:51 PM, John Baldwin wrote:
> gdb/ChangeLog:
> 
> 	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
> 	* NEWS: Mention new FreeBSD/riscv native configuration.
> 	* configure.host: Add riscv*-*-freebsd*.
> 	* configure.nat: Likewise.
> 	* riscv-fbsd-nat.c: New file.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Contributors): Add SRI International and University
> 	of Cambridge for FreeBSD/riscv.
> ---
>  gdb/ChangeLog        |   8 +++
>  gdb/Makefile.in      |   1 +
>  gdb/NEWS             |   1 +
>  gdb/configure.host   |   1 +
>  gdb/configure.nat    |   4 ++
>  gdb/doc/ChangeLog    |   5 ++
>  gdb/doc/gdb.texinfo  |   6 ++
>  gdb/riscv-fbsd-nat.c | 135 +++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 161 insertions(+)
>  create mode 100644 gdb/riscv-fbsd-nat.c
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 1367f37db7..19c63706e9 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2018-09-24  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
> +	* NEWS: Mention new FreeBSD/riscv native configuration.
> +	* configure.host: Add riscv*-*-freebsd*.
> +	* configure.nat: Likewise.
> +	* riscv-fbsd-nat.c: New file.
> +
>  2018-09-24  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 2e03bb956c..13a8e16eec 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -2308,6 +2308,7 @@ ALLDEPFILES = \
>  	procfs.c \
>  	ravenscar-thread.c \
>  	remote-sim.c \
> +	riscv-fbsd-nat.c \
>  	riscv-fbsd-tdep.c \
>  	riscv-linux-nat.c \
>  	riscv-linux-tdep.c \
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a191ae2714..4ae3adedc0 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -80,6 +80,7 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
>  * New native configurations
>  
>  GNU/Linux/RISC-V		riscv*-*-linux*
> +FreeBSD/riscv			riscv*-*-freebsd*

I just noticed that we write RISC-V in two different ways for GNU/Linux and
FreeBSD: RISC-V and riscv.  It's not a big deal, but it looks a bit inconsistent.

Otherwise everything looks good to me, but again I think the RISC-V ninjas should take a look.

Simon

Re: [PATCH v2 4/4] Add native target for FreeBSD/riscv.

[Jim Wilson]

On Thu, Sep 27, 2018 at 1:04 PM Simon Marchi <simon.marchi@ericsson.com> wrote:
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -80,6 +80,7 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
> >  * New native configurations
> >
> >  GNU/Linux/RISC-V             riscv*-*-linux*
> > +FreeBSD/riscv                        riscv*-*-freebsd*
>
> I just noticed that we write RISC-V in two different ways for GNU/Linux and
> FreeBSD: RISC-V and riscv.  It's not a big deal, but it looks a bit inconsistent.
>
> Otherwise everything looks good to me, but again I think the RISC-V ninjas should take a look.

RISC-V is the official spelling of the architecture name, and riscv is
what we use for configure triplets.  I try to use the official
spelling whenever possible which is why I used RISC-V here.

Jim

Re: [PATCH v2 2/4] Use the existing instruction to determine the RISC-V breakpoint kind.

[Andrew Burgess]

* John Baldwin <jhb@FreeBSD.org> [2018-09-24 13:51:49 -0700]:

> RISC-V supports instructions of varying lengths.  Standard existing
> instructions in the base ISA are 4 bytes in length, but the 'C'
> extension adds support for compressed, 2 byte instructions.  RISC-V
> supports two different breakpoint instructions: EBREAK is a 4 byte
> instruction in the base ISA, and C.EBREAK is a 2 byte instruction only
> available on processors implementing the 'C' extension.  Using EBREAK
> to set breakpoints on compressed instructions causes problems as the
> second half of EBREAK will overwrite the first 2 bytes of the
> following instruction breaking other threads in the process if their
> PC is the following instruction.  Thus, breakpoints on compressed
> instructions need to use C.EBREAK instead of EBREAK.
> 
> Previously, the riscv architecture checked the MISA register to
> determine if the 'C' extension was available.  If so, it used C.EBREAK
> for all breakpoints.  However, the MISA register is not necessarily
> available to supervisor mode operating systems.  While native targets
> could provide a fake MISA register value, this patch instead examines
> the existing instruction at a breakpoint target to determine which
> breakpoint instruction to use.  If the existing instruction is a
> compressed instruction, C.EBREAK is used, otherwise EBREAK is used.
> 
> gdb/ChangeLog:
> 
> 	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
> 	case with explicit breakpoint kind.
> 	* riscv-tdep.c (show_use_compressed_breakpoints): Remove
> 	'additional_info' and related logic.
> 	(riscv_debug_breakpoints): New variable.
> 	(riscv_breakpoint_kind_from_pc): Use the length of the existing
> 	instruction to determine the breakpoint kind.
> 	(_initialize_riscv_tdep): Add 'set/show debug riscv breakpoints'
> 	flag.  Update description of 'set/show riscv
> 	use-compressed-breakpoints' flag.
> ---
>  gdb/ChangeLog          | 13 +++++++++++
>  gdb/disasm-selftests.c |  7 +++---
>  gdb/riscv-tdep.c       | 50 ++++++++++++++++++++++++++++--------------
>  3 files changed, 51 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2624975146..f56b0487cc 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,16 @@
> +2018-09-24  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
> +	case with explicit breakpoint kind.
> +	* riscv-tdep.c (show_use_compressed_breakpoints): Remove
> +	'additional_info' and related logic.
> +	(riscv_debug_breakpoints): New variable.
> +	(riscv_breakpoint_kind_from_pc): Use the length of the existing
> +	instruction to determine the breakpoint kind.
> +	(_initialize_riscv_tdep): Add 'set/show debug riscv breakpoints'
> +	flag.  Update description of 'set/show riscv
> +	use-compressed-breakpoints' flag.
> +
>  2018-09-24  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* trad-frame.c (trad_frame_set_regmap, trad_frame_set_reg_regmap):
> diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
> index f7d0ecca0e..8cc267631e 100644
> --- a/gdb/disasm-selftests.c
> +++ b/gdb/disasm-selftests.c
> @@ -77,9 +77,10 @@ print_one_insn_test (struct gdbarch *gdbarch)
>        /* fall through */
>      case bfd_arch_nios2:
>      case bfd_arch_score:
> -      /* nios2 and score need to know the current instruction to select
> -	 breakpoint instruction.  Give the breakpoint instruction kind
> -	 explicitly.  */
> +    case bfd_arch_riscv:
> +      /* nios2, riscv, and score need to know the current instruction
> +	 to select breakpoint instruction.  Give the breakpoint
> +	 instruction kind explicitly.  */
>        int bplen;
>        insn = gdbarch_sw_breakpoint_from_kind (gdbarch, 4, &bplen);
>        len = bplen;
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 254914c9c7..7faa7d01f0 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -210,20 +210,9 @@ show_use_compressed_breakpoints (struct ui_file *file, int from_tty,
>  				 struct cmd_list_element *c,
>  				 const char *value)
>  {
> -  const char *additional_info;
> -  struct gdbarch *gdbarch = target_gdbarch ();
> -
> -  if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO)
> -    if (riscv_has_feature (gdbarch, 'C'))
> -      additional_info = _(" (currently on)");
> -    else
> -      additional_info = _(" (currently off)");
> -  else
> -    additional_info = "";
> -
>    fprintf_filtered (file,
>  		    _("Debugger's use of compressed breakpoints is set "
> -		      "to %s%s.\n"), value, additional_info);
> +		      "to %s.\n"), value);
>  }
>  
>  /* The set and show lists for 'set riscv' and 'show riscv' prefixes.  */
> @@ -284,6 +273,11 @@ show_riscv_debug_variable (struct ui_file *file, int from_tty,
>  		    c->name, value);
>  }
>  
> +/* When this is set to non-zero debugging information about breakpoint
> +   kinds will be printed.  */
> +
> +static unsigned int riscv_debug_breakpoints = 0;
> +
>  /* When this is set to non-zero debugging information about inferior calls
>     will be printed.  */
>  
> @@ -417,7 +411,21 @@ riscv_breakpoint_kind_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr)
>  {
>    if (use_compressed_breakpoints == AUTO_BOOLEAN_AUTO)
>      {
> -      if (riscv_has_feature (gdbarch, 'C'))
> +      gdb_byte buf[1];
> +      int status;
> +
> +      /* Read the opcode byte to determine the instruction length.  */
> +      status = target_read_code (*pcptr, buf, 1);
> +      if (status)
> +	memory_error (TARGET_XFER_E_IO, *pcptr);
> +
> +      if (riscv_debug_breakpoints)
> +	fprintf_unfiltered
> +	  (gdb_stdlog,
> +	   "Using %s for breakpoint at %s (instruction length %d)\n",
> +	   riscv_insn_length (buf[0]) == 2 ? "C.EBREAK" : "EBREAK",
> +	   paddress (gdbarch, *pcptr), riscv_insn_length (buf[0]));
> +      if (riscv_insn_length (buf[0]) == 2)
>  	return 2;
>        else
>  	return 4;
> @@ -2936,6 +2944,16 @@ _initialize_riscv_tdep (void)
>  		  &showdebugriscvcmdlist, "show debug riscv ", 0,
>  		  &showdebuglist);
>  
> +  add_setshow_zuinteger_cmd ("breakpoints", class_maintenance,
> +			     &riscv_debug_breakpoints,  _("\
> +Set riscv breakpoint debugging."), _("\
> +Show riscv breakpoint debugging."), _("\
> +When non-zero, print debugging information for the riscv specific parts\n\
> +of the breakpoint mechanism."),
> +			     NULL,
> +			     show_riscv_debug_variable,
> +			     &setdebugriscvcmdlist, &showdebugriscvcmdlist);
> +
>    add_setshow_zuinteger_cmd ("infcall", class_maintenance,
>  			     &riscv_debug_infcall,  _("\
>  Set riscv inferior call debugging."), _("\
> @@ -2973,9 +2991,9 @@ of the stack unwinding mechanism."),
>  Set debugger's use of compressed breakpoints."), _("	\
>  Show debugger's use of compressed breakpoints."), _("\
>  Debugging compressed code requires compressed breakpoints to be used. If\n \
> -left to 'auto' then gdb will use them if $misa indicates the C extension\n \
> -is supported. If that doesn't give the correct behavior, then this option\n\
> -can be used."),
> +left to 'auto' then gdb will use them if the existing instruction is a\n \
> +compressed instruction. If that doesn't give the correct behavior, then\n \
> +this option can be used."),

While you're editing these lines could you remove the trailing
whitespace please, so '\n\' instead of '\n \', the latter doesn't seem
to be the norm in GDB doc strings.

Feel free to switch 'read_code' as Simon suggested.

Otherwise this is fine to commit.

Thanks,
Andrew

>  				NULL,
>  				show_use_compressed_breakpoints,
>  				&setriscvcmdlist,
> -- 
> 2.18.0
> 

Re: [PATCH v2 3/4] Add FreeBSD/riscv architecture.

[Andrew Burgess]

* John Baldwin <jhb@FreeBSD.org> [2018-09-24 13:51:50 -0700]:

> Support for collecting and supplying general purpose and floating
> point register sets is provided along with signal frame unwinding.
> 
> FreeBSD only supports RV64 currently, so while some provision is made
> for RV32 in the general-purpose register set, the changes have only
> been tested on RV64.
> 
> gdb/ChangeLog:
> 
> 	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
> 	(HFILES_NO_SRCDIR): Add riscv-fbsd-tdep.h.
> 	(ALLDEPFILES): Add riscv-fbsd-tdep.c.
> 	* NEWS: Mention new FreeBSD/riscv target.
> 	* configure.tgt: Add riscv*-*-freebsd*.
> 	* riscv-fbsd-tdep.c: New file.
> 	* riscv-fbsd-tdep.h: New file.

I haven't tested the code yet, but I took a look through and I'm happy
with this being merged (with Simon's nit fixed).

Thanks,
Andrew




> ---
>  gdb/ChangeLog         |  10 ++
>  gdb/Makefile.in       |   3 +
>  gdb/NEWS              |   1 +
>  gdb/configure.tgt     |   5 +
>  gdb/riscv-fbsd-tdep.c | 206 ++++++++++++++++++++++++++++++++++++++++++
>  gdb/riscv-fbsd-tdep.h |  33 +++++++
>  6 files changed, 258 insertions(+)
>  create mode 100644 gdb/riscv-fbsd-tdep.c
>  create mode 100644 gdb/riscv-fbsd-tdep.h
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index f56b0487cc..1367f37db7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,13 @@
> +2018-09-24  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
> +	(HFILES_NO_SRCDIR): Add riscv-fbsd-tdep.h.
> +	(ALLDEPFILES): Add riscv-fbsd-tdep.c.
> +	* NEWS: Mention new FreeBSD/riscv target.
> +	* configure.tgt: Add riscv*-*-freebsd*.
> +	* riscv-fbsd-tdep.c: New file.
> +	* riscv-fbsd-tdep.h: New file.
> +
>  2018-09-24  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 3b158fa1db..2e03bb956c 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -745,6 +745,7 @@ ALL_TARGET_OBS = \
>  	ppc-sysv-tdep.o \
>  	ppc64-tdep.o \
>  	ravenscar-thread.o \
> +	riscv-fbsd-tdep.o \
>  	riscv-linux-tdep.o \
>  	riscv-tdep.o \
>  	rl78-tdep.o \
> @@ -1339,6 +1340,7 @@ HFILES_NO_SRCDIR = \
>  	remote.h \
>  	remote-fileio.h \
>  	remote-notif.h \
> +	riscv-fbsd-tdep.h \
>  	riscv-tdep.h \
>  	rs6000-aix-tdep.h \
>  	rs6000-tdep.h \
> @@ -2306,6 +2308,7 @@ ALLDEPFILES = \
>  	procfs.c \
>  	ravenscar-thread.c \
>  	remote-sim.c \
> +	riscv-fbsd-tdep.c \
>  	riscv-linux-nat.c \
>  	riscv-linux-tdep.c \
>  	riscv-tdep.c \
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a1936ca1cc..a191ae2714 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -86,6 +86,7 @@ GNU/Linux/RISC-V		riscv*-*-linux*
>  GNU/Linux/RISC-V		riscv*-*-linux*
>  CSKY ELF			csky*-*-elf
>  CSKY GNU/LINUX			csky*-*-linux
> +FreeBSD/riscv			riscv*-*-freebsd*
>  
>  * Python API
>  
> diff --git a/gdb/configure.tgt b/gdb/configure.tgt
> index 6d1a4df84a..3b94942732 100644
> --- a/gdb/configure.tgt
> +++ b/gdb/configure.tgt
> @@ -528,6 +528,11 @@ s390*-*-linux*)
>  	build_gdbserver=yes
>  	;;
>  
> +riscv*-*-freebsd*)
> +	# Target: FreeBSD/riscv
> +	gdb_target_obs="riscv-fbsd-tdep.o riscv-tdep.o"
> +	;;
> +
>  riscv*-*-linux*)
>  	# Target: Linux/RISC-V
>  	gdb_target_obs="riscv-linux-tdep.o riscv-tdep.o glibc-tdep.o \
> diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
> new file mode 100644
> index 0000000000..c09ec650ca
> --- /dev/null
> +++ b/gdb/riscv-fbsd-tdep.c
> @@ -0,0 +1,206 @@
> +/* Target-dependent code for FreeBSD on RISC-V processors.
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "fbsd-tdep.h"
> +#include "osabi.h"
> +#include "riscv-tdep.h"
> +#include "riscv-fbsd-tdep.h"
> +#include "solib-svr4.h"
> +#include "target.h"
> +#include "trad-frame.h"
> +#include "tramp-frame.h"
> +
> +/* Register maps.  */
> +
> +static const struct regcache_map_entry riscv_fbsd_gregmap[] =
> +  {
> +    { 1, RISCV_RA_REGNUM, 0 },
> +    { 1, RISCV_SP_REGNUM, 0 },
> +    { 1, RISCV_GP_REGNUM, 0 },
> +    { 1, RISCV_TP_REGNUM, 0 },
> +    { 3, 5, 0 },		/* t0 - t2 */
> +    { 4, 28, 0 },		/* t3 - t6 */
> +    { 2, RISCV_FP_REGNUM, 0 },	/* s0 - s1 */
> +    { 10, 18, 0 },		/* s2 - s11 */
> +    { 8, RISCV_A0_REGNUM, 0 },	/* a0 - a7 */
> +    { 1, RISCV_PC_REGNUM, 0 },
> +    { 1, RISCV_CSR_SSTATUS_REGNUM, 0 },
> +    { 0 }
> +  };
> +
> +static const struct regcache_map_entry riscv_fbsd_fpregmap[] =
> +  {
> +    { 32, RISCV_FIRST_FP_REGNUM, 16 },
> +    { 1, RISCV_CSR_FCSR_REGNUM, 8 },
> +    { 0 }
> +  };
> +
> +/* Supply the general-purpose registers stored in GREGS to REGCACHE.
> +   This function only exists to supply the always-zero x0 in addition
> +   to the registers in GREGS.  */
> +
> +static void
> +riscv_fbsd_supply_gregset (const struct regset *regset,
> +			   struct regcache *regcache, int regnum,
> +			   const void *gregs, size_t len)
> +{
> +  regcache->supply_regset (&riscv_fbsd_gregset, regnum, gregs, len);
> +  if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
> +    regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
> +}
> +
> +/* Register set definitions.  */
> +
> +const struct regset riscv_fbsd_gregset =
> +  {
> +    riscv_fbsd_gregmap,
> +    riscv_fbsd_supply_gregset, regcache_collect_regset
> +  };
> +
> +const struct regset riscv_fbsd_fpregset =
> +  {
> +    riscv_fbsd_fpregmap,
> +    regcache_supply_regset, regcache_collect_regset
> +  };
> +
> +/* Implement the "regset_from_core_section" gdbarch method.  */
> +
> +static void
> +riscv_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
> +					 iterate_over_regset_sections_cb *cb,
> +					 void *cb_data,
> +					 const struct regcache *regcache)
> +{
> +  cb (".reg", RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch),
> +      RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch),
> +      &riscv_fbsd_gregset, NULL, cb_data);
> +  cb (".reg2", RISCV_FBSD_SIZEOF_FPREGSET, RISCV_FBSD_SIZEOF_FPREGSET,
> +      &riscv_fbsd_fpregset, NULL, cb_data);
> +}
> +
> +/* In a signal frame, sp points to a 'struct sigframe' which is
> +   defined as:
> +
> +   struct sigframe {
> +	   siginfo_t	sf_si;
> +	   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 followed
> +   by the floating point register set.  The floating point register
> +   set is only valid if the _MC_FP_VALID flag is set in mc_flags.  */
> +
> +#define RISCV_SIGFRAME_UCONTEXT_OFFSET 		80
> +#define RISCV_UCONTEXT_MCONTEXT_OFFSET		16
> +#define RISCV_MCONTEXT_FLAG_FP_VALID		0x1
> +
> +/* Implement the "init" method of struct tramp_frame.  */
> +
> +static void
> +riscv_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, RISCV_SP_REGNUM);
> +  CORE_ADDR mcontext_addr =
> +    sp
> +    + RISCV_SIGFRAME_UCONTEXT_OFFSET
> +    + RISCV_UCONTEXT_MCONTEXT_OFFSET;
> +  gdb_byte buf[4];
> +  int i;
> +
> +  trad_frame_set_reg_regmap (this_cache, riscv_fbsd_gregmap, mcontext_addr,
> +			     RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch));
> +
> +  CORE_ADDR fpregs_addr
> +    = mcontext_addr + RISCV_FBSD_NUM_GREGS * riscv_isa_xlen (gdbarch);
> +  CORE_ADDR fp_flags_addr
> +    = fpregs_addr + RISCV_FBSD_SIZEOF_FPREGSET;
> +  if (target_read_memory (fp_flags_addr, buf, 4) == 0
> +      && (extract_unsigned_integer (buf, 4, byte_order)
> +	  & RISCV_MCONTEXT_FLAG_FP_VALID))
> +    trad_frame_set_reg_regmap (this_cache, riscv_fbsd_fpregmap, fpregs_addr,
> +			       RISCV_FBSD_SIZEOF_FPREGSET);
> +
> +  trad_frame_set_id (this_cache, frame_id_build (sp, func));
> +}
> +
> +/* RISC-V supports 16-bit instructions ("C") as well as 32-bit
> +   instructions.  The signal trampoline on FreeBSD uses a mix of
> +   these, but tramp_frame assumes a fixed instruction size.  To cope,
> +   claim that all instructions are 16 bits and use two "slots" for
> +   32-bit instructions.  */
> +
> +static const struct tramp_frame riscv_fbsd_sigframe =
> +{
> +  SIGTRAMP_FRAME,
> +  2,
> +  {
> +    {0x850a, ULONGEST_MAX},		/* mov  a0, sp  */
> +    {0x0513, ULONGEST_MAX},		/* addi a0, a0, #SF_UC  */
> +    {0x0505, ULONGEST_MAX},
> +    {0x0293, ULONGEST_MAX},		/* li   t0, #SYS_sigreturn  */
> +    {0x1a10, ULONGEST_MAX},
> +    {0x0073, ULONGEST_MAX},		/* ecall  */
> +    {0x0000, ULONGEST_MAX},
> +    {TRAMP_SENTINEL_INSN, ULONGEST_MAX}
> +  },
> +  riscv_fbsd_sigframe_init
> +};
> +
> +/* Implement the 'init_osabi' method of struct gdb_osabi_handler.  */
> +
> +static void
> +riscv_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> +{
> +  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> +
> +  /* Generic FreeBSD support.  */
> +  fbsd_init_abi (info, gdbarch);
> +
> +  set_gdbarch_software_single_step (gdbarch, riscv_software_single_step);
> +
> +  set_solib_svr4_fetch_link_map_offsets (gdbarch,
> +					 (riscv_isa_xlen (gdbarch) == 4
> +					  ? svr4_ilp32_fetch_link_map_offsets
> +					  : svr4_lp64_fetch_link_map_offsets));
> +
> +  tramp_frame_prepend_unwinder (gdbarch, &riscv_fbsd_sigframe);
> +
> +  set_gdbarch_iterate_over_regset_sections
> +    (gdbarch, riscv_fbsd_iterate_over_regset_sections);
> +}
> +
> +void
> +_initialize_riscv_fbsd_tdep (void)
> +{
> +  gdbarch_register_osabi (bfd_arch_riscv, 0, GDB_OSABI_FREEBSD,
> +			  riscv_fbsd_init_abi);
> +}
> diff --git a/gdb/riscv-fbsd-tdep.h b/gdb/riscv-fbsd-tdep.h
> new file mode 100644
> index 0000000000..8b6abd565b
> --- /dev/null
> +++ b/gdb/riscv-fbsd-tdep.h
> @@ -0,0 +1,33 @@
> +/* FreeBSD/riscv target support, prototypes.
> +
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "regset.h"
> +
> +/* The general-purpose regset consists of 31 X registers, EPC, and
> +   SSTATUS.  */
> +#define RISCV_FBSD_NUM_GREGS		33
> +
> +/* The fp regset always consists of 32 128-bit registers, plus a
> +   64-bit CSR_FCSR.  If 'Q' is not supported, only the low 64-bits of
> +   each floating point register are valid.  If 'D' is not supported,
> +   only the low 32-bits of each floating point register are valid.  */
> +#define RISCV_FBSD_SIZEOF_FPREGSET (32 * 16 + 8)
> +
> +extern const struct regset riscv_fbsd_gregset;
> +extern const struct regset riscv_fbsd_fpregset;
> -- 
> 2.18.0
> 

Re: [PATCH v2 4/4] Add native target for FreeBSD/riscv.

[Andrew Burgess]

* John Baldwin <jhb@FreeBSD.org> [2018-09-24 13:51:51 -0700]:

> gdb/ChangeLog:
> 
> 	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
> 	* NEWS: Mention new FreeBSD/riscv native configuration.
> 	* configure.host: Add riscv*-*-freebsd*.
> 	* configure.nat: Likewise.
> 	* riscv-fbsd-nat.c: New file.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Contributors): Add SRI International and University
> 	of Cambridge for FreeBSD/riscv.

I'm happy with these being merged.

Thanks,
Andrew



> ---
>  gdb/ChangeLog        |   8 +++
>  gdb/Makefile.in      |   1 +
>  gdb/NEWS             |   1 +
>  gdb/configure.host   |   1 +
>  gdb/configure.nat    |   4 ++
>  gdb/doc/ChangeLog    |   5 ++
>  gdb/doc/gdb.texinfo  |   6 ++
>  gdb/riscv-fbsd-nat.c | 135 +++++++++++++++++++++++++++++++++++++++++++
>  8 files changed, 161 insertions(+)
>  create mode 100644 gdb/riscv-fbsd-nat.c
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 1367f37db7..19c63706e9 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2018-09-24  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* Makefile.in (ALLDEPFILES): Add riscv-fbsd-nat.c.
> +	* NEWS: Mention new FreeBSD/riscv native configuration.
> +	* configure.host: Add riscv*-*-freebsd*.
> +	* configure.nat: Likewise.
> +	* riscv-fbsd-nat.c: New file.
> +
>  2018-09-24  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* Makefile.in (ALL_TARGET_OBS): Add riscv-fbsd-tdep.o.
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 2e03bb956c..13a8e16eec 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -2308,6 +2308,7 @@ ALLDEPFILES = \
>  	procfs.c \
>  	ravenscar-thread.c \
>  	remote-sim.c \
> +	riscv-fbsd-nat.c \
>  	riscv-fbsd-tdep.c \
>  	riscv-linux-nat.c \
>  	riscv-linux-tdep.c \
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a191ae2714..4ae3adedc0 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -80,6 +80,7 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
>  * New native configurations
>  
>  GNU/Linux/RISC-V		riscv*-*-linux*
> +FreeBSD/riscv			riscv*-*-freebsd*
>  
>  * New targets
>  
> diff --git a/gdb/configure.host b/gdb/configure.host
> index 23a2f16399..c87f997abc 100644
> --- a/gdb/configure.host
> +++ b/gdb/configure.host
> @@ -149,6 +149,7 @@ powerpc64*-*-linux*)	gdb_host=ppc64-linux
>  			;;
>  powerpc*-*-linux*)	gdb_host=linux ;;
>  
> +riscv*-*-freebsd*)	gdb_host=fbsd ;;
>  riscv*-*-linux*)	gdb_host=linux ;;
>  
>  s390*-*-linux*)		gdb_host=linux ;;
> diff --git a/gdb/configure.nat b/gdb/configure.nat
> index 10bf65fec3..200b716924 100644
> --- a/gdb/configure.nat
> +++ b/gdb/configure.nat
> @@ -177,6 +177,10 @@ case ${gdb_host} in
>  		# systems running FreeBSD.
>  		NATDEPFILES="${NATDEPFILES} ppc-fbsd-nat.o bsd-kvm.o"
>  		;;
> +	    riscv*)
> +		# Host: FreeBSD/riscv
> +		NATDEPFILES="${NATDEPFILES} riscv-fbsd-nat.o"
> +		;;
>  	    sparc)
>  		# Host: FreeBSD/sparc64
>  		NATDEPFILES="${NATDEPFILES} sparc-nat.o sparc64-nat.o \
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 4285a4d8bb..0197650c6a 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-09-24  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* gdb.texinfo (Contributors): Add SRI International and University
> +	of Cambridge for FreeBSD/riscv.
> +
>  2018-09-23  Tom Tromey  <tom@tromey.com>
>  
>  	PR python/18852:
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index b5b6089153..b8af74e949 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -546,6 +546,12 @@ was developed by SRI International and the University of Cambridge
>  Computer Laboratory under DARPA/AFRL contract FA8750-10-C-0237
>  ("CTSRD"), as part of the DARPA CRASH research programme.
>  
> +Initial support for the FreeBSD/riscv target and native configuration
> +was developed by SRI International and the University of Cambridge
> +Computer Laboratory (Department of Computer Science and Technology)
> +under DARPA contract HR0011-18-C-0016 ("ECATS"), as part of the DARPA
> +SSITH research programme.
> +
>  The original port to the OpenRISC 1000 is believed to be due to
>  Alessandro Forin and Per Bothner.  More recent ports have been the work
>  of Jeremy Bennett, Franck Jullien, Stefan Wallentowitz and
> diff --git a/gdb/riscv-fbsd-nat.c b/gdb/riscv-fbsd-nat.c
> new file mode 100644
> index 0000000000..ad4ea1e0ad
> --- /dev/null
> +++ b/gdb/riscv-fbsd-nat.c
> @@ -0,0 +1,135 @@
> +/* Native-dependent code for FreeBSD/riscv.
> +
> +   Copyright (C) 2018 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "defs.h"
> +#include "target.h"
> +
> +#include <sys/types.h>
> +#include <sys/ptrace.h>
> +#include <machine/reg.h>
> +
> +#include "fbsd-nat.h"
> +#include "riscv-tdep.h"
> +#include "riscv-fbsd-tdep.h"
> +#include "inf-ptrace.h"
> +
> +struct riscv_fbsd_nat_target final : public fbsd_nat_target
> +{
> +  void fetch_registers (struct regcache *, int) override;
> +  void store_registers (struct regcache *, int) override;
> +};
> +
> +static riscv_fbsd_nat_target the_riscv_fbsd_nat_target;
> +
> +/* Determine if PT_GETREGS fetches REGNUM.  */
> +
> +static bool
> +getregs_supplies (struct gdbarch *gdbarch, int regnum)
> +{
> +  return (regnum >= RISCV_RA_REGNUM && regnum <= RISCV_PC_REGNUM);
> +}
> +
> +/* Determine if PT_GETFPREGS fetches REGNUM.  */
> +
> +static bool
> +getfpregs_supplies (struct gdbarch *gdbarch, int regnum)
> +{
> +  return ((regnum >= RISCV_FIRST_FP_REGNUM && regnum <= RISCV_LAST_FP_REGNUM)
> +	  || regnum == RISCV_CSR_FCSR_REGNUM);
> +}
> +
> +/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
> +   for all registers.  */
> +
> +void
> +riscv_fbsd_nat_target::fetch_registers (struct regcache *regcache,
> +					int regnum)
> +{
> +  pid_t pid = get_ptrace_pid (regcache->ptid ());
> +
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
> +    regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
> +  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
> +    {
> +      struct reg regs;
> +
> +      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +
> +      regcache->supply_regset (&riscv_fbsd_gregset, regnum, &regs,
> +			       sizeof (regs));
> +    }
> +
> +  if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
> +    {
> +      struct fpreg fpregs;
> +
> +      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
> +	perror_with_name (_("Couldn't get floating point status"));
> +
> +      regcache->supply_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
> +			       sizeof (fpregs));
> +    }
> +}
> +
> +/* Store register REGNUM back into the inferior.  If REGNUM is -1, do
> +   this for all registers.  */
> +
> +void
> +riscv_fbsd_nat_target::store_registers (struct regcache *regcache,
> +					int regnum)
> +{
> +  pid_t pid = get_ptrace_pid (regcache->ptid ());
> +
> +  struct gdbarch *gdbarch = regcache->arch ();
> +  if (regnum == -1 || getregs_supplies (gdbarch, regnum))
> +    {
> +      struct reg regs;
> +
> +      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
> +	perror_with_name (_("Couldn't get registers"));
> +
> +      regcache->collect_regset (&riscv_fbsd_gregset, regnum, &regs,
> +			       sizeof (regs));
> +
> +      if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
> +	perror_with_name (_("Couldn't write registers"));
> +    }
> +
> +  if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
> +    {
> +      struct fpreg fpregs;
> +
> +      if (ptrace (PT_GETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
> +	perror_with_name (_("Couldn't get floating point status"));
> +
> +      regcache->collect_regset (&riscv_fbsd_fpregset, regnum, &fpregs,
> +				sizeof (fpregs));
> +
> +      if (ptrace (PT_SETFPREGS, pid, (PTRACE_TYPE_ARG3) &fpregs, 0) == -1)
> +	perror_with_name (_("Couldn't write floating point status"));
> +    }
> +}
> +
> +void
> +_initialize_riscv_fbsd_nat (void)
> +{
> +  add_inf_child_target (&the_riscv_fbsd_nat_target);
> +}
> -- 
> 2.18.0
> 

Re: [PATCH v2 4/4] Add native target for FreeBSD/riscv.

[John Baldwin]

On 9/27/18 1:04 PM, Simon Marchi wrote:
> On 2018-09-24 04:51 PM, John Baldwin wrote:
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index a191ae2714..4ae3adedc0 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -80,6 +80,7 @@ thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
>>  * New native configurations
>>  
>>  GNU/Linux/RISC-V		riscv*-*-linux*
>> +FreeBSD/riscv			riscv*-*-freebsd*
> 
> I just noticed that we write RISC-V in two different ways for GNU/Linux and
> FreeBSD: RISC-V and riscv.  It's not a big deal, but it looks a bit inconsistent.

To date when describing new FreeBSD targets in NEWS I have used the nomenclature
FreeBSD uses which is generally "FreeBSD/<arch>" where FreeBSD sometimes uses
different arch names (e.g. "amd64" instead of "x86-64", but also lowercase "mips"
vs "MIPS" as used for Linux or "aarch64" vs "AArch64").  It's the string that
shows up in most FreeBSD documentation for a given platform as well as at login
prompts on ttys, etc.  To that end it's kind of the closest thing FreeBSD has for
naming a port of FreeBSD to a given platform.  I had assumed that would be the
clearest way to describe FreeBSD targets.  It's just a cosmetic thing though.

-- 
John Baldwin

                                                                            

Re: [PATCH v2 2/4] Use the existing instruction to determine the RISC-V breakpoint kind.

[John Baldwin]

On 9/28/18 3:23 AM, Andrew Burgess wrote:
> While you're editing these lines could you remove the trailing
> whitespace please, so '\n\' instead of '\n \', the latter doesn't seem
> to be the norm in GDB doc strings.
> 
> Feel free to switch 'read_code' as Simon suggested.
> 
> Otherwise this is fine to commit.

Ok, I've pushed it with both of these fixes applied.

-- 
John Baldwin

                                                                            

Re: [PATCH v2 1/4] Add helper functions to trad_frame to support register cache maps.

[John Baldwin]

On 9/27/18 12:31 PM, Simon Marchi wrote:
> On 2018-09-24 04:51 PM, John Baldwin wrote:
>> Currently, signal frame handlers require explicitly coded calls to
>> trad_frame_set_reg_addr() to describe the location of saved registers
>> within a signal frame.  This change permits the regcache_map_entry
>> arrays used with regcache::supply_regset and regcache::collect_regset
>> to be used to describe a block of saved registers given an initial
>> address for the register block.
>>
>> Some systems use the same layout for registers in core dump notes,
>> native register sets with ptrace(), and the register contexts saved in
>> signal frames.  On these systems, a single register map can now be
>> used to describe the layout of registers in all three places.
>>
>> 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.
> 
> LGTM.  It sounds very good to de-duplicate the knowledge of the register
> layout in those structures.

After some discussions on IRC, I've modified this patch a bit.  If we want
to rename 'regcache_map_entry' to be less regcache-specific then I think
that might be something to do as a followup change.  The main changes
relative to the v2 patch are only providing a single new function (the
one taking a frame_cache) until a caller shows up needing to use the other
function (taking the saved register array), using a typed pointer for the
regache_map_entry argument to the new function, and adding some comments to
regcache.h to note that this structure is now also used by trad_frame as
well as document the semantics when a register's "slot" in the map doesn't
match the size of a register.  I've pasted it below rather than rerolling
the whole series:

Author: John Baldwin <jhb@FreeBSD.org>
Date:   Fri Sep 28 14:37:22 2018 -0700

    Add a helper function to trad_frame to support register cache maps.
    
    Currently, signal frame handlers require explicitly coded calls to
    trad_frame_set_reg_addr() to describe the location of saved registers
    within a signal frame.  This change permits the regcache_map_entry
    arrays used with regcache::supply_regset and regcache::collect_regset
    to be used to describe a block of saved registers given an initial
    address for the register block.
    
    Some systems use the same layout for registers in core dump notes,
    native register sets with ptrace(), and the register contexts saved in
    signal frames.  On these systems, a single register map can now be
    used to describe the layout of registers in all three places.
    
    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.
    
    While here, document the semantics for both regcache::transfer_regset
    and trad_frame with respect to register slot's whose size does not
    match the register's size.
    
    gdb/ChangeLog:
    
            * regcache.h (struct regcache_map_entry): Note that this type can
            be used with traditional frame caches.
            * trad-frame.c (trad_frame_set_reg_regmap): New.
            * trad-frame.h (trad_frame_set_reg_regmap): New.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56645e960f..f869713738 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-28  John Baldwin  <jhb@FreeBSD.org>
+
+       * regcache.h (struct regcache_map_entry): Note that this type can
+       be used with traditional frame caches.
+       * trad-frame.c (trad_frame_set_reg_regmap): New.
+       * trad-frame.h (trad_frame_set_reg_regmap): New.
+
 2018-09-28  John Baldwin  <jhb@FreeBSD.org>
 
        * disasm-selftests.c (print_one_insn_test): Add bfd_arch_riscv to
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 4a45f33871..57a4ab3488 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -72,14 +72,31 @@ extern void regcache_cooked_write_unsigned (struct regcache *regcache,
 extern void regcache_write_pc (struct regcache *regcache, CORE_ADDR pc);
 
 /* Mapping between register numbers and offsets in a buffer, for use
-   in the '*regset' functions below.  In an array of
-   'regcache_map_entry' each element is interpreted like follows:
+   in the '*regset' functions below and with traditional frame caches.
+   In an array of 'regcache_map_entry' each element is interpreted
+   like follows:
 
    - If 'regno' is a register number: Map register 'regno' to the
      current offset (starting with 0) and increase the current offset
      by 'size' (or the register's size, if 'size' is zero).  Repeat
      this with consecutive register numbers up to 'regno+count-1'.
 
+     For each described register, if 'size' is larger than the
+     register's size, the register's value is assumed to be stored in
+     the first N (where N is the register size) bytes at the current
+     offset.  The remaining 'size' - N bytes are filled with zeroes by
+     'regcache_collect_regset' and ignored by other consumers.
+
+     If 'size' is smaller than the register's size, only the first
+     'size' bytes of a register's value are assumed to be stored at
+     the current offset.  'regcache_collect_regset' copies the first
+     'size' bytes of a register's value to the output buffer.
+     'regcache_supply_regset' copies the bytes from the input buffer
+     into the low 'size' bytes of the register's value leaving the
+     remaining bytes of the register's value unchanged.  Frame caches
+     read the 'size' bytes from the stack frame and zero extend them
+     to generate the register's value.
+
    - If 'regno' is REGCACHE_MAP_SKIP: Add 'count*size' to the current
      offset.
 
diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c
index fd9f24084d..a3484d2b39 100644
--- a/gdb/trad-frame.c
+++ b/gdb/trad-frame.c
@@ -22,6 +22,7 @@
 #include "trad-frame.h"
 #include "regcache.h"
 #include "frame-unwind.h"
+#include "target.h"
 #include "value.h"
 
 struct trad_frame_cache
@@ -148,6 +149,62 @@ trad_frame_set_reg_addr (struct trad_frame_cache *this_trad_cache,
   trad_frame_set_addr (this_trad_cache->prev_regs, regnum, addr);
 }
 
+void
+trad_frame_set_reg_regmap (struct trad_frame_cache *this_trad_cache,
+                          const struct regcache_map_entry *regmap,
+                          CORE_ADDR addr, size_t size)
+{
+  struct gdbarch *gdbarch = get_frame_arch (this_trad_cache->this_frame);
+  int offs = 0, count;
+
+  for (; (count = regmap->count) != 0; regmap++)
+    {
+      int regno = regmap->regno;
+      int slot_size = regmap->size;
+
+      if (slot_size == 0 && regno != REGCACHE_MAP_SKIP)
+       slot_size = register_size (gdbarch, regno);
+
+      if (offs + slot_size > size)
+       break;
+
+      if (regno == REGCACHE_MAP_SKIP)
+       offs += count * slot_size;
+      else
+       for (; count--; regno++, offs += slot_size)
+         {
+           /* Mimic the semantics of regcache::transfer_regset if a
+              register slot's size does not match the size of a
+              register.
+
+              If a register slot is larger than a register, assume
+              the register's value is stored in the first N bytes of
+              the slot and ignore the remaining bytes.
+
+              If the register slot is smaller than the register,
+              assume that the slot contains the low N bytes of the
+              register's value.  Since trad_frame assumes that
+              registers stored by address are sized according to the
+              register, read the low N bytes and zero-extend them to
+              generate a register value.  */
+           if (slot_size >= register_size (gdbarch, regno))
+             trad_frame_set_reg_addr (this_trad_cache, regno, addr + offs);
+           else
+             {
+               enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
+               gdb_byte buf[slot_size];
+
+               if (target_read_memory (addr + offs, buf, sizeof buf) == 0)
+                 {
+                   LONGEST val
+                     = extract_unsigned_integer (buf, sizeof buf, byte_order);
+                   trad_frame_set_reg_value (this_trad_cache, regno, val);
+                 }
+             }
+         }
+    }
+}
+
 void
 trad_frame_set_unknown (struct trad_frame_saved_reg this_saved_regs[],
                        int regnum)
diff --git a/gdb/trad-frame.h b/gdb/trad-frame.h
index a11a2c29c9..0d04bf4127 100644
--- a/gdb/trad-frame.h
+++ b/gdb/trad-frame.h
@@ -23,6 +23,7 @@
 #include "frame.h"             /* For "struct frame_id".  */
 
 struct frame_info;
+struct regcache_map_entry;
 struct trad_frame_cache;
 
 /* A simple, or traditional frame cache.
@@ -45,6 +46,9 @@ void trad_frame_set_reg_realreg (struct trad_frame_cache *this_trad_cache,
                                 int regnum, int realreg);
 void trad_frame_set_reg_addr (struct trad_frame_cache *this_trad_cache,
                              int regnum, CORE_ADDR addr);
+void trad_frame_set_reg_regmap (struct trad_frame_cache *this_trad_cache,
+                               const struct regcache_map_entry *regmap,
+                               CORE_ADDR addr, size_t size);
 void trad_frame_set_reg_value (struct trad_frame_cache *this_cache,
                               int regnum, LONGEST val);
 
-- 
John Baldwin

                                                                            

Re: [PATCH v2 1/4] Add helper functions to trad_frame to support register cache maps.

[Simon Marchi]

On 2018-09-28 05:44 PM, John Baldwin wrote:
> On 9/27/18 12:31 PM, Simon Marchi wrote:
>> On 2018-09-24 04:51 PM, John Baldwin wrote:
>>> Currently, signal frame handlers require explicitly coded calls to
>>> trad_frame_set_reg_addr() to describe the location of saved registers
>>> within a signal frame.  This change permits the regcache_map_entry
>>> arrays used with regcache::supply_regset and regcache::collect_regset
>>> to be used to describe a block of saved registers given an initial
>>> address for the register block.
>>>
>>> Some systems use the same layout for registers in core dump notes,
>>> native register sets with ptrace(), and the register contexts saved in
>>> signal frames.  On these systems, a single register map can now be
>>> used to describe the layout of registers in all three places.
>>>
>>> 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.
>>
>> LGTM.  It sounds very good to de-duplicate the knowledge of the register
>> layout in those structures.
> 
> After some discussions on IRC, I've modified this patch a bit.  If we want
> to rename 'regcache_map_entry' to be less regcache-specific then I think
> that might be something to do as a followup change.  The main changes
> relative to the v2 patch are only providing a single new function (the
> one taking a frame_cache) until a caller shows up needing to use the other
> function (taking the saved register array), using a typed pointer for the
> regache_map_entry argument to the new function, and adding some comments to
> regcache.h to note that this structure is now also used by trad_frame as
> well as document the semantics when a register's "slot" in the map doesn't
> match the size of a register.  I've pasted it below rather than rerolling
> the whole series:
> 
> Author: John Baldwin <jhb@FreeBSD.org>
> Date:   Fri Sep 28 14:37:22 2018 -0700
> 
>     Add a helper function to trad_frame to support register cache maps.
>     
>     Currently, signal frame handlers require explicitly coded calls to
>     trad_frame_set_reg_addr() to describe the location of saved registers
>     within a signal frame.  This change permits the regcache_map_entry
>     arrays used with regcache::supply_regset and regcache::collect_regset
>     to be used to describe a block of saved registers given an initial
>     address for the register block.
>     
>     Some systems use the same layout for registers in core dump notes,
>     native register sets with ptrace(), and the register contexts saved in
>     signal frames.  On these systems, a single register map can now be
>     used to describe the layout of registers in all three places.
>     
>     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.
>     
>     While here, document the semantics for both regcache::transfer_regset
>     and trad_frame with respect to register slot's whose size does not
>     match the register's size.
>     
>     gdb/ChangeLog:
>     
>             * regcache.h (struct regcache_map_entry): Note that this type can
>             be used with traditional frame caches.
>             * trad-frame.c (trad_frame_set_reg_regmap): New.
>             * trad-frame.h (trad_frame_set_reg_regmap): New.

Thanks, this LGTM with a nit:

>  /* Mapping between register numbers and offsets in a buffer, for use
> -   in the '*regset' functions below.  In an array of
> -   'regcache_map_entry' each element is interpreted like follows:
> +   in the '*regset' functions below and with traditional frame caches.
> +   In an array of 'regcache_map_entry' each element is interpreted
> +   like follows:
>  
>     - If 'regno' is a register number: Map register 'regno' to the
>       current offset (starting with 0) and increase the current offset
>       by 'size' (or the register's size, if 'size' is zero).  Repeat
>       this with consecutive register numbers up to 'regno+count-1'.
>  
> +     For each described register, if 'size' is larger than the
> +     register's size, the register's value is assumed to be stored in
> +     the first N (where N is the register size) bytes at the current
> +     offset.  The remaining 'size' - N bytes are filled with zeroes by
> +     'regcache_collect_regset' and ignored by other consumers.
> +
> +     If 'size' is smaller than the register's size, only the first
> +     'size' bytes of a register's value are assumed to be stored at
> +     the current offset.  'regcache_collect_regset' copies the first
> +     'size' bytes of a register's value to the output buffer.
> +     'regcache_supply_regset' copies the bytes from the input buffer
> +     into the low 'size' bytes of the register's value leaving the
> +     remaining bytes of the register's value unchanged.  Frame caches
> +     read the 'size' bytes from the stack frame and zero extend them
> +     to generate the register's value.

In that case, regcache_supply_part will set the first 'size' bytes of the
register.  In the case of big-endian, does that mean the high 'size' bytes
will be set?  Perhaps you could say first 'size' bytes instead.

Simon