[PATCH, V2] gas: sframe: fix PR gas/33756

[Jens Remus]

On 1/8/2026 10:37 AM, Indu Bhagat wrote:
> Previously, sframe_xlate_do_remember_state () relied exclusively on
> last_fre (the last Frame Row Entry for the previous PC range block).  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 in the current context).
> 
> 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.  While at at, also
> add a new testcase for s390x.
> 
> Co-Authored-by: Jens Remus <jremus@linux.ibm.com>
> 
> gas/
> 	PR gas/33756
> 	* gen-sframe.c (sframe_xlate_do_remember_state): Use cur_fre.
> gas/testsuite/
> 	* gas/cfi-sframe/cfi-sframe.exp: Adjust common-empty-1 test to
> 	be repurposed as cfi-sframe-x86_64-pr33756.  Add s390x testcase.
> 	* 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.
> 	* gas/cfi-sframe/cfi-sframe-s390x-pr33756.d: New test.
> 	* gas/cfi-sframe/cfi-sframe-s390x-pr33756.s: New test.
> 
> ---
> [Changes in V2]
>   - s390: sframe: test case PR gas/33756 [Jens].
>   - bugfix: Using cur_fre suffices [Jens].
> [End of changes in V2]
> ---
>  gas/gen-sframe.c                              | 12 +++++-----
>  .../gas/cfi-sframe/cfi-sframe-s390x-pr33756.d | 23 ++++++++++++++++++
>  .../gas/cfi-sframe/cfi-sframe-s390x-pr33756.s | 23 ++++++++++++++++++
>  .../cfi-sframe/cfi-sframe-x86_64-pr33756.d    | 24 +++++++++++++++++++
>  .../cfi-sframe/cfi-sframe-x86_64-pr33756.s    | 13 ++++++++++
>  gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  3 ++-
>  gas/testsuite/gas/cfi-sframe/common-empty-1.d | 17 -------------
>  gas/testsuite/gas/cfi-sframe/common-empty-1.s |  5 ----
>  8 files changed, 91 insertions(+), 29 deletions(-)
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-pr33756.d
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-pr33756.s
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-pr33756.d
>  create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-x86_64-pr33756.s
>  delete mode 100644 gas/testsuite/gas/cfi-sframe/common-empty-1.d
>  delete mode 100644 gas/testsuite/gas/cfi-sframe/common-empty-1.s

Reviewed-by: Jens Remus <jremus@linux.ibm.com>

Perhaps rename prev_fre to cur_fre though?

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

> @@ -1349,12 +1349,12 @@ 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;
> +  const struct sframe_row_entry *prev_fre = xlate_ctx->cur_fre;

Nit: Maybe name the local variable "cur_fre"?  .cfi_remember_state does
save the current state.

>  
> -  /* 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)
> +  /* 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"));
> @@ -1363,7 +1363,7 @@ sframe_xlate_do_remember_state (struct sframe_xlate_ctx *xlate_ctx)
>  
>    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;
>  }
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/