RE: [PATCH] i386: Don't generate ENDBR if function is only called directly

["Tsimbalist, Igor V" <igor dot v dot tsimbalist at intel dot com>]

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


> -----Original Message-----
> From: Uros Bizjak [mailto:ubizjak@gmail.com]
> Sent: Monday, October 23, 2017 9:26 PM
> To: H.J. Lu <hjl.tools@gmail.com>
> Cc: gcc-patches@gcc.gnu.org; Tsimbalist, Igor V
> <igor.v.tsimbalist@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.tools@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
> >