[V2 18/36] [SFrame-V3] gas: sframe: output new FDE type SFRAME_FDE_TYPE_FLEX
Jens Remus
jremus@linux.ibm.com
Thu Jan 8 15:43:14 GMT 2026
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;
> +{
> + 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)?
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.
Or are there targets where these are not defined (e.g. 32-bit ones
lacking INT64_{MIN|MAX} and UINT64_MAX?
> + 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.
>
> - 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;
> + 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. How would a reg_data value of
SFRAME_V3_FLEX_FDE_REG_ENCODE (0, 0, 0) indicate that there is no
offset_data? If you introduce a fourth tracked item (e.g. SP), how
would the single padding "offset" for RA and FP work?
> + }
> +
> + 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?
> cur_fre->merge_candidate = false;
> }
> else if (sframe_ra_tracking_p ()
> && cfi_insn->u.ri.reg == SFRAME_CFA_RA_REG)
> {
> sframe_fre_set_ra_track (cur_fre, cfi_insn->u.ri.offset);
> + cur_fre->ra_reg = SFRAME_FRE_REG_INVALID;
> + cur_fre->ra_deref_p = true;
Same for sframe_fre_set_ra_track()?
> cur_fre->merge_candidate = false;
> }
> /* This is used to track changes to non-rsp registers, skip all others
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