On Mon, Sep 24, 2018 at 10:23 AM, H.J. Lu <hjl.to...@gmail.com> wrote: > On Mon, Sep 24, 2018 at 9:55 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >>> On Mon, Sep 24, 2018 at 9:50 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >>> >> On Mon, Sep 24, 2018 at 9:19 AM, Jan Hubicka <hubi...@ucw.cz> wrote: >>> >> >> >> PING. >>> >> >> > >>> >> >> > Hi, Jan Uros, >>> >> >> > >>> >> >> > Can you review this patch? >>> >> >> >>> >> >> I don't know CET stuff, so I'm not able to review functionality of >>> >> >> CET patches. >>> >> > >>> >> > My (very partial) understanding is that ENDBR is used to mark places >>> >> > where one >>> >> > can jump/call. So we need to always arrange it first. Normally this is >>> >> > done >>> >> > simply by inserting it very first in the instruction stream, but in >>> >> > cases >>> >> > where profiling code is inserted this breaks because profiling code is >>> >> > output as string rather than real instructions because it needs the >>> >> > code label >>> >> > to be referred from mloc_count section. >>> >> > >>> >> > It is ugly, I wonder how much work would be tu turn profiler insertion >>> >> > to also >>> >> > use RTL representation? >>> >> > >>> >> >>> >> We will investigate it. >>> > Thanks, do we need to backport this fix into release braches? >>> > (I think current patch is more suitable for backporting) >>> >>> Yes, we need to backport it to GCC 8 branch. >> >> OK, then I guess the patch is fine for mainline and for branch after a week >> (I am not sure how much of practical CET testing we have though). Please >> however >> investigate the cleanup for the profiler code. It is not first time it hit us >> and it would be nice to have less code output to function bodies that is not >> visible to RTL passes. >> > > This is the patch I checked into trunk. I will backport it to GCC 8 branch > next Monday. I also opened: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87419 > > to improve -mfentry.
Oops. Wrong patch. Here is the right one. -- H.J.
From 9a6325624e2ffe478c03453f6777abad77969501 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.to...@gmail.com> Date: Tue, 24 Oct 2017 08:47:19 -0700 Subject: [PATCH] i386: Insert ENDBR before the profiling counter call ENDBR must be the first instruction of a function. This patch queues ENDBR if we need to put the profiling counter call before the prologue and generate ENDBR before the profiling counter call. gcc/ PR target/82699 * config/i386/i386.c (rest_of_insert_endbranch): Set endbr_queued_at_entrance to true and don't insert ENDBR if x86_function_profiler will be called. (x86_function_profiler): Insert ENDBR if endbr_queued_at_entrance is true. * config/i386/i386.h (machine_function): Add endbr_queued_at_entrance. gcc/testsuite/ PR target/82699 * gcc.target/i386/pr82699-1.c: New file. * gcc.target/i386/pr82699-2.c: Likewise. * gcc.target/i386/pr82699-3.c: Likewise. * gcc.target/i386/pr82699-4.c: Likewise. * gcc.target/i386/pr82699-5.c: Likewise. * gcc.target/i386/pr82699-6.c: Likewise. --- gcc/config/i386/i386.c | 18 ++++++++++++++---- gcc/config/i386/i386.h | 3 +++ gcc/testsuite/gcc.target/i386/pr82699-1.c | 11 +++++++++++ gcc/testsuite/gcc.target/i386/pr82699-2.c | 11 +++++++++++ gcc/testsuite/gcc.target/i386/pr82699-3.c | 11 +++++++++++ gcc/testsuite/gcc.target/i386/pr82699-4.c | 11 +++++++++++ gcc/testsuite/gcc.target/i386/pr82699-5.c | 11 +++++++++++ gcc/testsuite/gcc.target/i386/pr82699-6.c | 11 +++++++++++ 8 files changed, 83 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-2.c create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-3.c create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-4.c create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-5.c create mode 100644 gcc/testsuite/gcc.target/i386/pr82699-6.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 6dd31309495..052ca63e460 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -2593,11 +2593,17 @@ rest_of_insert_endbranch (void) TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl))) && !cgraph_node::get (cfun->decl)->only_called_directly_p ()) { - cet_eb = gen_nop_endbr (); + /* Queue ENDBR insertion to x86_function_profiler. */ + if (crtl->profile && flag_fentry) + cfun->machine->endbr_queued_at_entrance = true; + else + { + cet_eb = gen_nop_endbr (); - bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; - insn = BB_HEAD (bb); - emit_insn_before (cet_eb, insn); + bb = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; + insn = BB_HEAD (bb); + emit_insn_before (cet_eb, insn); + } } bb = 0; @@ -41203,6 +41209,10 @@ x86_function_profiler (FILE *file, int labelno ATTRIBUTE_UNUSED) { const char *mcount_name = (flag_fentry ? MCOUNT_NAME_BEFORE_PROLOGUE : MCOUNT_NAME); + + if (cfun->machine->endbr_queued_at_entrance) + fprintf (file, "\t%s\n", TARGET_64BIT ? "endbr64" : "endbr32"); + if (TARGET_64BIT) { #ifndef NO_PROFILE_COUNTERS diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 2fa9f2d53c4..e77dac7ef38 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2747,6 +2747,9 @@ struct GTY(()) machine_function { /* Nonzero if the function places outgoing arguments on stack. */ BOOL_BITFIELD outgoing_args_on_stack : 1; + /* If true, ENDBR is queued at function entrance. */ + BOOL_BITFIELD endbr_queued_at_entrance : 1; + /* The largest alignment, in bytes, of stack slot actually used. */ unsigned int max_used_stack_alignment; diff --git a/gcc/testsuite/gcc.target/i386/pr82699-1.c b/gcc/testsuite/gcc.target/i386/pr82699-1.c new file mode 100644 index 00000000000..272d0797ff8 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-1.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-O2 -fno-pic -fcf-protection -pg -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} diff --git a/gcc/testsuite/gcc.target/i386/pr82699-2.c b/gcc/testsuite/gcc.target/i386/pr82699-2.c new file mode 100644 index 00000000000..07a4ccbdbf4 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-2.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-O2 -fno-pic -fcf-protection -pg -mfentry -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} diff --git a/gcc/testsuite/gcc.target/i386/pr82699-3.c b/gcc/testsuite/gcc.target/i386/pr82699-3.c new file mode 100644 index 00000000000..08fa0e7fa5c --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-3.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-O2 -fpic -fcf-protection -pg -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} diff --git a/gcc/testsuite/gcc.target/i386/pr82699-4.c b/gcc/testsuite/gcc.target/i386/pr82699-4.c new file mode 100644 index 00000000000..3cc03dbf13e --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-4.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target { *-*-linux* && { ! ia32 } } } } */ +/* { dg-options "-O2 -fpic -fcf-protection -pg -mfentry -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} diff --git a/gcc/testsuite/gcc.target/i386/pr82699-5.c b/gcc/testsuite/gcc.target/i386/pr82699-5.c new file mode 100644 index 00000000000..e0fe0188d56 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-5.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-O2 -fcf-protection -mfentry -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} diff --git a/gcc/testsuite/gcc.target/i386/pr82699-6.c b/gcc/testsuite/gcc.target/i386/pr82699-6.c new file mode 100644 index 00000000000..cacf0ab3db2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr82699-6.c @@ -0,0 +1,11 @@ +/* { dg-do compile { target *-*-linux* } } */ +/* { dg-options "-O2 -fcf-protection -pg -mfentry -mrecord-mcount -mnop-mcount -fasynchronous-unwind-tables" } */ +/* { dg-final { scan-assembler-times {\t\.cfi_startproc\n\tendbr} 1 } } */ + +extern int bar (int); + +int +foo (int i) +{ + return bar (i); +} -- 2.17.1