[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