* Re: [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen @ 2022-12-06 15:46 Andrea Corallo 2022-12-06 16:24 ` Richard Earnshaw 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-12-06 15:46 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Andrea Corallo via Gcc-patches, Richard Earnshaw, nd Hi Richard, thanks for reviewing. Just one clarification before I complete the respin of this patch. Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: [...] > Also, I think (out of an abundance of caution) we really need a > scheduling barrier placed before calls to gen_aut_nop() pattern is > emitted, to ensure that the scheduler never tries to move this > instruction away from the position we place it. Use gen_blockage() > for that (see TARGET_SCHED_PROLOG). Alternatively, we could make the > UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC) > without needing an additional insn - if you use this approach, then > please make sure this is explained in a comment. > > +(define_insn "pacbti_nop" > + [(set (reg:SI IP_REGNUM) > + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] > + UNSPEC_PACBTI_NOP))] > + "arm_arch8m_main" > + "pacbti\t%|ip, %|lr, %|sp" > + [(set_attr "conds" "unconditional")]) > > The additional side-effect of this being a BTI landing pad means that > we mustn't move any other instruction before it. So I think this > needs to be an unspec_volatile as well. IIUC from this we want to make all the three (UNSPEC_PAC_NOP, UNSPEC_PACBTI_NOP, UNSPEC_AUT_NOP) unspec volatile, correct? IIUC correctly the scheduler should not reorder them as we have expressed which register they consume and produce but is for double caution correct? > On the tests, they are OK as they stand, but we lack anything that > will be tested when suitable hardware is unavailable (all tests are > "dg-do run"). Can we please have some compile-only tests as well? Ack. BR Andrea ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen 2022-12-06 15:46 [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen Andrea Corallo @ 2022-12-06 16:24 ` Richard Earnshaw 2022-12-06 17:19 ` Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Richard Earnshaw @ 2022-12-06 16:24 UTC (permalink / raw) To: Andrea Corallo; +Cc: Andrea Corallo via Gcc-patches, Richard Earnshaw, nd On 06/12/2022 15:46, Andrea Corallo wrote: > Hi Richard, > > thanks for reviewing. > > Just one clarification before I complete the respin of this patch. > > Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: > > [...] > >> Also, I think (out of an abundance of caution) we really need a >> scheduling barrier placed before calls to gen_aut_nop() pattern is >> emitted, to ensure that the scheduler never tries to move this >> instruction away from the position we place it. Use gen_blockage() >> for that (see TARGET_SCHED_PROLOG). Alternatively, we could make the >> UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC) >> without needing an additional insn - if you use this approach, then >> please make sure this is explained in a comment. >> >> +(define_insn "pacbti_nop" >> + [(set (reg:SI IP_REGNUM) >> + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] >> + UNSPEC_PACBTI_NOP))] >> + "arm_arch8m_main" >> + "pacbti\t%|ip, %|lr, %|sp" >> + [(set_attr "conds" "unconditional")]) >> >> The additional side-effect of this being a BTI landing pad means that >> we mustn't move any other instruction before it. So I think this >> needs to be an unspec_volatile as well. > > IIUC from this we want to make all the three (UNSPEC_PAC_NOP, > UNSPEC_PACBTI_NOP, UNSPEC_AUT_NOP) unspec volatile, correct? UNSPEC_PAC_NOP doesn't need to be volatile. The register constraints will be enough to ensure it is run before any instruction that consumes the result it produces. UNSPEC_PAC_BTI_NOP needs to be volatile, as it's essential that when we have an instruction (for example ldr r3, [r3]) early in the program that doesn't interact with the prologue then it cannot be migrated before the BTI as the BTI is a landing pad and must be the first instruction in the function. This is why UNSPEC_BTI_NOP is volatile. UNSPEC_AUT_NOP must be volatile because we want to ensure that no instruction is moved after this one and before the return as that might expose a ROP gadget to hackers. R. > > IIUC correctly the scheduler should not reorder them as we have > expressed which register they consume and produce but is for double > caution correct? > >> On the tests, they are OK as they stand, but we lack anything that >> will be tested when suitable hardware is unavailable (all tests are >> "dg-do run"). Can we please have some compile-only tests as well? > > Ack. > > BR > > Andrea ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen 2022-12-06 16:24 ` Richard Earnshaw @ 2022-12-06 17:19 ` Andrea Corallo 0 siblings, 0 replies; 5+ messages in thread From: Andrea Corallo @ 2022-12-06 17:19 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Andrea Corallo via Gcc-patches, Richard Earnshaw, nd Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: > On 06/12/2022 15:46, Andrea Corallo wrote: >> Hi Richard, >> thanks for reviewing. >> Just one clarification before I complete the respin of this patch. >> Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: >> [...] >> >>> Also, I think (out of an abundance of caution) we really need a >>> scheduling barrier placed before calls to gen_aut_nop() pattern is >>> emitted, to ensure that the scheduler never tries to move this >>> instruction away from the position we place it. Use gen_blockage() >>> for that (see TARGET_SCHED_PROLOG). Alternatively, we could make the >>> UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC) >>> without needing an additional insn - if you use this approach, then >>> please make sure this is explained in a comment. >>> >>> +(define_insn "pacbti_nop" >>> + [(set (reg:SI IP_REGNUM) >>> + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] >>> + UNSPEC_PACBTI_NOP))] >>> + "arm_arch8m_main" >>> + "pacbti\t%|ip, %|lr, %|sp" >>> + [(set_attr "conds" "unconditional")]) >>> >>> The additional side-effect of this being a BTI landing pad means that >>> we mustn't move any other instruction before it. So I think this >>> needs to be an unspec_volatile as well. >> IIUC from this we want to make all the three (UNSPEC_PAC_NOP, >> UNSPEC_PACBTI_NOP, UNSPEC_AUT_NOP) unspec volatile, correct? > > UNSPEC_PAC_NOP doesn't need to be volatile. The register constraints > will be enough to ensure it is run before any instruction that > consumes the result it produces. > > UNSPEC_PAC_BTI_NOP needs to be volatile, as it's essential that when > we have an instruction (for example ldr r3, [r3]) early in the program > that doesn't interact with the prologue then it cannot be migrated > before the BTI as the BTI is a landing pad and must be the first > instruction in the function. This is why UNSPEC_BTI_NOP is volatile. > > UNSPEC_AUT_NOP must be volatile because we want to ensure that no > instruction is moved after this one and before the return as that > might expose a ROP gadget to hackers. > > R. Understood now, thanks. Andrea ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 0/15] arm: Enables return address verification and branch target identification on Cortex-M @ 2022-08-12 14:26 Andrea Corallo 2022-08-12 15:36 ` [PATCH 10/15] arm: Implement cortex-M return signing address codegen Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-08-12 14:26 UTC (permalink / raw) To: gcc-patches; +Cc: nd, Richard Earnshaw, Kyrylo.Tkachov Hi all, as I respinned few patches, dropped one and added another, I'm reposting this series thant enables return address verification and branch target identification based on Armv8.1-M Pointer Authentication and Branch Target Identification Extension [1] for Arm Cortex-M. This feature is controlled by the newly introduced '-mbranch-protection' option, contextually the Armv8.1-M Mainline target feature '+pacbti' is added. Best Regards Andrea [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 10/15] arm: Implement cortex-M return signing address codegen 2022-08-12 14:26 [PATCH 0/15] arm: Enables return address verification and branch target identification on Cortex-M Andrea Corallo @ 2022-08-12 15:36 ` Andrea Corallo 2022-09-14 14:20 ` [PATCH 10/15 V2] " Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-08-12 15:36 UTC (permalink / raw) To: Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, nd [-- Attachment #1: Type: text/plain, Size: 2487 bytes --] Hi all, this patch enables address return signature and verification based on Armv8.1-M Pointer Authentication [1]. To sign the return address, we use the PAC R12, LR, SP instruction upon function entry. This is signing LR using SP and storing the result in R12. R12 will be pushed into the stack. During function epilogue R12 will be popped and AUT R12, LR, SP will be used to verify that the content of LR is still valid before return. Here an example of PAC instrumented function prologue and epilogue: void foo (void); int main() { foo (); return 0; } Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret -mthumb' translates into: main: pac ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr The patch also takes care of generating a PACBTI instruction in place of the sequence BTI+PAC when Branch Target Identification is enabled contextually. Ex. the previous example compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret+bti -mthumb' translates into: main: pacbti ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr As part of previous upstream suggestions a test for varargs has been added and '-mtpcs-frame' is deemed being incompatible with this return signing address feature being introduced. [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> gcc/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * config/arm/arm.c: (arm_compute_frame_layout) (arm_expand_prologue, thumb2_expand_return, arm_expand_epilogue) (arm_conditional_register_usage): Update for pac codegen. (arm_current_function_pac_enabled_p): New function. * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): Add new patterns. * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. gcc/testsuite/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * gcc.target/arm/pac.h : New file. * gcc.target/arm/pac-1.c : New test case. * gcc.target/arm/pac-2.c : Likewise. * gcc.target/arm/pac-3.c : Likewise. * gcc.target/arm/pac-4.c : Likewise. * gcc.target/arm/pac-5.c : Likewise. * gcc.target/arm/pac-6.c : Likewise. * gcc.target/arm/pac-7.c : Likewise. * gcc.target/arm/pac-8.c : Likewise. [-- Attachment #2: 10-15.patch --] [-- Type: text/plain, Size: 14112 bytes --] diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index cff7ff1da2a..84764bf27ce 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -379,6 +379,7 @@ extern int vfp3_const_double_for_bits (rtx); extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, rtx); extern bool arm_fusion_enabled_p (tune_params::fuse_ops); +extern bool arm_current_function_pac_enabled_p (void); extern bool arm_valid_symbolic_address_p (rtx); extern bool arm_validize_comparison (rtx *, rtx *, rtx *); extern bool arm_expand_vector_compare (rtx, rtx_code, rtx, rtx, bool); diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index a5cf4225aa2..31c6bcdea55 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -3209,6 +3209,9 @@ arm_option_override_internal (struct gcc_options *opts, arm_stack_protector_guard_offset = offs; } + if (arm_current_function_pac_enabled_p () && !(arm_arch7 && arm_arch_cmse)) + error ("This architecture does not support branch protection instructions"); + #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; #endif @@ -21139,6 +21142,9 @@ arm_compute_save_core_reg_mask (void) save_reg_mask |= arm_compute_save_reg0_reg12_mask (); + if (arm_current_function_pac_enabled_p ()) + save_reg_mask |= 1 << IP_REGNUM; + /* Decide if we need to save the link register. Interrupt routines have their own banked link register, so they never need to save it. @@ -23362,6 +23368,12 @@ output_probe_stack_range (rtx reg1, rtx reg2) return ""; } +static bool +aarch_bti_enabled () +{ + return false; +} + /* Generate the prologue instructions for entry into an ARM or Thumb-2 function. */ void @@ -23440,12 +23452,13 @@ arm_expand_prologue (void) /* The static chain register is the same as the IP register. If it is clobbered when creating the frame, we need to save and restore it. */ - clobber_ip = IS_NESTED (func_type) - && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) - || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK - || flag_stack_clash_protection) - && !df_regs_ever_live_p (LR_REGNUM) - && arm_r3_live_at_start_p ())); + clobber_ip = (IS_NESTED (func_type) + && (((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) + || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK + || flag_stack_clash_protection) + && !df_regs_ever_live_p (LR_REGNUM) + && arm_r3_live_at_start_p ())) + || (arm_current_function_pac_enabled_p ()))); /* Find somewhere to store IP whilst the frame is being created. We try the following places in order: @@ -23521,6 +23534,14 @@ arm_expand_prologue (void) } } + if (arm_current_function_pac_enabled_p ()) + { + if (aarch_bti_enabled ()) + emit_insn (gen_pacbti_nop ()); + else + emit_insn (gen_pac_nop ()); + } + if (TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) { if (IS_INTERRUPT (func_type)) @@ -27309,7 +27330,7 @@ thumb2_expand_return (bool simple_return) to assert it for now to ensure that future code changes do not silently change this behavior. */ gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); - if (num_regs == 1) + if (num_regs == 1 && !arm_current_function_pac_enabled_p ()) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); rtx reg = gen_rtx_REG (SImode, PC_REGNUM); @@ -27324,10 +27345,20 @@ thumb2_expand_return (bool simple_return) } else { - saved_regs_mask &= ~ (1 << LR_REGNUM); - saved_regs_mask |= (1 << PC_REGNUM); - arm_emit_multi_reg_pop (saved_regs_mask); - } + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } + else + { + saved_regs_mask &= ~ (1 << LR_REGNUM); + saved_regs_mask |= (1 << PC_REGNUM); + arm_emit_multi_reg_pop (saved_regs_mask); + } + } } else { @@ -27733,7 +27764,8 @@ arm_expand_epilogue (bool really_return) && really_return && crtl->args.pretend_args_size == 0 && saved_regs_mask & (1 << LR_REGNUM) - && !crtl->calls_eh_return) + && !crtl->calls_eh_return + && !arm_current_function_pac_enabled_p ()) { saved_regs_mask &= ~(1 << LR_REGNUM); saved_regs_mask |= (1 << PC_REGNUM); @@ -27847,6 +27879,9 @@ arm_expand_epilogue (bool really_return) } } + if (arm_current_function_pac_enabled_p ()) + emit_insn (gen_aut_nop ()); + if (!really_return) return; @@ -32941,6 +32976,15 @@ arm_fusion_enabled_p (tune_params::fuse_ops op) return current_tune->fusible_ops & op; } +/* Return TRUE if return address signing mechanism is enabled. */ +bool +arm_current_function_pac_enabled_p (void) +{ + return aarch_ra_sign_scope == AARCH_FUNCTION_ALL + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF + && !crtl->is_leaf); +} + /* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be scheduled for speculative execution. Reject the long-running division and square-root instructions. */ diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 60468f6182c..92269a7819a 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -11518,7 +11518,7 @@ arm_expand_prologue (); else thumb1_expand_prologue (); - DONE; + DONE; " ) @@ -12890,6 +12890,29 @@ (set_attr "length" "8")] ) +(define_insn "pac_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PAC_NOP))] + "arm_arch7 && arm_arch_cmse" + "pac\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + +(define_insn "pacbti_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PACBTI_NOP))] + "arm_arch7 && arm_arch_cmse" + "pacbti\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + +(define_insn "aut_nop" + [(unspec:SI [(reg:SI IP_REGNUM) (reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_AUT_NOP)] + "arm_arch7 && arm_arch_cmse" + "aut\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + ;; Vector bits common to IWMMXT, Neon and MVE (include "vec-common.md") ;; Load the Intel Wireless Multimedia Extension patterns diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md index 7748e784379..dbe243a03f6 100644 --- a/gcc/config/arm/unspecs.md +++ b/gcc/config/arm/unspecs.md @@ -159,6 +159,9 @@ UNSPEC_VCDE ; Custom Datapath Extension instruction. UNSPEC_VCDEA ; Custom Datapath Extension instruction. UNSPEC_DLS ; Used for DLS (Do Loop Start), Armv8.1-M Mainline instruction + UNSPEC_PAC_NOP ; Represents PAC signing LR + UNSPEC_PACBTI_NOP ; Represents PAC signing LR + valid landing pad + UNSPEC_AUT_NOP ; Represents PAC verifying LR ]) diff --git a/gcc/testsuite/gcc.target/arm/pac-1.c b/gcc/testsuite/gcc.target/arm/pac-1.c new file mode 100644 index 00000000000..6cd64dbd014 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-1.c @@ -0,0 +1,12 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ + diff --git a/gcc/testsuite/gcc.target/arm/pac-2.c b/gcc/testsuite/gcc.target/arm/pac-2.c new file mode 100644 index 00000000000..945ce938592 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-2.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-3.c b/gcc/testsuite/gcc.target/arm/pac-3.c new file mode 100644 index 00000000000..47e290a5840 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-3.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=bti+pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pacbti\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-4.c b/gcc/testsuite/gcc.target/arm/pac-4.c new file mode 100644 index 00000000000..cf915cdba50 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-4.c @@ -0,0 +1,10 @@ +/* Testing return address signing. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-not "\tbti\t" } } */ +/* { dg-final { scan-assembler-not "\tpac\t" } } */ +/* { dg-final { scan-assembler-not "\tpacbti\t" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-5.c b/gcc/testsuite/gcc.target/arm/pac-5.c new file mode 100644 index 00000000000..c70087eb6b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-5.c @@ -0,0 +1,28 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int square (int z) { return z * z; } + return square (a) + square (b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 5) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-6.c b/gcc/testsuite/gcc.target/arm/pac-6.c new file mode 100644 index 00000000000..c5329f0ef48 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-6.c @@ -0,0 +1,18 @@ +/* Check that GCC does .save and .cfi_offset directives with RA_AUTH_CODE pseudo hard-register. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+fp -mbranch-protection=pac-ret+leaf -mthumb --save-temps -O0 -g" } */ + +int i; + +void foo (int); + +int bar() +{ + foo (i); + return 0; +} + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "bti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-7.c b/gcc/testsuite/gcc.target/arm/pac-7.c new file mode 100644 index 00000000000..cdaebca5cfa --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-7.c @@ -0,0 +1,32 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int x = 4; + int foo2 (int a, int b) + { + return a + b + x; + } + return foo2 (a, b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 7) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-8.c b/gcc/testsuite/gcc.target/arm/pac-8.c new file mode 100644 index 00000000000..3f37dcfa5c8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-8.c @@ -0,0 +1,34 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdarg.h> +#include <stdlib.h> + +int acc (int n, ...) +{ + int sum = 0; + va_list ptr; + + va_start (ptr, n); + + for (int i = 0; i < n; i++) + sum += va_arg (ptr, int); + va_end (ptr); + + return sum; +} + +int main() +{ + if (acc (3, 1, 2, 3) != 6) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac.h b/gcc/testsuite/gcc.target/arm/pac.h new file mode 100644 index 00000000000..7355e6b2954 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac.h @@ -0,0 +1,17 @@ +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + return a + b; +} + +int +main (void) +{ + if (foo1 (1, 2) != 3) + abort (); + + return 0; +} ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 10/15 V2] arm: Implement cortex-M return signing address codegen 2022-08-12 15:36 ` [PATCH 10/15] arm: Implement cortex-M return signing address codegen Andrea Corallo @ 2022-09-14 14:20 ` Andrea Corallo 2022-10-21 12:58 ` Richard Earnshaw 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-09-14 14:20 UTC (permalink / raw) To: Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, nd [-- Attachment #1: Type: text/plain, Size: 2487 bytes --] Hi all, this patch enables address return signature and verification based on Armv8.1-M Pointer Authentication [1]. To sign the return address, we use the PAC R12, LR, SP instruction upon function entry. This is signing LR using SP and storing the result in R12. R12 will be pushed into the stack. During function epilogue R12 will be popped and AUT R12, LR, SP will be used to verify that the content of LR is still valid before return. Here an example of PAC instrumented function prologue and epilogue: void foo (void); int main() { foo (); return 0; } Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret -mthumb' translates into: main: pac ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr The patch also takes care of generating a PACBTI instruction in place of the sequence BTI+PAC when Branch Target Identification is enabled contextually. Ex. the previous example compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret+bti -mthumb' translates into: main: pacbti ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr As part of previous upstream suggestions a test for varargs has been added and '-mtpcs-frame' is deemed being incompatible with this return signing address feature being introduced. [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> gcc/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * config/arm/arm.c: (arm_compute_frame_layout) (arm_expand_prologue, thumb2_expand_return, arm_expand_epilogue) (arm_conditional_register_usage): Update for pac codegen. (arm_current_function_pac_enabled_p): New function. * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): Add new patterns. * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. gcc/testsuite/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * gcc.target/arm/pac.h : New file. * gcc.target/arm/pac-1.c : New test case. * gcc.target/arm/pac-2.c : Likewise. * gcc.target/arm/pac-3.c : Likewise. * gcc.target/arm/pac-4.c : Likewise. * gcc.target/arm/pac-5.c : Likewise. * gcc.target/arm/pac-6.c : Likewise. * gcc.target/arm/pac-7.c : Likewise. * gcc.target/arm/pac-8.c : Likewise. [-- Attachment #2: 10-15.patch --] [-- Type: text/plain, Size: 14546 bytes --] diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index cff7ff1da2a..84764bf27ce 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -379,6 +379,7 @@ extern int vfp3_const_double_for_bits (rtx); extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, rtx); extern bool arm_fusion_enabled_p (tune_params::fuse_ops); +extern bool arm_current_function_pac_enabled_p (void); extern bool arm_valid_symbolic_address_p (rtx); extern bool arm_validize_comparison (rtx *, rtx *, rtx *); extern bool arm_expand_vector_compare (rtx, rtx_code, rtx, rtx, bool); diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index a5cf4225aa2..0ba9c0b9960 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -3209,6 +3209,9 @@ arm_option_override_internal (struct gcc_options *opts, arm_stack_protector_guard_offset = offs; } + if (arm_current_function_pac_enabled_p () && !(arm_arch7 && arm_arch_cmse)) + error ("This architecture does not support branch protection instructions"); + #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; #endif @@ -21139,6 +21142,9 @@ arm_compute_save_core_reg_mask (void) save_reg_mask |= arm_compute_save_reg0_reg12_mask (); + if (arm_current_function_pac_enabled_p ()) + save_reg_mask |= 1 << IP_REGNUM; + /* Decide if we need to save the link register. Interrupt routines have their own banked link register, so they never need to save it. @@ -23362,6 +23368,12 @@ output_probe_stack_range (rtx reg1, rtx reg2) return ""; } +static bool +aarch_bti_enabled () +{ + return false; +} + /* Generate the prologue instructions for entry into an ARM or Thumb-2 function. */ void @@ -23440,12 +23452,13 @@ arm_expand_prologue (void) /* The static chain register is the same as the IP register. If it is clobbered when creating the frame, we need to save and restore it. */ - clobber_ip = IS_NESTED (func_type) - && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) - || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK - || flag_stack_clash_protection) - && !df_regs_ever_live_p (LR_REGNUM) - && arm_r3_live_at_start_p ())); + clobber_ip = (IS_NESTED (func_type) + && (((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) + || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK + || flag_stack_clash_protection) + && !df_regs_ever_live_p (LR_REGNUM) + && arm_r3_live_at_start_p ())) + || (arm_current_function_pac_enabled_p ()))); /* Find somewhere to store IP whilst the frame is being created. We try the following places in order: @@ -23470,7 +23483,8 @@ arm_expand_prologue (void) { rtx addr, dwarf; - gcc_assert(arm_compute_static_chain_stack_bytes() == 4); + gcc_assert(arm_compute_static_chain_stack_bytes() == 4 + || arm_current_function_pac_enabled_p ()); saved_regs += 4; addr = gen_rtx_PRE_DEC (Pmode, stack_pointer_rtx); @@ -23521,6 +23535,14 @@ arm_expand_prologue (void) } } + if (arm_current_function_pac_enabled_p ()) + { + if (aarch_bti_enabled ()) + emit_insn (gen_pacbti_nop ()); + else + emit_insn (gen_pac_nop ()); + } + if (TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) { if (IS_INTERRUPT (func_type)) @@ -27309,7 +27331,7 @@ thumb2_expand_return (bool simple_return) to assert it for now to ensure that future code changes do not silently change this behavior. */ gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); - if (num_regs == 1) + if (num_regs == 1 && !arm_current_function_pac_enabled_p ()) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); rtx reg = gen_rtx_REG (SImode, PC_REGNUM); @@ -27324,10 +27346,20 @@ thumb2_expand_return (bool simple_return) } else { - saved_regs_mask &= ~ (1 << LR_REGNUM); - saved_regs_mask |= (1 << PC_REGNUM); - arm_emit_multi_reg_pop (saved_regs_mask); - } + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } + else + { + saved_regs_mask &= ~ (1 << LR_REGNUM); + saved_regs_mask |= (1 << PC_REGNUM); + arm_emit_multi_reg_pop (saved_regs_mask); + } + } } else { @@ -27733,7 +27765,8 @@ arm_expand_epilogue (bool really_return) && really_return && crtl->args.pretend_args_size == 0 && saved_regs_mask & (1 << LR_REGNUM) - && !crtl->calls_eh_return) + && !crtl->calls_eh_return + && !arm_current_function_pac_enabled_p ()) { saved_regs_mask &= ~(1 << LR_REGNUM); saved_regs_mask |= (1 << PC_REGNUM); @@ -27847,6 +27880,9 @@ arm_expand_epilogue (bool really_return) } } + if (arm_current_function_pac_enabled_p ()) + emit_insn (gen_aut_nop ()); + if (!really_return) return; @@ -32941,6 +32977,15 @@ arm_fusion_enabled_p (tune_params::fuse_ops op) return current_tune->fusible_ops & op; } +/* Return TRUE if return address signing mechanism is enabled. */ +bool +arm_current_function_pac_enabled_p (void) +{ + return aarch_ra_sign_scope == AARCH_FUNCTION_ALL + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF + && !crtl->is_leaf); +} + /* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be scheduled for speculative execution. Reject the long-running division and square-root instructions. */ diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 60468f6182c..92269a7819a 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -11518,7 +11518,7 @@ (define_expand "prologue" arm_expand_prologue (); else thumb1_expand_prologue (); - DONE; + DONE; " ) @@ -12890,6 +12890,29 @@ (define_insn "*speculation_barrier_insn" (set_attr "length" "8")] ) +(define_insn "pac_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PAC_NOP))] + "arm_arch7 && arm_arch_cmse" + "pac\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + +(define_insn "pacbti_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PACBTI_NOP))] + "arm_arch7 && arm_arch_cmse" + "pacbti\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + +(define_insn "aut_nop" + [(unspec:SI [(reg:SI IP_REGNUM) (reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_AUT_NOP)] + "arm_arch7 && arm_arch_cmse" + "aut\t%|ip, %|lr, %|sp" + [(set_attr "length" "4")]) + ;; Vector bits common to IWMMXT, Neon and MVE (include "vec-common.md") ;; Load the Intel Wireless Multimedia Extension patterns diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md index 7748e784379..dbe243a03f6 100644 --- a/gcc/config/arm/unspecs.md +++ b/gcc/config/arm/unspecs.md @@ -159,6 +159,9 @@ (define_c_enum "unspec" [ UNSPEC_VCDE ; Custom Datapath Extension instruction. UNSPEC_VCDEA ; Custom Datapath Extension instruction. UNSPEC_DLS ; Used for DLS (Do Loop Start), Armv8.1-M Mainline instruction + UNSPEC_PAC_NOP ; Represents PAC signing LR + UNSPEC_PACBTI_NOP ; Represents PAC signing LR + valid landing pad + UNSPEC_AUT_NOP ; Represents PAC verifying LR ]) diff --git a/gcc/testsuite/gcc.target/arm/pac-1.c b/gcc/testsuite/gcc.target/arm/pac-1.c new file mode 100644 index 00000000000..6cd64dbd014 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-1.c @@ -0,0 +1,12 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ + diff --git a/gcc/testsuite/gcc.target/arm/pac-2.c b/gcc/testsuite/gcc.target/arm/pac-2.c new file mode 100644 index 00000000000..945ce938592 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-2.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-3.c b/gcc/testsuite/gcc.target/arm/pac-3.c new file mode 100644 index 00000000000..47e290a5840 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-3.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=bti+pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pacbti\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-4.c b/gcc/testsuite/gcc.target/arm/pac-4.c new file mode 100644 index 00000000000..cf915cdba50 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-4.c @@ -0,0 +1,10 @@ +/* Testing return address signing. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-not "\tbti\t" } } */ +/* { dg-final { scan-assembler-not "\tpac\t" } } */ +/* { dg-final { scan-assembler-not "\tpacbti\t" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-5.c b/gcc/testsuite/gcc.target/arm/pac-5.c new file mode 100644 index 00000000000..c70087eb6b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-5.c @@ -0,0 +1,28 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int square (int z) { return z * z; } + return square (a) + square (b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 5) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-6.c b/gcc/testsuite/gcc.target/arm/pac-6.c new file mode 100644 index 00000000000..c5329f0ef48 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-6.c @@ -0,0 +1,18 @@ +/* Check that GCC does .save and .cfi_offset directives with RA_AUTH_CODE pseudo hard-register. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+fp -mbranch-protection=pac-ret+leaf -mthumb --save-temps -O0 -g" } */ + +int i; + +void foo (int); + +int bar() +{ + foo (i); + return 0; +} + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "bti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-7.c b/gcc/testsuite/gcc.target/arm/pac-7.c new file mode 100644 index 00000000000..cdaebca5cfa --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-7.c @@ -0,0 +1,32 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int x = 4; + int foo2 (int a, int b) + { + return a + b + x; + } + return foo2 (a, b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 7) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-8.c b/gcc/testsuite/gcc.target/arm/pac-8.c new file mode 100644 index 00000000000..3f37dcfa5c8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-8.c @@ -0,0 +1,34 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdarg.h> +#include <stdlib.h> + +int acc (int n, ...) +{ + int sum = 0; + va_list ptr; + + va_start (ptr, n); + + for (int i = 0; i < n; i++) + sum += va_arg (ptr, int); + va_end (ptr); + + return sum; +} + +int main() +{ + if (acc (3, 1, 2, 3) != 6) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac.h b/gcc/testsuite/gcc.target/arm/pac.h new file mode 100644 index 00000000000..7355e6b2954 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac.h @@ -0,0 +1,17 @@ +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + return a + b; +} + +int +main (void) +{ + if (foo1 (1, 2) != 3) + abort (); + + return 0; +} ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 10/15 V2] arm: Implement cortex-M return signing address codegen 2022-09-14 14:20 ` [PATCH 10/15 V2] " Andrea Corallo @ 2022-10-21 12:58 ` Richard Earnshaw 2022-10-26 15:48 ` Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Richard Earnshaw @ 2022-10-21 12:58 UTC (permalink / raw) To: Andrea Corallo, Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, nd On 14/09/2022 15:20, Andrea Corallo via Gcc-patches wrote: > Hi all, > > this patch enables address return signature and verification based on > Armv8.1-M Pointer Authentication [1]. > > To sign the return address, we use the PAC R12, LR, SP instruction > upon function entry. This is signing LR using SP and storing the > result in R12. R12 will be pushed into the stack. > > During function epilogue R12 will be popped and AUT R12, LR, SP will > be used to verify that the content of LR is still valid before return. > > Here an example of PAC instrumented function prologue and epilogue: > > void foo (void); > > int main() > { > foo (); > return 0; > } > > Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret > -mthumb' translates into: > > main: > pac ip, lr, sp > push {r3, r7, ip, lr} > add r7, sp, #0 > bl foo > movs r3, #0 > mov r0, r3 > pop {r3, r7, ip, lr} > aut ip, lr, sp > bx lr > > The patch also takes care of generating a PACBTI instruction in place > of the sequence BTI+PAC when Branch Target Identification is enabled > contextually. > > Ex. the previous example compiled with '-march=armv8.1-m.main > -mbranch-protection=pac-ret+bti -mthumb' translates into: > > main: > pacbti ip, lr, sp > push {r3, r7, ip, lr} > add r7, sp, #0 > bl foo > movs r3, #0 > mov r0, r3 > pop {r3, r7, ip, lr} > aut ip, lr, sp > bx lr > > As part of previous upstream suggestions a test for varargs has been > added and '-mtpcs-frame' is deemed being incompatible with this return > signing address feature being introduced. > > [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> > > gcc/Changelog > > 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> > > * config/arm/arm.c: (arm_compute_frame_layout) > (arm_expand_prologue, thumb2_expand_return, arm_expand_epilogue) > (arm_conditional_register_usage): Update for pac codegen. > (arm_current_function_pac_enabled_p): New function. > * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): > Add new patterns. > * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) > (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. > > gcc/testsuite/Changelog > > 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> > > * gcc.target/arm/pac.h : New file. > * gcc.target/arm/pac-1.c : New test case. > * gcc.target/arm/pac-2.c : Likewise. > * gcc.target/arm/pac-3.c : Likewise. > * gcc.target/arm/pac-4.c : Likewise. > * gcc.target/arm/pac-5.c : Likewise. > * gcc.target/arm/pac-6.c : Likewise. > * gcc.target/arm/pac-7.c : Likewise. > * gcc.target/arm/pac-8.c : Likewise. > + if (arm_current_function_pac_enabled_p () && !(arm_arch7 && arm_arch_cmse)) + error ("This architecture does not support branch protection instructions"); This test feels wrong. What does having cmse give us? I suspect you want a test that ensures we have at least v8-m.main so that the NOP instructions are correctly defined as NOPs (or, in this case, PACBTI instructions) rather than unpredictable; but if that's the case then I think you really want to write the test that way here (perhaps in a macro) and then move this test into that so that it becomes self-documenting - but don't we have a v8-m.main test anyway? + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } The assert is using indents that are just spaces, but the other lines use tabs. Please use tabs everywhere rather than mixing like this. +/* Return TRUE if return address signing mechanism is enabled. */ +bool +arm_current_function_pac_enabled_p (void) +{ + return aarch_ra_sign_scope == AARCH_FUNCTION_ALL + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF + && !crtl->is_leaf); +} This is a case where you should use parenthesis around the expression so that the continuation lines are correctly indented. @@ -11518,7 +11518,7 @@ (define_expand "prologue" arm_expand_prologue (); else thumb1_expand_prologue (); - DONE; + DONE; " ) Although this is a trivial cleanup, it has nothing to do with this patch. Please remove. + "arm_arch7 && arm_arch_cmse" See my comments earlier about this test; the same applies here. + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PAC_NOP))] + Again you have a mix of lines indented with tabs and lines indented with just spaces. Similarly with pacbti_nop and aut_nop. Do you have a test for the nested functions case (I can't see it, but perhaps I've missed it somewhere)? R. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 10/15 V2] arm: Implement cortex-M return signing address codegen 2022-10-21 12:58 ` Richard Earnshaw @ 2022-10-26 15:48 ` Andrea Corallo 2022-10-28 16:34 ` [PATCH 10/15 V3] " Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-10-26 15:48 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Andrea Corallo via Gcc-patches, Richard Earnshaw, nd Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes: > On 14/09/2022 15:20, Andrea Corallo via Gcc-patches wrote: >> Hi all, >> >> this patch enables address return signature and verification based on >> Armv8.1-M Pointer Authentication [1]. >> >> To sign the return address, we use the PAC R12, LR, SP instruction >> upon function entry. This is signing LR using SP and storing the >> result in R12. R12 will be pushed into the stack. >> >> During function epilogue R12 will be popped and AUT R12, LR, SP will >> be used to verify that the content of LR is still valid before return. >> >> Here an example of PAC instrumented function prologue and epilogue: >> >> void foo (void); >> >> int main() >> { >> foo (); >> return 0; >> } >> >> Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret >> -mthumb' translates into: >> >> main: >> pac ip, lr, sp >> push {r3, r7, ip, lr} >> add r7, sp, #0 >> bl foo >> movs r3, #0 >> mov r0, r3 >> pop {r3, r7, ip, lr} >> aut ip, lr, sp >> bx lr >> >> The patch also takes care of generating a PACBTI instruction in place >> of the sequence BTI+PAC when Branch Target Identification is enabled >> contextually. >> >> Ex. the previous example compiled with '-march=armv8.1-m.main >> -mbranch-protection=pac-ret+bti -mthumb' translates into: >> >> main: >> pacbti ip, lr, sp >> push {r3, r7, ip, lr} >> add r7, sp, #0 >> bl foo >> movs r3, #0 >> mov r0, r3 >> pop {r3, r7, ip, lr} >> aut ip, lr, sp >> bx lr >> >> As part of previous upstream suggestions a test for varargs has been >> added and '-mtpcs-frame' is deemed being incompatible with this return >> signing address feature being introduced. >> >> [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> >> >> gcc/Changelog >> >> 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> >> >> * config/arm/arm.c: (arm_compute_frame_layout) >> (arm_expand_prologue, thumb2_expand_return, arm_expand_epilogue) >> (arm_conditional_register_usage): Update for pac codegen. >> (arm_current_function_pac_enabled_p): New function. >> * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): >> Add new patterns. >> * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) >> (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. >> >> gcc/testsuite/Changelog >> >> 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> >> >> * gcc.target/arm/pac.h : New file. >> * gcc.target/arm/pac-1.c : New test case. >> * gcc.target/arm/pac-2.c : Likewise. >> * gcc.target/arm/pac-3.c : Likewise. >> * gcc.target/arm/pac-4.c : Likewise. >> * gcc.target/arm/pac-5.c : Likewise. >> * gcc.target/arm/pac-6.c : Likewise. >> * gcc.target/arm/pac-7.c : Likewise. >> * gcc.target/arm/pac-8.c : Likewise. >> > > + if (arm_current_function_pac_enabled_p () && !(arm_arch7 && > arm_arch_cmse)) > + error ("This architecture does not support branch protection > instructions"); > > This test feels wrong. What does having cmse give us? I suspect you > want a test that ensures we have at least v8-m.main so that the NOP > instructions are correctly defined as NOPs (or, in this case, PACBTI > instructions) rather than unpredictable; but if that's the case then I > think you really want to write the test that way here (perhaps in a > macro) and then move this test into that so that it becomes > self-documenting - but don't we have a v8-m.main test anyway? Yep > + if (arm_current_function_pac_enabled_p ()) > + { > + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); > + arm_emit_multi_reg_pop (saved_regs_mask); > + emit_insn (gen_aut_nop ()); > + emit_jump_insn (simple_return_rtx); > + } > > The assert is using indents that are just spaces, but the other lines > use tabs. Please use tabs everywhere rather than mixing like this. Ack. > +/* Return TRUE if return address signing mechanism is enabled. */ > +bool > +arm_current_function_pac_enabled_p (void) > +{ > + return aarch_ra_sign_scope == AARCH_FUNCTION_ALL > + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF > + && !crtl->is_leaf); > +} > > This is a case where you should use parenthesis around the expression so > that the continuation lines are correctly indented. Ack. > @@ -11518,7 +11518,7 @@ (define_expand "prologue" > arm_expand_prologue (); > else > thumb1_expand_prologue (); > - DONE; > + DONE; > " > ) > > Although this is a trivial cleanup, it has nothing to do with this > patch. Please remove. Okay. > + "arm_arch7 && arm_arch_cmse" > > See my comments earlier about this test; the same applies here. > > + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] > + UNSPEC_PAC_NOP))] > + > Again you have a mix of lines indented with tabs and lines indented with > just spaces. Similarly with pacbti_nop and aut_nop. > > Do you have a test for the nested functions case (I can't see it, but > perhaps I've missed it somewhere)? We have gcc/testsuite/gcc.target/arm/pac-7.c added by this patch. > R. Andrea ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 10/15 V3] arm: Implement cortex-M return signing address codegen 2022-10-26 15:48 ` Andrea Corallo @ 2022-10-28 16:34 ` Andrea Corallo 2022-11-07 8:57 ` [PATCH 10/15 V4] " Andrea Corallo 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-10-28 16:34 UTC (permalink / raw) To: Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, Richard Earnshaw, nd [-- Attachment #1: Type: text/plain, Size: 101 bytes --] Hi all, the third iteration of this patch is attached addresing review comments. Thanks Andrea [-- Attachment #2: 0001-PATCH-10-15-arm-Implement-cortex-M-return-signing-ad.patch --] [-- Type: text/plain, Size: 19985 bytes --] From b42e28be75f374a4e1a5943c8c9002e07dbcc567 Mon Sep 17 00:00:00 2001 From: Andrea Corallo <andrea.corallo@arm.com> Date: Thu, 20 Jan 2022 15:36:23 +0100 Subject: [PATCH] [PATCH 10/15] arm: Implement cortex-M return signing address codegen Hi all, this patch enables address return signature and verification based on Armv8.1-M Pointer Authentication [1]. To sign the return address, we use the PAC R12, LR, SP instruction upon function entry. This is signing LR using SP and storing the result in R12. R12 will be pushed into the stack. During function epilogue R12 will be popped and AUT R12, LR, SP will be used to verify that the content of LR is still valid before return. Here an example of PAC instrumented function prologue and epilogue: void foo (void); int main() { foo (); return 0; } Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret -mthumb' translates into: main: pac ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr The patch also takes care of generating a PACBTI instruction in place of the sequence BTI+PAC when Branch Target Identification is enabled contextually. Ex. the previous example compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret+bti -mthumb' translates into: main: pacbti ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr As part of previous upstream suggestions a test for varargs has been added and '-mtpcs-frame' is deemed being incompatible with this return signing address feature being introduced. [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> gcc/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * config/arm/arm.h (arm_arch8m_main): Declare it. * config/arm/arm.cc (arm_arch8m_main): Define it. (arm_option_reconfigure_globals): Set arm_arch8m_main. (arm_compute_frame_layout, arm_expand_prologue) (thumb2_expand_return, arm_expand_epilogue) (arm_conditional_register_usage): Update for pac codegen. (arm_current_function_pac_enabled_p): New function. * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): Add new patterns. * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. gcc/testsuite/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * gcc.target/arm/pac.h : New file. * gcc.target/arm/pac-1.c : New test case. * gcc.target/arm/pac-2.c : Likewise. * gcc.target/arm/pac-3.c : Likewise. * gcc.target/arm/pac-4.c : Likewise. * gcc.target/arm/pac-5.c : Likewise. * gcc.target/arm/pac-6.c : Likewise. * gcc.target/arm/pac-7.c : Likewise. * gcc.target/arm/pac-8.c : Likewise. --- gcc/config/arm/arm-protos.h | 1 + gcc/config/arm/arm.cc | 77 +++++++++++++++++++++++----- gcc/config/arm/arm.h | 4 ++ gcc/config/arm/arm.md | 23 +++++++++ gcc/config/arm/unspecs.md | 3 ++ gcc/testsuite/gcc.target/arm/pac-1.c | 12 +++++ gcc/testsuite/gcc.target/arm/pac-2.c | 11 ++++ gcc/testsuite/gcc.target/arm/pac-3.c | 11 ++++ gcc/testsuite/gcc.target/arm/pac-4.c | 10 ++++ gcc/testsuite/gcc.target/arm/pac-5.c | 28 ++++++++++ gcc/testsuite/gcc.target/arm/pac-6.c | 18 +++++++ gcc/testsuite/gcc.target/arm/pac-7.c | 32 ++++++++++++ gcc/testsuite/gcc.target/arm/pac-8.c | 34 ++++++++++++ gcc/testsuite/gcc.target/arm/pac.h | 17 ++++++ 14 files changed, 268 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/pac-1.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-2.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-3.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-4.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-5.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-6.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-7.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-8.c create mode 100644 gcc/testsuite/gcc.target/arm/pac.h diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index cff7ff1da2a..84764bf27ce 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -379,6 +379,7 @@ extern int vfp3_const_double_for_bits (rtx); extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, rtx); extern bool arm_fusion_enabled_p (tune_params::fuse_ops); +extern bool arm_current_function_pac_enabled_p (void); extern bool arm_valid_symbolic_address_p (rtx); extern bool arm_validize_comparison (rtx *, rtx *, rtx *); extern bool arm_expand_vector_compare (rtx, rtx_code, rtx, rtx, bool); diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index a5cf4225aa2..fa0f9a61498 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -927,6 +927,11 @@ int arm_arch8_3 = 0; /* Nonzero if this chip supports the ARM Architecture 8.4 extensions. */ int arm_arch8_4 = 0; + +/* Nonzero if this chip supports the ARM Architecture 8-M Mainline + extensions. */ +int arm_arch8m_main = 0; + /* Nonzero if this chip supports the ARM Architecture 8.1-M Mainline extensions. */ int arm_arch8_1m_main = 0; @@ -3209,6 +3214,9 @@ arm_option_override_internal (struct gcc_options *opts, arm_stack_protector_guard_offset = offs; } + if (arm_current_function_pac_enabled_p () && !arm_arch8m_main) + error ("This architecture does not support branch protection instructions"); + #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; #endif @@ -3855,6 +3863,7 @@ arm_option_reconfigure_globals (void) arm_arch_arm_hwdiv = bitmap_bit_p (arm_active_target.isa, isa_bit_adiv); arm_arch_crc = bitmap_bit_p (arm_active_target.isa, isa_bit_crc32); arm_arch_cmse = bitmap_bit_p (arm_active_target.isa, isa_bit_cmse); + arm_arch8m_main = arm_arch7 && arm_arch_cmse; arm_arch_lpae = bitmap_bit_p (arm_active_target.isa, isa_bit_lpae); arm_arch_i8mm = bitmap_bit_p (arm_active_target.isa, isa_bit_i8mm); arm_arch_bf16 = bitmap_bit_p (arm_active_target.isa, isa_bit_bf16); @@ -21139,6 +21148,9 @@ arm_compute_save_core_reg_mask (void) save_reg_mask |= arm_compute_save_reg0_reg12_mask (); + if (arm_current_function_pac_enabled_p ()) + save_reg_mask |= 1 << IP_REGNUM; + /* Decide if we need to save the link register. Interrupt routines have their own banked link register, so they never need to save it. @@ -23362,6 +23374,12 @@ output_probe_stack_range (rtx reg1, rtx reg2) return ""; } +static bool +aarch_bti_enabled () +{ + return false; +} + /* Generate the prologue instructions for entry into an ARM or Thumb-2 function. */ void @@ -23440,12 +23458,13 @@ arm_expand_prologue (void) /* The static chain register is the same as the IP register. If it is clobbered when creating the frame, we need to save and restore it. */ - clobber_ip = IS_NESTED (func_type) - && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) - || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK - || flag_stack_clash_protection) - && !df_regs_ever_live_p (LR_REGNUM) - && arm_r3_live_at_start_p ())); + clobber_ip = (IS_NESTED (func_type) + && (((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) + || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK + || flag_stack_clash_protection) + && !df_regs_ever_live_p (LR_REGNUM) + && arm_r3_live_at_start_p ())) + || (arm_current_function_pac_enabled_p ()))); /* Find somewhere to store IP whilst the frame is being created. We try the following places in order: @@ -23470,7 +23489,8 @@ arm_expand_prologue (void) { rtx addr, dwarf; - gcc_assert(arm_compute_static_chain_stack_bytes() == 4); + gcc_assert(arm_compute_static_chain_stack_bytes() == 4 + || arm_current_function_pac_enabled_p ()); saved_regs += 4; addr = gen_rtx_PRE_DEC (Pmode, stack_pointer_rtx); @@ -23521,6 +23541,14 @@ arm_expand_prologue (void) } } + if (arm_current_function_pac_enabled_p ()) + { + if (aarch_bti_enabled ()) + emit_insn (gen_pacbti_nop ()); + else + emit_insn (gen_pac_nop ()); + } + if (TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) { if (IS_INTERRUPT (func_type)) @@ -27309,7 +27337,7 @@ thumb2_expand_return (bool simple_return) to assert it for now to ensure that future code changes do not silently change this behavior. */ gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); - if (num_regs == 1) + if (num_regs == 1 && !arm_current_function_pac_enabled_p ()) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); rtx reg = gen_rtx_REG (SImode, PC_REGNUM); @@ -27324,10 +27352,20 @@ thumb2_expand_return (bool simple_return) } else { - saved_regs_mask &= ~ (1 << LR_REGNUM); - saved_regs_mask |= (1 << PC_REGNUM); - arm_emit_multi_reg_pop (saved_regs_mask); - } + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } + else + { + saved_regs_mask &= ~ (1 << LR_REGNUM); + saved_regs_mask |= (1 << PC_REGNUM); + arm_emit_multi_reg_pop (saved_regs_mask); + } + } } else { @@ -27733,7 +27771,8 @@ arm_expand_epilogue (bool really_return) && really_return && crtl->args.pretend_args_size == 0 && saved_regs_mask & (1 << LR_REGNUM) - && !crtl->calls_eh_return) + && !crtl->calls_eh_return + && !arm_current_function_pac_enabled_p ()) { saved_regs_mask &= ~(1 << LR_REGNUM); saved_regs_mask |= (1 << PC_REGNUM); @@ -27847,6 +27886,9 @@ arm_expand_epilogue (bool really_return) } } + if (arm_current_function_pac_enabled_p ()) + emit_insn (gen_aut_nop ()); + if (!really_return) return; @@ -32941,6 +32983,15 @@ arm_fusion_enabled_p (tune_params::fuse_ops op) return current_tune->fusible_ops & op; } +/* Return TRUE if return address signing mechanism is enabled. */ +bool +arm_current_function_pac_enabled_p (void) +{ + return (aarch_ra_sign_scope == AARCH_FUNCTION_ALL + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF + && !crtl->is_leaf)); +} + /* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be scheduled for speculative execution. Reject the long-running division and square-root instructions. */ diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 3495ab857ea..e33425bbf42 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -510,6 +510,10 @@ extern int arm_arch8_3; /* Nonzero if this chip supports the ARM Architecture 8.4 extensions. */ extern int arm_arch8_4; +/* Nonzero if this chip supports the ARM Architecture 8-M Mainline + extensions. */ +extern int arm_arch8m_main; + /* Nonzero if this chip supports the ARM Architecture 8.1-M Mainline extensions. */ extern int arm_arch8_1m_main; diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 60468f6182c..7255fa98f5d 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -12890,6 +12890,29 @@ (define_insn "*speculation_barrier_insn" (set_attr "length" "8")] ) +(define_insn "pac_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PAC_NOP))] + "arm_arch8m_main" + "pac\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + +(define_insn "pacbti_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PACBTI_NOP))] + "arm_arch8m_main" + "pacbti\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + +(define_insn "aut_nop" + [(unspec:SI [(reg:SI IP_REGNUM) (reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_AUT_NOP)] + "arm_arch8m_main" + "aut\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + ;; Vector bits common to IWMMXT, Neon and MVE (include "vec-common.md") ;; Load the Intel Wireless Multimedia Extension patterns diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md index 7748e784379..dbe243a03f6 100644 --- a/gcc/config/arm/unspecs.md +++ b/gcc/config/arm/unspecs.md @@ -159,6 +159,9 @@ (define_c_enum "unspec" [ UNSPEC_VCDE ; Custom Datapath Extension instruction. UNSPEC_VCDEA ; Custom Datapath Extension instruction. UNSPEC_DLS ; Used for DLS (Do Loop Start), Armv8.1-M Mainline instruction + UNSPEC_PAC_NOP ; Represents PAC signing LR + UNSPEC_PACBTI_NOP ; Represents PAC signing LR + valid landing pad + UNSPEC_AUT_NOP ; Represents PAC verifying LR ]) diff --git a/gcc/testsuite/gcc.target/arm/pac-1.c b/gcc/testsuite/gcc.target/arm/pac-1.c new file mode 100644 index 00000000000..6cd64dbd014 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-1.c @@ -0,0 +1,12 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ + diff --git a/gcc/testsuite/gcc.target/arm/pac-2.c b/gcc/testsuite/gcc.target/arm/pac-2.c new file mode 100644 index 00000000000..945ce938592 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-2.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-3.c b/gcc/testsuite/gcc.target/arm/pac-3.c new file mode 100644 index 00000000000..47e290a5840 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-3.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=bti+pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pacbti\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-4.c b/gcc/testsuite/gcc.target/arm/pac-4.c new file mode 100644 index 00000000000..cf915cdba50 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-4.c @@ -0,0 +1,10 @@ +/* Testing return address signing. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-not "\tbti\t" } } */ +/* { dg-final { scan-assembler-not "\tpac\t" } } */ +/* { dg-final { scan-assembler-not "\tpacbti\t" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-5.c b/gcc/testsuite/gcc.target/arm/pac-5.c new file mode 100644 index 00000000000..c70087eb6b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-5.c @@ -0,0 +1,28 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int square (int z) { return z * z; } + return square (a) + square (b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 5) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-6.c b/gcc/testsuite/gcc.target/arm/pac-6.c new file mode 100644 index 00000000000..c5329f0ef48 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-6.c @@ -0,0 +1,18 @@ +/* Check that GCC does .save and .cfi_offset directives with RA_AUTH_CODE pseudo hard-register. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+fp -mbranch-protection=pac-ret+leaf -mthumb --save-temps -O0 -g" } */ + +int i; + +void foo (int); + +int bar() +{ + foo (i); + return 0; +} + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "bti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-7.c b/gcc/testsuite/gcc.target/arm/pac-7.c new file mode 100644 index 00000000000..cdaebca5cfa --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-7.c @@ -0,0 +1,32 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int x = 4; + int foo2 (int a, int b) + { + return a + b + x; + } + return foo2 (a, b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 7) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-8.c b/gcc/testsuite/gcc.target/arm/pac-8.c new file mode 100644 index 00000000000..3f37dcfa5c8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-8.c @@ -0,0 +1,34 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdarg.h> +#include <stdlib.h> + +int acc (int n, ...) +{ + int sum = 0; + va_list ptr; + + va_start (ptr, n); + + for (int i = 0; i < n; i++) + sum += va_arg (ptr, int); + va_end (ptr); + + return sum; +} + +int main() +{ + if (acc (3, 1, 2, 3) != 6) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac.h b/gcc/testsuite/gcc.target/arm/pac.h new file mode 100644 index 00000000000..7355e6b2954 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac.h @@ -0,0 +1,17 @@ +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + return a + b; +} + +int +main (void) +{ + if (foo1 (1, 2) != 3) + abort (); + + return 0; +} -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen 2022-10-28 16:34 ` [PATCH 10/15 V3] " Andrea Corallo @ 2022-11-07 8:57 ` Andrea Corallo 2022-12-05 16:38 ` Richard Earnshaw 0 siblings, 1 reply; 5+ messages in thread From: Andrea Corallo @ 2022-11-07 8:57 UTC (permalink / raw) To: Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, nd, Richard Earnshaw [-- Attachment #1: Type: text/plain, Size: 150 bytes --] Hi all, please find attached the lastest version of this patch incorporating some more improvents. Feel free to ignore V3. Best Regards Andrea [-- Attachment #2: 0001-PATCH-10-15-arm-Implement-cortex-M-return-signing-ad.patch --] [-- Type: text/plain, Size: 20180 bytes --] From 869630801de2d3df03ce2f2551fd801dd59a640c Mon Sep 17 00:00:00 2001 From: Andrea Corallo <andrea.corallo@arm.com> Date: Thu, 20 Jan 2022 15:36:23 +0100 Subject: [PATCH] [PATCH 10/15] arm: Implement cortex-M return signing address codegen Hi all, this patch enables address return signature and verification based on Armv8.1-M Pointer Authentication [1]. To sign the return address, we use the PAC R12, LR, SP instruction upon function entry. This is signing LR using SP and storing the result in R12. R12 will be pushed into the stack. During function epilogue R12 will be popped and AUT R12, LR, SP will be used to verify that the content of LR is still valid before return. Here an example of PAC instrumented function prologue and epilogue: void foo (void); int main() { foo (); return 0; } Compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret -mthumb' translates into: main: pac ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr The patch also takes care of generating a PACBTI instruction in place of the sequence BTI+PAC when Branch Target Identification is enabled contextually. Ex. the previous example compiled with '-march=armv8.1-m.main -mbranch-protection=pac-ret+bti -mthumb' translates into: main: pacbti ip, lr, sp push {r3, r7, ip, lr} add r7, sp, #0 bl foo movs r3, #0 mov r0, r3 pop {r3, r7, ip, lr} aut ip, lr, sp bx lr As part of previous upstream suggestions a test for varargs has been added and '-mtpcs-frame' is deemed being incompatible with this return signing address feature being introduced. [1] <https://community.arm.com/developer/ip-products/processors/b/processors-ip-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension> gcc/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * config/arm/arm.h (arm_arch8m_main): Declare it. * config/arm/arm.cc (arm_arch8m_main): Define it. (arm_option_reconfigure_globals): Set arm_arch8m_main. (arm_compute_frame_layout, arm_expand_prologue) (thumb2_expand_return, arm_expand_epilogue) (arm_conditional_register_usage): Update for pac codegen. (arm_current_function_pac_enabled_p): New function. * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): Add new patterns. * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. gcc/testsuite/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * gcc.target/arm/pac.h : New file. * gcc.target/arm/pac-1.c : New test case. * gcc.target/arm/pac-2.c : Likewise. * gcc.target/arm/pac-3.c : Likewise. * gcc.target/arm/pac-4.c : Likewise. * gcc.target/arm/pac-5.c : Likewise. * gcc.target/arm/pac-6.c : Likewise. * gcc.target/arm/pac-7.c : Likewise. * gcc.target/arm/pac-8.c : Likewise. --- gcc/config/arm/arm-protos.h | 1 + gcc/config/arm/arm.cc | 80 +++++++++++++++++++++++----- gcc/config/arm/arm.h | 4 ++ gcc/config/arm/arm.md | 23 ++++++++ gcc/config/arm/unspecs.md | 3 ++ gcc/testsuite/gcc.target/arm/pac-1.c | 12 +++++ gcc/testsuite/gcc.target/arm/pac-2.c | 11 ++++ gcc/testsuite/gcc.target/arm/pac-3.c | 11 ++++ gcc/testsuite/gcc.target/arm/pac-4.c | 10 ++++ gcc/testsuite/gcc.target/arm/pac-5.c | 28 ++++++++++ gcc/testsuite/gcc.target/arm/pac-6.c | 18 +++++++ gcc/testsuite/gcc.target/arm/pac-7.c | 32 +++++++++++ gcc/testsuite/gcc.target/arm/pac-8.c | 34 ++++++++++++ gcc/testsuite/gcc.target/arm/pac.h | 17 ++++++ 14 files changed, 271 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/pac-1.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-2.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-3.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-4.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-5.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-6.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-7.c create mode 100644 gcc/testsuite/gcc.target/arm/pac-8.c create mode 100644 gcc/testsuite/gcc.target/arm/pac.h diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index cff7ff1da2a..84764bf27ce 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -379,6 +379,7 @@ extern int vfp3_const_double_for_bits (rtx); extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, rtx); extern bool arm_fusion_enabled_p (tune_params::fuse_ops); +extern bool arm_current_function_pac_enabled_p (void); extern bool arm_valid_symbolic_address_p (rtx); extern bool arm_validize_comparison (rtx *, rtx *, rtx *); extern bool arm_expand_vector_compare (rtx, rtx_code, rtx, rtx, bool); diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index a5cf4225aa2..be5229ffd1d 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -927,6 +927,11 @@ int arm_arch8_3 = 0; /* Nonzero if this chip supports the ARM Architecture 8.4 extensions. */ int arm_arch8_4 = 0; + +/* Nonzero if this chip supports the ARM Architecture 8-M Mainline + extensions. */ +int arm_arch8m_main = 0; + /* Nonzero if this chip supports the ARM Architecture 8.1-M Mainline extensions. */ int arm_arch8_1m_main = 0; @@ -3209,6 +3214,9 @@ arm_option_override_internal (struct gcc_options *opts, arm_stack_protector_guard_offset = offs; } + if (arm_current_function_pac_enabled_p () && !arm_arch8m_main) + error ("This architecture does not support branch protection instructions"); + #ifdef SUBTARGET_OVERRIDE_INTERNAL_OPTIONS SUBTARGET_OVERRIDE_INTERNAL_OPTIONS; #endif @@ -3855,6 +3863,7 @@ arm_option_reconfigure_globals (void) arm_arch_arm_hwdiv = bitmap_bit_p (arm_active_target.isa, isa_bit_adiv); arm_arch_crc = bitmap_bit_p (arm_active_target.isa, isa_bit_crc32); arm_arch_cmse = bitmap_bit_p (arm_active_target.isa, isa_bit_cmse); + arm_arch8m_main = arm_arch7 && arm_arch_cmse; arm_arch_lpae = bitmap_bit_p (arm_active_target.isa, isa_bit_lpae); arm_arch_i8mm = bitmap_bit_p (arm_active_target.isa, isa_bit_i8mm); arm_arch_bf16 = bitmap_bit_p (arm_active_target.isa, isa_bit_bf16); @@ -21139,6 +21148,9 @@ arm_compute_save_core_reg_mask (void) save_reg_mask |= arm_compute_save_reg0_reg12_mask (); + if (arm_current_function_pac_enabled_p ()) + save_reg_mask |= 1 << IP_REGNUM; + /* Decide if we need to save the link register. Interrupt routines have their own banked link register, so they never need to save it. @@ -23362,6 +23374,12 @@ output_probe_stack_range (rtx reg1, rtx reg2) return ""; } +static bool +aarch_bti_enabled () +{ + return false; +} + /* Generate the prologue instructions for entry into an ARM or Thumb-2 function. */ void @@ -23440,12 +23458,13 @@ arm_expand_prologue (void) /* The static chain register is the same as the IP register. If it is clobbered when creating the frame, we need to save and restore it. */ - clobber_ip = IS_NESTED (func_type) - && ((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) - || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK - || flag_stack_clash_protection) - && !df_regs_ever_live_p (LR_REGNUM) - && arm_r3_live_at_start_p ())); + clobber_ip = (IS_NESTED (func_type) + && (((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) + || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK + || flag_stack_clash_protection) + && !df_regs_ever_live_p (LR_REGNUM) + && arm_r3_live_at_start_p ())) + || (arm_current_function_pac_enabled_p ()))); /* Find somewhere to store IP whilst the frame is being created. We try the following places in order: @@ -23470,7 +23489,8 @@ arm_expand_prologue (void) { rtx addr, dwarf; - gcc_assert(arm_compute_static_chain_stack_bytes() == 4); + gcc_assert(arm_compute_static_chain_stack_bytes() == 4 + || arm_current_function_pac_enabled_p ()); saved_regs += 4; addr = gen_rtx_PRE_DEC (Pmode, stack_pointer_rtx); @@ -23521,6 +23541,17 @@ arm_expand_prologue (void) } } + if (arm_current_function_pac_enabled_p ()) + { + /* If IP was clobbered we only emit a PAC instruction as the BTI + one will be added before the push of the clobbered IP (if + necessary) by the bti pass. */ + if (aarch_bti_enabled () && !clobber_ip) + emit_insn (gen_pacbti_nop ()); + else + emit_insn (gen_pac_nop ()); + } + if (TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) { if (IS_INTERRUPT (func_type)) @@ -27309,7 +27340,7 @@ thumb2_expand_return (bool simple_return) to assert it for now to ensure that future code changes do not silently change this behavior. */ gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); - if (num_regs == 1) + if (num_regs == 1 && !arm_current_function_pac_enabled_p ()) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); rtx reg = gen_rtx_REG (SImode, PC_REGNUM); @@ -27324,10 +27355,20 @@ thumb2_expand_return (bool simple_return) } else { - saved_regs_mask &= ~ (1 << LR_REGNUM); - saved_regs_mask |= (1 << PC_REGNUM); - arm_emit_multi_reg_pop (saved_regs_mask); - } + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } + else + { + saved_regs_mask &= ~ (1 << LR_REGNUM); + saved_regs_mask |= (1 << PC_REGNUM); + arm_emit_multi_reg_pop (saved_regs_mask); + } + } } else { @@ -27733,7 +27774,8 @@ arm_expand_epilogue (bool really_return) && really_return && crtl->args.pretend_args_size == 0 && saved_regs_mask & (1 << LR_REGNUM) - && !crtl->calls_eh_return) + && !crtl->calls_eh_return + && !arm_current_function_pac_enabled_p ()) { saved_regs_mask &= ~(1 << LR_REGNUM); saved_regs_mask |= (1 << PC_REGNUM); @@ -27847,6 +27889,9 @@ arm_expand_epilogue (bool really_return) } } + if (arm_current_function_pac_enabled_p ()) + emit_insn (gen_aut_nop ()); + if (!really_return) return; @@ -32941,6 +32986,15 @@ arm_fusion_enabled_p (tune_params::fuse_ops op) return current_tune->fusible_ops & op; } +/* Return TRUE if return address signing mechanism is enabled. */ +bool +arm_current_function_pac_enabled_p (void) +{ + return (aarch_ra_sign_scope == AARCH_FUNCTION_ALL + || (aarch_ra_sign_scope == AARCH_FUNCTION_NON_LEAF + && !crtl->is_leaf)); +} + /* Implement TARGET_SCHED_CAN_SPECULATE_INSN. Return true if INSN can be scheduled for speculative execution. Reject the long-running division and square-root instructions. */ diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 3495ab857ea..e33425bbf42 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -510,6 +510,10 @@ extern int arm_arch8_3; /* Nonzero if this chip supports the ARM Architecture 8.4 extensions. */ extern int arm_arch8_4; +/* Nonzero if this chip supports the ARM Architecture 8-M Mainline + extensions. */ +extern int arm_arch8m_main; + /* Nonzero if this chip supports the ARM Architecture 8.1-M Mainline extensions. */ extern int arm_arch8_1m_main; diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md index 60468f6182c..7255fa98f5d 100644 --- a/gcc/config/arm/arm.md +++ b/gcc/config/arm/arm.md @@ -12890,6 +12890,29 @@ (define_insn "*speculation_barrier_insn" (set_attr "length" "8")] ) +(define_insn "pac_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PAC_NOP))] + "arm_arch8m_main" + "pac\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + +(define_insn "pacbti_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PACBTI_NOP))] + "arm_arch8m_main" + "pacbti\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + +(define_insn "aut_nop" + [(unspec:SI [(reg:SI IP_REGNUM) (reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_AUT_NOP)] + "arm_arch8m_main" + "aut\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) + ;; Vector bits common to IWMMXT, Neon and MVE (include "vec-common.md") ;; Load the Intel Wireless Multimedia Extension patterns diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md index 7748e784379..dbe243a03f6 100644 --- a/gcc/config/arm/unspecs.md +++ b/gcc/config/arm/unspecs.md @@ -159,6 +159,9 @@ (define_c_enum "unspec" [ UNSPEC_VCDE ; Custom Datapath Extension instruction. UNSPEC_VCDEA ; Custom Datapath Extension instruction. UNSPEC_DLS ; Used for DLS (Do Loop Start), Armv8.1-M Mainline instruction + UNSPEC_PAC_NOP ; Represents PAC signing LR + UNSPEC_PACBTI_NOP ; Represents PAC signing LR + valid landing pad + UNSPEC_AUT_NOP ; Represents PAC verifying LR ]) diff --git a/gcc/testsuite/gcc.target/arm/pac-1.c b/gcc/testsuite/gcc.target/arm/pac-1.c new file mode 100644 index 00000000000..6cd64dbd014 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-1.c @@ -0,0 +1,12 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ + diff --git a/gcc/testsuite/gcc.target/arm/pac-2.c b/gcc/testsuite/gcc.target/arm/pac-2.c new file mode 100644 index 00000000000..945ce938592 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-2.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-3.c b/gcc/testsuite/gcc.target/arm/pac-3.c new file mode 100644 index 00000000000..47e290a5840 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-3.c @@ -0,0 +1,11 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=bti+pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-times "pacbti\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-4.c b/gcc/testsuite/gcc.target/arm/pac-4.c new file mode 100644 index 00000000000..cf915cdba50 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-4.c @@ -0,0 +1,10 @@ +/* Testing return address signing. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mthumb -mfloat-abi=hard --save-temps -O2" } */ + +#include "pac.h" + +/* { dg-final { scan-assembler-not "\tbti\t" } } */ +/* { dg-final { scan-assembler-not "\tpac\t" } } */ +/* { dg-final { scan-assembler-not "\tpacbti\t" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-5.c b/gcc/testsuite/gcc.target/arm/pac-5.c new file mode 100644 index 00000000000..c70087eb6b6 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-5.c @@ -0,0 +1,28 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int square (int z) { return z * z; } + return square (a) + square (b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 5) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-6.c b/gcc/testsuite/gcc.target/arm/pac-6.c new file mode 100644 index 00000000000..c5329f0ef48 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-6.c @@ -0,0 +1,18 @@ +/* Check that GCC does .save and .cfi_offset directives with RA_AUTH_CODE pseudo hard-register. */ +/* { dg-do compile } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-options "-march=armv8.1-m.main+fp -mbranch-protection=pac-ret+leaf -mthumb --save-temps -O0 -g" } */ + +int i; + +void foo (int); + +int bar() +{ + foo (i); + return 0; +} + +/* { dg-final { scan-assembler "pac\tip, lr, sp" } } */ +/* { dg-final { scan-assembler "aut\tip, lr, sp" } } */ +/* { dg-final { scan-assembler-not "bti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-7.c b/gcc/testsuite/gcc.target/arm/pac-7.c new file mode 100644 index 00000000000..cdaebca5cfa --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-7.c @@ -0,0 +1,32 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + int x = 4; + int foo2 (int a, int b) + { + return a + b + x; + } + return foo2 (a, b); +} + +int +main (void) +{ + if (foo1 (1, 2) != 7) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 3 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac-8.c b/gcc/testsuite/gcc.target/arm/pac-8.c new file mode 100644 index 00000000000..3f37dcfa5c8 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac-8.c @@ -0,0 +1,34 @@ +/* Testing return address signing. */ +/* { dg-do run } */ +/* { dg-require-effective-target mbranch_protection_ok } */ +/* { dg-require-effective-target arm_pacbti_hw } */ +/* { dg-options "-march=armv8.1-m.main+pacbti+fp -mbranch-protection=pac-ret+leaf -mthumb -mfloat-abi=hard --save-temps -O0" } */ + +#include <stdarg.h> +#include <stdlib.h> + +int acc (int n, ...) +{ + int sum = 0; + va_list ptr; + + va_start (ptr, n); + + for (int i = 0; i < n; i++) + sum += va_arg (ptr, int); + va_end (ptr); + + return sum; +} + +int main() +{ + if (acc (3, 1, 2, 3) != 6) + abort (); + + return 0; +} + +/* { dg-final { scan-assembler-times "pac\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-times "aut\tip, lr, sp" 2 } } */ +/* { dg-final { scan-assembler-not "\tbti" } } */ diff --git a/gcc/testsuite/gcc.target/arm/pac.h b/gcc/testsuite/gcc.target/arm/pac.h new file mode 100644 index 00000000000..7355e6b2954 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pac.h @@ -0,0 +1,17 @@ +#include <stdlib.h> + +int +__attribute__((noinline)) +foo1 (int a, int b) +{ + return a + b; +} + +int +main (void) +{ + if (foo1 (1, 2) != 3) + abort (); + + return 0; +} -- 2.25.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen 2022-11-07 8:57 ` [PATCH 10/15 V4] " Andrea Corallo @ 2022-12-05 16:38 ` Richard Earnshaw 0 siblings, 0 replies; 5+ messages in thread From: Richard Earnshaw @ 2022-12-05 16:38 UTC (permalink / raw) To: Andrea Corallo, Andrea Corallo via Gcc-patches; +Cc: Richard Earnshaw, nd On 07/11/2022 08:57, Andrea Corallo via Gcc-patches wrote: > Hi all, > > please find attached the lastest version of this patch incorporating some > more improvents. Feel free to ignore V3. > > Best Regards > > Andrea > > As part of previous upstream suggestions a test for varargs has been > added and '-mtpcs-frame' is deemed being incompatible with this return > signing address feature being introduced. I don't see any check for the tpcs-frame incompatibility? What happens if a user does combine the options? gcc/Changelog 2021-11-03 Andrea Corallo <andrea.corallo@arm.com> * config/arm/arm.h (arm_arch8m_main): Declare it. * config/arm/arm.cc (arm_arch8m_main): Define it. (arm_option_reconfigure_globals): Set arm_arch8m_main. (arm_compute_frame_layout, arm_expand_prologue) (thumb2_expand_return, arm_expand_epilogue) (arm_conditional_register_usage): Update for pac codegen. (arm_current_function_pac_enabled_p): New function. * config/arm/arm.md (pac_ip_lr_sp, pacbti_ip_lr_sp, aut_ip_lr_sp): Add new patterns. * config/arm/unspecs.md (UNSPEC_PAC_IP_LR_SP) (UNSPEC_PACBTI_IP_LR_SP, UNSPEC_AUT_IP_LR_SP): Add unspecs. You're missing an entry for aarch_bti_enabled () - yes I realize that's just a placeholder at present and will be fully defined in patch 12. +static bool +aarch_bti_enabled () +{ + return false; +} + No comment on this function (and in patch 12 it moves to a different location). It would be best to have it in the right place at this point in time. + clobber_ip = (IS_NESTED (func_type) + && (((TARGET_APCS_FRAME && frame_pointer_needed && TARGET_ARM) + || ((flag_stack_check == STATIC_BUILTIN_STACK_CHECK + || flag_stack_clash_protection) + && !df_regs_ever_live_p (LR_REGNUM) + && arm_r3_live_at_start_p ())) + || (arm_current_function_pac_enabled_p ()))); Redundant parenthesis around arm_current_function_pac_enabled_p () call. + gcc_assert(arm_compute_static_chain_stack_bytes() == 4 + || arm_current_function_pac_enabled_p ()); I wonder if this assert is now really serving a useful purpose. I'd consider removing it. @@ -27309,7 +27340,7 @@ thumb2_expand_return (bool simple_return) to assert it for now to ensure that future code changes do not silently change this behavior. */ gcc_assert (!IS_CMSE_ENTRY (arm_current_func_type ())); - if (num_regs == 1) + if (num_regs == 1 && !arm_current_function_pac_enabled_p ()) { rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2)); rtx reg = gen_rtx_REG (SImode, PC_REGNUM); @@ -27324,10 +27355,20 @@ thumb2_expand_return (bool simple_return) } else { - saved_regs_mask &= ~ (1 << LR_REGNUM); - saved_regs_mask |= (1 << PC_REGNUM); - arm_emit_multi_reg_pop (saved_regs_mask); - } + if (arm_current_function_pac_enabled_p ()) + { + gcc_assert (!(saved_regs_mask & (1 << PC_REGNUM))); + arm_emit_multi_reg_pop (saved_regs_mask); + emit_insn (gen_aut_nop ()); + emit_jump_insn (simple_return_rtx); + } + else + { + saved_regs_mask &= ~ (1 << LR_REGNUM); + saved_regs_mask |= (1 << PC_REGNUM); + arm_emit_multi_reg_pop (saved_regs_mask); + } + } } else The logic for these blocks would, I think, be better expressed as if (pac_enabled) ... else if (num_regs == 1) ... // existing code else ... // existing code Also, I think (out of an abundance of caution) we really need a scheduling barrier placed before calls to gen_aut_nop() pattern is emitted, to ensure that the scheduler never tries to move this instruction away from the position we place it. Use gen_blockage() for that (see TARGET_SCHED_PROLOG). Alternatively, we could make the UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC) without needing an additional insn - if you use this approach, then please make sure this is explained in a comment. +(define_insn "pacbti_nop" + [(set (reg:SI IP_REGNUM) + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] + UNSPEC_PACBTI_NOP))] + "arm_arch8m_main" + "pacbti\t%|ip, %|lr, %|sp" + [(set_attr "conds" "unconditional")]) The additional side-effect of this being a BTI landing pad means that we mustn't move any other instruction before it. So I think this needs to be an unspec_volatile as well. On the tests, they are OK as they stand, but we lack anything that will be tested when suitable hardware is unavailable (all tests are "dg-do run"). Can we please have some compile-only tests as well? R. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-06 17:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-06 15:46 [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen Andrea Corallo 2022-12-06 16:24 ` Richard Earnshaw 2022-12-06 17:19 ` Andrea Corallo -- strict thread matches above, loose matches on Subject: below -- 2022-08-12 14:26 [PATCH 0/15] arm: Enables return address verification and branch target identification on Cortex-M Andrea Corallo 2022-08-12 15:36 ` [PATCH 10/15] arm: Implement cortex-M return signing address codegen Andrea Corallo 2022-09-14 14:20 ` [PATCH 10/15 V2] " Andrea Corallo 2022-10-21 12:58 ` Richard Earnshaw 2022-10-26 15:48 ` Andrea Corallo 2022-10-28 16:34 ` [PATCH 10/15 V3] " Andrea Corallo 2022-11-07 8:57 ` [PATCH 10/15 V4] " Andrea Corallo 2022-12-05 16:38 ` Richard Earnshaw
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).