Re: [PATCH v2 4/5] gdb/aarch64: core file support for FPMR

[Luis <luis.machado.foss@gmail.com>]

On 07/10/2025 13:31, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> Add support for FPMR dumps/reads for core files.
> ---
> Changes from v1->v2:
> * Addressed comments/whitespace/formatting issues.
> 
> Ezra
> 
>   gdb/aarch64-linux-tdep.c | 66 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 66 insertions(+)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index e905be40a8d..5b4bc56210f 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -1518,6 +1518,55 @@ aarch64_linux_collect_zt_regset (const struct regset *regset,
>   			    AARCH64_SME2_ZT0_SIZE);
>   }
>   
> +/* Supply register REGNUM from BUF to REGCACHE, using the register map
> +   in REGSET.  If REGNUM is -1, do this for all registers in REGSET.
> +   If BUF is NULL, set the registers to "unavailable" status.  */
> +
> +static void
> +aarch64_linux_supply_fpmr_regset (const struct regset *regset,
> +				  struct regcache *regcache, int regnum,
> +				  const void *buf, size_t size)
> +{
> +  /* Read the FPMR note from a core file into the register buffer.  */
> +
> +  /* Make sure the buffer contains at least the expected amount of data we are
> +     supposed to get.  */
> +  gdb_assert (size >= sizeof (uint64_t));
> +
> +  /* Handle an empty buffer.  */
> +  if (buf == nullptr)
> +    return regcache->supply_regset (regset, regnum, nullptr, size);
> +
> +  aarch64_gdbarch_tdep *tdep
> +      = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +  /* Supply the FPMR register contents.  */
> +  regcache->raw_supply (tdep->fpmr_regnum, buf);
> +}
> +
> +/* Collect register REGNUM from REGCACHE to BUF, using the register
> +   map in REGSET.  If REGNUM is -1, do this for all registers in
> +   REGSET.  */
> +
> +static void
> +aarch64_linux_collect_fpmr_regset (const struct regset *regset,
> +				   const struct regcache *regcache, int regnum,
> +				   void *buf, size_t size)
> +{
> +  /* Read the FPMR contents from the register buffer into the core
> +     file section.  */
> +
> +  /* Make sure the buffer can hold the data we need to return.  */
> +  gdb_assert (size >= sizeof (uint64_t));
> +  gdb_assert (buf != nullptr);
> +
> +  aarch64_gdbarch_tdep *tdep
> +      = gdbarch_tdep<aarch64_gdbarch_tdep> (regcache->arch ());
> +
> +  /* Dump the register cache contents for the FPMR to the buffer.  */
> +  regcache->collect_regset (regset, tdep->fpmr_regnum, buf, sizeof (uint64_t));
> +}
> +
>   /* Implement the "iterate_over_regset_sections" gdbarch method.  */
>   
>   static void
> @@ -1635,6 +1684,23 @@ aarch64_linux_iterate_over_regset_sections (struct gdbarch *gdbarch,
>   	}
>       }
>   
> +  if (tdep->has_fpmr ())
> +    {
> +      const struct regcache_map_entry fpmr_regmap[] =
> +	{
> +	  { 1, tdep->fpmr_regnum, sizeof (uint64_t) }
> +	};
> +
> +	const struct regset aarch64_linux_fpmr_regset =
> +	{
> +	  fpmr_regmap, aarch64_linux_supply_fpmr_regset,
> +	  aarch64_linux_collect_fpmr_regset
> +	};
> +
> +	cb (".reg-aarch-fpmr", sizeof (uint64_t), sizeof (uint64_t),
> +	    &aarch64_linux_fpmr_regset, "FPMR", cb_data);
> +    }
> +
>     if (tdep->has_pauth ())
>       {
>         /* Create this on the fly in order to handle the variable location.  */

Given the FPMR register set only has a single register, do you really 
need custom supply/collect functions for it? See the example of the MTE 
register set, where we just provide the register set but use the generic 
collect/supply functions.

I think we could do the same for FPMR here, unless there is a reason for 
the custom functions, which I'm not seeing at the moment.