[PATCH v1 8/8] aarch64: refactor binary approach for RA signing method to make it extendable

[Richard Earnshaw (lists)]

On 23/10/2024 11:48, Matthieu Longo wrote:
> ---
>  gas/gen-sframe.c | 22 ++++++++++++++--------
>  gas/gen-sframe.h | 17 ++++++++++++++---
>  include/sframe.h |  4 ++--
>  3 files changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index 626dc33b71d..565148d2842 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -254,11 +254,13 @@ static struct sframe_version_ops sframe_ver_ops;
>  
>  static unsigned char
>  sframe_v1_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
> -			unsigned int offset_size, bool mangled_ra_p)
> +			unsigned int offset_size,
> +			ra_signing_method_t ra_signing_info)
>  {
>    unsigned char fre_info;
>    fre_info = SFRAME_V1_FRE_INFO (base_reg, num_offsets, offset_size);
> -  fre_info = SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P (mangled_ra_p, fre_info);
> +  fre_info = SFRAME_V1_FRE_INFO_UPDATE_RA_SIGNING_INFO (ra_signing_info,
> +							fre_info);
>    return fre_info;
>  }
>  
> @@ -292,10 +294,11 @@ sframe_set_version (uint32_t sframe_version ATTRIBUTE_UNUSED)
>  
>  static unsigned char
>  sframe_set_fre_info (unsigned int base_reg, unsigned int num_offsets,
> -		     unsigned int offset_size, bool mangled_ra_p)
> +		     unsigned int offset_size,
> +		     ra_signing_method_t ra_signing_info)
>  {
>    return sframe_ver_ops.set_fre_info (base_reg, num_offsets,
> -				      offset_size, mangled_ra_p);
> +				      offset_size, ra_signing_info);
>  }
>  
>  /* SFrame set func info. */
> @@ -526,7 +529,7 @@ output_sframe_row_entry (symbolS *fde_start_addr,
>    fre_num_offsets = get_fre_num_offsets (sframe_fre);
>    fre_offset_size = sframe_get_fre_offset_size (sframe_fre);
>    fre_info = sframe_set_fre_info (fre_base_reg, fre_num_offsets,
> -				  fre_offset_size, sframe_fre->mangled_ra_p);
> +				  fre_offset_size, sframe_fre->ra_signing_info);
>    out_one (fre_info);
>  
>    idx = sframe_fre_offset_func_map_index (fre_offset_size);
> @@ -879,7 +882,7 @@ sframe_row_entry_new (void)
>    fre->merge_candidate = true;
>    /* Reset the mangled RA status bit to zero by default.  We will initialize it in
>       sframe_row_entry_initialize () with the sticky bit if set.  */
> -  fre->mangled_ra_p = false;
> +  fre->ra_signing_info = ra_no_signing;
>  
>    return fre;
>  }
> @@ -927,7 +930,7 @@ sframe_row_entry_initialize (struct sframe_row_entry *cur_fre,
>    cur_fre->ra_offset = prev_fre->ra_offset;
>    /* Treat RA mangling as a sticky bit.  It retains its value until another
>       .cfi_negate_ra_state is seen.  */
> -  cur_fre->mangled_ra_p = prev_fre->mangled_ra_p;
> +  cur_fre->ra_signing_info = prev_fre->ra_signing_info;
>  }
>  
>  /* Return SFrame register name for SP, FP, and RA, or NULL if other.  */
> @@ -1276,7 +1279,10 @@ sframe_xlate_do_aarch64_negate_ra_state (struct sframe_xlate_ctx *xlate_ctx,
>  
>    gas_assert (cur_fre);
>    /* Toggle the mangled RA status bit.  */
> -  cur_fre->mangled_ra_p = !cur_fre->mangled_ra_p;
> +  cur_fre->ra_signing_info
> +    = ((cur_fre->ra_signing_info == ra_no_signing)
> +       ? ra_signing_sp
> +       : ra_no_signing);
>    cur_fre->merge_candidate = false;
>  
>    return SFRAME_XLATE_OK;
> diff --git a/gas/gen-sframe.h b/gas/gen-sframe.h
> index 8ed46dbb087..35cc01ff0be 100644
> --- a/gas/gen-sframe.h
> +++ b/gas/gen-sframe.h
> @@ -21,11 +21,22 @@
>  #ifndef GENSFRAME_H
>  #define GENSFRAME_H
>  
> +#include "ansidecl.h"
> +
>  #define SFRAME_FRE_ELEM_LOC_REG		0
>  #define SFRAME_FRE_ELEM_LOC_STACK	1
>  
>  #define SFRAME_FRE_BASE_REG_INVAL	((unsigned int)-1)
>  
> +/* Signing method used for return address authentication.
> +   (AArch64 extension)  */
> +
> +typedef enum ATTRIBUTE_PACKED
> +{
> +  ra_no_signing = 0x0,
> +  ra_signing_sp = 0x1,
> +} ra_signing_method_t;
> +
>  /* SFrame Frame Row Entry (FRE).
>  
>     A frame row entry is a slice of the frame and can be valid for a set of
> @@ -52,8 +63,8 @@ struct sframe_row_entry
>       on it.  */
>    bool merge_candidate;
>  
> -  /* Whether the return address is mangled with pauth code.  */
> -  bool mangled_ra_p;
> +  /* Track the PAuth mangling information for RA.  */
> +  ra_signing_method_t ra_signing_info;
>  
>    /* Track CFA base (architectural) register ID.  */
>    unsigned int cfa_base_reg;
> @@ -146,7 +157,7 @@ struct sframe_version_ops
>    unsigned char format_version;    /* SFrame format version.  */
>    /* set SFrame FRE info.  */
>    unsigned char (*set_fre_info) (unsigned int, unsigned int, unsigned int,
> -				 bool);
> +				 ra_signing_method_t);
>    /* set SFrame Func info.  */
>    unsigned char (*set_func_info) (unsigned int, unsigned int, unsigned int);
>  };
> diff --git a/include/sframe.h b/include/sframe.h
> index 347dbc73787..4e7092d0bbe 100644
> --- a/include/sframe.h
> +++ b/include/sframe.h
> @@ -264,8 +264,8 @@ typedef struct sframe_fre_info
>     (((offset_num) & 0xf) << 1) | ((base_reg_id) & 0x1))
>  
>  /* Set the mangled_ra_p bit as indicated.  */
> -#define SFRAME_V1_FRE_INFO_UPDATE_MANGLED_RA_P(mangled_ra_p, fre_info) \
> -  ((((mangled_ra_p) & 0x1) << 7) | ((fre_info) & 0x7f))
> +#define SFRAME_V1_FRE_INFO_UPDATE_RA_SIGNING_INFO(ra_signing_info, fre_info) \
> +  ((((ra_signing_info) & 0x1) << 7) | ((fre_info) & 0x7f))
>  
>  #define SFRAME_V1_FRE_CFA_BASE_REG_ID(data)	  ((data) & 0x1)
>  #define SFRAME_V1_FRE_OFFSET_COUNT(data)	  (((data) >> 1) & 0xf)

This patch is out of scope for me (it's general sframe code, not aarch64), you probably need Indu to review it as she is the primary libsframe maintainer.  However, it would really benefit from some motivational commentary as part of the commit message: why would you want to make this more extendable?  Is there some forthcoming patch that is going to do that?

R.