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

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Luis <luis.machado.foss@gmail.com> writes:

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

It isn't. This method is needed when gdbarch can change from one frame
to the next (or previous, rather). In the case of GCS, its state can be
enabled or disabled between consecutive frames, but in any case the GCS
feature will still be there because its presence only depends on whether
AT_HWCAP has the GCS bit set, which doesn't change at runtime.

From what I understood of this patch series, it looks like FPMR works
the same way so I think it also doesn't need this change in
aarch64_linux_sigframe_prev_arch either.

-- 
Thiago