You are right. The functions in the tests should be changed to static scope to trigger the check in the patch. After that I expect there should be no endbr generated at all for the static functions and that's is wrong.
Igor > -----Original Message----- > From: H.J. Lu [mailto:hjl.to...@gmail.com] > Sent: Tuesday, October 24, 2017 12:06 AM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > Cc: Uros Bizjak <ubiz...@gmail.com>; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] i386: Don't generate ENDBR if function is only called > directly > > On Mon, Oct 23, 2017 at 3:01 PM, Tsimbalist, Igor V > <igor.v.tsimbal...@intel.com> wrote: > > Existing tests cet-label.c cet-switch-2.c cet-sjlj-1.c cet-sjlj-3.c should > > catch > this. > > There are no regressions with my patch. Did I miss something? > > > Igor > > > > > >> -----Original Message----- > >> From: H.J. Lu [mailto:hjl.to...@gmail.com] > >> Sent: Monday, October 23, 2017 11:50 PM > >> To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com> > >> Cc: Uros Bizjak <ubiz...@gmail.com>; gcc-patches@gcc.gnu.org > >> Subject: Re: [PATCH] i386: Don't generate ENDBR if function is only called > >> directly > >> > >> On Mon, Oct 23, 2017 at 2:44 PM, Tsimbalist, Igor V > >> <igor.v.tsimbal...@intel.com> wrote: > >> > The change will skip a whole function from endbr processing by > >> rest_of_insert_endbranch, > >> > which inserts endbr not only at the beginning of the function but inside > the > >> function's > >> > body also. For example, tests with setjmp should fail. > >> > > >> > I would suggest to insert the check in rest_of_insert_endbranch > function, > >> something like this > >> > > >> > if (!(lookup_attribute ("nocf_check", > >> > TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl))) > >> > || cgraph_node::get (fun->decl)->only_called_directly_p ()) > >> > > >> > Igor > >> > >> Can you provide one test for each case to cover all of them? > >> > >> > >> > > >> >> -----Original Message----- > >> >> From: Uros Bizjak [mailto:ubiz...@gmail.com] > >> >> Sent: Monday, October 23, 2017 9:26 PM > >> >> To: H.J. Lu <hjl.to...@gmail.com> > >> >> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V > >> >> <igor.v.tsimbal...@intel.com> > >> >> Subject: Re: [PATCH] i386: Don't generate ENDBR if function is only > called > >> >> directly > >> >> > >> >> On Sun, Oct 22, 2017 at 4:13 PM, H.J. Lu <hjl.to...@gmail.com> wrote: > >> >> > There is no need to insert ENDBR instruction if function is only > >> >> > called > >> >> > directly. > >> >> > > >> >> > OK for trunk if there is no regressions? > >> >> > >> >> Patch needs to be OK'd by Igor first. > >> >> > >> >> Uros. > >> >> > >> >> > H.J. > >> >> > ---- > >> >> > gcc/ > >> >> > > >> >> > PR target/82659 > >> >> > * config/i386/i386.c (pass_insert_endbranch::gate): Return > >> >> > false if function is only called directly. > >> >> > > >> >> > gcc/testsuite/ > >> >> > > >> >> > PR target/82659 > >> >> > * gcc.target/i386/pr82659-1.c: New test. > >> >> > * gcc.target/i386/pr82659-2.c: Likewise. > >> >> > * gcc.target/i386/pr82659-3.c: Likewise. > >> >> > * gcc.target/i386/pr82659-4.c: Likewise. > >> >> > * gcc.target/i386/pr82659-5.c: Likewise. > >> >> > * gcc.target/i386/pr82659-6.c: Likewise. > >> >> > --- > >> >> > gcc/config/i386/i386.c | 6 ++++-- > >> >> > gcc/testsuite/gcc.target/i386/pr82659-1.c | 19 > +++++++++++++++++++ > >> >> > gcc/testsuite/gcc.target/i386/pr82659-2.c | 18 > ++++++++++++++++++ > >> >> > gcc/testsuite/gcc.target/i386/pr82659-3.c | 21 > >> +++++++++++++++++++++ > >> >> > gcc/testsuite/gcc.target/i386/pr82659-4.c | 15 +++++++++++++++ > >> >> > gcc/testsuite/gcc.target/i386/pr82659-5.c | 10 ++++++++++ > >> >> > gcc/testsuite/gcc.target/i386/pr82659-6.c | 19 > +++++++++++++++++++ > >> >> > 7 files changed, 106 insertions(+), 2 deletions(-) > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-1.c > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-2.c > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-3.c > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-4.c > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-5.c > >> >> > create mode 100644 gcc/testsuite/gcc.target/i386/pr82659-6.c > >> >> > > >> >> > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > >> >> > index fb0b7e71469..b86504378ae 100644 > >> >> > --- a/gcc/config/i386/i386.c > >> >> > +++ b/gcc/config/i386/i386.c > >> >> > @@ -2693,9 +2693,11 @@ public: > >> >> > {} > >> >> > > >> >> > /* opt_pass methods: */ > >> >> > - virtual bool gate (function *) > >> >> > + virtual bool gate (function *fun) > >> >> > { > >> >> > - return ((flag_cf_protection & CF_BRANCH) && TARGET_IBT); > >> >> > + return ((flag_cf_protection & CF_BRANCH) > >> >> > + && TARGET_IBT > >> >> > + && !cgraph_node::get (fun->decl)->only_called_directly_p > ()); > >> >> > } > >> >> > > >> >> > virtual unsigned int execute (function *) > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-1.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-1.c > >> >> > new file mode 100644 > >> >> > index 00000000000..8f0a6906815 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-1.c > >> >> > @@ -0,0 +1,19 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } > >> >> > } } > } */ > >> >> > + > >> >> > +extern int x; > >> >> > + > >> >> > +static void > >> >> > +__attribute__ ((noinline, noclone)) > >> >> > +test (int i) > >> >> > +{ > >> >> > + x = i; > >> >> > +} > >> >> > + > >> >> > +void > >> >> > +bar (int i) > >> >> > +{ > >> >> > + test (i); > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-2.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-2.c > >> >> > new file mode 100644 > >> >> > index 00000000000..228a20006b6 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-2.c > >> >> > @@ -0,0 +1,18 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } > >> >> > } } > } */ > >> >> > + > >> >> > +extern int x; > >> >> > + > >> >> > +void > >> >> > +test (int i) > >> >> > +{ > >> >> > + x = i; > >> >> > +} > >> >> > + > >> >> > +void > >> >> > +bar (int i) > >> >> > +{ > >> >> > + test (i); > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-3.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-3.c > >> >> > new file mode 100644 > >> >> > index 00000000000..6ae23e40abc > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-3.c > >> >> > @@ -0,0 +1,21 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } > >> >> > } } > } */ > >> >> > + > >> >> > +extern int x; > >> >> > + > >> >> > +static void > >> >> > +__attribute__ ((noinline, noclone)) > >> >> > +test (int i) > >> >> > +{ > >> >> > + x = i; > >> >> > +} > >> >> > + > >> >> > +extern __typeof (test) foo __attribute__ ((alias ("test"))); > >> >> > + > >> >> > +void > >> >> > +bar (int i) > >> >> > +{ > >> >> > + test (i); > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-4.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-4.c > >> >> > new file mode 100644 > >> >> > index 00000000000..ca87264e98b > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-4.c > >> >> > @@ -0,0 +1,15 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } > >> >> > } } > } */ > >> >> > + > >> >> > +static void > >> >> > +test (void) > >> >> > +{ > >> >> > +} > >> >> > + > >> >> > +void * > >> >> > +bar (void) > >> >> > +{ > >> >> > + return test; > >> >> > +} > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-5.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-5.c > >> >> > new file mode 100644 > >> >> > index 00000000000..c34eade0f90 > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-5.c > >> >> > @@ -0,0 +1,10 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 1 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 1 { target { ! ia32 } > >> >> > } } > } */ > >> >> > +static void > >> >> > +test (void) > >> >> > +{ > >> >> > +} > >> >> > + > >> >> > +void (*test_p) (void) = test; > >> >> > diff --git a/gcc/testsuite/gcc.target/i386/pr82659-6.c > >> >> b/gcc/testsuite/gcc.target/i386/pr82659-6.c > >> >> > new file mode 100644 > >> >> > index 00000000000..b4ffd65c74f > >> >> > --- /dev/null > >> >> > +++ b/gcc/testsuite/gcc.target/i386/pr82659-6.c > >> >> > @@ -0,0 +1,19 @@ > >> >> > +/* { dg-do compile } */ > >> >> > +/* { dg-options "-O2 -fcf-protection -mcet" } */ > >> >> > +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } > >> >> > */ > >> >> > +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } > >> >> > } } > } */ > >> >> > + > >> >> > +extern int x; > >> >> > + > >> >> > + __attribute__ ((visibility ("hidden"))) > >> >> > +void > >> >> > +test (int i) > >> >> > +{ > >> >> > + x = i; > >> >> > +} > >> >> > + > >> >> > +void > >> >> > +bar (int i) > >> >> > +{ > >> >> > + test (i); > >> >> > +} > >> >> > -- > >> >> > 2.13.6 > >> >> > > >> > >> > >> > >> -- > >> H.J. > > > > -- > H.J.
