[PATCH] libsframe: use proper FRE type when flipping SFrame V3 FREs

[Indu Bhagat]

On 1/19/26 7:46 AM, Jens Remus wrote:
> flip_sframe_fdes_with_fres_v3 was erroneously using a fixed FRE type
> of 0 (= SFRAME_FRE_TYPE_ADDR1) when endianness byte-flipping the FREs,
> regardless of the actual FRE type (i.e. ADDR1, ADDR2, or ADDR4).  This
> only affected cross builds, where the .sframe section data may needed
> to be endianness byte-flipped (e.g. binutils cross-built for s390
> 64-bit (s390x) on x86-64).
> 
> As a consequence objdump/readelf with option --sframe could fail to dump
> e.g. s390 64-bit (s390x) .sframe section data on x86-64 with the
> following error message:
> 
>    Error: SFrame decode failure: Buffer does not contain SFrame data.
> 
> The linker ld-sframe test "PR ld/33401 (Step 1: Create relocatable
> object and check R_*_NONE)" cross-build for s390 64-bit (s390x) on
> x86-64 could fail with ld error message:
> 
>    error in tmpdir/StatePlaying.o(.sframe); no .sframe will be created
> 
> The linker ld-sframe test "PR ld/33401 (Step 1: Create relocatable
> object and check R_*_NONE)" cros-build for s390 64-bit (s390x) on
> x86-64 could fail with BDF assertion:
> 
>    BFD (GNU Binutils) ... assertion fail .../bfd/elf-sframe.c:153
> 
> Add a common cfi-sframe assembler test, that forces a FDE type of
> SFRAME_FRE_TYPE_ADDR2.  When the test is run cross-build it may
> exercise the SFrame FDE and FDE endianness byte flipping.
> 

LGTM.  Thanks!

> libsframe/
> 	PR ld/33810
> 	* sframe.c (sframe_decode_fde_attr_v3): Decode and return FRE
> 	type from FDE attributes.
> 	(flip_sframe_fdes_with_fres_v3): Use proper FRE type from
> 	FDE attributes when flipping FREs.
> 
> gas/testsuite/
> 	PR ld/33810
> 	* gas/cfi-sframe/cfi-sframe.exp (cfi-sframe-common-pr33810): Run
> 	new test.
> 	* gas/cfi-sframe/cfi-sframe-common-pr33810.d: New test.
> 	* gas/cfi-sframe/cfi-sframe-common-pr33810.s: Likewise.
> 
> Bug: https://sourceware.org/pr33810
> Fixes: 8ab6e4c72ab6 ("[SFrame-V3] include: gas: libsframe: split FDE into idx and attr")
> Reported-by: Alan Modra <amodra@gmail.com>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> ---
>   .../cfi-sframe/cfi-sframe-common-pr33810.d    | 23 +++++++++++++++++++
>   .../cfi-sframe/cfi-sframe-common-pr33810.s    |  7 ++++++
>   gas/testsuite/gas/cfi-sframe/cfi-sframe.exp   |  2 ++
>   libsframe/sframe.c                            | 13 +++++------
>   4 files changed, 38 insertions(+), 7 deletions(-)
>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.d
>   create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.s
> 
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.d
> new file mode 100644
> index 000000000000..ae8b76a6effb
> --- /dev/null
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.d
> @@ -0,0 +1,23 @@
> +#as: --gsframe
> +#objdump: --sframe=.sframe
> +#name: SFrame FRE type SFRAME_FRE_TYPE_ADDR2 (PR 33810)
> +#...
> +Contents of the SFrame section .sframe:
> +
> +  Header :
> +
> +    Version: SFRAME_VERSION_3
> +    Flags: SFRAME_F_FDE_FUNC_START_PCREL
> +#?    CFA fixed FP offset: \-?\d+
> +#?    CFA fixed RA offset: \-?\d+
> +    Num FDEs: 1
> +    Num FREs: 3
> +
> +  Function Index :
> +    func idx \[0\]: pc = 0x0, size = 264 bytes
> +    STARTPC + CFA + FP + RA +
> +#...
> +    0+0004 +sp\+16 +u +[uf] +
> +    0+0104 +sp\+32 +u +[uf] +
> +
> +#pass
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.s
> new file mode 100644
> index 000000000000..0cbbd991ca38
> --- /dev/null
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-pr33810.s
> @@ -0,0 +1,7 @@
> +	.cfi_startproc
> +	.long 0
> +	.cfi_def_cfa_offset 16
> +	.ds.b 0x100
> +	.cfi_def_cfa_offset 32
> +	.long 0
> +	.cfi_endproc
> diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> index 2e0942657ae9..a73e5c84033e 100644
> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp
> @@ -53,6 +53,8 @@ if  { ([istarget "x86_64-*-*"] || [istarget "aarch64*-*-*"]
>   
>       run_dump_test "common-empty-2"
>       run_dump_test "common-empty-3"
> +
> +    run_dump_test "cfi-sframe-common-pr33810"
>   }
>   
>   # x86-64 specific tests
> diff --git a/libsframe/sframe.c b/libsframe/sframe.c
> index 52788d8ac152..3f76629665cb 100644
> --- a/libsframe/sframe.c
> +++ b/libsframe/sframe.c
> @@ -647,15 +647,14 @@ sframe_decode_fde_idx_v3 (const char *buf, size_t buf_size,
>   
>   static int
>   sframe_decode_fde_attr_v3 (const char *buf, size_t buf_size,
> -			   uint16_t *num_fres)
> +			   uint16_t *num_fres, uint32_t *fre_type)
>   {
>     if (buf_size < sizeof (sframe_func_desc_attr_v3))
>       return SFRAME_ERR;
>   
> -  /* sfda_func_num_fres is the first member of sframe_func_desc_attr_v3.  */
> -  const struct { uint16_t x; } ATTRIBUTE_PACKED *p = (void *)buf;
> -  *num_fres = p->x;
> -
> +  const sframe_func_desc_attr_v3 *fdap = (sframe_func_desc_attr_v3 *) buf;
> +  *num_fres = fdap->sfda_func_num_fres;
> +  *fre_type = SFRAME_V3_FDE_FRE_TYPE (fdap->sfda_func_info);
>     return 0;
>   }
>   static int
> @@ -855,7 +854,7 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
>         /* Handle FDE attr (only in V3).  */
>         fp = fres + fre_offset;
>         if (to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
> -						   &num_fres))
> +						   &num_fres, &fre_type))
>   	goto bad;
>   
>         if (flip_fde_attr_v3 (fp, buf_end - fp))
> @@ -864,7 +863,7 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
>         fre_bytes_flipped += sizeof (sframe_func_desc_attr_v3);
>   
>         if (!to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
> -						    &num_fres))
> +						    &num_fres, &fre_type))
>   	goto bad;
>   
>         /* Handle FREs.  */