[PATCH 5/6] gas: scfi: allow additional workflow for split sp update code pattern
Alice Carlotti
alice.carlotti@arm.com
Thu Jun 5 10:51:47 GMT 2025
On Thu, Aug 08, 2024 at 08:38:52AM -0700, Indu Bhagat wrote:
> In aarch64, the following code pattern may be used for managing stack
> statically:
>
> mov x16, IMM
> add sp, sp, x16
>
> To allow for such a pattern, the SCFI machinery needs to book-keep some
> more data flow and associated constants. This is done by using the
> scratch reg state in the SCFI state object. Without this special
> treatment to memorize the fact that x16 holds a constant, the update to
> stack would otherwise be deemed untraceable.
>
> Add new APIs to manipulate state in split_sp_state and use them to allow
> these patterns in SCFI.
>
> When a GINSN_TYPE_OTHER is seen, the scratch reg state corresponding to
> the destination register is reset. When any change of flow instruction is
> seen, the scratch reg state of all registers in state REG_SCRATCH_VALUE
> is reset: this is to say that we do not propagate this information across
> control flow.
>
> The previously present testcase of scfi-unsupported-1.s is now
> repurposed as a testcase for a static stack usage with split sp update
> code pattern.
>
> gas/
> * scfi.c (scfi_state_scratch_reg_value_p): New definition.
> (scfi_state_scratch_reg_get_value): Likewise.
> (scfi_state_scratch_reg_reset): Likewise.
> (scfi_state_scratch_reg_reset_value): Likewise.
> (verify_heuristic_traceable_stack_manipulation): Update
> heuristics to allow selected addsub ops with two reg operands.
> (gen_scfi_ops): Add handling for the involved ops.
>
> gas/testsuite/
> * gas/scfi/aarch64/scfi-aarch64.exp: Adjust tests.
> * gas/scfi/aarch64/scfi-static-stack-1.s: ...here.
> * gas/scfi/aarch64/scfi-static-stack-1.d: New test.
> * gas/scfi/aarch64/scfi-static-stack-1.l: New test.
> * gas/scfi/aarch64/scfi-unsupported-1.l: Remove.
> * gas/scfi/aarch64/scfi-unsupported-1.s: Move to...
> ---
> gas/scfi.c | 128 +++++++++++++++---
> .../gas/scfi/aarch64/scfi-aarch64.exp | 2 +-
> .../gas/scfi/aarch64/scfi-static-stack-1.d | 33 +++++
> .../gas/scfi/aarch64/scfi-static-stack-1.l | 2 +
> ...-unsupported-1.s => scfi-static-stack-1.s} | 23 ++--
> .../gas/scfi/aarch64/scfi-unsupported-1.l | 4 -
> 6 files changed, 154 insertions(+), 38 deletions(-)
> create mode 100644 gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.d
> create mode 100644 gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.l
> rename gas/testsuite/gas/scfi/aarch64/{scfi-unsupported-1.s => scfi-static-stack-1.s} (54%)
> delete mode 100644 gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.l
>
> diff --git a/gas/scfi.c b/gas/scfi.c
> index d9a5d59587d..f14118e16af 100644
> --- a/gas/scfi.c
> +++ b/gas/scfi.c
There's a comment on scfi_state that also needs updating to say that rsp = rsp
op reg instructions don't always make te stack size untraceable.
> @@ -219,6 +219,63 @@ scfi_state_scratch_reg_get_loc (scfi_stateS *state, unsigned int reg)
> return value;
> }
>
> +/* Whether REG has previously been saved in the scratch reg state due to
> + a mov imm, reg insn. */
> +
> +static bool
> +scfi_state_scratch_reg_value_p (scfi_stateS *state, unsigned int reg)
> +{
> + bool value_p;
> +
> + if (reg >= MAX_NUM_SCFI_REGS)
> + return false;
> +
> + value_p = state->scratch[reg].state == REG_SCRATCH_VALUE;
> +
> + return value_p;
> +}
> +
> +/* Return the constant value of REG stashed away in the scratch reg state.
> + IMP: Caller _must_ ensure this is done when scfi_state_scratch_reg_value_p ()
> + is true for this to be the intended value. */
> +
> +static offsetT
> +scfi_state_scratch_reg_get_value (scfi_stateS *state, unsigned int reg)
> +{
> + offsetT value = 0;
> +
> + if (scfi_state_scratch_reg_value_p (state, reg))
> + value = state->scratch[reg].r.value;
> +
> + return value;
> +}
> +
> +/* Reset the scratch reg state of the given REG if in state
> + REG_SCRATCH_VALUE. */
> +
> +static void
> +scfi_state_scratch_reg_value_reset (scfi_stateS *state, unsigned int reg)
> +{
> + if (reg >= MAX_NUM_SCFI_REGS)
> + return;
> +
> + if (scfi_state_scratch_reg_value_p (state, reg))
> + {
> + state->scratch[reg].state = REG_SCRATCH_UNKNOWN;
> + state->scratch[reg].r.value = 0;
> + }
> +}
> +
> +/* Reset the scratch reg state of all registers with a state of
> + REG_SCRATCH_VALUE. */
> +
> +static void
> +scfi_state_scratch_reg_value_reset_all (scfi_stateS *state)
> +{
> + for (int i = 0; i < MAX_NUM_SCFI_REGS; i++)
> + scfi_state_scratch_reg_value_reset (state, i);
> +}
> +
> /* Initialize a new SCFI op. */
>
> static scfi_opS *
> @@ -663,6 +720,7 @@ verify_heuristic_traceable_stack_manipulation (ginsnS *ginsn,
> struct ginsn_src *src1;
> struct ginsn_src *src2;
> unsigned int src1_reg;
> + unsigned int src2_reg;
> unsigned int dst_reg;
> enum ginsn_src_type src1_type;
> enum ginsn_src_type src2_type;
> @@ -675,6 +733,7 @@ verify_heuristic_traceable_stack_manipulation (ginsnS *ginsn,
> dst = ginsn_get_dst (ginsn);
>
> src1_reg = ginsn_get_src_reg (src1);
> + src2_reg = ginsn_get_src_reg (src2);
> dst_reg = ginsn_get_dst_reg (dst);
>
> src1_type = ginsn_get_src_type (src1);
> @@ -704,13 +763,17 @@ verify_heuristic_traceable_stack_manipulation (ginsnS *ginsn,
> if (!scfi_state_scratch_reg_loc_p (state, src1_reg))
> possibly_untraceable = true;
> }
> - /* Check add/sub/and insn usage when CFA base register is REG_SP.
> - Any stack size manipulation, including stack realignment is not allowed
> - if CFA base register is REG_SP. */
> + /* A non constant operand with add/sub insn when CFA base register is REG_SP
> + is not OK. */
> else if (dst_type == GINSN_DST_REG && dst_reg == REG_SP
> - && (((gtype == GINSN_TYPE_ADD || gtype == GINSN_TYPE_SUB)
> + && ((ginsn->type == GINSN_TYPE_ADD || ginsn->type == GINSN_TYPE_SUB)
> && src2_type != GINSN_SRC_IMM)
> - || gtype == GINSN_TYPE_AND || gtype == GINSN_TYPE_OTHER))
> + && !scfi_state_scratch_reg_value_p (state, src2_reg))
> + possibly_untraceable = true;
> + /* Any stack size manipulation, including stack realignment is not allowed
> + if CFA base register is REG_SP. */
> + else if (dst_type == GINSN_DST_REG && dst_reg == REG_SP
> + && (gtype == GINSN_TYPE_AND || gtype == GINSN_TYPE_OTHER))
> possibly_untraceable = true;
> /* If a register save operation is seen when REG_SP is untraceable,
> CFI cannot be synthesized for register saves, hence bail out. */
> @@ -830,6 +893,7 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state)
> struct ginsn_src *src2;
> struct ginsn_dst *dst;
> unsigned int src1_reg;
> + unsigned int src2_reg;
> unsigned int dst_reg;
> enum ginsn_src_type src1_type;
> enum ginsn_src_type src2_type;
> @@ -853,6 +917,7 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state)
> dst = ginsn_get_dst (ginsn);
>
> src1_reg = ginsn_get_src_reg (src1);
> + src2_reg = ginsn_get_src_reg (src2);
> dst_reg = ginsn_get_dst_reg (dst);
>
> src1_type = ginsn_get_src_type (src1);
> @@ -933,36 +998,50 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state)
> #endif
>
> }
> + else if (src1_type == GINSN_SRC_IMM && dst_type == GINSN_DST_REG)
> + {
> + scfi_state_update_scratch_reg (state, dst_reg,
> + ginsn_get_src_imm (src1), false);
> + }
> break;
> case GINSN_TYPE_SUB:
> if (src1_type == GINSN_SRC_REG && src1_reg == REG_SP
> - && dst_type == GINSN_DST_REG && dst_reg == REG_SP
> - && src2_type == GINSN_SRC_IMM)
> + && dst_type == GINSN_DST_REG && dst_reg == REG_SP)
> {
> /* Stack inc/dec offset, when generated due to stack push and pop is
> target-specific. Use the value encoded in the ginsn. */
> - state->stack_size += ginsn_get_src_imm (src2);
> + if (src2_type == GINSN_SRC_IMM)
> + offset = ginsn_get_src_imm (src2);
> + else if (src2_type == GINSN_SRC_REG
> + && scfi_state_scratch_reg_value_p (state, src2_reg))
> + offset = scfi_state_scratch_reg_get_value (state, src2_reg);
> + else
> + break;
> +
> + state->stack_size += offset;
> if (state->regs[REG_CFA].base == REG_SP)
> - {
> - /* push reg. */
> - scfi_op_add_cfa_offset_dec (state, ginsn, ginsn_get_src_imm (src2));
> - }
> + scfi_op_add_cfa_offset_dec (state, ginsn, offset);
> }
> break;
> case GINSN_TYPE_ADD:
> if (src1_type == GINSN_SRC_REG && src1_reg == REG_SP
> - && dst_type == GINSN_DST_REG && dst_reg == REG_SP
> - && src2_type == GINSN_SRC_IMM)
> + && dst_type == GINSN_DST_REG && dst_reg == REG_SP)
> {
> /* Stack inc/dec offset is target-specific. Use the value
> encoded in the ginsn. */
> - state->stack_size -= ginsn_get_src_imm (src2);
> + if (src2_type == GINSN_SRC_IMM)
> + offset = ginsn_get_src_imm (src2);
> + else if (src2_type == GINSN_SRC_REG
> + && scfi_state_scratch_reg_value_p (state, src2_reg))
> + offset = scfi_state_scratch_reg_get_value (state, src2_reg);
> + else
> + break;
> +
> + state->stack_size -= offset;
> /* pop %reg affects CFA offset only if CFA is currently
> stack-pointer based. */
> if (state->regs[REG_CFA].base == REG_SP)
> - {
> - scfi_op_add_cfa_offset_inc (state, ginsn, ginsn_get_src_imm (src2));
> - }
> + scfi_op_add_cfa_offset_inc (state, ginsn, offset);
> }
> else if (src1_type == GINSN_SRC_REG && src1_reg == REG_FP
> && dst_type == GINSN_DST_REG && dst_reg == REG_SP
> @@ -1000,6 +1079,9 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state)
> }
> }
> break;
> + case GINSN_TYPE_OTHER:
> + scfi_state_scratch_reg_value_reset (state, dst_reg);
> + break;
> default:
> break;
> }
> @@ -1039,9 +1121,15 @@ gen_scfi_ops (ginsnS *ginsn, scfi_stateS *state)
> }
> break;
>
> + case GINSN_DST_UNKNOWN:
> + /* Demarcate the end of the SPLIT_SP_UPDATE pattern if a control flow
> + instruction is seen. */
> + if (ginsn_cofi_p (ginsn))
> + scfi_state_scratch_reg_value_reset_all (state);
> + break;
> +
> default:
> - /* Skip GINSN_DST_UNKNOWN and GINSN_DST_MEM as they are uninteresting
> - currently for SCFI. */
> + /* Skip GINSN_DST_MEM as they are uninteresting currently for SCFI. */
> break;
> }
>
> diff --git a/gas/testsuite/gas/scfi/aarch64/scfi-aarch64.exp b/gas/testsuite/gas/scfi/aarch64/scfi-aarch64.exp
> index 9a0b2856cb4..b4cd7e22f2b 100644
> --- a/gas/testsuite/gas/scfi/aarch64/scfi-aarch64.exp
> +++ b/gas/testsuite/gas/scfi/aarch64/scfi-aarch64.exp
> @@ -33,13 +33,13 @@ if { ([istarget "aarch64-*-*"]) } then {
> run_list_test "scfi-diag-2" "--scfi=experimental"
> run_list_test "scfi-diag-3" "--scfi=experimental"
>
> - run_list_test "scfi-unsupported-1" "--scfi=experimental"
> run_list_test "scfi-unsupported-2" "--scfi=experimental"
>
> run_dump_test "scfi-callee-saved-fp-1"
> run_list_test "scfi-callee-saved-fp-1" "--scfi=experimental --warn"
> run_dump_test "scfi-callee-saved-fp-2"
> run_list_test "scfi-callee-saved-fp-2" "--scfi=experimental --warn"
> + run_list_test "scfi-static-stack-1" "--scfi=experimental"
>
> run_dump_test "scfi-ldrp-1"
> run_list_test "scfi-ldrp-1" "--scfi=experimental --warn"
> diff --git a/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.d b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.d
> new file mode 100644
> index 00000000000..c353158114e
> --- /dev/null
> +++ b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.d
> @@ -0,0 +1,33 @@
> +#as: --scfi=experimental -W
> +#objdump: -Wf
> +#name: Synthesize CFI for ldp ldr instructions
> +#...
> +Contents of the .eh_frame section:
> +
> +00000000 0+0010 00000000 CIE
> + Version: 1
> + Augmentation: "zR"
> + Code alignment factor: 4
> + Data alignment factor: -8
> + Return address column: 30
> + Augmentation data: 1b
> + DW_CFA_def_cfa: r31 (sp) ofs 0
> +
> +00000014 0+0028 00000018 FDE cie=00000000 pc=0+0000..0+002c
> + DW_CFA_advance_loc: 8 to 0000000000000008
> + DW_CFA_def_cfa_offset: 4384
> + DW_CFA_advance_loc: 4 to 000000000000000c
> + DW_CFA_offset: r29 \(x29\) at cfa-4384
> + DW_CFA_offset: r30 \(x30\) at cfa-4376
> + DW_CFA_advance_loc: 4 to 0000000000000010
> + DW_CFA_offset: r19 \(x19\) at cfa-4368
> + DW_CFA_advance_loc: 16 to 0000000000000020
> + DW_CFA_restore: r29 \(x29\)
> + DW_CFA_restore: r30 \(x30\)
> + DW_CFA_advance_loc: 4 to 0000000000000024
> + DW_CFA_restore: r19 \(x19\)
> + DW_CFA_advance_loc: 4 to 0000000000000028
> + DW_CFA_def_cfa_offset: 0
> + DW_CFA_nop
> +
> +#pass
> diff --git a/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.l b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.l
> new file mode 100644
> index 00000000000..6ec24387b8c
> --- /dev/null
> +++ b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.l
> @@ -0,0 +1,2 @@
> +.*Assembler messages:
> +.*7: Warning: SCFI ignores most user-specified CFI directives
> diff --git a/gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.s b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.s
> similarity index 54%
> rename from gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.s
> rename to gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.s
> index c143185d03e..a19854158a3 100644
> --- a/gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.s
> +++ b/gas/testsuite/gas/scfi/aarch64/scfi-static-stack-1.s
> @@ -1,6 +1,6 @@
> -# Testcase where immediate used for stack allocation is a wide
> -# one. Since SCFI does not currently have any data-flow
> -# capabilities, this is currently not supported.
> +# Testcase where stack pointer update is done using a reg-based insn
> +# SCFI must not choke; it is not necessary for the mov and add/sub insn to be
> +# consequent insns
> .global foo
> .type foo, %function
> foo:
> @@ -11,20 +11,17 @@ foo:
> stp x29, x30, [sp]
> .cfi_offset 29, -4384
> .cfi_offset 30, -4376
> - mov x29, sp
> - str x0, [sp, 24]
> - str x1, [sp, 16]
> - add x0, sp, 4096
> - add x0, x0, 112
> + str x19, [sp, 16]
> + .cfi_offset 19, -4368
> + mov x19, x0
> bl bar
> -.L1:
> - str xzr, [sp, 4376]
> -.L2:
> - ldp x29, x30, [sp]
> mov x16, 4384
> - add sp, sp, x16
> + ldp x29, x30, [sp]
> .cfi_restore 29
> .cfi_restore 30
> + ldr x19, [sp, 16]
> + .cfi_restore 19
> + add sp, sp, x16
> .cfi_def_cfa_offset 0
> ret
> .cfi_endproc
> diff --git a/gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.l b/gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.l
> deleted file mode 100644
> index de3ed86250b..00000000000
> --- a/gas/testsuite/gas/scfi/aarch64/scfi-unsupported-1.l
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -.*Assembler messages:
> -.*7: Warning: SCFI ignores most user-specified CFI directives
> -.*9: Error: SCFI: unsupported stack manipulation pattern
> -.*31: Error: SCFI: forward pass failed for func 'foo'
> --
> 2.43.0
>
More information about the Binutils
mailing list