Re: [PATCH 1/8] gdb/riscv: use a single regset supply function for riscv fbsd & linux

Andrew Burgess
2021-01-18T14:16:03Z

* Andrew Burgess <andrew.burgess@embecosm.com> [2020-12-02 17:39:25 +0000]:

> The RISC-V x0 register is hard-coded to zero. As such neither Linux
> or FreeBSD supply the value of the register x0 in their core dump
> files.
>
> For FreeBSD we take care of this by manually supplying the value of x0
> in riscv_fbsd_supply_gregset, however we don't do this for Linux. As
> a result after loading a core file on Linux we see this behaviour:
>
> (gdb) p $x0
> $1 = <unavailable>
>
> In this commit I make riscv_fbsd_supply_gregset a common function that
> can be shared between RISC-V for FreeBSD and Linux, this resolves the
> above issue.
>
> There is a similar problem for the two registers `fflags` and `frm`.
> These two floating point related CSRs are a little weird. They are
> separate CSRs in the RISC-V specification, but are actually sub-fields
> of the `fcsr` CSR.
>
> As a result neither Linux or FreeBSD supply the `fflags` or `frm`
> registers as separate fields in their core dumps, and so, after
> restoring a core dump these register are similarly unavailable.
>
> In this commit I supply `fflags` and `frm` by first asking for the
> value of `fcsr`, extracting the two fields, and using these to supply
> the values for `fflags` and `frm`.
>
> gdb/ChangeLog:
>
> * riscv-fbsd-tdep.c (riscv_fbsd_supply_gregset): Delete.
> (riscv_fbsd_gregset): Use riscv_supply_regset.
> (riscv_fbsd_fpregset): Likewise.
> * riscv-linux-tdep.c (riscv_linux_gregset): Likewise.
> (riscv_linux_fregset): Likewise.
> * riscv-tdep.c (riscv_supply_regset): Define new function.
> * riscv-tdep.h (riscv_supply_regset): Declare new function.

I have now pushed this patch only from this series.

I'm going to revise the rest of this series and post a new version.

Thanks,
Andrew

