[PATCH] gas: sframe: fix PR gas/33756
Jens Remus
jremus@linux.ibm.com
Wed Jan 7 13:04:48 GMT 2026
On 1/7/2026 9:11 AM, Indu Bhagat via Binutils wrote:
> Previously, sframe_xlate_do_remember_state () relied exclusively on
> last_fre (the last fully constructed Frame Row Entry). At the start of
> a function (before an advance in location), last_fre is NULL. The
> intent of DW_CFA_remember_state, however, is to simply snapshot the
> state of the call frame information at the time. In terms of SFrame
> generation, this means that we should simply look at cur_fre (the
> scratchpad FRE currently being built from initial CFI instructions),
> should last_fre be NULL.
>
> Remove the common-empty-1.s which assumed that .cfi_startproc,
> immediately followed by a .cfi_remember_state is not present out in the
> wild. Its best to not make such an assumption, as such a sequence is
> valid CFI, FWIW.
>
> Initial CFI are arch-specific, so add a new testcase for x86_64 to check
> .cfi_remember_state handling for SFrame is sensible.
I'll send a respective s390x test case in a separate patch.
>
> gas/
> PR gas/33756
> * gen-sframe.c (sframe_xlate_do_remember_state): Use cur_fre if
> last_fre is not set.
> gas/testsuite/
> * gas/cfi-sframe/cfi-sframe.exp: Adjust common-empty-1 test to
> be repurposed as cfi-sframe-x86_64-pr33756.
> * gas/cfi-sframe/common-empty-1.d: Removed.
> * gas/cfi-sframe/common-empty-1.s: Removed.
> * gas/cfi-sframe/cfi-sframe-x86_64-pr33756.d: New test.
> * gas/cfi-sframe/cfi-sframe-x86_64-pr33756.s: New test.
Thanks for fixing!
> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
> @@ -1349,21 +1349,25 @@ sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx,
> static int
> sframe_xlate_do_remember_state (struct sframe_xlate_ctx *xlate_ctx)
> {
> - const struct sframe_row_entry *last_fre = xlate_ctx->last_fre;
> + if (!xlate_ctx->remember_fre)
> + xlate_ctx->remember_fre = sframe_row_entry_new ();
>
> - /* If there is no FRE state to remember, nothing to do here. Return
> - early with non-zero error code, this will cause no SFrame stack trace
> - info for the function involved. */
> - if (!last_fre)
> + const struct sframe_row_entry *prev_fre = xlate_ctx->last_fre;
One could argue whether the local variables should be defined at the top
of the function.
> + /* If there is no previous saved FRE, use the state so far for the snapshot.
I found "previous saved FRE" kind of confusing, but don't have a good
alternative.
> + .cfi_startproc followed by .cfi_remember_state is a valid sequence. */
> + if (!prev_fre)
> + prev_fre = xlate_ctx->cur_fre;
> + /* If there is no previous FRE state to remember, nothing to do here. Return
> + early with non-zero error code, this will cause no SFrame stack trace info
> + for the function involved. */
> + if (!prev_fre)
> {
> as_warn (_("no SFrame FDE emitted; "
> ".cfi_remember_state without prior SFrame FRE state"));
> return SFRAME_XLATE_ERR_INVAL;
> }
>
> - if (!xlate_ctx->remember_fre)
> - xlate_ctx->remember_fre = sframe_row_entry_new ();
> - sframe_row_entry_initialize (xlate_ctx->remember_fre, last_fre);
> + sframe_row_entry_initialize (xlate_ctx->remember_fre, prev_fre);
>
> return SFRAME_XLATE_OK;
> }
This needs to be simplified as follows. sframe_xlate_do_advance_loc
allocates a new current FRE and initializes it with the state of the
previous FRE. So always remembering the current FRE should be the
right thing if I am not mistaken:
@@ -1349,14 +1349,11 @@ sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx,
static int
sframe_xlate_do_remember_state (struct sframe_xlate_ctx *xlate_ctx)
{
+ const struct sframe_row_entry *prev_fre = xlate_ctx->cur_fre;
+
if (!xlate_ctx->remember_fre)
xlate_ctx->remember_fre = sframe_row_entry_new ();
- const struct sframe_row_entry *prev_fre = xlate_ctx->last_fre;
- /* If there is no previous saved FRE, use the state so far for the snapshot.
- .cfi_startproc followed by .cfi_remember_state is a valid sequence. */
- if (!prev_fre)
- prev_fre = xlate_ctx->cur_fre;
/* If there is no previous FRE state to remember, nothing to do here. Return
early with non-zero error code, this will cause no SFrame stack trace info
for the function involved. */
Only with this change GAS' DWARF CFI and SFrame generation behave
truly the same for both of the following variants:
foo:
.cfi_startproc
.cfi_def_cfa_offset 32
.long 0
.cfi_def_cfa_offset 64
.cfi_remember_state
.long 0
.cfi_def_cfa_offset 16
.long 0
.long 0
.cfi_restore_state
.long 0
.cfi_endproc
and
foo:
.cfi_startproc
.cfi_def_cfa_offset 32
.long 0
.cfi_remember_state
.cfi_def_cfa_offset 64
.long 0
.cfi_def_cfa_offset 16
.long 0
.long 0
.cfi_restore_state
.long 0
.cfi_endproc
Otherwise SFrame differs from DWARF CFI in the first case as follows,
because it would remember the previous FRE instead of the current FRE:
.eh_frame:
00000018 000000000000001c 0000001c FDE cie=00000000 pc=0000000000000000..0000000000000014
LOC CFA ra
0000000000000000 rsp+32 c-8
0000000000000004 rsp+64 c-8
0000000000000008 rsp+16 c-8
0000000000000010 rsp+64 c-8 <-- DWARF CFI has +64
.sframe:
func idx [0]: pc = 0x0, size = 20 bytes
STARTPC CFA FP RA
0000000000000000 sp+32 u f
0000000000000004 sp+64 u f
0000000000000008 sp+16 u f
0000000000000010 sp+32 u f <-- SFrame has +32 (because it remembered the previous FRE)
Thanks and 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