Re: [PATCH v3 15/15] gas: Validate SFrame RA tracking and fixed RA offset

[Indu Bhagat <indu.bhagat@oracle.com>]

On 4/12/24 07:47, Jens Remus wrote:
> If an architecture uses SFrame return-address (RA) tracking it must
> specify the fixed RA offset as invalid. Otherwise, if an architecture
> does not use RA tracking, it must specify a valid fixed RA offset.
> 
> gas/
> 	* gen-sframe.c: Validate SFrame RA tracking and fixed
> 	RA offset.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
> 
> Notes (jremus):
>      Changes v2 -> v3:
>      - New patch.
>      
>      This could be made dependent on ENABLE_CHECKING (configure option
>      --enable-checking).
> 
>   gas/gen-sframe.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index ca6565b0e45e..7e815f9603ef 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -1532,6 +1532,18 @@ output_sframe (segT sframe_seg)
>     /* Setup the version specific access functions.  */
>     sframe_set_version (SFRAME_VERSION_2);
>   
> +#ifdef SFRAME_FRE_RA_TRACKING
> +  if (sframe_ra_tracking_p ())
> +    /* With RA tracking the fixed RA offset must be invalid.  */
> +    gas_assert (sframe_cfa_ra_offset () == SFRAME_CFA_FIXED_RA_INVALID);
> +  else
> +    /* Without RA tracking the fixed RA offset may not be invalid.  */
> +    gas_assert (sframe_cfa_ra_offset () != SFRAME_CFA_FIXED_RA_INVALID);
> +#else
> +  /* Without RA tracking the fixed RA offset may not be invalid.  */
> +  gas_assert (sframe_cfa_ra_offset () != SFRAME_CFA_FIXED_RA_INVALID);
> +#endif
> +

I am not sure if the detailed checks are worth it here (simply because 
of code patterns that follow).

We use the sframe_cfa_ra_offset () function later and only in 
output_sframe_internal () (shown below).  How about we simply put an 
assert there (and get rid of the proposed thunk above):

#ifdef sframe_ra_tracking_p
   if (!sframe_ra_tracking_p ())
     {
       fixed_ra_offset = sframe_cfa_ra_offset ();
       gas_assert (fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID);
     }
#endif
   out_one (fixed_ra_offset);

fixed_ra_offset is initialized to SFRAME_CFA_FIXED_RA_INVALID in 
output_sframe_internal ().

>     /* Process all fdes and create SFrame stack trace information.  */
>     create_sframe_all ();
>