> ---
> gdb/ChangeLog | 10 +++++++++
> gdb/riscv-fbsd-tdep.c | 20 ++---------------
> gdb/riscv-linux-tdep.c | 4 ++--
> gdb/riscv-tdep.c | 50 ++++++++++++++++++++++++++++++++++++++++++
> gdb/riscv-tdep.h | 23 +++++++++++++++++++
> 5 files changed, 87 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/riscv-fbsd-tdep.c b/gdb/riscv-fbsd-tdep.c
> index 6e0eb2bb788..f9eaaa16b25 100644
> --- a/gdb/riscv-fbsd-tdep.c
> +++ b/gdb/riscv-fbsd-tdep.c
> @@ -53,32 +53,16 @@ static const struct regcache_map_entry riscv_fbsd_fpregmap[] =
> { 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
> + riscv_fbsd_gregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> const struct regset riscv_fbsd_fpregset =
> {
> - riscv_fbsd_fpregmap,
> - regcache_supply_regset, regcache_collect_regset
> + riscv_fbsd_fpregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Implement the "iterate_over_regset_sections" gdbarch method. */
> diff --git a/gdb/riscv-linux-tdep.c b/gdb/riscv-linux-tdep.c
> index fce838097e2..86dd536edf4 100644
> --- a/gdb/riscv-linux-tdep.c
> +++ b/gdb/riscv-linux-tdep.c
> @@ -52,14 +52,14 @@ static const struct regcache_map_entry riscv_linux_fregmap[] =
>
> static const struct regset riscv_linux_gregset =
> {
> - riscv_linux_gregmap, regcache_supply_regset, regcache_collect_regset
> + riscv_linux_gregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Define the FP register regset. */
>
> static const struct regset riscv_linux_fregset =
> {
> - riscv_linux_fregmap, regcache_supply_regset, regcache_collect_regset
> + riscv_linux_fregmap, riscv_supply_regset, regcache_collect_regset
> };
>
> /* Define hook for core file support. */
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 4e255056863..a428f79940d 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -3693,6 +3693,56 @@ riscv_init_reggroups ()
> csr_reggroup = reggroup_new ("csr", USER_REGGROUP);
> }
>
> +/* See riscv-tdep.h. */
> +
> +void
> +riscv_supply_regset (const struct regset *regset,
> + struct regcache *regcache, int regnum,
> + const void *regs, size_t len)
> +{
> + regcache->supply_regset (regset, regnum, regs, len);
> +
> + if (regnum == -1 || regnum == RISCV_ZERO_REGNUM)
> + regcache->raw_supply_zeroed (RISCV_ZERO_REGNUM);
> +
> + if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM
> + || regnum == RISCV_CSR_FRM_REGNUM)
> + {
> + int fcsr_regnum = RISCV_CSR_FCSR_REGNUM;
> +
> + /* Ensure that FCSR has been read into REGCACHE. */
> + if (regnum != -1)
> + regcache->supply_regset (regset, fcsr_regnum, regs, len);
> +
> + /* Grab the FCSR value if it is now in the regcache. We must check
> + the status first as, if the register was not supplied by REGSET,
> + this call will trigger a recursive attempt to fetch the
> + registers. */
> + if (regcache->get_register_status (fcsr_regnum) == REG_VALID)
> + {
> + ULONGEST fcsr_val;
> + regcache->raw_read (fcsr_regnum, &fcsr_val);
> +
> + /* Extract the fflags and frm values. */
> + ULONGEST fflags_val = fcsr_val & 0x1f;
> + ULONGEST frm_val = (fcsr_val >> 5) & 0x7;
> +
> + /* And supply these if needed. */
> + if (regnum == -1 || regnum == RISCV_CSR_FFLAGS_REGNUM)
> + regcache->raw_supply_integer (RISCV_CSR_FFLAGS_REGNUM,
> + (gdb_byte *) &fflags_val,
> + sizeof (fflags_val),
> + /* is_signed */ false);
> +
> + if (regnum == -1 || regnum == RISCV_CSR_FRM_REGNUM)
> + regcache->raw_supply_integer (RISCV_CSR_FRM_REGNUM,
> + (gdb_byte *)&frm_val,
> + sizeof (fflags_val),
> + /* is_signed */ false);
> + }
> + }
> +}
> +
> void _initialize_riscv_tdep ();
> void
> _initialize_riscv_tdep ()
> diff --git a/gdb/riscv-tdep.h b/gdb/riscv-tdep.h
> index 5bd3314d450..1064ced1192 100644
> --- a/gdb/riscv-tdep.h
> +++ b/gdb/riscv-tdep.h
> @@ -132,4 +132,27 @@ extern int riscv_abi_flen (struct gdbarch *gdbarch);
> extern std::vector<CORE_ADDR> riscv_software_single_step
> (struct regcache *regcache);
>
> +/* Supply register REGNUM from the buffer REGS (length LEN) into
> + REGCACHE. REGSET describes the layout of the buffer. If REGNUM is -1
> + then all registers described by REGSET are supplied.
> +
> + The register RISCV_ZERO_REGNUM should not be described by REGSET,
> + however, this register (which always has the value 0) will be supplied
> + by this function if requested.
> +
> + The registers RISCV_CSR_FFLAGS_REGNUM and RISCV_CSR_FRM_REGNUM should
> + not be described by REGSET, however, these register will be provided if
> + requested assuming either:
> + (a) REGCACHE already contains the value of RISCV_CSR_FCSR_REGNUM, or
> + (b) REGSET describes the location of RISCV_CSR_FCSR_REGNUM in the REGS
> + buffer.
> +
> + This function can be used as the supply function for either x-regs or
> + f-regs when loading corefiles, and doesn't care which abi is currently
> + in use. */
> +
> +extern void riscv_supply_regset (const struct regset *regset,
> + struct regcache *regcache, int regnum,
> + const void *regs, size_t len);
> +
> #endif /* RISCV_TDEP_H */
> --
> 2.25.4
>