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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* 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
>