[PATCH 5/6] gas: scfi: allow additional workflow for split sp update code pattern
Indu Bhagat
indu.bhagat@oracle.com
Thu Aug 8 15:38:52 GMT 2024
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
@@ -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