This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PING^6: [PATCH] i386: Insert ENDBR before the profiling counter call
On Mon, Sep 24, 2018 at 10:23 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Sep 24, 2018 at 9:55 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> On Mon, Sep 24, 2018 at 9:50 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> >> On Mon, Sep 24, 2018 at 9:19 AM, Jan Hubicka <hubicka@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.tools@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