Re: [RFC PATCH 3/5] gdb/aarch64: signal frame support for fpmr

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

On 05/09/2025 14:17, Ezra.Sitorus@arm.com wrote:
> From: Ezra Sitorus <ezra.sitorus@arm.com>
> 
> Add support for FPMR in signal frames and restore contents of FPMR.
> 
> Manually validated with Shrinkwrap (Arm FVP).
> ---
>   gdb/aarch64-linux-tdep.c | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/gdb/aarch64-linux-tdep.c b/gdb/aarch64-linux-tdep.c
> index acb650db514..b4911d36410 100644
> --- a/gdb/aarch64-linux-tdep.c
> +++ b/gdb/aarch64-linux-tdep.c
> @@ -168,6 +168,7 @@
>   #define AARCH64_TPIDR2_MAGIC			0x54504902
>   #define AARCH64_ZT_MAGIC			0x5a544e01
>   #define AARCH64_GCS_MAGIC			0x47435300
> +#define AARCH64_FPMR_MAGIC			0x46504d52
>   
>   /* Defines for the extra_context that follows an AARCH64_EXTRA_MAGIC.  */
>   #define AARCH64_EXTRA_DATAP_OFFSET		8
> @@ -214,6 +215,9 @@
>   /* features_enabled value offset in the GCS signal frame context.  */
>   #define AARCH64_GCS_CONTEXT_FEATURES_ENABLED_OFFSET	16
>   
> +/* FPMR constants. */
> +#define AARCH64_FPMR_OFFSET			8
> +
>   /* Holds information about the signal frame.  */
>   struct aarch64_linux_sigframe
>   {
> @@ -261,6 +265,12 @@ struct aarch64_linux_sigframe
>     uint64_t gcspr;
>     /* Flags indicating which GCS features are enabled for the thread.  */
>     uint64_t gcs_features_enabled;
> +
> +  /* True if we have an FPMR entry in the signal context, false otherwise.  */
> +  bool fpmr_available = false;
> +  /* The Floating Point Mode Register.  */
> +  CORE_ADDR fpmr = 0;
> +
>   };
>   
>   /* Read an aarch64_ctx, returning the magic value, and setting *SIZE to the
> @@ -577,6 +587,22 @@ aarch64_linux_read_signal_frame_info (const frame_info_ptr &this_frame,
>   	    section += size;
>   	    break;
>   	  }
> +	case AARCH64_FPMR_MAGIC:
> +	  {
> +	    gdb_byte buf[8];
> +	    if (target_read_memory (section + AARCH64_FPMR_OFFSET,
> +				    buf, 8) != 0)
> +	      {
> +		warning (_("Failed to read the FPMR section address from the"
> +			   " signal frame context."));
> +		section += size;
> +		break;
> +	      }
> +	    signal_frame.fpmr = extract_unsigned_integer (buf, 8, byte_order);
> +	    signal_frame.fpmr_available = true;
> +	    section += size;
> +	    break;
> +	  }
>   	case AARCH64_EXTRA_MAGIC:
>   	  {
>   	    /* Extra is always the last valid section in reserved and points to
> @@ -740,6 +766,13 @@ aarch64_linux_sigframe_init (const struct tramp_frame *self,
>   	}
>       }
>   
> +      /* Handle FPMR.  */
> +  if (tdep->has_fpmr () && signal_frame.fpmr_available != 0)
> +  {
> +    trad_frame_set_reg_value (this_cache, tdep->fpmr_regnum,
> +			      signal_frame.fpmr);
> +  }
> +
>     /* Restore the tpidr2 register, if the target supports it and if there is
>        an entry for it.  */
>     if (signal_frame.tpidr2_section != 0 && tdep->has_tls ()
> @@ -792,6 +825,7 @@ aarch64_linux_sigframe_prev_arch (const frame_info_ptr &this_frame,
>     aarch64_features features = aarch64_features_from_target_desc (tdesc);
>     features.vq = sve_vq_from_vl (signal_frame.vl);
>     features.svq = (uint8_t) sve_vq_from_vl (signal_frame.svl);
> +  features.fpmr = signal_frame.fpmr_available;

Unrelated, but looks like we're not handling gcs in 
aarch64_linux_sigframe_prev_arch. Is that something we need to do Thiago?

>   
>     struct gdbarch_info info;
>     info.bfd_arch_info = bfd_lookup_arch (bfd_arch_aarch64, bfd_mach_aarch64);

Otherwise, I don't have comments on this patch.

Reviewed-By: Luis Machado <luis.machado.foss@gmail.com>