[V2 19/36] [SFrame-V3] libsframe: testsuite: add new argument to offset access APIs

[Indu Bhagat]

On 1/8/26 7:59 AM, Jens Remus wrote:
> On 1/7/2026 9:42 AM, Indu Bhagat wrote:
>> For FDE type SFRAME_FDE_TYPE_FLEX, the offsets are not only laid out
>> differently, they also have different encoding.
>>
>> Adjust the APIs in libsframe to get stack frame offsets by adding a new
>> argument type.
>>
>> ATM, the stack tracer testsuite is not using this newly externalized API
>> sframe_get_fre_offset.  So not exposing this via the libsframe.ver file
>> is OK for now.
>>
>> At the moment, like the generation routines in GAS, the textual dump
>> routines in sframe-dump.c are also unaware of the FDE type
>> SFRAME_FDE_TYPE_FLEX.  In the next commits, these capabilities will be
>> added.
>>
>> include/
>> 	* sframe-api.h (MAX_NUM_STACK_OFFSETS): Increase the number of
>> 	stack offsets to 6 to accommodate the FDE type
>> 	SFRAME_FDE_TYPE_FLEX.
>> 	(sframe_get_fre_offset): Make extern.
>> 	(sframe_fre_get_cfa_offset): Add new arg.
>> 	(sframe_fre_get_fp_offset): Likewise.
>> 	(sframe_fre_get_ra_offset): Likewise.
>> libsframe/
>> 	* libsframe/sframe-dump.c (dump_sframe_func_with_fres): Pass
>> 	SFRAME_FDE_TYPE_DEFAULT for FDE type.
>> 	* sframe.c (sframe_fre_get_cfa_offset): Handle FDE type.
>> 	(sframe_fre_get_fp_offset): Likewise.
>> 	(sframe_fre_get_ra_offset): Likewise.
>> libsframe/testsuite/
>> 	* libsframe.find/findfre-1.c: Pass SFRAME_FDE_TYPE_DEFAULT for
>> 	FDE type.
>> 	* libsframe.find/findfunc-1.c: Likewise.
>> 	* libsframe.find/plt-findfre-1.c: Likewise.
>> 	* libsframe.find/plt-findfre-2.c: Likewise.
>>
>> ---
>> [Changes in V1]
>>    - Now that the RA offset for flex FDE is not two dummy offsets when
>>      padding, adjust the offset access APIs.
>> [End of changes in V1]
>>
>> [Changes in V2]
>>    - Remove !flex_p check for s390x.  It was initially added when flex
>>      FDE generation was limited to AMD64.  But even if flex FDE is not
>>      generated for s390x, this is unnecessarily restrictive [Indu].
>>    - Use var initialized with SFRAME_FDE_TYPE_DEFAULT instead of magic
>>      constant 0 in testcases [Indu].
>> [End of changes in V2]
> 
>> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> 
>> @@ -970,28 +975,51 @@ sframe_fre_get_cfa_offset (const sframe_decoder_ctx *dctx,
>>   
>>   int32_t
>>   sframe_fre_get_fp_offset (const sframe_decoder_ctx *dctx,
>> -			  const sframe_frame_row_entry *fre, int *errp)
>> +			  const sframe_frame_row_entry *fre,
>> +			  uint32_t fde_type,
>> +			  int *errp)
>>   {
>> -  uint32_t fp_offset_idx = 0;
>> -  int8_t fp_offset = sframe_decoder_get_fixed_fp_offset (dctx);
>> -  /* If the FP offset is not being tracked, return the fixed FP offset
>> -     from the SFrame header.  */
>> -  if (fp_offset != SFRAME_CFA_FIXED_FP_INVALID
>> +  int fp_err = 0;
>> +  int8_t fixed_fp_offset = sframe_decoder_get_fixed_fp_offset (dctx);
>> +  bool flex_p = (fde_type == SFRAME_FDE_TYPE_FLEX);
>> +
>> +  /* Although we need the offset first only for flex FDE, just get the FP
>> +     offset from the FRE for all FDE types now.
> 
> I fail to understand above comment.
> 

This is a bit misplaced.  This was intended to be done with the 
sframe_get_fre_offset call. I thought it might be confusing to see a 
call to get the offset which we return in select cases.

What I meant to say was:
   Although we return the fp_offset from the FRE data in select cases 
(see the path returning fixed_fp_offset below), just get the FP offset 
from the FRE for all FDE types now.

I think its over-explaining the codeflow.  Not necessary, I will remove it.

>> +     In some ABIs, the stack offset to recover RA (using the CFA) from is
>> +     fixed (like AMD64).  In such cases, the stack offset to recover FP will
>> +     appear at the second index.  */
>> +  uint32_t fp_offset_idx = ((sframe_decoder_get_fixed_ra_offset (dctx)
>> +			     != SFRAME_CFA_FIXED_RA_INVALID)
>> +			    ? SFRAME_FRE_RA_OFFSET_IDX
>> +			    : SFRAME_FRE_FP_OFFSET_IDX);
>> +  if (flex_p)
>> +    {
> 
> This needs a comment that it handles the case of a flex FDE single
> padding "offset" for RA.  I still don't understand why
> flex_ra_reg_data == SFRAME_FRE_RA_OFFSET_INVALID cannot be valid
> location information for RA.
> 

OK.

Now I have:

   /* Initialize fp_offset_idx for default FDEs.  In some ABIs, the 
stack offset
      to recover RA (using the CFA) from is fixed (like AMD64).  In such 
cases,
      the stack offset to recover FP will appear at the second index.  */
   uint32_t fp_offset_idx = ((sframe_decoder_get_fixed_ra_offset (dctx)
                              != SFRAME_CFA_FIXED_RA_INVALID)
                             ? SFRAME_FRE_RA_OFFSET_IDX
                             : SFRAME_FRE_FP_OFFSET_IDX);
   if (flex_p)
     {
       uint32_t flex_ra_reg_data
         = sframe_get_fre_udata (fre, SFRAME_FRE_RA_OFFSET_IDX * 2, errp);
       /* In presence of RA padding SFRAME_FRE_RA_OFFSET_INVALID 
(instead of RA
          offsets), adjust the expected index of the FP offset.  */
       if (errp && *errp == 0
           && flex_ra_reg_data == SFRAME_FRE_RA_OFFSET_INVALID)
         fp_offset_idx = SFRAME_FRE_FP_OFFSET_IDX * 2;
       else
         fp_offset_idx = SFRAME_FRE_FP_OFFSET_IDX * 2 + 1;
     }

   /* NB: This errp must be retained if returning fp_offset.  */
   int32_t fp_offset = sframe_get_fre_offset (fre, fp_offset_idx, &fp_err);

   /* If the FP offset is not being tracked, return the fixed FP offset 
from the
      SFrame header:
        - For default FDEs (!flex_p)
        - For flex FDEs, if there were no FP offsets found.  */
   if ((!flex_p || (flex_p && fp_err))
       && fixed_fp_offset != SFRAME_CFA_FIXED_FP_INVALID
       && !sframe_get_fre_ra_undefined_p (fre->fre_info))

      ...

Also removed the stale function-level comment, now that s390x will not 
encode register number in default FDE.

-/* Get the FP offset from the FRE.  If the offset is invalid, sets errp.
-
-   For s390x the offset may be an encoded register number, indicated by
-   LSB set to one, which is only valid in the topmost frame.  */
+/* Get the FP offset from the FRE.  If the offset is invalid, sets 
errp.  */


>> +      int32_t flex_ra_reg_data
>> +	= sframe_get_fre_offset (fre, SFRAME_FRE_RA_OFFSET_IDX * 2, errp);
>> +      if (errp && *errp == 0
>> +	  && flex_ra_reg_data == SFRAME_FRE_RA_OFFSET_INVALID)
>> +	fp_offset_idx = SFRAME_FRE_FP_OFFSET_IDX * 2;
>> +      else
>> +	fp_offset_idx = SFRAME_FRE_FP_OFFSET_IDX * 2 + 1;
>> +    }
>> +
>> +  /* NB: This errp must be retained if returning fp_offset.  */
>> +  int32_t fp_offset = sframe_get_fre_offset (fre, fp_offset_idx, &fp_err);
>> +
>> +  /* For default FDEs, if the FP offset is not being tracked, return the fixed
>> +     FP offset from the SFrame header.  */
>> +  if ((!flex_p || (flex_p && fp_err))
>> +      && fixed_fp_offset != SFRAME_CFA_FIXED_FP_INVALID
>>         && !sframe_get_fre_ra_undefined_p (fre->fre_info))
>>       {
>>         if (errp)
>>   	*errp = 0;
>> -      return fp_offset;
>> +      return fixed_fp_offset;
>>       }
>>   
>> -  /* In some ABIs, the stack offset to recover RA (using the CFA) from is
>> -     fixed (like AMD64).  In such cases, the stack offset to recover FP will
>> -     appear at the second index.  */
>> -  fp_offset_idx = ((sframe_decoder_get_fixed_ra_offset (dctx)
>> -		    != SFRAME_CFA_FIXED_RA_INVALID)
>> -		   ? SFRAME_FRE_RA_OFFSET_IDX
>> -		   : SFRAME_FRE_FP_OFFSET_IDX);
>> -  return sframe_get_fre_offset (fre, fp_offset_idx, errp);
>> +  if (errp)
>> +    *errp = fp_err;
>> +  return fp_offset;
>>   }
>>   
>>   /* Get the RA offset from the FRE.  If the offset is invalid, sets errp.
>> @@ -1003,21 +1031,40 @@ sframe_fre_get_fp_offset (const sframe_decoder_ctx *dctx,
>>   
>>   int32_t
>>   sframe_fre_get_ra_offset (const sframe_decoder_ctx *dctx,
>> -			  const sframe_frame_row_entry *fre, int *errp)
>> -{
>> -  int8_t ra_offset = sframe_decoder_get_fixed_ra_offset (dctx);
>> -  /* If the RA offset was not being tracked, return the fixed RA offset
>> -     from the SFrame header.  */
>> -  if (ra_offset != SFRAME_CFA_FIXED_RA_INVALID
>> +			  const sframe_frame_row_entry *fre,
>> +			  uint32_t fde_type,
>> +			  int *errp)
>> +{
>> +  int ra_err = 0;
>> +  int8_t fixed_ra_offset = sframe_decoder_get_fixed_ra_offset (dctx);
>> +  bool flex_p = (fde_type == SFRAME_FDE_TYPE_FLEX);
>> +
>> +  /* Although we need the offset first only for flex FDE, just get the RA
>> +     offset from the FRE for all FDE types now.  */
> 
> Dito.
> 

Removed this too.

>> +  uint32_t ra_offset_idx = (flex_p
>> +			    ? SFRAME_FRE_RA_OFFSET_IDX * 2 + 1
>> +			    : SFRAME_FRE_RA_OFFSET_IDX);
>> +  /* NB: This errp must be retained if returning ra_offset.  */
>> +  int32_t ra_offset = sframe_get_fre_offset (fre, ra_offset_idx, &ra_err);
>> +
>> +  /* For ABIs where RA offset was not being tracked, return the fixed RA offset
>> +     specified in the the SFrame header, when:
>> +       - for default FDEs (!flex_p)
>> +       - for flex FDEs, if RA offset is solely padding or not present.  */
>> +  if ((!flex_p || (flex_p && ra_err))
>> +      && fixed_ra_offset != SFRAME_CFA_FIXED_RA_INVALID
>>         && !sframe_get_fre_ra_undefined_p (fre->fre_info))
>>       {
>>         if (errp)
>>   	*errp = 0;
>> -      return ra_offset;
>> +      return fixed_ra_offset;
>>       }
>>   
>> -  /* Otherwise, get the RA offset from the FRE.  */
>> -  return sframe_get_fre_offset (fre, SFRAME_FRE_RA_OFFSET_IDX, errp);
>> +  /* Otherwise, return the RA offset from the FRE.  The corresponding errp was
>> +     set earlier.  */
>> +  if (errp)
>> +    *errp = ra_err;
>> +  return ra_offset;
>>   }
> Regards,
> Jens