[v4,15/15] gas: Validate SFrame RA tracking and fixed RA offset

Message ID 20240624142334.3283823-16-jremus@linux.ibm.com
State New
Headers
Series sframe: Enhancements to SFrame info generation |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jens Remus June 24, 2024, 2:23 p.m. UTC
  Verify all architectures participating in SFrame generation do define
the mandatory SFrame return address (RA) tracking predicate function
sframe_ra_tracking_p. Do so by explicitly not testing for the macro
SFRAME_FRE_RA_TRACKING as otherwise required.

Verify that architectures not using SFrame RA tracking specify a valid
fixed RA offset.

gas/
	* gen-sframe.c (output_sframe_internal): Validate SFrame
	RA tracking and fixed RA offset.

Suggested-by: Indu Bhagat <indu.bhagat@oracle.com>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes v3 -> v4:
    - Implement review feedback and suggestions from Indu.
    
    Changes v2 -> v3:
    - New patch.
    
    This could be made dependent on ENABLE_CHECKING (configure option
    --enable-checking).

 gas/gen-sframe.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
  

Comments

Indu Bhagat June 25, 2024, 11:57 p.m. UTC | #1
On 6/24/24 07:23, Jens Remus wrote:
> Verify all architectures participating in SFrame generation do define
> the mandatory SFrame return address (RA) tracking predicate function
> sframe_ra_tracking_p. Do so by explicitly not testing for the macro
> SFRAME_FRE_RA_TRACKING as otherwise required.
> 
> Verify that architectures not using SFrame RA tracking specify a valid
> fixed RA offset.
> 
> gas/
> 	* gen-sframe.c (output_sframe_internal): Validate SFrame
> 	RA tracking and fixed RA offset.
> 
> Suggested-by: Indu Bhagat <indu.bhagat@oracle.com>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>

LGTM.

Thanks

> ---
> 
> Notes (jremus):
>      Changes v3 -> v4:
>      - Implement review feedback and suggestions from Indu.
>      
>      Changes v2 -> v3:
>      - New patch.
>      
>      This could be made dependent on ENABLE_CHECKING (configure option
>      --enable-checking).
> 
>   gas/gen-sframe.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> index f83a64518c28..626dc33b71dc 100644
> --- a/gas/gen-sframe.c
> +++ b/gas/gen-sframe.c
> @@ -667,12 +667,16 @@ output_sframe_internal (void)
>        -fno-omit-frame-pointer is used.  */
>     out_one (fixed_fp_offset);
>   
> -  /* Offset for the return address from CFA is fixed for some ABIs
> -     (e.g., AMD64), output a SFRAME_CFA_FIXED_RA_INVALID otherwise.  */
> -#ifdef sframe_ra_tracking_p
> +  /* All ABIs participating in SFrame generation must define
> +     sframe_ra_tracking_p.
> +     When RA tracking (in FREs) is not needed (e.g., AMD64), SFrame assumes
> +     the RA is going to be at a fixed offset from CFA.  Check that the fixed RA
> +     offset is appropriately defined in all cases.  */
>     if (!sframe_ra_tracking_p ())
> -    fixed_ra_offset = sframe_cfa_ra_offset ();
> -#endif
> +    {
> +      fixed_ra_offset = sframe_cfa_ra_offset ();
> +      gas_assert (fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID);
> +    }
>     out_one (fixed_ra_offset);
>   
>     /* None of the AMD64, or AARCH64 ABIs need the auxiliary header.
  

Patch

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index f83a64518c28..626dc33b71dc 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -667,12 +667,16 @@  output_sframe_internal (void)
      -fno-omit-frame-pointer is used.  */
   out_one (fixed_fp_offset);
 
-  /* Offset for the return address from CFA is fixed for some ABIs
-     (e.g., AMD64), output a SFRAME_CFA_FIXED_RA_INVALID otherwise.  */
-#ifdef sframe_ra_tracking_p
+  /* All ABIs participating in SFrame generation must define
+     sframe_ra_tracking_p.
+     When RA tracking (in FREs) is not needed (e.g., AMD64), SFrame assumes
+     the RA is going to be at a fixed offset from CFA.  Check that the fixed RA
+     offset is appropriately defined in all cases.  */
   if (!sframe_ra_tracking_p ())
-    fixed_ra_offset = sframe_cfa_ra_offset ();
-#endif
+    {
+      fixed_ra_offset = sframe_cfa_ra_offset ();
+      gas_assert (fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID);
+    }
   out_one (fixed_ra_offset);
 
   /* None of the AMD64, or AARCH64 ABIs need the auxiliary header.