Re: [PATCH] Remove use of deprecated core functions (in NetBSD/ARM)

[John Baldwin <jhb at FreeBSD dot org>]

On 3/6/20 12:59 PM, Christian Biesinger via gdb-patches wrote:
> This is in preparation for deleting deprecated_add_core_fns and
> related code.
> 
> As a side-effect, this makes it possible to read NetBSD/ARM
> core files on non-NetBSD/ARM platforms, subject to PR corefiles/25638.
> 
> I have removed this comment:
> -  /* This is ok: we're running native...  */
> Since we are using the gdbarch from the regcache, we should be
> guaranteed to be calling the right function here, so it shouldn't
> matter whether we are running native.
> 
> Tested by reading a NetBSD/ARM core file on Linux/x86-64 and NetBSD/ARM;
> the "info registers" output matches the one from the system GDB.
> 
> diff --git a/gdb/arm-nbsd-tdep.c b/gdb/arm-nbsd-tdep.c
> index 2642024022..689c4b6219 100644
> --- a/gdb/arm-nbsd-tdep.c
> +++ b/gdb/arm-nbsd-tdep.c
> @@ -35,6 +37,70 @@ static const gdb_byte arm_nbsd_arm_be_breakpoint[] = {0xe6, 0x00, 0x00, 0x11};
>  static const gdb_byte arm_nbsd_thumb_le_breakpoint[] = {0xfe, 0xde};
>  static const gdb_byte arm_nbsd_thumb_be_breakpoint[] = {0xde, 0xfe};
>  
> +/* This matches struct reg from NetBSD's sys/arch/arm/include/reg.h */
> +struct arm_nbsd_reg {
> +  uint32_t reg[13];
> +  uint32_t sp;
> +  uint32_t lr;
> +  uint32_t pc;
> +  uint32_t cpsr;
> +};

I would encourage you to use a regcache_map instead.  It's a bit simpler to
work with, and if NetBSD copies the same structure out as part of the
sigcontext on the signal stack, you can reuse the regcache_map in your
signal frame unwinder.  I've updated the arm, aarch64, and risc-v FreeBSD
architectures to use this if you want to see a reference on how to use a
map, e.g. from arm-fbsd-tdep.c:

/* Register maps.  */

static const struct regcache_map_entry arm_fbsd_gregmap[] =
  {
    { 13, ARM_A1_REGNUM, 4 }, /* r0 ... r12 */
    { 1, ARM_SP_REGNUM, 4 },
    { 1, ARM_LR_REGNUM, 4 },
    { 1, ARM_PC_REGNUM, 4 },
    { 1, ARM_PS_REGNUM, 4 },
    { 0 }
  };

...

/* Register set definitions.  */

const struct regset arm_fbsd_gregset =
  {
    arm_fbsd_gregmap,
    regcache_supply_regset, regcache_collect_regset
  };

...

/* Implement the "iterate_over_regset_sections" gdbarch method.  */

static void
arm_fbsd_iterate_over_regset_sections (struct gdbarch *gdbarch,
				       iterate_over_regset_sections_cb *cb,
				       void *cb_data,
				       const struct regcache *regcache)
{
  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

  cb (".reg", ARM_FBSD_SIZEOF_GREGSET, ARM_FBSD_SIZEOF_GREGSET,
      &arm_fbsd_gregset, NULL, cb_data);


The native target is also able to use the regcache_map, e.g. from
arm-fbsd-nat.c:

/* Fetch register REGNUM from the inferior.  If REGNUM is -1, do this
   for all registers.  */

void
arm_fbsd_nat_target::fetch_registers (struct regcache *regcache, int regnum)
{
  pid_t pid = get_ptrace_pid (regcache->ptid ());

  if (regnum == -1 || getregs_supplies (regnum))
    {
      struct reg regs;

      if (ptrace (PT_GETREGS, pid, (PTRACE_TYPE_ARG3) &regs, 0) == -1)
	perror_with_name (_("Couldn't get registers"));

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

...

And for the signal frame unwinder in arm-fbsd-tdep.c, the single
call to trad_frame_set_reg_regmap() lets you pull in all the register
from the sigcontext in one go:

/* Implement the "init" method of struct tramp_frame.  */

static void
arm_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, ARM_SP_REGNUM);
  CORE_ADDR mcontext_addr = (sp
			     + ARM_SIGFRAME_UCONTEXT_OFFSET
			     + ARM_UCONTEXT_MCONTEXT_OFFSET);
  ULONGEST mcontext_vfp_addr;

  trad_frame_set_reg_regmap (this_cache, arm_fbsd_gregmap, mcontext_addr,
			     regcache_map_entry_size (arm_fbsd_gregmap));


-- 
John Baldwin