From 0ad24c8d6f93db36b78b34b2aad2a913d347d15e Mon Sep 17 00:00:00 2001 From: Indu Bhagat Date: Tue, 30 Dec 2025 14:26:01 -0800 Subject: [PATCH] [SFrame-V3][RFC] sframe: gas: testsuite: enable flex FDE for s390x This commit amalgamates a patch set proposed by Jens Remus to enable the SFrame Version 3 Flexible FDE Type (SFRAME_FDE_TYPE_FLEX) generation for the s390x ABI. Previously, s390x relied on architecture-specific encoding (shifting register numbers into offset fields) to represent register recovery rules. This limited the complexity of CFI that could be supported. With Flex FDE enabled: - s390x can now represent .cfi_def_cfa using non-SP/FP registers. - The architecture-specific function s390_sframe_xlate_do_register () in GAS is replaced by the generic Flex FDE generation path. - The SFrame V3 specific macros for s390x register encoding are removed from libsframe/include, as the generic Flex FDE format handles explicit register columns natively. The testsuite is updated to replace negative tests (which asserted warnings or empty SFrame generation for these patterns) with positive tests verifying valid Flex FDE generation. Co-authored-by: Jens Remus TBD: - Discuss with s390x maintainer. gas/ * config/tc-s390.c (s390_support_flex_fde_p): Return true to enable Flex FDE generation. * gen-sframe.c (s390_sframe_xlate_do_register): Disable s390x specific implementation. (sframe_xlate_do_register): Invoke generic Flex FDE path now that flex FDE generation is supported. gas/testsuite/ * gas/cfi-sframe/cfi-sframe-s390x-err-1.d: Removed. * gas/cfi-sframe/cfi-sframe-s390x-err-1.s: Moved to... * gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.s: ...here. * gas/cfi-sframe/cfi-sframe-s390x-err-2.d: Removed. * gas/cfi-sframe/cfi-sframe-s390x-err-2.s: Moved to... * gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.s: ...here. * gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d: Update to expect Flex FDE output. * gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d: Likewise. * gas/cfi-sframe/cfi-sframe.exp: Run renamed tests. include/ * sframe.h (SFRAME_V3_S390X_OFFSET_IS_REGNUM): Remove. (SFRAME_V3_S390X_OFFSET_ENCODE_REGNUM): Remove. (SFRAME_V3_S390X_OFFSET_DECODE_REGNUM): Remove. libsframe/ * sframe-dump.c (sframe_s390x_offset_regnum_p): Return false for SFrame V3. (sframe_s390x_offset_decode_regnum): Remove V3 support. --- [New in V2] [No changes in V3] This commit is an amalgamation of a set of patches proposed by Jens to use flex FDE for s390x. For ease of review, here are the details of the diffs brought over: makeshift s390x flex FDE use gas/ * config/tc-s390.c (s390_support_flex_fde_p): Return true to enable FLEX FDE generation for s390x. * gen-sframe.c (s390_sframe_xlate_do_register): Remove definition. (sframe_xlate_do_register): Use sframe_support_flex_fde_p () and invoke the FLEX FDE generation path. s390x: Jens' patch to move err tests to flex ones Bring over the testsuite specific component from: commit b4e5e9e6700aec17a36c189f19ae587ded31640c Author: Jens Remus Date: Fri Dec 12 16:03:33 2025 +0100 s390: sframe: gas: Represent .cfi_def_cfa[_register] for non-SP/FP reg. using FLEX_TOPMOST_FRAME Signed-off-by: Jens Remus ChangeLog: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.s: Move to... * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.s: ...here. * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.s: Move to... * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.s: ...here. * gas/testsuite/gas/cfi-sframe/cfi-sframe.exp: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.d: Removed. * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.d: Removed. * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d: New test. * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d: New test. TODO: fixup! s390: sframe: gas: Represent .cfi_def_cfa[_register] for non-SP/FP reg. using FLEX_TOPMOST_FRAME cherry-pick of the following commit commit 9a39216b87e44464718da78bf315fd95f32f5473 Author: Jens Remus Date: Mon Dec 15 15:54:17 2025 +0100 TODO: fixup! s390: sframe: gas: Represent .cfi_def_cfa[_register] for non-SP/FP reg. using FLEX_TOPMOST_FRAME Why does FLEX_TOPMOST_FRAME always have RA tracking information? ChangeLog: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d: bring Jens fix fpra-register-1.d fpra-register-2.d commit 69c3a613a48dc77c5f41b8bd2369662cd8237927 Author: Jens Remus Date: Fri Dec 12 16:05:01 2025 +0100 s390: gas: sframe: Represent .cfi_register FP/RA using FLEX_TOPMOST_FRAME Signed-off-by: Jens Remus still needs adjustment ChangeLog: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d: testsuite: fixup s390x tests now that 2 RA offsets are not always forcefully emitted ChangeLog: * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d * gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d cleanup sframe.h and sframe-dump.c based on Jens' patch commit 69c3a613a48dc77c5f41b8bd2369662cd8237927 Author: Jens Remus Date: Fri Dec 12 16:05:01 2025 +0100 s390: gas: sframe: Represent .cfi_register FP/RA using FLEX_TOPMOST_FRAME Signed-off-by: Jens Remus include/ * sframe.h (SFRAME_V3_S390X_OFFSET_IS_REGNUM): Remove. (SFRAME_V3_S390X_OFFSET_ENCODE_REGNUM): Remove. (SFRAME_V3_S390X_OFFSET_DECODE_REGNUM): Remove. libsframe/ * sframe-dump.c (sframe_s390x_offset_regnum_p): No more necessary for V3, now that it uses FLEX FDE instead. (sframe_s390x_offset_decode_regnum): Likewise. --- gas/config/tc-s390.c | 2 +- gas/gen-sframe.c | 42 +------------------ .../gas/cfi-sframe/cfi-sframe-s390x-err-1.d | 15 ------- .../gas/cfi-sframe/cfi-sframe-s390x-err-2.d | 15 ------- .../cfi-sframe-s390x-fpra-register-1.d | 10 ++--- .../cfi-sframe-s390x-fpra-register-2.d | 8 ++-- .../cfi-sframe-s390x-non-spfp-cfa-1.d | 24 +++++++++++ ...-1.s => cfi-sframe-s390x-non-spfp-cfa-1.s} | 0 .../cfi-sframe-s390x-non-spfp-cfa-2.d | 24 +++++++++++ ...-2.s => cfi-sframe-s390x-non-spfp-cfa-2.s} | 0 gas/testsuite/gas/cfi-sframe/cfi-sframe.exp | 4 +- include/sframe.h | 13 ------ libsframe/sframe-dump.c | 4 +- 13 files changed, 62 insertions(+), 99 deletions(-) delete mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.d delete mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.d create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d rename gas/testsuite/gas/cfi-sframe/{cfi-sframe-s390x-err-1.s => cfi-sframe-s390x-non-spfp-cfa-1.s} (100%) create mode 100644 gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d rename gas/testsuite/gas/cfi-sframe/{cfi-sframe-s390x-err-2.s => cfi-sframe-s390x-non-spfp-cfa-2.s} (100%) diff --git a/gas/config/tc-s390.c b/gas/config/tc-s390.c index 7d7c56dc8a9..535c424ca04 100644 --- a/gas/config/tc-s390.c +++ b/gas/config/tc-s390.c @@ -2910,7 +2910,7 @@ s390_sframe_ra_tracking_p (void) bool s390_support_flex_fde_p (void) { - return false; + return true; } /* Specify the fixed offset to recover RA from CFA. diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c index 3c82bb4e3e7..34e2cd1e39f 100644 --- a/gas/gen-sframe.c +++ b/gas/gen-sframe.c @@ -1485,43 +1485,6 @@ sframe_xlate_do_val_offset (const struct sframe_xlate_ctx *xlate_ctx ATTRIBUTE_U return SFRAME_XLATE_OK; } -/* S390-specific translate DW_CFA_register into SFrame context. - Return SFRAME_XLATE_OK if success. */ - -static int -s390_sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx, - const struct cfi_insn_data *cfi_insn) -{ - /* The scratchpad FRE currently being updated with each cfi_insn - being interpreted. This FRE eventually gets linked in into the - list of FREs for the specific function. */ - struct sframe_row_entry *cur_fre = xlate_ctx->cur_fre; - - gas_assert (cur_fre); - - /* Change the rule for the register indicated by the register number to - be the specified register. Encode the register number as offset by - shifting it to the left by one and setting the least-significant bit - (LSB). The LSB can be used to differentiate offsets from register - numbers, as offsets from CFA are always a multiple of -8 on s390x. */ - if (cfi_insn->u.rr.reg1 == SFRAME_CFA_FP_REG) - sframe_fre_set_fp_track (cur_fre, - SFRAME_V3_S390X_OFFSET_ENCODE_REGNUM (cfi_insn->u.rr.reg2)); - else if (sframe_ra_tracking_p () - && cfi_insn->u.rr.reg1 == SFRAME_CFA_RA_REG) - sframe_fre_set_ra_track (cur_fre, - SFRAME_V3_S390X_OFFSET_ENCODE_REGNUM (cfi_insn->u.rr.reg2)); - /* SFrame does not track SP explicitly. */ - else if (cfi_insn->u.rr.reg1 == SFRAME_CFA_SP_REG) - { - as_warn (_("no SFrame FDE emitted; %s register %u in .cfi_register"), - sframe_register_name (cfi_insn->u.rr.reg1), cfi_insn->u.rr.reg1); - return SFRAME_XLATE_ERR_NOTREPRESENTED; /* Not represented. */ - } - - return SFRAME_XLATE_OK; -} - /* Translate DW_CFA_register into SFrame context. This opcode indicates: Previous value of register1 is register2. This is @@ -1541,10 +1504,7 @@ static int sframe_xlate_do_register (struct sframe_xlate_ctx *xlate_ctx, const struct cfi_insn_data *cfi_insn) { - /* Conditionally invoke S390-specific implementation. */ - if (sframe_get_abi_arch () == SFRAME_ABI_S390X_ENDIAN_BIG) - return s390_sframe_xlate_do_register (xlate_ctx, cfi_insn); - else if (sframe_support_flex_fde_p ()) + if (sframe_support_flex_fde_p ()) { struct sframe_row_entry *cur_fre = xlate_ctx->cur_fre; diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.d deleted file mode 100644 index 5cd31c7da00..00000000000 --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.d +++ /dev/null @@ -1,15 +0,0 @@ -#name: SFrame generation on s390x - .cfi_def_cfa_register with non-SP/FP register -#as: --gsframe -#warning: non-SP/FP register 10 in \.cfi_def_cfa_register -#objdump: --sframe=.sframe -#... -Contents of the SFrame section .sframe: - - Header : - - Version: SFRAME_VERSION_3 - Flags: SFRAME_F_FDE_FUNC_START_PCREL - Num FDEs: 0 - Num FREs: 0 - -#pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.d deleted file mode 100644 index 0397cd84c05..00000000000 --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.d +++ /dev/null @@ -1,15 +0,0 @@ -#name: SFrame generation on s390x - .cfi_def_cfa with non-SP/FP register -#as: --gsframe -#warning: non-SP/FP register 10 in \.cfi_def_cfa -#objdump: --sframe=.sframe -#... -Contents of the SFrame section .sframe: - - Header : - - Version: SFRAME_VERSION_3 - Flags: SFRAME_F_FDE_FUNC_START_PCREL - Num FDEs: 0 - Num FREs: 0 - -#pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d index c53b2477cdc..63219dcae99 100644 --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-1.d @@ -1,4 +1,4 @@ -#name: SFrame generation on s390x - RA and then FP saved in registers +#name: SFrame generation on s390x - RA and then FP saved in FPR registers #objdump: --sframe=.sframe #... Contents of the SFrame section .sframe: @@ -12,11 +12,11 @@ Contents of the SFrame section .sframe: Function Index : - func idx \[0\]: pc = 0x0, size = 26 bytes + func idx \[0\]: pc = 0x0, size = 26 bytes, attr = "F" STARTPC +CFA +FP +RA + 0+0000 +sp\+160 +u +u + - 0+0004 +sp\+160 +u +r16 + - 0+0008 +sp\+160 +r17 +r16 + - 0+0014 +sp\+160 +u +r16 + + 0+0004 +sp\+160 +u +r16\+0 + + 0+0008 +sp\+160 +r17\+0 +r16\+0 + + 0+0014 +sp\+160 +u +r16\+0 + 0+0018 +sp\+160 +u +u + #pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d index f781fb1dee9..d82b4557d94 100644 --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-fpra-register-2.d @@ -12,11 +12,11 @@ Contents of the SFrame section .sframe: Function Index : - func idx \[0\]: pc = 0x0, size = 26 bytes + func idx \[0\]: pc = 0x0, size = 26 bytes, attr = "F" STARTPC +CFA +FP +RA + 0+0000 +sp\+160 +u +u + - 0+0004 +sp\+160 +r17 +U + - 0+0008 +sp\+160 +r17 +r16 + - 0+0014 +sp\+160 +r17 +U + + 0+0004 +sp\+160 +r17\+0 +U + + 0+0008 +sp\+160 +r17\+0 +r16\+0 + + 0+0014 +sp\+160 +r17\+0 +U + 0+0018 +sp\+160 +u +u + #pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d new file mode 100644 index 00000000000..adf92e13232 --- /dev/null +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.d @@ -0,0 +1,24 @@ +#name: SFrame generation on s390x - .cfi_def_cfa_register with non-SP/FP register +#as: --gsframe +#objdump: --sframe=.sframe +#... +Contents of the SFrame section .sframe: + + Header : + + Version: SFRAME_VERSION_3 + Flags: SFRAME_F_FDE_FUNC_START_PCREL + Num FDEs: 1 + Num FREs: 6 + + Function Index : + + func idx \[0\]: pc = 0x0, size = 40 bytes, attr = "F" + STARTPC +CFA +FP +RA + + 0+0000 +sp\+160 +u +u + + 0+0006 +sp\+160 +c\-72 +c\-48 + + 0+000c +sp\+320 +c\-72 +c\-48 + + 0+0010 +r10\+320 +c\-72 +c\-48 + + 0+001c +sp\+160 +u +u + + 0+001e +r10\+320 +c\-72 +c\-48 + +#pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.s similarity index 100% rename from gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-1.s rename to gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-1.s diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d new file mode 100644 index 00000000000..5415d8431fc --- /dev/null +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.d @@ -0,0 +1,24 @@ +#name: SFrame generation on s390x - .cfi_def_cfa with non-SP/FP register +#as: --gsframe +#objdump: --sframe=.sframe +#... +Contents of the SFrame section .sframe: + + Header : + + Version: SFRAME_VERSION_3 + Flags: SFRAME_F_FDE_FUNC_START_PCREL + Num FDEs: 1 + Num FREs: 6 + + Function Index : + + func idx \[0\]: pc = 0x0, size = 40 bytes, attr = "F" + STARTPC +CFA +FP +RA + + 0+0000 +sp\+160 +u +u + + 0+0006 +sp\+160 +c\-72 +c\-48 + + 0+000c +sp\+320 +c\-72 +c\-48 + + 0+0010 +r10\+320 +c\-72 +c\-48 + + 0+001c +sp\+160 +u +u + + 0+001e +r10\+320 +c\-72 +c\-48 + +#pass diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.s b/gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.s similarity index 100% rename from gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-err-2.s rename to gas/testsuite/gas/cfi-sframe/cfi-sframe-s390x-non-spfp-cfa-2.s diff --git a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp index aa2ab4e904c..3aa59b4f8bf 100644 --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe.exp @@ -98,8 +98,6 @@ if { [istarget "s390x*-*-*"] && [gas_sframe_check] } then { run_dump_test "cfi-sframe-s390x-1" run_dump_test "cfi-sframe-s390x-2" run_dump_test "cfi-sframe-s390x-3" - run_dump_test "cfi-sframe-s390x-err-1" - run_dump_test "cfi-sframe-s390x-err-2" run_dump_test "cfi-sframe-s390x-err-3" run_dump_test "cfi-sframe-s390x-err-4" run_dump_test "cfi-sframe-s390x-fpra-offset-1" @@ -108,4 +106,6 @@ if { [istarget "s390x*-*-*"] && [gas_sframe_check] } then { run_dump_test "cfi-sframe-s390x-fpra-register-2" run_dump_test "cfi-sframe-s390x-ra-undefined-1" run_dump_test "cfi-sframe-s390x-pr33756" + run_dump_test "cfi-sframe-s390x-non-spfp-cfa-1" + run_dump_test "cfi-sframe-s390x-non-spfp-cfa-2" } diff --git a/include/sframe.h b/include/sframe.h index aea589e8704..0dc3bd6def4 100644 --- a/include/sframe.h +++ b/include/sframe.h @@ -529,19 +529,6 @@ typedef struct sframe_frame_row_entry_addr4 #define SFRAME_V2_S390X_OFFSET_DECODE_REGNUM(offset) \ ((offset) >> 1) -/* In SFrame V3, change the encoding of register numbers in the SFrame offsets - on s390x by keeping the lower 3 bits aside. - - LSB=0: Stack offset. The s390x ELF ABI mandates that stack register - slots must be 8-byte aligned. - - LSB=1: DWARF register number shifted to the left by three. - Bits 1 and 2 are currently unused. */ -#define SFRAME_V3_S390X_OFFSET_IS_REGNUM(offset) \ - ((offset) & 1) -#define SFRAME_V3_S390X_OFFSET_ENCODE_REGNUM(regnum) \ - (((regnum) << 3) | 1) -#define SFRAME_V3_S390X_OFFSET_DECODE_REGNUM(offset) \ - ((offset) >> 3) - #ifdef __cplusplus } #endif diff --git a/libsframe/sframe-dump.c b/libsframe/sframe-dump.c index cd538564f8e..3d7bbafcdbc 100644 --- a/libsframe/sframe-dump.c +++ b/libsframe/sframe-dump.c @@ -122,7 +122,7 @@ sframe_s390x_offset_regnum_p (int32_t offset, uint8_t ver) if (ver == SFRAME_VERSION_2) return SFRAME_V2_S390X_OFFSET_IS_REGNUM (offset); else if (ver == SFRAME_VERSION_3) - return SFRAME_V3_S390X_OFFSET_IS_REGNUM (offset); + return false; else /* No other version is supported yet. */ sframe_assert (false); @@ -133,8 +133,6 @@ sframe_s390x_offset_decode_regnum (int32_t offset, uint8_t ver) { if (ver == SFRAME_VERSION_2) return SFRAME_V2_S390X_OFFSET_DECODE_REGNUM (offset); - else if (ver == SFRAME_VERSION_3) - return SFRAME_V3_S390X_OFFSET_DECODE_REGNUM (offset); else /* No other version is supported yet. */ sframe_assert (false);