[PATCH] i386: Don't generate ENDBR if function is only called directly
Tsimbalist, Igor V
igor.v.tsimbalist@intel.com
Tue Oct 24 08:50:00 GMT 2017
OK.
Igor
> -----Original Message-----
> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> Sent: Tuesday, October 24, 2017 1:01 AM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> Cc: Uros Bizjak <ubizjak@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:19 PM, Tsimbalist, Igor V
> <igor.v.tsimbalist@intel.com> wrote:
> > 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.
> >
>
> Here is the updated patch with new testcases. OK for trunk if there are
> no regressions?
>
> Thanks.
>
> H.J.
> >
> >> -----Original Message-----
> >> From: H.J. Lu [mailto:hjl.tools@gmail.com]
> >> Sent: Tuesday, October 24, 2017 12:06 AM
> >> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> >> Cc: Uros Bizjak <ubizjak@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.tsimbalist@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.tools@gmail.com]
> >> >> Sent: Monday, October 23, 2017 11:50 PM
> >> >> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>
> >> >> Cc: Uros Bizjak <ubizjak@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.tsimbalist@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: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
> >> >> >> >
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> H.J.
> >>
> >>
> >>
> >> --
> >> H.J.
>
>
>
> --
> H.J.
More information about the Gcc-patches
mailing list