v2: - emit bti based on feedback from Richard Sandiford (dont copy varasm logic). - add testcases. - kept bti outside the patch area if possible, i.e. option (b) in earlier discussion.
This fix does not update the documentation of the generic option, I think some text would be useful there about patch area layout with indirect branch hardening (but i wanted to keep this fix target specific). gcc/ChangeLog: 2020-01-21 Szabolcs Nagy <szabolcs.n...@arm.com> * config/aarch64/aarch64.c (aarch64_declare_function_name): Set cfun->machine->label_is_assembled. (aarch64_print_patchable_function_entry): New. (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): Define. * config/aarch64/aarch64.h (struct machine_function): New field, label_is_assembled. gcc/testsuite/ChangeLog: 2020-01-21 Szabolcs Nagy <szabolcs.n...@arm.com> * gcc.target/aarch64/pr92424-1.c: New test. * gcc.target/aarch64/pr92424-2.c: New test. * gcc.target/aarch64/pr92424-3.c: New test.
From 65a60c0dc4318a33e0a0e0a6573084d84bd18a88 Mon Sep 17 00:00:00 2001 From: Szabolcs Nagy <szabolcs.n...@arm.com> Date: Wed, 15 Jan 2020 12:23:40 +0000 Subject: [PATCH] [AArch64] PR92424: Fix -fpatchable-function-entry=N,M with BTI This is a workaround that emits a BTI after the function label if that is followed by a patch area. We try to remove the BTI that follows the patch area (this may fail e.g. if the first instruction is a PACIASP). So before this commit -fpatchable-function-entry=3,1 with bti generates .section __patchable_function_entries .8byte .LPFE .text .LPFE: nop foo: nop nop bti c // or paciasp ... and after this commit .section __patchable_function_entries .8byte .LPFE .text .LPFE: nop foo: bti c nop nop // may be paciasp ... and with -fpatchable-function-entry=1 (M=0) the code now is foo: bti c .section __patchable_function_entries .8byte .LPFE .text .LPFE: nop // may be paciasp ... There is a new bti insn in the middle of the patchable area users need to be aware of unless M=0 (patch area is after the new bti) or M=N (patch area is before the label, no new bti). Note: bti is not added to all functions consistently (it can be turned off per function using a target attribute or the compiler may detect that the function is never called indirectly), so if bti is inserted in the middle of a patch area then user code needs to deal with detecting it. Tested on aarch64-none-linux-gnu. gcc/ChangeLog: * config/aarch64/aarch64.c (aarch64_declare_function_name): Set cfun->machine->label_is_assembled. (aarch64_print_patchable_function_entry): New. (TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): Define. * config/aarch64/aarch64.h (struct machine_function): New field, label_is_assembled. gcc/testsuite/ChangeLog: * gcc.target/aarch64/pr92424-1.c: New test. * gcc.target/aarch64/pr92424-2.c: New test. * gcc.target/aarch64/pr92424-3.c: New test. --- gcc/config/aarch64/aarch64.c | 31 +++++ gcc/config/aarch64/aarch64.h | 1 + gcc/testsuite/gcc.target/aarch64/pr92424-1.c | 122 +++++++++++++++++++ gcc/testsuite/gcc.target/aarch64/pr92424-2.c | 12 ++ gcc/testsuite/gcc.target/aarch64/pr92424-3.c | 12 ++ 5 files changed, 178 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/pr92424-1.c create mode 100644 gcc/testsuite/gcc.target/aarch64/pr92424-2.c create mode 100644 gcc/testsuite/gcc.target/aarch64/pr92424-3.c diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e40750380cc..ef037e226a7 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -18123,6 +18123,34 @@ aarch64_declare_function_name (FILE *stream, const char* name, /* Don't forget the type directive for ELF. */ ASM_OUTPUT_TYPE_DIRECTIVE (stream, name, "function"); ASM_OUTPUT_LABEL (stream, name); + + cfun->machine->label_is_assembled = true; +} + +/* Implement PRINT_PATCHABLE_FUNCTION_ENTRY. Check if the patch area is after + the function label and emit a BTI if necessary. */ + +void +aarch64_print_patchable_function_entry (FILE *file, + unsigned HOST_WIDE_INT patch_area_size, + bool record_p) +{ + if (cfun->machine->label_is_assembled + && aarch64_bti_enabled () + && !cgraph_node::get (cfun->decl)->only_called_directly_p ()) + { + /* Remove the BTI that follows the patch area and insert a new BTI + before the patch area right after the function label. */ + rtx_insn *insn = next_real_nondebug_insn (get_insns ()); + if (insn + && INSN_P (insn) + && GET_CODE (PATTERN (insn)) == UNSPEC_VOLATILE + && XINT (PATTERN (insn), 1) == UNSPECV_BTI_C) + delete_insn (insn); + asm_fprintf (file, "\thint\t34 // bti c\n"); + } + + default_print_patchable_function_entry (file, patch_area_size, record_p); } /* Implement ASM_OUTPUT_DEF_FROM_DECLS. Output .variant_pcs for aliases. */ @@ -21970,6 +21998,9 @@ aarch64_run_selftests (void) #undef TARGET_ASM_TRAMPOLINE_TEMPLATE #define TARGET_ASM_TRAMPOLINE_TEMPLATE aarch64_asm_trampoline_template +#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY +#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY aarch64_print_patchable_function_entry + #undef TARGET_BUILD_BUILTIN_VA_LIST #define TARGET_BUILD_BUILTIN_VA_LIST aarch64_build_builtin_va_list diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 342fe29bd91..297d197297e 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -842,6 +842,7 @@ typedef struct GTY (()) machine_function struct aarch64_frame frame; /* One entry for each hard register. */ bool reg_is_wrapped_separately[LAST_SAVED_REGNUM]; + bool label_is_assembled; } machine_function; #endif diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-1.c b/gcc/testsuite/gcc.target/aarch64/pr92424-1.c new file mode 100644 index 00000000000..c413a2c306e --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-1.c @@ -0,0 +1,122 @@ +/* { dg-do "compile" } */ +/* { dg-options "-O1" } */ +/* { dg-final { check-function-bodies "**" "" } } */ + +/* Note: this test only checks the instructions in the function bodies, + not the placement of the patch label or nops before the futncion. */ + +/* +**f10_none: +** nop +** ret +*/ +void +__attribute__ ((target("branch-protection=none"), + patchable_function_entry (1, 0))) +f10_none () +{ +} + +/* +**f10_pac: +** hint 34 // bti c +** nop +** hint 25 // paciasp +** hint 29 // autiasp +** ret +*/ +void +__attribute__ ((target("branch-protection=bti+pac-ret+leaf"), + patchable_function_entry (1, 0))) +f10_pac () +{ +} + +/* +**f10_bti: +** hint 34 // bti c +** nop +** ret +*/ +void +__attribute__ ((target("branch-protection=bti"), + patchable_function_entry (1, 0))) +f10_bti () +{ +} + +/* +**f11_none: +** ret +*/ +void +__attribute__ ((target("branch-protection=none"), + patchable_function_entry (1, 1))) +f11_none () +{ +} + +/* +**f11_pac: +** hint 25 // paciasp +** hint 29 // autiasp +** ret +*/ +void +__attribute__ ((target("branch-protection=bti+pac-ret+leaf"), + patchable_function_entry (1, 1))) +f11_pac () +{ +} + +/* +**f11_bti: +** hint 34 // bti c +** ret +*/ +void +__attribute__ ((target("branch-protection=bti"), + patchable_function_entry (1, 1))) +f11_bti () +{ +} + +/* +**f21_none: +** nop +** ret +*/ +void +__attribute__ ((target("branch-protection=none"), + patchable_function_entry (2, 1))) +f21_none () +{ +} + +/* +**f21_pac: +** hint 34 // bti c +** nop +** hint 25 // paciasp +** hint 29 // autiasp +** ret +*/ +void +__attribute__ ((target("branch-protection=bti+pac-ret+leaf"), + patchable_function_entry (2, 1))) +f21_pac () +{ +} + +/* +**f21_bti: +** hint 34 // bti c +** nop +** ret +*/ +void +__attribute__ ((target("branch-protection=bti"), + patchable_function_entry (2, 1))) +f21_bti () +{ +} diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-2.c b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c new file mode 100644 index 00000000000..0e75657a153 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-2.c @@ -0,0 +1,12 @@ +/* { dg-do "compile" } */ +/* { dg-options "-O1" } */ + +/* Test the placement of the .LPFE1 label. */ + +void +__attribute__ ((target("branch-protection=bti"), + patchable_function_entry (1, 0))) +f10_bti () +{ +} +/* { dg-final { scan-assembler "f10_bti:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\tret\n" } } */ diff --git a/gcc/testsuite/gcc.target/aarch64/pr92424-3.c b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c new file mode 100644 index 00000000000..0a1f74d4096 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/pr92424-3.c @@ -0,0 +1,12 @@ +/* { dg-do "compile" } */ +/* { dg-options "-O1" } */ + +/* Test the placement of the .LPFE1 label. */ + +void +__attribute__ ((target("branch-protection=bti+pac-ret+leaf"), + patchable_function_entry (1, 0))) +f10_pac () +{ +} +/* { dg-final { scan-assembler "f10_pac:\n\thint\t34 // bti c\n.*\.LPFE1:\n\tnop\n.*\thint\t25 // paciasp\n.*\thint\t29 // autiasp\n.*\tret\n" } } */ -- 2.17.1