Home Original page

[PATCH v2 04/11] s390: Represent FP/RA saved in register in SFrame

Jens Remus jremus@linux.ibm.com
Tue Jun 3 14:29:12 GMT 2025 
Hello Indu,

thank you for the review feedback!

On 02.06.2025 22:36, Indu Bhagat via Binutils wrote:
> On 5/27/25 4:07 AM, Jens Remus wrote:

>> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi
>> index 62040c3fa64f..f576ea988139 100644
>> --- a/libsframe/doc/sframe-spec.texi
>> +++ b/libsframe/doc/sframe-spec.texi
> 
> There is a statement "Currently, these bytes are only used for
> representing stack offsets (for all the currently
> supported ABIs). " in the document that also needs to be amended
> with the current patch series.
> 
> I think we should change that to say something like:
> Currently, these bytes are only used for representing stack offsets
> (for AMD64, and AARCH64 ABIs).  For s390x ABI, the interpretation of
> these bytes may be as stack offsets or even as register numbers.

Good catch!  Will do.

> 
>> @@ -861,16 +861,30 @@ It is recommended that a stack tracer implementation performs the required
>>   checks to ensure that RA remains unchanged only for the topmost stack frame
>>   in the callchain.
>>   -Given the nature of things, the number of stack offsets seen on s390x per
>> -SFrame FRE is either 1, 2, or 3.
>> +In leaf functions the RA and FP may be saved in other registers, such as
>> +floating-point registers (FPRs), instead of on the stack.  To represent this
>> +in the SFrame stack trace format the DWARF register number is encoded as
>> +RA/FP offset using the least-significant bit (LSB) as indication:
> 
> I see that you use language such that you always refer to the var len
> bytes in SFrame FRE as "offsets" for s390x, but weave in the 'DWARF reg
> number storage into the offsets'.

IIUC the "var len bytes in SFrame FRE" contain an amount of
fre_offset_count and fre_offset_size sized signed (8-bit, 16-bit, or
32-bit) offsets and nothing else.  That is why I am referring to them
as the offsets following a FRE (or offsets part of a FRE).

> 
> This is just a note, the statements are clear at least to me.

Sorry, I cannot follow you.  Is my above understanding wrong or is there
other wording you would like to be changed?

> 
>> +offset = (regnum << 1) | 1.  A LSB of zero indicates a stack slot offset.
>> +A LSB of one indicates a DWARF register number, which is interpreted as:
>> +regnum = offset >> 1.  Given the nature of leaf functions, this can only occur
>> +in the topmost frame during stack tracing.  It is recommended that a stack
>> +tracer implementation performs the required checks to ensure that restoring
>> +FP and RA from the said register locations is done only for topmost stack
>> +frame in the callchain.
>> +
>> +Given the nature of things, the number of stack offsets and/or register numbers
>> +seen on s390x per SFrame FRE is either 1, 2, or 3.
>>     Hence, in summary:
>>   -@multitable {Offset ID} {Interpretation in s390x in X}
>> +@multitable @columnfractions .15 .85
>>   @headitem Offset ID @tab Interpretation in s390x
>>   @item 1 @tab CFA = @code{BASE_REG} + offset1
>> -@item 2 @tab RA = CFA + offset2
>> -@item 3 @tab FP = CFA + offset3
>> +@item 2 @tab RA stack slot = CFA + offset2, if (offset2 & 1 == 0)
>> +           @*RA register number = offset2 >> 1, if (offset2 & 1 == 1)
>> +@item 3 @tab FP stack slot = CFA + offset3, if (offset3 & 1 == 0)
>> +           @*FP register number = offset3 >> 1, if (offset3 & 1 == 1)
>>   @end multitable
>>     The s390x ELF ABI defines the CFA as stack pointer (SP) at call site +160.  The
>> diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c
>> index 1fa508d9bad2..09ecd4faab54 100644
>> --- a/libsframe/sframe-dump.c
>> +++ b/libsframe/sframe-dump.c
>> @@ -40,6 +40,14 @@ is_sframe_abi_arch_aarch64 (sframe_decoder_ctx *sfd_ctx)
>>     return aarch64_p;
>>   }
>>   +/* Return TRUE if the SFrame section is associated with the s390x ABI.  */
>> +
>> +static bool
>> +is_sframe_abi_arch_s390x (sframe_decoder_ctx *sfd_ctx)
>> +{
>> +  return sframe_decoder_get_abi_arch (sfd_ctx) == SFRAME_ABI_S390X_ENDIAN_BIG;
>> +}
>> +
>>   static void
>>   dump_sframe_header (sframe_decoder_ctx *sfd_ctx)
>>   {
>> @@ -175,7 +183,13 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
>>           /* Dump SP/FP info.  */
>>         if (err[1] == 0)
>> -    sprintf (temp, "c%+d", fp_offset);
>> +    {
>> +      if (is_sframe_abi_arch_s390x (sfd_ctx)
>> +          && SFRAME_S390X_OFFSET_IS_REGNUM (fp_offset))
>> +        sprintf (temp, "r%d", SFRAME_S390X_OFFSET_DECODE_REGNUM (fp_offset));
>> +      else
>> +        sprintf (temp, "c%+d", fp_offset);
>> +    }
>>         else
>>       strcpy (temp, "u");
>>         printf ("%-10s", temp);
>> @@ -187,7 +201,13 @@ dump_sframe_func_with_fres (sframe_decoder_ctx *sfd_ctx,
>>         != SFRAME_CFA_FIXED_RA_INVALID)
>>       strcpy (temp, "f");
>>         else if (err[2] == 0)
>> -    sprintf (temp, "c%+d", ra_offset);
>> +    {
>> +      if (is_sframe_abi_arch_s390x (sfd_ctx)
>> +          && SFRAME_S390X_OFFSET_IS_REGNUM (ra_offset))
>> +        sprintf (temp, "r%d", SFRAME_S390X_OFFSET_DECODE_REGNUM (ra_offset));
>> +      else
>> +        sprintf (temp, "c%+d", ra_offset);
>> +    }
>>         else
>>       strcpy (temp, "u");
>>   diff --git a/libsframe/sframe.c b/libsframe/sframe.c
>> index d7acfeccdc24..712551b0ed1a 100644
>> --- a/libsframe/sframe.c
>> +++ b/libsframe/sframe.c
>> @@ -679,7 +679,10 @@ sframe_fre_get_cfa_offset (sframe_decoder_ctx *dctx ATTRIBUTE_UNUSED,
>>     return sframe_get_fre_offset (fre, SFRAME_FRE_CFA_OFFSET_IDX, errp);
>>   }
>>   -/* Get the FP offset from the FRE.  If the offset is invalid, sets errp.  */
>> +/* 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.  */
>>     int32_t
>>   sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
>> @@ -706,7 +709,12 @@ sframe_fre_get_fp_offset (sframe_decoder_ctx *dctx,
>>     return sframe_get_fre_offset (fre, fp_offset_idx, errp);
>>   }
>>   -/* Get the RA offset from the FRE.  If the offset is invalid, sets errp.  */
>> +/* Get the RA offset from the FRE.  If the offset is invalid, sets errp.
>> +
>> +   For s390x a RA offset value of SFRAME_FRE_RA_OFFSET_INVALID indicates
> 
> Nit: "an RA offset".  One more place in this patch.

Will grep for "RA offset" in my patches and fix all instances.

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
More information about the Binutils mailing list