[PATCH v2 06/11] s390: Add SFrame stack trace information for .plt section

[Jens Remus]

Hello Indu,

thank you for the review feedback!

On 02.06.2025 22:37, Indu Bhagat via Binutils wrote:
> On 5/27/25 4:07 AM, Jens Remus wrote:
>> Enable SFrame stack tracing through PLT entries.  Based on x86-64.
>>
>> On s390x both PLT0 and PLTn entries are 32-bytes in size.  Their code
>> neither alters the stack pointer (SP), frame pointer (FP), nor return
>> address (RA) registers.  Therefore the PLT0 can be represented using
>> a SFrame FDE of type PCINC with a SFrame "null" FRE and the PLTn can
>> be represented using a SFrame FDE of type PCMASK, with a repetition
>> block size of 32 (PLTn size), and a SFrame "null" FRE.
>>
>> Note that as both the PLT0 entry and the PLTn entries have equal size
>> and can both be represented using the identical SFrame "null" FRE, the
>> whole .plt section on s390x could be represented using a single SFrame
>> FDE of type PCMASK, with a repetition block size of 32 (PLT0 and PLTn
>> size), and a single SFrame "null" FRE.  Keep the x86-64 logic with
>> separate SFrame FDEs for PLT0 and PLTn, to ease potential generalization
>> of the .sframe for .plt generation logic among architectures.
>>
> 
> 'SFrame "null" FRE' caught my attention.  Can you add a one liner in
> the commit log explaining that term ? Basically, IIUC, you mean FRE
> with one (CFA) offset of 0 (updated by later commits which scale down
> and adjust the CFA offset).

The term 'SFrame "null" FRE' originates from the x86-64 implementation
elf_x86_64_sframe_null_fre.  I potentially misinterpreted that as the
kind of most basic default FRE that describes any function at entry.
Reviewing the x86-64 code again, I now come to the conclusion that the
elf_x86_64_sframe_null_fre might rather be intended as a NULL value to
initialize any unused FRE pointer array fields.  If my new understanding
is correct, then I would submit a separate cleanup patch that removes
elf_x86_64_sframe_null_fre, as it is not continuously used for that
purpose and it is even misused as FRE for PLT GOT entries.

On s390x the PTLN (and similar for the PLT0) are described by a FDE with
one FRE with one CFA offset of 160.  Subsequent commits first adjust it
by -160, so that the stored offset value is 0, and then additionally
scaled down by 8 (which has no additional effect on the stored value, as
0 / 8 = 0).

I think it would be best if I change the commit message to not use the
term 'SFrame "null" FRE', as it is wrong in this context.

> 
> Overall the SFrame specific logic looks good to me.
> 
> Reviewed-by: Indu Bhagat <indu.bhagat@oracle.com>

Thank you!

>> diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c

>> +/* .sframe FRE covering the PLT0/PLTn .plt section entry.  */
>> +static const sframe_frame_row_entry elf_s390x_sframe_plt_fre =
>> +{
>> +  0, /* SFrame FRE start address.  */
>> +  {0, 160, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
>> +  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_2B) /* FRE info.  */
>> +};
>> +
>> +/* SFrame helper object for PLT.  */
>> +static const struct elf_s390x_sframe_plt elf_s390x_sframe_plt =
>> +{
>> +  PLT_FIRST_ENTRY_SIZE,
>> +  1, /* Number of FREs for PLT0.  */
>> +  /* Array of SFrame FREs for PLT0.  */
>> +  { &elf_s390x_sframe_plt_fre },
>> +
>> +  PLT_ENTRY_SIZE,
>> +  1, /* Number of FREs for PLTn.  */
>> +  /* Array of SFrame FREs for PLTn.  */
>> +  { &elf_s390x_sframe_plt_fre },
>> +};

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/