[PATCH v3 08/11] s390: Store SFrame CFA offset adjusted

Mon Jul 7 15:07:55 GMT 2025

Hello Indu,

thank you for the review and feedback!

On 06.07.2025 01:25, Indu Bhagat via Binutils wrote:
> On 6/27/25 4:08 AM, Jens Remus wrote:
>> In SFrame V2 the size of the offsets following an SFrame FRE can be
>> either signed 8-bit, 16-bit, or 32-bit integer, which the largest offset
> 
> Typo: "with" the largest offset

Sure.  My bad.

>> determining their size:
>>    1. CFA offset from CFA base register
>>    2. RA (stack save slot) offset from CFA, usually -48 on s390x if saved
>>    3. FP (stack save slot) offset from CFA, usually -72 on s390x if saved
>> The FP and RA offsets from CFA, when FP/RA saved on the stack, usually
>> have fixed values that fit into signed 8-bit SFrame offsets.  Likewise
>> the DWARF register numbers on s390x of general registers (GR; 0-15) and
>> floating-point registers (FPR; 16-31), when FP/RA saved in registers.
>> With that the CFA offset from CFA base register has the greatest impact
>> on the signed SFrame offset size.
>>
>> The s390x ELF ABI [1] defines the CFA as stack pointer (SP) at call
>> site +160. [2]  Therefore the minimum CFA offset from CFA base register
>> on s390x is 160.  This does not fit into a signed 8-bit integer and
>> therefore effectively prevents any use of signed 8-bit SFrame offsets
>> on s390x.
>>
>> For s390x store the CFA offset from CFA base register adjusted by -160
>> to enable the use of signed 8-bit SFrame offsets.
>>
>> [1]: s390x ELF ABI, https://github.com/IBM/s390x-abi/releases
>> [2]: s390x ELF ABI, commit 4e38ad9c8a88 ("Document the CFA"),
>>       https://github.com/IBM/s390x-abi/commit/4e38ad9c8a88
>>
>> include/
>>     * sframe.h (SFRAME_S390X_CFA_OFFSET_ADJUSTMENT): Define
>>     s390x-specific CFA offset adjustment.
>>     (SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE,
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE): New s390x-specific
>>     macros.  Use SFRAME_S390X_CFA_OFFSET_ADJUSTMENT to en-/decode
>>     CFA offset.
>>
>> bfd/
>>     * elf64-s390.c (elf_s390x_sframe_plt_fre): Use
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE on CFA offset to store it
>>     adjusted and switch to 8-bit offsets.
>>
>> gas/
>>     * gen-sframe.c (sframe_fre_set_cfa_offset): For s390x use
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE on CFA offset to store it
>>     adjusted.
>>     (sframe_fre_get_cfa_offset): New helper.  For s390x use
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE on CFA offset to undo its
>>     adjustment.
>>     (sframe_xlate_do_def_cfa_register): Use new helper
>>     sframe_fre_get_cfa_offset.
>>
>> libsframe/
>>     * sframe.c (sframe_fre_get_cfa_offset): For s390x use
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE on CFA offset to undo its
>>     adjustment.
>>     * doc/sframe-spec.texi (s390x,
>>     SFRAME_S390X_CFA_OFFSET_ADJUSTMENT,
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE,
>>     SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE): Document s390x-specific
>>     adjustment of CFA offset.
>>
>> libsframe/testsuite/
>>     * libsframe.find/plt-findfre-2.c (add_plt0_fde, add_pltn_fde):
>>     Use SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE to enable use of
>>     1-byte SFrame offsets.
>>
>> Suggested-by: Indu Bhagat <indu.bhagat@oracle.com>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> 
> One comment for sframe-spec.texi below.
> 
> Otherwise, LGTM.
> 
> Reviewed-by: Indu Bhagat <indu.bhagat@oracle.com>

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

