[V2 18/36] [SFrame-V3] gas: sframe: output new FDE type SFRAME_FDE_TYPE_FLEX
Jens Remus
jremus@linux.ibm.com
Fri Jan 9 10:23:29 GMT 2026
On 1/9/2026 9:21 AM, Indu Bhagat wrote:
> On 1/8/26 7:43 AM, Jens Remus wrote:
>> On 1/7/2026 9:42 AM, Indu Bhagat wrote:
>>> This patch updates the routines for emission of the new FDE type
>>> SFRAME_FDE_TYPE_FLEX in the SFrame output section. The support for
>>> generating these flex FDEs themselves is added in a subsequent commit.
>>>
>>> For flex FDE type, output_sframe_row_entry_offsets () emits two
>>> offsets for RA tracking, irrespective of the ABI/arch, i.e.,
>>> irrespecitve of whether RA-tracking is enabled. This is because, for
>>> flex FDE type, RA is allowed "non-standard" recovery rules,
>>> e.g., RA = REG or RA = *(REG+offset). In a follow-up patch, when
>>> handling for .cfi_register RA, REG is added, emission code will be
>>> patched up accodingly.
>>>
>>> TBD - gas/gen-sframe.c code can be improved ?
>>>
>>> gas/
>>> * gen-sframe.c (get_udata_size_in_bytes): Get size of unsigned
>>> int data in bytes.
>>> (get_fre_num_offsets): Get number of offsets for
>>> SFRAME_FDE_TYPE_FLEX fde type as well (new argument).
>>> (output_sframe_row_entry_offsets): Output the SFrame FREs for
>>> two different cases (fde type SFRAME_FDE_TYPE_FLEX or the
>>> default).
>>> (output_sframe_row_entry): Update caller.
>>> (output_sframe_funcdesc): Emit func type in func_info2 field.
>>> (sframe_xlate_ctx_init): Initialize flex_p in translation
>>> context object.
>>> (sframe_xlate_ctx_finalize): Likewise.
>>> (sframe_row_entry_initialize): Copy over new data members too.
>>> * gen-sframe.h (struct sframe_row_entry): Add tracking for CFA
>>> dereferencing, frame pointer dereferencing.
>>> (struct sframe_func_entry): Add tracking for flexible FDE type.
>>> (struct sframe_xlate_ctx): Likewise.
>>>
>>> ---
>>> [Changes in V1]
>>> - Rename the FDE type to SFRAME_FDE_TYPE_FLEX. [Jens]
>>> - Fix generation of RA offsets for flex FDE. In RFC, we were emitting
>>> two offsets for flex FDE, irrespctive of whether RA tracking location
>>> was REG/STACK or NONE. This is now changed to: emit RA padding offset
>>> - Bring proper implementation in now and remove FIXME in
>>> output_sframe_row_entry_offsets [Jens].
>>> - gas: sframe: must reset and init ra_reg appropriately.
>>> - Allow reg to be any sized in the offset. In RFC, there was a limit
>>> of 5-bits. Now its lifted. The calculation of the `size of offset`
>>> is adjusted to account for the minimum necessary size, just like the
>>> other offsets.
>>> [End of changes in V1]
>>>
>>> [Changes in V2]
>>> - Bring stub resetting ra_reg, ra_deref_p, fp_deref_p to the rightful
>>> place (from previous commit) [Indu].
>>> - Remove stale comments from output_sframe_row_entry_offsets () that
>>> two offsets are emitted for RA irrespective of whether its tracked
>>> or not [Indu].
>>> - Other minor touch ups.
>>> [End of changes in V2]
>>
>>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>>
>>> @@ -233,6 +233,22 @@ get_offset_size_in_bytes (offsetT value)
>>> return size;
>>> }
>>> +/* Given an unsigned item, return the size in bytes needed to represent it. */
>>> +
>>> +static unsigned int
>>> ++get_udata_size_in_bytes (unsigned int value)
>>
>> What about unsigned long value and adding another case for 64-bit to
>> align to get_offset_size_in_bytes?
>>
>> else if ((sizeof (unsigned long) > 4) && (value <= UINT64_MAX))
>> return 8;
>>
>
> SFrame doesnt allow "offsets" of size > 4 bytes.
>
> /* Size of stack frame offsets in an SFrame Frame Row Entry. A single
> SFrame FRE has all offsets of the same size. Offset size may vary
> across frame row entries. */
> #define SFRAME_FRE_OFFSET_1B 0
> #define SFRAME_FRE_OFFSET_2B 1
> #define SFRAME_FRE_OFFSET_4B 2
I know. get_offset_size_in_bytes() does support signed 8-byte offsets.
>>> +{
>>> + unsigned int size = 0;
>>> +
>>> + if (value <= VALUE_8BIT + 1)
>>
>> While this could be rewritten as (value < VALUE_8BIT) this seems
>> wrong to me:
>>
>> VALUE_8BIT = 0x7F = 127
>> VALUE_8BIT + 1 = 0x80 = 128
>>
>> Shouldn't the test be for (value <= 0xFF)?
>>
>
> Yes, it should be. I messed up.
>
> Together with this, we need to define a new access API for reading the
> register/control data word as unsigned int.
What about:
extern uint32_t
sframe_get_fre_udata (const sframe_frame_row_entry *fre, int idx, int *errp);
>
>> Using the proper condition here reveals the broken "Read the Register/
>> Control Data as unsigned" logic in a subsequent patch. See my feedback
>> there.
>>
>> Is there a reason to use VALUE_{8|16}BIT instead of the standard
>> UINT8_MAX and UINT16_MAX constants?
>>
>> Same for get_offset_size_in_bytes(), which could use the standard
>> INT{8|16|32|64}_MIN and INT{8|16|32|64}_MAX constants.
>>
>
> I am not sure why I didnt use the INTXX_MIN and INTXX_MAX back then. I
> see that stdint.h is fine to use in gas.
>
>> Or are there targets where these are not defined (e.g. 32-bit ones
>> lacking INT64_{MIN|MAX} and UINT64_MAX?
>>
>
> IIUC, this should be fine too.
I'll send a patch.
>
>>> + size = 1;
>>> + else if (value <= VALUE_16BIT + 1)
>>
>> Dito.
>>
>>> + size = 2;
>>> + else
>>> + size = 4;
>>> +
>>> + return size;
>>> +}
>>> #define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_1B 0 /* SFRAME_FRE_OFFSET_1B. */
>>> #define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_2B 1 /* SFRAME_FRE_OFFSET_2B. */
>>> #define SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_4B 2 /* SFRAME_FRE_OFFSET_4B. */
>>> @@ -380,19 +396,36 @@ get_fre_base_reg_id (const struct sframe_row_entry *sframe_fre)
>>> /* Get number of offsets necessary for the SFrame Frame Row Entry. */
>>> static unsigned int
>>> -get_fre_num_offsets (const struct sframe_row_entry *sframe_fre)
>>> +get_fre_num_offsets (const struct sframe_row_entry *sframe_fre,
>>> + bool flex_p)
>>> {
>>> - /* Atleast 1 must always be present (to recover CFA). */
>>> - unsigned int fre_num_offsets = 1;
>>> + /* For SFRAME_FDE_TYPE_FLEX FDE type, each entity (CFA, FP, RA) may carry up
>>> + to two offsets. */
>>> + unsigned int count = flex_p ? 2 : 1;
>>
>> Up to two or exactly two? See my feedback on RA padding location data
>> below.
>>
>
> Up to two.
>
> Following cases may occur for flex FDEs:
> - For ABIs tracking not RA, e.g. AMD64, for some FREs, there may be only
> 2 "offsets" if only CFA recovery rules are specified.
> - If RA recovery rules are available, there will be 4 offsets (2 for CFA,
> 2 for RA)
> - If no RA recovery rule is applicable, but there is an FP recovery rule,
> there will be 5 offsets (2 for CFA, 1 Padding for RA, 2 for FP)
>
> So for RA, there may be 0, 1, or 2 "offsets".
>
> My understanding is that you indicated preference for the above scheme over
> the previous RFC where we were always emitting 2 offsets for RA.
I had thought about two padding "offsets" (one for reg_data and one for
offset_data). But unless .cfi_val_offset is to be supported for FP and
RA (and SP if the need arises), this would only unneccessarily increase
the FDE data size. I now also realize that two padding "offsets" would
not be distinguishable from .cfi_val_offset {SP|FP|RA}, 0 either.
>
> In principle, the scheme in V1/V2 series aligns better with how you approached the matter for default FDEs for s390x.
>
>>> - if (sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> - fre_num_offsets++;
>>> - if (sframe_ra_tracking_p ()
>>> - && (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK
>>> - /* For s390x account padding RA offset, if FP without RA saved. */
>>> - || (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG
>>> - && sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)))
>>> + /* CFA offset (or offsets when flex_p) must always be present. */
>>> + unsigned int fre_num_offsets = count;
>>> +
>>> + /* For flexible frames encoding, there will be two offsets for RA (if RA is
>>> + being tracked). 1 padding offset otherwise. */
>>
>> How can a stack tracer differentiate "1 padding offset" from the
>> following valid RA location information:
>>
>> reg_data = SFRAME_V3_FLEX_FDE_REG_ENCODE (0 /* reg */, 0 /* deref_p */, 0 /* reg_p */);
>> offset_data = sframe_fre->ra_offset;
>>
>
> Above means RA = CFA + offset, i.e., .cfi_val_offset semantics. This is not representable in flex FDEs.
> [Note: this is not RA = *(CFA + offset), for which one will see SFRAME_V3_FLEX_FDE_REG_ENCODE ( 0, 1, 0)]
>
> Similarly FP = CFA + offset is not representable in flex FDEs.
>
> Such values are simply stack locations.
I agree that .cfi_val_offset RA, offset does not seem sensible. Why
should the RA point into the stack?
I also cannot think of a .cfi_val_offset FP, offset use case. But maybe
I am missing something? How would a function know how the FP value at
entry relates to the CFA, except if it does not rely on knowledge on
how it is called (e.g. SP == FP).
For .cfi_val_offset SP, offset the offset value should always be the
architecture/ABI-specific fixed offset used in the implicit SP rule,
which then does not need to be represented in SFRame. But searching
GitHub I found the following different use:
https://github.com/Keyronex/Keyronex/blob/0b70078b0b0bcd0d8b1f10eb4cde8c1f7b07f646/kernel/kern/aarch64/trap.S#L84
Changes SP but instead of adjusting the CFA definition accordingly
defines the CFA as SP+0 and thus uses a non-default .cfi_val_offset SP
offset value.
>>> + if (flex_p)
>>> + {
>>> + if (sframe_fre->ra_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> + fre_num_offsets += count;
>>> + else if (sframe_fre->fp_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> + fre_num_offsets += 1;
>>> + }
>>> + else if (sframe_ra_tracking_p ()
>>> + && (sframe_fre->ra_loc != SFRAME_FRE_ELEM_LOC_NONE
>>> + /* For s390x account padding RA offset, if FP without RA
>>> + saved. */
>>> + || (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG
>>> + && sframe_fre->fp_loc != SFRAME_FRE_ELEM_LOC_NONE)))
>>> fre_num_offsets++;
>>> +
>>> + if (sframe_fre->fp_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> + fre_num_offsets += count;
>>> +
>>> return fre_num_offsets;
>>> }
>>
>>> @@ -588,39 +649,91 @@ sframe_fde_free (struct sframe_func_entry *sframe_fde)
>>> size FRE_OFFSET_SIZE. Write out the offsets in order - CFA, RA, FP. */
>>> static unsigned int
>>> -output_sframe_row_entry_offsets (const struct sframe_func_entry *sframe_fde ATTRIBUTE_UNUSED,
>>> +output_sframe_row_entry_offsets (const struct sframe_func_entry *sframe_fde,
>>> const struct sframe_row_entry *sframe_fre,
>>> unsigned int fre_offset_size)
>>> -
>>> {
>>> unsigned int fre_write_offsets = 0;
>>> unsigned int idx = sframe_fre_offset_func_map_index (fre_offset_size);
>>> gas_assert (idx < SFRAME_FRE_OFFSET_FUNC_MAP_INDEX_MAX);
>>> - /* Write out the offsets in order - cfa, fp, ra. */
>>> - fre_offset_func_map[idx].out_func (sframe_fre->cfa_offset);
>>> - fre_write_offsets++;
>>> -
>>> - if (sframe_ra_tracking_p ())
>>> + if (sframe_fde->fde_flex_p)
>>> {
>>> - if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + /* SFrame FDE of type SFRAME_FDE_TYPE_FLEX. */
>>> + /* Output CFA related FRE offsets. */
>>> + uint32_t reg_data
>>> + = SFRAME_V3_FLEX_FDE_REG_ENCODE (sframe_fre->cfa_base_reg,
>>> + sframe_fre->cfa_deref_p,
>>> + 1 /* reg_p. */);
>>> + offsetT offset_data = sframe_fre->cfa_offset;
>>> + fre_offset_func_map[idx].out_func (reg_data);
>>> + fre_offset_func_map[idx].out_func (offset_data);
>>> + fre_write_offsets += 2;
>>> +
>>> + bool reg_p = false;
>>> + if (sframe_fre->ra_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> {
>>> - fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
>>> - fre_write_offsets++;
>>> + /* Output RA related FRE offsets. */
>>> + reg_p = sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_REG;
>>> + reg_data = SFRAME_V3_FLEX_FDE_REG_ENCODE (sframe_fre->ra_reg,
>>> + sframe_fre->ra_deref_p,
>>> + reg_p);
>>> + offset_data = sframe_fre->ra_offset;
>>> + fre_offset_func_map[idx].out_func (reg_data);
>>> + fre_offset_func_map[idx].out_func (offset_data);
>>> + fre_write_offsets += 2;
>>> }
>>> - /* For s390x write padding RA offset, if FP without RA saved. */
>>> - else if (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG
>>> - && sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + else if (sframe_fre->fp_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> {
>>> + /* If RA is not in REG/STACK, emit RA padding if there are more
>>> + offsets to follow. Note that, emitting
>>> + SFRAME_FRE_RA_OFFSET_INVALID is equivalent to emitting
>>> + SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0). */
>>> fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
>>> - fre_write_offsets++;
>>> + fre_write_offsets += 1;
>>
>> Shouldn't two SFRAME_FRE_RA_OFFSET_INVALID be written? One for reg_data
>> and another for offset_data?
>>
>> reg_data = SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0);
>> offset_data = SFRAME_FRE_RA_OFFSET_INVALID;
>>
>> Otherwise a stack tracer needs to cope with flex FDEs where a FRE's
>> tracked location item can consist of either one or two "offsets", at
>> least for RA.
>
> Yes, it does make the life of stack tracer burdened some additional conditionals to figure out the exact index to read data RA/FP data from. But as you pointed out for the RFC series, emitted two offsets always will be a more bytes emitted than the minimum necessary.
>
> How would a reg_data value of
>> SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0) indicate that there is no
>> offset_data?
>
> The same way we used SFRAME_FRE_RA_OFFSET_INVALID as padding ?
>
> If you introduce a fourth tracked item (e.g. SP), how
>> would the single padding "offset" for RA and FP work?
>>
>
> SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0) can be used as invalid FP offset/padding too. This is because the following:
> fp_reg_data = SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0)
> fp_offset = XX
> means:
> FP = CFA + offset [not FP = *(CFA + offset)]
> The former is not representable in flex FDE (and should not be necessary for CFA, FP, SP, or RA if I am not mistaken). The latter will have SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 1, 0) to indicate deref_p.
>
> IOW, flex FDE do not represent .cfi_val_offset still.
I did not consider that .cfi_val_offset should not be representable
in SFrame V3.
>
>>> + }
>>> +
>>> + if (sframe_fre->fp_loc != SFRAME_FRE_ELEM_LOC_NONE)
>>> + {
>>> + /* Output FP related FRE offsets. */
>>> + reg_p = sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_REG;
>>> + reg_data = SFRAME_V3_FLEX_FDE_REG_ENCODE (sframe_fre->fp_reg,
>>> + sframe_fre->fp_deref_p,
>>> + reg_p);
>>> + offset_data = sframe_fre->fp_offset;
>>> + fre_offset_func_map[idx].out_func (reg_data);
>>> + fre_offset_func_map[idx].out_func (offset_data);
>>> + fre_write_offsets += 2;
>>> }
>>> }
>>> - if (sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + else
>>> {
>>> - fre_offset_func_map[idx].out_func (sframe_fre->fp_offset);
>>> + /* SFrame FDE of type SFRAME_FDE_TYPE_DEFAULT. */
>>> + /* Output CFA related FRE offsets. */
>>> + fre_offset_func_map[idx].out_func (sframe_fre->cfa_offset);
>>> fre_write_offsets++;
>>> +
>>> + if (sframe_ra_tracking_p ())
>>> + {
>>> + if (sframe_fre->ra_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + {
>>> + fre_offset_func_map[idx].out_func (sframe_fre->ra_offset);
>>> + fre_write_offsets++;
>>> + }
>>> + /* For s390x write padding RA offset, if FP without RA saved. */
>>> + else if (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG
>>> + && sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + {
>>> + fre_offset_func_map[idx].out_func (SFRAME_FRE_RA_OFFSET_INVALID);
>>> + fre_write_offsets++;
>>> + }
>>> + }
>>> + if (sframe_fre->fp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>>> + {
>>> + fre_offset_func_map[idx].out_func (sframe_fre->fp_offset);
>>> + fre_write_offsets++;
>>> + }
>>> }
>>> return fre_write_offsets;
>>> @@ -1255,12 +1380,15 @@ sframe_xlate_do_offset (struct sframe_xlate_ctx *xlate_ctx,
>>> {
>>> sframe_fre_set_fp_track (cur_fre, cfi_insn->u.ri.offset);
>>> cur_fre->fp_reg = SFRAME_FRE_REG_INVALID;
>>> + cur_fre->fp_deref_p = true;
>>
>> Why did you decide to set this here instead of in
>> sframe_fre_set_fp_track()? Shouldn't sframe_fre_set_fp_track actually
>> do both, reset fp_reg and set fp_deref_p, as it already sets ra_loc to
>> SFRAME_FRE_ELEM_LOC_STACK?
>>
>
> I wanted to see what abstraction/API will work eventually, until then I
> continued with explicit assignments everywhere. I planned to clean this
> up later.
Ok.
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com
IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/
More information about the Binutils
mailing list