[V2 26/36] [SFrame-V3] gas: sframe: testsuite: handle .cfi_offset for RA

[Jens Remus]

On 1/7/2026 9:42 AM, Indu Bhagat wrote:
> With the introduction of flex FDE type, handling .cfi_offset for
> RA needs adjustment.
> 
> On architectures like x86_64, the return address (RA) is typically saved
> at a fixed offset from the CFA. Previous versions of the SFrame format
> assumed this fixed offset was invariant for the entire function on such
> architectures. Consequently, GAS would warn and suppress SFrame
> generation if it encountered a .cfi_offset directive for the RA
> register that deviated from this fixed default.
> 
> SFrame V3 introduces "Flex FDEs" which allows tracking the RA location
> explicitly even on architectures where it is usually fixed.
> 
> This patch updates sframe_xlate_do_offset () to leverage Flex FDEs. When
> processing a .cfi_offset for the RA register:
>   - The check for non-representable RA offsets is relaxed. If the ABI
>     supports Flex FDEs (SFrame V3), GAS proceeds instead of issuing a
>     warning.
>   - For ABIs without explicit RA tracking (like AMD64), if the RA
>     offset differs from the default fixed offset, the FDE is marked as a
>     Flex FDE, and the new stack location is recorded.
>   - Logic is added to detect when the RA is restored to its standard
>     fixed offset. In this case, the tracking state is reset (ra_loc set
>     to SFRAME_FRE_ELEM_LOC_NONE), deferring to the ABI's default fixed RA
>     offset behavior.
> 
> gas/
> 	* gen-sframe.c (sframe_xlate_do_offset): Support .cfi_offset for RA
> 	by switching to Flex FDEs when necessary.
> gas/testsuite/
> 	* gas/cfi-sframe/cfi-sframe-x86_64-6.d: New test.
> 	* gas/cfi-sframe/cfi-sframe-x86_64-6.s: New test to check
> 	transition of location of REG_RA from register to CFA-8 (default
> 	location on AMD64).  Flex FDE in effect.
> 	* gas/cfi-sframe/cfi-sframe.exp: Add new test.

Reviewed-by: Jens Remus <jremus@linux.ibm.com>

> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c

> @@ -1390,8 +1390,10 @@ sframe_xlate_do_offset (struct sframe_xlate_ctx *xlate_ctx,
>  
>    /* For ABIs not tracking RA, the return address is expected to be in a
>       specific location.  Explicit manourvering to a different offset (than the
> -     default offset) is non-representable in SFrame.  */
> -  if (!sframe_ra_tracking_p () && cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG
> +     default offset) is non-representable in SFrame, unless flex FDE generation
> +     is supported for the ABI.  */
> +  if (!sframe_support_flex_fde_p () && !sframe_ra_tracking_p ()
> +      && cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG
>        && cfi_insn->u.ri.offset != sframe_cfa_ra_offset ())
>      {
>        as_warn (_("no SFrame FDE emitted; %s register %u in .cfi_offset"),
> @@ -1409,13 +1411,42 @@ sframe_xlate_do_offset (struct sframe_xlate_ctx *xlate_ctx,
>        cur_fre->fp_deref_p = true;
>        cur_fre->merge_candidate = false;
>      }
> -  else if (sframe_ra_tracking_p ()
> -	   && cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG)
> +  /* Either the ABI has enabled RA tracking, in which case we must process the
> +     DW_CFA_offset opcode for REG_RA like usual.  Or if the ABI has not enabled
> +     RA tracking, but flex FDE generation is supported, dintinguish between

Nit: s/dintinguish/distinguish/

> +     whether its time to reset the ra tracking state or not.  */

Nit: s/ra/RA/

> +  else if (cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG)
>      {
> -      sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
> -      cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
> -      cur_fre->ra_deref_p = true;
> -      cur_fre->merge_candidate = false;
> +      if (!sframe_ra_tracking_p ())
> +	{
> +	  /* RA is restored to its standard fixed offset.  */

Maybe: s/RA/RA tracking info/

The comment is kind of confusing as what it states is not performed
unconditionally.  Maybe change as follows:

	  /* Reset RA tracking info, if fixed offset.  */

> +	  if (cfi_insn->u.ri.offset == sframe_cfa_ra_offset ())
> +	    {
> +	      cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
> +	      cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_NONE;
> +	      cur_fre->ra_deref_p = false;
> +	      cur_fre->merge_candidate = false;
> +	    }
> +	  /* If flex FDE is supported, update the ra tracking info.  */

Nit: s/ra/RA/

> +	  else if (sframe_support_flex_fde_p ())
> +	    {
> +	      sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
> +	      cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
> +	      cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_STACK;
> +	      cur_fre->ra_deref_p = true;
> +	      cur_fre->merge_candidate = false;
> +
> +	      xlate_ctx->flex_p = true;
> +	    }
> +	}
> +      else if (sframe_ra_tracking_p ())

Can't this be simplified to:

      else
or
      else /* sframe_ra_tracking_p ()  */

If the sframe_ra_tracking_p () case is handled first then the else
would be close to the if condition:

      if (!sframe_ra_tracking_p ())
	{
	  sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
	  cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
	  cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_STACK;
	  cur_fre->ra_deref_p = true;
	  cur_fre->merge_candidate = false;
	}
      else
	{
...
	}

> +	{
> +	  sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
> +	  cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
> +	  cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_STACK;

If I am not mistaken this was not part of the old implementation.  Why
is that required?

> +	  cur_fre->ra_deref_p = true;
> +	  cur_fre->merge_candidate = false;
> +	}
>      }
>    /* This is used to track changes to non-rsp registers, skip all others

Unrelated to this patch: What are non-rsp registers?

>       except FP / RA for now.  */
Maybe the SFRAME_CFA_RA_REG case can be simplified as follows:

  else if (cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG)
    {
      if (!sframe_ra_tracking_p ()
          && cfi_insn->u.ri.offset == sframe_cfa_ra_offset ())
        {
          /* Reset RA tracking info to fixed offset.  */
          cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
          cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_NONE;
          cur_fre->ra_deref_p = false;
          cur_fre->merge_candidate = false;
        }
      else
        {
          sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
          cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
          cur_fre->ra_loc = SFRAME_FRE_ELEM_LOC_STACK;
          cur_fre->ra_deref_p = true;
          cur_fre->merge_candidate = false;

          if (!sframe_ra_tracking_p () && sframe_support_flex_fde_p ())
            xlate_ctx->flex_p = true;
        }
    }

The following common statements could be moved from the inner to the
outer clause.  I left them duplicated as I found that more readable.

  cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
  cur_fre->merge_candidate = false;

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/