EH_RETURN_DATA_REGNO is r0/r1 on arm, and r0 is also the return-value
register. When a function calls __builtin_eh_return, r0/r1 are saved in the
prologue so the unwinder can install the landing-pad values into their stack
slots, and the epilogue -- shared at runtime between the normal and the
exceptional return -- restored them and applied the exceptional stack
adjustment "add sp, sp, r2". A normal (non-exceptional) return through such a
function, the rare fall-through case of _Unwind_RaiseException and friends,
therefore reloaded the incoming r0/r1 over the computed result and adjusted sp
by a meaningless r2.
Fix it the same way as the committed aarch64 code: aarch64.h defines
EH_RETURN_TAKEN_RTX (a flag register the generic code sets on the
exceptional path) and aarch64's epilogue, gated on crtl->calls_eh_return,
branches over the EH stack adjustment when the flag is clear. Define
EH_RETURN_TAKEN_RTX on arm likewise and gate the return-path-only work
on a runtime test of that flag.
For ARM and Thumb-2 (arm_expand_epilogue and the APCS variant) this mirrors
aarch64_expand_epilogue: r0/r1 are peeled out of the shared pop and reloaded,
and the "add sp, sp, r2" is applied, only on the exceptional path. The two
data-register slots are discarded on both paths so sp and the remaining
register pop are unchanged. On a normal return the flag is clear, so neither
the reload nor the stack adjustment runs -- the latter matters because
expand_eh_return clobbers EH_RETURN_STACKADJ_RTX (r2) on the normal path (so
the old, always-zero "mov r2, #0" can be deleted), leaving it undefined.
Thumb-1 shares one text-emitted exit (thumb_exit) that applies "add sp, r2"
unconditionally and is too register-starved to test the flag at that late
point. There the epilogue expander instead consumes the flag early -- while it
is still live, before force_register_use anchors r2 into the epilogue -- and
re-zeroes r2 on the normal path, which makes the later unconditional adjustment
a no-op there while the exceptional path keeps the real adjustment.
arm implements the handler transfer with the eh_return pattern rather than
EH_RETURN_HANDLER_RTX, which is left at its NULL default; cast the operands of
the clobber loop in expand_eh_return so its initializer_list type can still be
deduced.
Normal functions are unaffected: every change is gated on
crtl->calls_eh_return, and code generation is byte-identical for every
gcc.c-torture/execute test at -O2 in -marm, -mthumb (Thumb-2) and Thumb-1. The
new run test forces a nonzero value into the r2 argument position and checks,
with an sp canary, that a normal return preserves both the result and sp; it
aborts before the fix and passes after on ARM, Thumb-2 and Thumb-1 under QEMU,
while the exceptional __builtin_eh_return path is unchanged.
gcc/ChangeLog:
PR target/114847
* config/arm/arm.h (ARM_EH_TAKEN_REGNUM): New macro.
(EH_RETURN_TAKEN_RTX): Define.
* config/arm/arm.cc (arm_emit_eh_data_reg_restore): New function.
(arm_emit_eh_stack_adjust): New function.
(arm_expand_epilogue_apcs_frame): Restore the EH data registers and
apply the exceptional stack adjustment only on the exceptional return
path.
(arm_expand_epilogue): Likewise, and cope with an empty saved register
mask once r0/r1 have been peeled out.
* config/arm/arm.md (epilogue): For Thumb-1, zero
EH_RETURN_STACKADJ_RTX on the normal return path so thumb_exit's
unconditional stack adjustment is a no-op there.
* except.cc (expand_eh_return): Cast EH_RETURN_STACKADJ_RTX and
EH_RETURN_HANDLER_RTX to rtx in the clobber loop.
gcc/testsuite/ChangeLog:
PR target/114847
* gcc.target/arm/eh_return-pr114847.c: New test.
* gcc.target/arm/eh_return-pr114847-2.c: New test.
* gcc.target/arm/eh_return-pr114847-3.c: New test.
Signed-off-by: Dominic P <[email protected]>
---
Related work: the PR filer later posted an aarch64 series going the other
way -- deleting EH_RETURN_TAKEN_RTX for a separate exceptional epilogue
(an eh_return_internal insn split after epilogue_completed, the shape of
the 2018 RISC-V fix and of x86). That series has not landed (master still
has EH_RETURN_TAKEN_RTX), and its generic eh_return-1..5.c torture tests
were recorded as failing on arm -- as this very PR. This patch keeps arm
on the committed EH_RETURN_TAKEN_RTX design, which except.cc supports
today and which is the minimal arm-side delta; it should also let those
generic torture tests pass on arm, and arm can follow along if aarch64
later migrates to the two-epilogue design.
This patch was prepared with the assistance of an AI coding tool. Every line of
code, every test and every measurement was written, reviewed and verified by the
author, who takes responsibility for the patch; the Signed-off-by above
certifies
the Developer Certificate of Origin.
gcc/config/arm/arm.cc | 118 ++++++++++++++++--
gcc/config/arm/arm.h | 13 ++
gcc/config/arm/arm.md | 22 +++-
gcc/except.cc | 6 +-
.../gcc.target/arm/eh_return-pr114847-2.c | 34 +++++
.../gcc.target/arm/eh_return-pr114847-3.c | 24 ++++
.../gcc.target/arm/eh_return-pr114847.c | 60 +++++++++
7 files changed, 268 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arm/eh_return-pr114847-2.c
create mode 100644 gcc/testsuite/gcc.target/arm/eh_return-pr114847-3.c
create mode 100644 gcc/testsuite/gcc.target/arm/eh_return-pr114847.c
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc
index 0bc66abe2..dbccf1d8b 100644
--- a/gcc/config/arm/arm.cc
+++ b/gcc/config/arm/arm.cc
@@ -27974,6 +27974,91 @@ thumb1_expand_epilogue (void)
cmse_nonsecure_entry_clear_before_return ();
}
+/* PR target/114847. Emit the conditional restore of the EH data registers
+ (r0, r1) in the epilogue of a function that calls __builtin_eh_return.
+ Those registers are saved in the prologue so that the unwinder can install
+ the landing-pad values into their stack slots, and the shared epilogue
+ would normally pop them. But r0 (and r1 for a 64-bit result) is also the
+ return-value register, so popping them on a *normal* return corrupts the
+ result. Restore them only on the exceptional return path, guarded by a
+ runtime test of EH_RETURN_TAKEN_RTX.
+
+ On entry SP points at the saved r0 slot (r0/r1 are the two lowest-numbered
+ saved registers, hence the two lowest slots). On exit SP has been advanced
+ past both slots on either path, and r0/r1 have been removed from
+ *SAVED_REGS_MASK so that the remaining registers can be popped by the usual
+ emitters. */
+
+static void
+arm_emit_eh_data_reg_restore (unsigned long *saved_regs_mask)
+{
+ rtx_code_label *label = gen_label_rtx ();
+ rtx cond, x, insn;
+
+ gcc_assert ((*saved_regs_mask & ((1 << R0_REGNUM) | (1 << R1_REGNUM)))
+ == ((1 << R0_REGNUM) | (1 << R1_REGNUM)));
+
+ /* if (EH_RETURN_TAKEN_RTX == 0) goto label; -- on a normal return skip the
+ reload so that the result held in r0/r1 is preserved. */
+ cond = arm_gen_compare_reg (EQ, EH_RETURN_TAKEN_RTX, const0_rtx, NULL_RTX);
+ x = gen_rtx_EQ (VOIDmode, cond, const0_rtx);
+ x = gen_rtx_IF_THEN_ELSE (VOIDmode, x,
+ gen_rtx_LABEL_REF (Pmode, label), pc_rtx);
+ emit_jump_insn (gen_rtx_SET (pc_rtx, x));
+
+ /* Exceptional return: reload r0/r1 from their slots, which the unwinder has
+ patched with the values the landing pad expects. */
+ emit_insn (gen_movsi (gen_rtx_REG (SImode, R0_REGNUM),
+ gen_frame_mem (SImode, stack_pointer_rtx)));
+ emit_insn (gen_movsi (gen_rtx_REG (SImode, R1_REGNUM),
+ gen_frame_mem (SImode,
+ plus_constant (Pmode, stack_pointer_rtx,
+ UNITS_PER_WORD))));
+
+ emit_label (label);
+
+ /* Both paths: discard the two EH data-register slots. */
+ insn = emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx,
+ GEN_INT (2 * UNITS_PER_WORD)));
+ arm_add_cfa_adjust_cfa_note (insn, 2 * UNITS_PER_WORD,
+ stack_pointer_rtx, stack_pointer_rtx);
+
+ *saved_regs_mask &= ~((1 << R0_REGNUM) | (1 << R1_REGNUM));
+}
+
+/* PR target/114847. Emit the exception-handler stack adjustment
+ (add sp, sp, r2) in the epilogue of a function that calls
+ __builtin_eh_return, guarded by EH_RETURN_TAKEN_RTX so that it runs only on
+ the exceptional return path. On a normal return the flag is clear and r2
+ (EH_RETURN_STACKADJ_RTX) holds a dataflow-undefined value -- the generic
+ code in expand_eh_return clobbers it on that path so the old, always-zero
+ "mov r2, #0" can be deleted -- so the adjustment must be skipped rather than
+ applied. This mirrors the flag-guarded stack adjustment in
+ aarch64_expand_epilogue. */
+
+static void
+arm_emit_eh_stack_adjust (void)
+{
+ rtx_code_label *label = gen_label_rtx ();
+ rtx cond, x;
+
+ /* if (EH_RETURN_TAKEN_RTX == 0) goto label; -- on a normal return skip the
+ stack adjustment, leaving SP where the register pops left it. */
+ cond = arm_gen_compare_reg (EQ, EH_RETURN_TAKEN_RTX, const0_rtx, NULL_RTX);
+ x = gen_rtx_EQ (VOIDmode, cond, const0_rtx);
+ x = gen_rtx_IF_THEN_ELSE (VOIDmode, x,
+ gen_rtx_LABEL_REF (Pmode, label), pc_rtx);
+ emit_jump_insn (gen_rtx_SET (pc_rtx, x));
+
+ /* Exceptional return: unwind the intervening frames. Letting the CFA move
+ during this adjustment is as correct as retaining it, since we are on the
+ way out to the handler; no CFI note is needed. */
+ emit_insn (gen_addsi3 (stack_pointer_rtx, stack_pointer_rtx,
+ gen_rtx_REG (SImode, ARM_EH_STACKADJ_REGNUM)));
+
+ emit_label (label);
+}
+
/* Epilogue code for APCS frame. */
static void
arm_expand_epilogue_apcs_frame (bool really_return)
@@ -28082,6 +28167,11 @@ arm_expand_epilogue_apcs_frame (bool really_return)
stack_pointer_rtx, hard_frame_pointer_rtx);
}
+ /* PR target/114847: restore the EH data registers (r0, r1) only on the
+ exceptional return path; SP now points at their slots. */
+ if (crtl->calls_eh_return && !IS_STACKALIGN (func_type))
+ arm_emit_eh_data_reg_restore (&saved_regs_mask);
+
arm_emit_multi_reg_pop (saved_regs_mask);
if (IS_INTERRUPT (func_type))
@@ -28102,10 +28192,10 @@ arm_expand_epilogue_apcs_frame (bool really_return)
if (!really_return || (saved_regs_mask & (1 << PC_REGNUM)))
return;
+ /* PR target/114847: apply the exceptional stack adjustment only on the EH
+ return path; r2 is undefined on a normal return. */
if (crtl->calls_eh_return)
- emit_insn (gen_addsi3 (stack_pointer_rtx,
- stack_pointer_rtx,
- gen_rtx_REG (SImode, ARM_EH_STACKADJ_REGNUM)));
+ arm_emit_eh_stack_adjust ();
if (IS_STACKALIGN (func_type))
/* Restore the original stack pointer. Before prologue, the stack was
@@ -28265,6 +28355,16 @@ arm_expand_epilogue (bool really_return)
rtx insn;
bool return_in_pc = false;
+ /* PR target/114847: peel the EH data registers (r0, r1) out of the
+ shared pop and restore them only on the exceptional return path, so a
+ normal return does not clobber the function result held in r0/r1.
+ SP currently points at their slots. */
+ if (crtl->calls_eh_return && !IS_STACKALIGN (func_type))
+ {
+ arm_emit_eh_data_reg_restore (&saved_regs_mask);
+ num_regs = bit_count (saved_regs_mask);
+ }
+
if (ARM_FUNC_TYPE (func_type) != ARM_FT_INTERWORKED
&& (TARGET_ARM || ARM_FUNC_TYPE (func_type) == ARM_FT_NORMAL)
&& !IS_CMSE_ENTRY (func_type)
@@ -28280,7 +28380,11 @@ arm_expand_epilogue (bool really_return)
return_in_pc = true;
}
- if (num_regs == 1
+ if (saved_regs_mask == 0)
+ /* PR target/114847: the only saved core registers were the EH data
+ registers (r0, r1), already handled above; nothing left to pop. */
+ ;
+ else if (num_regs == 1
&& !optimize_size
&& current_tune->prefer_ldrd_strd
&& !(IS_INTERRUPT (func_type) && return_in_pc))
@@ -28396,10 +28500,10 @@ arm_expand_epilogue (bool really_return)
if (!really_return)
return;
+ /* PR target/114847: apply the exceptional stack adjustment only on the EH
+ return path; r2 is undefined on a normal return. */
if (crtl->calls_eh_return)
- emit_insn (gen_addsi3 (stack_pointer_rtx,
- stack_pointer_rtx,
- gen_rtx_REG (SImode, ARM_EH_STACKADJ_REGNUM)));
+ arm_emit_eh_stack_adjust ();
if (IS_STACKALIGN (func_type))
/* Restore the original stack pointer. Before prologue, the stack was
diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 10786ec91..cb99e036e 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -939,6 +939,19 @@ extern const int arm_arch_cde_coproc_bits[];
#define ARM_EH_STACKADJ_REGNUM 2
#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (SImode, ARM_EH_STACKADJ_REGNUM)
+/* Flag register used to distinguish, in the shared return path of a function
+ that calls __builtin_eh_return, an exceptional return (when the EH data
+ registers r0/r1 and the stack adjustment in r2 are meaningful) from a
+ normal return (when r0, and r1 for a 64-bit result, hold the function
+ value and must be preserved). The generic code (except.cc) sets this to 0
+ on the normal path and 1 on the EH path; the epilogue reloads r0/r1 and
+ applies the exceptional stack adjustment (add sp, sp, r2) only when it is
+ set. r3 is a call-clobbered scratch that is free at the point the flag is
+ written (function exit) and is kept live to the epilogue by df-scan. See
+ PR target/114847. */
+#define ARM_EH_TAKEN_REGNUM 3
+#define EH_RETURN_TAKEN_RTX gen_rtx_REG (SImode, ARM_EH_TAKEN_REGNUM)
+
#ifndef ARM_TARGET2_DWARF_FORMAT
#define ARM_TARGET2_DWARF_FORMAT DW_EH_PE_pcrel
#endif
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index febff17df..e82d6806d 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -11639,7 +11639,27 @@
"TARGET_EITHER"
"
if (crtl->calls_eh_return)
- emit_insn (gen_force_register_use (gen_rtx_REG (Pmode, 2)));
+ {
+ /* PR target/114847: the Thumb-1 exit (thumb_exit) emits the exceptional
+ stack adjustment \"add sp, r2\" unconditionally, and it is too
+ register-starved to test EH_RETURN_TAKEN_RTX at that late point. With
+ the flag defined, the generic code leaves r2 (EH_RETURN_STACKADJ_RTX)
+ dataflow-undefined on the normal return path, so zero it here -- while
+ the flag is still live, before force_register_use anchors it into the
+ epilogue -- to keep that adjustment a no-op on a normal return. The
+ ARM/Thumb-2 epilogue instead guards the adjustment itself, mirroring
+ aarch64. */
+ if (TARGET_THUMB1)
+ {
+ rtx_code_label *label = gen_label_rtx ();
+ rtx test = gen_rtx_NE (VOIDmode, EH_RETURN_TAKEN_RTX, const0_rtx);
+ emit_jump_insn (gen_cbranchsi4 (test, EH_RETURN_TAKEN_RTX,
+ const0_rtx, label));
+ emit_insn (gen_movsi (gen_rtx_REG (SImode, 2), const0_rtx));
+ emit_label (label);
+ }
+ emit_insn (gen_force_register_use (gen_rtx_REG (Pmode, 2)));
+ }
if (TARGET_THUMB1)
{
thumb1_expand_epilogue ();
diff --git a/gcc/except.cc b/gcc/except.cc
index 98876833e..f7d99046f 100644
--- a/gcc/except.cc
+++ b/gcc/except.cc
@@ -2333,7 +2333,11 @@ expand_eh_return (void)
emit_label (around_label);
#ifdef EH_RETURN_TAKEN_RTX
- for (rtx tmp : { EH_RETURN_STACKADJ_RTX, EH_RETURN_HANDLER_RTX })
+ /* Cast to rtx so the initializer_list type can be deduced even on targets
+ such as arm that define EH_RETURN_TAKEN_RTX but leave
EH_RETURN_HANDLER_RTX
+ as its NULL default because they implement the handler transfer with the
+ eh_return instruction pattern instead. */
+ for (rtx tmp : { (rtx) EH_RETURN_STACKADJ_RTX, (rtx) EH_RETURN_HANDLER_RTX })
if (tmp && REG_P (tmp))
emit_clobber (tmp);
emit_label (eh_done_label);
diff --git a/gcc/testsuite/gcc.target/arm/eh_return-pr114847-2.c
b/gcc/testsuite/gcc.target/arm/eh_return-pr114847-2.c
new file mode 100644
index 000000000..5d2124d1e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/eh_return-pr114847-2.c
@@ -0,0 +1,34 @@
+/* PR target/114847: check that in the shared epilogue of a function that also
+ calls __builtin_eh_return, both the EH data-register reload (r0, r1) and the
+ exceptional stack adjustment (add sp, sp, r2) are guarded by the
+ EH_RETURN_TAKEN flag, so a normal return neither pops r0/r1 over the
function
+ result nor adjusts sp by the dataflow-undefined r2. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -marm" } */
+/* { dg-require-effective-target arm_arch_v7a_ok } */
+/* { dg-add-options arm_arch_v7a } */
+
+int __attribute__((noipa))
+foo (int sel, unsigned long off, void *handler)
+{
+ if (sel)
+ __builtin_eh_return (off, handler);
+ return 0x1234;
+}
+
+/* The epilogue tests the EH-return flag ... */
+/* { dg-final { scan-assembler {cmp\tr3, #0} } } */
+/* ... reloads r0/r1 only on the exceptional path ... */
+/* { dg-final { scan-assembler {ldrdne\tr0, \[sp\]} } } */
+/* ... discards the two EH data-register slots on both paths ... */
+/* { dg-final { scan-assembler {add\tsp, sp, #8} } } */
+/* ... and applies the exceptional stack adjustment only on that path. */
+/* { dg-final { scan-assembler {addne\tsp, sp, r2} } } */
+
+/* The buggy shapes must be absent: no unconditional "add sp, sp, r2" that
+ would corrupt sp on a normal return, no unconditional reload, and r0/r1 are
+ never popped over the result. */
+/* { dg-final { scan-assembler-not {\tadd\tsp, sp, r2} } } */
+/* { dg-final { scan-assembler-not {\tldrd\tr0, \[sp\]} } } */
+/* { dg-final { scan-assembler-not {\tpop\t\{r0} } } */
diff --git a/gcc/testsuite/gcc.target/arm/eh_return-pr114847-3.c
b/gcc/testsuite/gcc.target/arm/eh_return-pr114847-3.c
new file mode 100644
index 000000000..3c52ae5a0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/eh_return-pr114847-3.c
@@ -0,0 +1,24 @@
+/* PR target/114847: the Thumb-1 exit (thumb_exit) is too register-starved to
+ test the EH-return flag where it emits the exceptional "add sp, r2", so the
+ epilogue instead consumes the flag early and re-zeroes r2 on the normal
+ return path (making that adjustment a no-op there) while keeping the real
+ stack adjustment on the exceptional path. Check the guarded r2 fix-up. */
+
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_arch_v6m_ok } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_arch_v6m } */
+
+int __attribute__((noipa))
+foo (int sel, unsigned long off, void *handler)
+{
+ if (sel)
+ __builtin_eh_return (off, handler);
+ return 0x1234;
+}
+
+/* The epilogue tests the EH-return flag ... */
+/* { dg-final { scan-assembler {cmp\tr3, #0} } } */
+/* ... and zeroes r2 on the normal return path so the unconditional
+ "add sp, r2" that follows is a no-op there. */
+/* { dg-final { scan-assembler {movs\tr2, #0} } } */
diff --git a/gcc/testsuite/gcc.target/arm/eh_return-pr114847.c
b/gcc/testsuite/gcc.target/arm/eh_return-pr114847.c
new file mode 100644
index 000000000..a78ba2276
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/eh_return-pr114847.c
@@ -0,0 +1,60 @@
+/* PR target/114847: a normal (non-exceptional) return from a function that
+ also calls __builtin_eh_return must corrupt neither the return value nor the
+ stack pointer. On arm the epilogue is shared between the two returns: the
EH
+ data registers r0/r1 are reloaded from their slots and the exceptional stack
+ adjustment "add sp, sp, r2" is applied. Both must be gated on the EH-return
+ flag. r0 (and r1 for a 64-bit result) is the return-value register, and on
a
+ normal return r2 (EH_RETURN_STACKADJ) is dataflow-undefined, so an unguarded
+ epilogue returned the wrong value and/or corrupted sp. */
+
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-inline" } */
+
+extern void abort (void);
+
+/* noipa + a runtime-opaque argument: otherwise the mid-end proves
+ foo (0, ...) == 0x1234 and deletes the check, hiding the backend bug. */
+int __attribute__((noipa))
+foo (int sel, unsigned long off, void *handler)
+{
+ if (sel)
+ __builtin_eh_return (off, handler);
+ return 0x1234;
+}
+
+long long __attribute__((noipa))
+foo64 (int sel, unsigned long off, void *handler)
+{
+ if (sel)
+ __builtin_eh_return (off, handler);
+ return 0x1234567890abcdefLL;
+}
+
+volatile int zero = 0; /* sel == 0 -> normal return */
+volatile unsigned long r2_value = 0x40; /* nonzero value for the r2
slot */
+
+int
+main (void)
+{
+ unsigned long sp_before, sp_after;
+ /* A nonzero value delivered in the r2 (EH_RETURN_STACKADJ) argument
position:
+ on the buggy normal return the deleted "mov r2, #0" leaves it live into
the
+ unconditional "add sp, sp, r2", so sp is silently adjusted by this
amount. */
+ void *handler = (void *) r2_value;
+
+ __asm__ volatile ("mov %0, sp" : "=r" (sp_before));
+ if (foo (zero, 0, handler) != 0x1234)
+ abort (); /* return value clobbered */
+ __asm__ volatile ("mov %0, sp" : "=r" (sp_after));
+ if (sp_before != sp_after)
+ abort (); /* sp corrupted by add sp, sp, r2 */
+
+ __asm__ volatile ("mov %0, sp" : "=r" (sp_before));
+ if (foo64 (zero, 0, handler) != 0x1234567890abcdefLL)
+ abort ();
+ __asm__ volatile ("mov %0, sp" : "=r" (sp_after));
+ if (sp_before != sp_after)
+ abort ();
+
+ return 0;
+}
--
2.55.0