>> @@ -134,10 +134,26 @@ sframe_fre_set_cfa_base_reg (struct sframe_row_entry *fre,
>>     fre->merge_candidate = false;
>>   }
>>   +static offsetT
>> +sframe_fre_get_cfa_offset (const struct sframe_row_entry * fre)

The recent fix of relocatable SFrame links now include "sframe-api.h".
This cases above new internal helper to clash with the external SFrame
API function of same name.  For V4 I have prefixed the internal helper
with "__", which I somewhat dislike.  Any better suggestion to resolve?

>> +{
>> +  offsetT offset = fre->cfa_offset;
>> +
>> +  /* For s390x undo adjustment of CFA offset (to enable 8-bit offsets).  */
>> +  if (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG)
>> +    offset = SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE (offset);
>> +
>> +  return offset;
>> +}
>> +
>>   static void
>>   sframe_fre_set_cfa_offset (struct sframe_row_entry *fre,
>>                  offsetT cfa_offset)
>>   {
>> +  /* For s390x adjust CFA offset to enable 8-bit offsets.  */
>> +  if (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG)
>> +    cfa_offset = SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE (cfa_offset);
>> +
>>     fre->cfa_offset = cfa_offset;
>>     fre->merge_candidate = false;
>>   }
>> @@ -1036,7 +1052,7 @@ sframe_xlate_do_def_cfa_register (struct sframe_xlate_ctx *xlate_ctx,
>>       }
>>     sframe_fre_set_cfa_base_reg (cur_fre, cfi_insn->u.r);
>>     if (last_fre)
>> -    sframe_fre_set_cfa_offset (cur_fre, last_fre->cfa_offset);
>> +    sframe_fre_set_cfa_offset (cur_fre, sframe_fre_get_cfa_offset (last_fre));
>>       cur_fre->merge_candidate = false;

>> diff --git a/libsframe/doc/sframe-spec.texi b/libsframe/doc/sframe-spec.texi

>> @@ -144,6 +144,12 @@ an FP/RA offset.
>>   @item SFRAME_FRE_RA_OFFSET_INVALID: Invalid RA offset value (like
>>   SFRAME_CFA_FIXED_RA_INVALID).  Used on s390x as padding offset to represent
>>   FP without RA saved.
>> +@item SFRAME_S390X_CFA_OFFSET_ADJUSTMENT: CFA offset (from CFA base register)
>> +adjustment value.  Used to enable use of 8-bit SFrame offsets on s390x.
>> +@item SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE: Encode CFA offset (i.e. apply
>> +CFA offset adjustment).
>> +@item SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE: Decode CFA offset (i.e. unapply
>> +CFA offset adjustment).
> 
> Comma after i.e. : "i.e.,"
> Instead of unapply, perhaps "revert the CFA offset adjustment" may be better.

Ok.

>>   @end itemize
>>   @end itemize
>>   @@ -865,7 +871,15 @@ case in the topmost stack frame of the callchain.  For FP this may be the case
>>   in any stack frame.
>>     Irrespective of the ABI, the first stack offset is always used to locate the
>> -CFA, by interpreting it as: CFA = @code{BASE_REG} + offset1.
>> +CFA.  On s390x the value of the offset is stored adjusted by the s390x-specific
>> +@code{SFRAME_S390X_CFA_OFFSET_ADJUSTMENT} to enable the use of signed 8-bit
>> +offsets on s390x.
>> +s390x-specific helpers @code{SFRAME_V2_FRE_S390X_CFA_OFFSET_ENCODE} and
>> +@code{SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE} are provided to perform and undo
>> +the adjustment.  The CFA offset can therefore be interpreted as:
>> +CFA = @code{BASE_REG} + @code{SFRAME_V2_FRE_S390X_CFA_OFFSET_DECODE(offset1)}
>> +or
>> +CFA = @code{BASE_REG} + offset1 - @code{SFRAME_S390X_CFA_OFFSET_ADJUSTMENT}.
>>   The identification of the @code{BASE_REG} is done by using the
>>   @code{fre_cfa_base_reg_id} field in the SFrame FRE info word.

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/