diff mbox series

i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY

Message ID CAMe9rOrY_2k1oixJgXk1m=ig-CamNLwAmUcYAdEPnVHv=HZkUQ@mail.gmail.com
State New
Headers show
Series i386: Define TARGET_ASM_PRINT_PATCHABLE_FUNCTION_ENTRY | expand

Commit Message

H.J. Lu Feb. 3, 2020, 6:35 p.m. UTC
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.

Comments

H.J. Lu Feb. 4, 2020, 12:02 a.m. UTC | #1
On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu <hjl.tools@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. Lu Feb. 4, 2020, 2:10 a.m. UTC | #2
On Mon, Feb 3, 2020 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Feb 3, 2020 at 10:35 AM H.J. Lu <hjl.tools@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.
>

-g -fpatchable-function-entry=1  doesn't work together:

[hjl@gnu-cfl-1 pr93492]$ cat y.i
void f(){}
[hjl@gnu-cfl-1 pr93492]$ make y.s
/export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/tools-build/gcc-gitlab-debug/build-x86_64-linux/gcc/
-g -fpatchable-function-entry=1 -S y.i
[hjl@gnu-cfl-1 pr93492]$ cat y.s
.file "y.i"
.text
.Ltext0:
.globl f
.type f, @function
f:
.section __patchable_function_entries,"aw",@progbits
.align 8
.quad .LPFE1
.text
.LPFE1:
nop
.LFB0:
.file 1 "y.i"
.loc 1 1 9
.cfi_startproc
pushq %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq %rsp, %rbp
.cfi_def_cfa_register 6
.loc 1 1 1
nop
popq %rbp
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE0:
.size f, .-f

I will update my patch to handle it.
diff mbox series

Patch

From 5363c0289e3525139939bb678deeda98d06b2556 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@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 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__".

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.
---
 gcc/config/i386/i386.c                    | 46 ++++++++++++++
 gcc/config/i386/i386.h                    |  3 +
 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 ++++
 5 files changed, 147 insertions(+)
 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.c b/gcc/config/i386/i386.c
index ffda3e8fd21..dc9bd095e9a 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,45 @@  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 ())
+	{
+	  /* Remove ENDBR that follows the patch area.  */
+	  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)
+	    delete_insn (insn);
+
+	  /* Remove the queued ENDBR.  */
+	  cfun->machine->endbr_queued_at_entrance = false;
+
+	  /* Insert a ENDBR before the patch area right after the
+	     function label and the .cfi_startproc directive.  */
+	  asm_fprintf (file, "\t%s\n",
+		       TARGET_64BIT ? "endbr64" : "endbr32");
+	}
+    }
+
+  default_print_patchable_function_entry (file, patch_area_size,
+					  record_p);
+}
+
 /* 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.
@@ -22744,6 +22786,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..46a809afb96 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -2844,6 +2844,9 @@  struct GTY(()) machine_function {
   /* If true, ENDBR is queued at function entrance.  */
   BOOL_BITFIELD endbr_queued_at_entrance : 1;
 
+  /* 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/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..18a302784df
--- /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" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr ()
+{
+}
+
+/* { dg-final { scan-assembler "_?f10_endbr:\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..cfec546085f
--- /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 -fno-asynchronous-unwind-tables" } */
+
+/* Test the placement of the .LPFE1 label.  */
+
+void
+__attribute__ ((cf_check,patchable_function_entry (1, 0)))
+f10_endbr ()
+{
+}
+
+/* { dg-final { scan-assembler "_?f10_endbr:\n\tendbr(32|64)\n.*\.LPFE1:\n\tnop\n1:\tcall\t__fentry__\n\tret\n" } } */
-- 
2.24.1