On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu <hjl.to...@gmail.com> wrote:
>
> Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to make sure that the
> ENDBR are emitted before the patch area. When -mfentry -pg is also used
> together, there should be no ENDBR before "call __fentry__".
>
> OK for master if there is no regression?
>
> Thanks.
>
> H.J.
> --
> gcc/
>
> PR target/93492
> * config/i386/i386.c (ix86_asm_output_function_label): Set
> function_label_emitted to true.
> (ix86_print_patchable_function_entry): New function.
>
> gcc/testsuite/
>
> PR target/93492
> * gcc.target/i386/pr93492-1.c: New test.
> * gcc.target/i386/pr93492-2.c: Likewise.
> * gcc.target/i386/pr93492-3.c: Likewise.
>
This version works with both .cfi_startproc and DWARF debug info.
--
H.J.
From c4660acd1555f90f0f76f32a59f043a51c866553 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@gmail.com>
Date: Mon, 3 Feb 2020 10:22:57 -0800
Subject: [PATCH] i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY to delay patchable
area generation to ENDBR generation. It works with both .cfi_startproc
and DWARF debug info.
gcc/
PR target/93492
* config/i386/i386-features.c (rest_of_insert_endbranch): Set
endbr_queued_at_entrance to TYPE_ENDBR.
* config/i386/i386-protos.h (ix86_output_endbr): New.
* config/i386/i386.c (ix86_asm_output_function_label): Set
function_label_emitted to true.
(ix86_print_patchable_function_entry): New function.
(ix86_output_endbr): Likewise.
(x86_function_profiler): Call ix86_output_endbr to generate
ENDBR.
(TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY): New.
* i386.h (endbr_type): New.
(machine_function): Add patch_area_size, record_patch_area and
function_label_emitted. Change endbr_queued_at_entrance to
enum.
* config/i386/i386.md (UNSPECV_PATCH_ENDBR): New.
(patch_endbr): New.
gcc/testsuite/
PR target/93492
* gcc.target/i386/pr93492-1.c: New test.
* gcc.target/i386/pr93492-2.c: Likewise.
* gcc.target/i386/pr93492-3.c: Likewise.
---
gcc/config/i386/i386-features.c | 2 +-
gcc/config/i386/i386-protos.h | 2 +
gcc/config/i386/i386.c | 77 ++++++++++++++++++++++-
gcc/config/i386/i386.h | 18 +++++-
gcc/config/i386/i386.md | 9 +++
gcc/testsuite/gcc.target/i386/pr93492-1.c | 73 +++++++++++++++++++++
gcc/testsuite/gcc.target/i386/pr93492-2.c | 12 ++++
gcc/testsuite/gcc.target/i386/pr93492-3.c | 13 ++++
8 files changed, 203 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr93492-3.c
diff --git a/gcc/config/i386/i386-features.c b/gcc/config/i386/i386-features.c
index b49e6f8d408..4d3d36e9ade 100644
--- a/gcc/config/i386/i386-features.c
+++ b/gcc/config/i386/i386-features.c
@@ -1963,7 +1963,7 @@ rest_of_insert_endbranch (void)
{
/* Queue ENDBR insertion to x86_function_profiler. */
if (crtl->profile && flag_fentry)
- cfun->machine->endbr_queued_at_entrance = true;
+ cfun->machine->endbr_queued_at_entrance = TYPE_ENDBR;
else
{
cet_eb = gen_nop_endbr ();
diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h
index 266381ca5a6..f9f5a243714 100644
--- a/gcc/config/i386/i386-protos.h
+++ b/gcc/config/i386/i386-protos.h
@@ -38,6 +38,8 @@ extern void ix86_expand_split_stack_prologue (void);
extern void ix86_output_addr_vec_elt (FILE *, int);
extern void ix86_output_addr_diff_elt (FILE *, int, int);
+extern void ix86_output_endbr (bool);
+
extern enum calling_abi ix86_cfun_abi (void);
extern enum calling_abi ix86_function_type_abi (const_tree);
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index ffda3e8fd21..e5b2565d5bd 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1563,6 +1563,9 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname,
{
bool is_ms_hook = ix86_function_ms_hook_prologue (decl);
+ if (cfun)
+ cfun->machine->function_label_emitted = true;
+
if (is_ms_hook)
{
int i, filler_count = (TARGET_64BIT ? 32 : 16);
@@ -9118,6 +9121,73 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED)
}
}
+/* Implement TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY. */
+
+void
+ix86_print_patchable_function_entry (FILE *file,
+ unsigned HOST_WIDE_INT patch_area_size,
+ bool record_p)
+{
+ if (cfun->machine->function_label_emitted)
+ {
+ if ((flag_cf_protection & CF_BRANCH)
+ && !lookup_attribute ("nocf_check",
+ TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+ && (!flag_manual_endbr
+ || lookup_attribute ("cf_check",
+ DECL_ATTRIBUTES (cfun->decl)))
+ && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
+ {
+ /* Change the queued and normal ENDBR to ENDBR with
+ patchable area. */
+ if (cfun->machine->endbr_queued_at_entrance)
+ cfun->machine->endbr_queued_at_entrance = TYPE_PATCH_ENDBR;
+ else
+ {
+ /* Replace UNSPECV_NOP_ENDBR with UNSPECV_PATCH_ENDBR. */
+ 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_NOP_ENDBR)
+ {
+ PATTERN (insn) = gen_patch_endbr ();
+ INSN_CODE (insn) = -1;
+ recog_memoized (insn);
+ }
+ }
+
+ /* Record the patchable area. */
+ cfun->machine->patch_area_size = patch_area_size;
+ cfun->machine->record_patch_area = record_p;
+
+ return;
+ }
+ }
+
+ default_print_patchable_function_entry (file, patch_area_size,
+ record_p);
+}
+
+void
+ix86_output_endbr (bool patchable)
+{
+ if (patchable & !cfun->machine->patch_area_size)
+ sorry ("only one patchable area is supported at functon entry");
+
+ fprintf (asm_out_file, "\t%s\n",
+ TARGET_64BIT ? "endbr64" : "endbr32");
+
+ if (patchable)
+ {
+ default_print_patchable_function_entry
+ (asm_out_file, cfun->machine->patch_area_size,
+ cfun->machine->record_patch_area);
+
+ cfun->machine->patch_area_size = 0;
+ }
+}
+
/* Return a scratch register to use in the split stack prologue. The
split stack prologue is used for -fsplit-stack. It is the first
instructions in the function, even before the regular prologue.
@@ -20152,7 +20222,8 @@ void
x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED)
{
if (cfun->machine->endbr_queued_at_entrance)
- fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32");
+ ix86_output_endbr (cfun->machine->endbr_queued_at_entrance
+ == TYPE_PATCH_ENDBR);
const char *mcount_name = MCOUNT_NAME;
@@ -22744,6 +22815,10 @@ ix86_run_selftests (void)
#undef TARGET_ASM_FUNCTION_EPILOGUE
#define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue
+#undef TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY
+#define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY \
+ ix86_print_patchable_function_entry
+
#undef TARGET_ENCODE_SECTION_INFO
#ifndef SUBTARGET_ENCODE_SECTION_INFO
#define TARGET_ENCODE_SECTION_INFO ix86_encode_section_info
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 943e9a5c783..bf6855124a4 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2751,6 +2751,13 @@ enum function_type
TYPE_EXCEPTION
};
+enum endbr_type
+{
+ TYPE_NONE = 0,
+ TYPE_ENDBR,
+ TYPE_PATCH_ENDBR
+};
+
struct GTY(()) machine_function {
struct stack_local_entry *stack_locals;
int varargs_gpr_size;
@@ -2767,6 +2774,12 @@ struct GTY(()) machine_function {
structure. */
rtx split_stack_varargs_pointer;
+ /* The size of the patchable area at function entry. */
+ unsigned HOST_WIDE_INT patch_area_size;
+
+ /* If true, record patchable area at function entry. */
+ BOOL_BITFIELD record_patch_area : 1;
+
/* This value is used for amd64 targets and specifies the current abi
to be used. MS_ABI means ms abi. Otherwise SYSV_ABI means sysv abi. */
ENUM_BITFIELD(calling_abi) call_abi : 8;
@@ -2842,7 +2855,10 @@ struct GTY(()) machine_function {
BOOL_BITFIELD outgoing_args_on_stack : 1;
/* If true, ENDBR is queued at function entrance. */
- BOOL_BITFIELD endbr_queued_at_entrance : 1;
+ ENUM_BITFIELD(endbr_type) endbr_queued_at_entrance : 2;
+
+ /* If true, the function label has been emitted. */
+ BOOL_BITFIELD function_label_emitted : 1;
/* True if the function needs a stack frame. */
BOOL_BITFIELD stack_frame_required : 1;
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 46b442dae51..ee476080182 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -271,6 +271,7 @@
;; For CET support
UNSPECV_NOP_ENDBR
+ UNSPECV_PATCH_ENDBR
UNSPECV_NOP_RDSSP
UNSPECV_INCSSP
UNSPECV_SAVEPREVSSP
@@ -20991,6 +20992,14 @@
(set_attr "length_immediate" "0")
(set_attr "modrm" "0")])
+(define_insn "patch_endbr"
+ [(unspec_volatile [(const_int 0)] UNSPECV_PATCH_ENDBR)]
+ "(flag_cf_protection & CF_BRANCH)"
+{
+ ix86_output_endbr (true);
+ return "";
+})
+
;; For RTM support
(define_expand "xbegin"
[(set (match_operand:SI 0 "register_operand")
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-1.c b/gcc/testsuite/gcc.target/i386/pr93492-1.c
new file mode 100644
index 00000000000..478c9ba7c04
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-1.c
@@ -0,0 +1,73 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr" } */
+/* { 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 function. */
+
+/*
+**f10_none:
+** nop
+** ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 0)))
+f10_none ()
+{
+}
+
+/*
+**f10_endbr:
+** endbr(32|64)
+** nop
+** ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr (void)
+{
+}
+
+/*
+**f11_none:
+** ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (1, 1)))
+f11_none ()
+{
+}
+
+/*
+**f11_endbr:
+** endbr(32|64)
+** ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 1)))
+f11_endbr (void)
+{
+}
+
+/*
+**f21_none:
+** nop
+** ret
+*/
+void
+__attribute__ ((nocf_check,patchable_function_entry (2, 1)))
+f21_none ()
+{
+}
+
+/*
+**f21_endbr:
+** endbr(32|64)
+** nop
+** ret
+*/
+void
+__attribute__ ((cf_check,patchable_function_entry (2, 1)))
+f21_endbr ()
+{
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-2.c b/gcc/testsuite/gcc.target/i386/pr93492-2.c
new file mode 100644
index 00000000000..77279ee957f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-2.c
@@ -0,0 +1,12 @@
+/* { dg-do "compile" } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -fasynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label. */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr ()
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n\tret\n" } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr93492-3.c b/gcc/testsuite/gcc.target/i386/pr93492-3.c
new file mode 100644
index 00000000000..ed5129a6a67
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr93492-3.c
@@ -0,0 +1,13 @@
+/* { dg-do "compile" } */
+/* { dg-require-effective-target mfentry } */
+/* { dg-options "-O1 -fcf-protection -mmanual-endbr -mfentry -pg -fasynchronous-unwind-tables" } */
+
+/* Verify that there is no ENDBR before "call __fentry__". */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr ()
+{
+}
+
+/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
--
2.24.1
