public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] i386: Don't generate ENDBR if function is only called directly
@ 2017-10-22 16:44 H.J. Lu
  2017-10-23 19:43 ` Uros Bizjak
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2017-10-22 16:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: Igor Tsimbalist, Uros Bizjak

There is no need to insert ENDBR instruction if function is only called
directly.

OK for trunk if there is no regressions?

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-22 16:44 [PATCH] i386: Don't generate ENDBR if function is only called directly H.J. Lu
@ 2017-10-23 19:43 ` Uros Bizjak
  2017-10-23 21:50   ` Tsimbalist, Igor V
  0 siblings, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2017-10-23 19:43 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Igor Tsimbalist

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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 19:43 ` Uros Bizjak
@ 2017-10-23 21:50   ` Tsimbalist, Igor V
  2017-10-23 21:56     ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Tsimbalist, Igor V @ 2017-10-23 21:50 UTC (permalink / raw)
  To: Uros Bizjak, H.J. Lu; +Cc: gcc-patches, Tsimbalist, Igor V

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
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 21:50   ` Tsimbalist, Igor V
@ 2017-10-23 21:56     ` H.J. Lu
  2017-10-23 22:02       ` Tsimbalist, Igor V
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2017-10-23 21:56 UTC (permalink / raw)
  To: Tsimbalist, Igor V; +Cc: Uros Bizjak, gcc-patches

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 21:56     ` H.J. Lu
@ 2017-10-23 22:02       ` Tsimbalist, Igor V
  2017-10-23 22:07         ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Tsimbalist, Igor V @ 2017-10-23 22:02 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Tsimbalist, Igor V

Existing tests cet-label.c cet-switch-2.c cet-sjlj-1.c cet-sjlj-3.c should catch this.

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 22:02       ` Tsimbalist, Igor V
@ 2017-10-23 22:07         ` H.J. Lu
  2017-10-23 23:00           ` Tsimbalist, Igor V
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2017-10-23 22:07 UTC (permalink / raw)
  To: Tsimbalist, Igor V; +Cc: Uros Bizjak, gcc-patches

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 22:07         ` H.J. Lu
@ 2017-10-23 23:00           ` Tsimbalist, Igor V
  2017-10-23 23:09             ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Tsimbalist, Igor V @ 2017-10-23 23:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Tsimbalist, Igor V

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.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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 23:00           ` Tsimbalist, Igor V
@ 2017-10-23 23:09             ` H.J. Lu
  2017-10-24  8:50               ` Tsimbalist, Igor V
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2017-10-23 23:09 UTC (permalink / raw)
  To: Tsimbalist, Igor V; +Cc: Uros Bizjak, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 10609 bytes --]

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.

[-- Attachment #2: 0001-i386-Don-t-insert-ENDBR-at-function-entrance-when-ca.patch --]
[-- Type: text/x-patch, Size: 10347 bytes --]

From c954069cb97f33535ed273701188ab4941fc0ba8 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 22 Oct 2017 06:09:27 -0700
Subject: [PATCH] i386: Don't insert ENDBR at function entrance when called
 directly

There is no need to insert ENDBR instruction at function entrance if
function is only called directly.

gcc/

	PR target/82659
	* config/i386/i386.c (rest_of_insert_endbranch): Don't insert
	ENDBR instruction at function entrance if function is only
	called directly.

gcc/testsuite/

	PR target/82659
	* gcc.target/i386/cet-label-2.c: New test.
	* gcc.target/i386/cet-sjlj-4.c: Likewise.
	* gcc.target/i386/cet-sjlj-5.c: Likewise.
	* gcc.target/i386/cet-switch-3.c: Likewise.
	* gcc.target/i386/pr82659-1.c: Likewise.
	* 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                       |  3 +-
 gcc/testsuite/gcc.target/i386/cet-label-2.c  | 24 ++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-sjlj-4.c   | 45 ++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-sjlj-5.c   | 48 ++++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-switch-3.c | 34 ++++++++++++++++++++
 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 +++++++++++
 11 files changed, 255 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-switch-3.c
 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 72caf62bbf8..c75c8ef95e5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2587,7 +2587,8 @@ rest_of_insert_endbranch (void)
      nocf_check attribute.  This will allow to reduce the number of EB.  */
 
   if (!lookup_attribute ("nocf_check",
-			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl))))
+			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+      && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
     {
       cet_eb = gen_nop_endbr ();
 
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-2.c b/gcc/testsuite/gcc.target/i386/cet-label-2.c
new file mode 100644
index 00000000000..c7f79819079
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-2.c
@@ -0,0 +1,24 @@
+/* Verify that CET works.  */
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */
+
+__attribute__ ((noinline, noclone))
+static int
+func (int arg)
+{
+  static void *array[] = { &&foo, &&bar };
+
+  goto *array[arg];
+foo:
+  return arg*111;
+bar:
+  return arg*777;
+}
+
+int
+foo (int arg)
+{
+  return func (arg);
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
new file mode 100644
index 00000000000..d41406fde1f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
@@ -0,0 +1,45 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "rdssp\[dq]" 2 } } */
+/* { dg-final { scan-assembler-times "incssp\[dq]" 1 } } */
+
+/* Based on gcc.dg/setjmp-3.c.  */
+
+void *buf[5];
+
+extern void abort (void);
+
+void
+raise0 (void)
+{
+  __builtin_longjmp (buf, 1);
+}
+
+__attribute__ ((noinline, noclone))
+static int
+execute (int cmd)
+{
+  int last = 0;
+
+  if (__builtin_setjmp (buf) == 0)
+    while (1)
+      {
+	last = 1;
+	raise0 ();
+      }
+
+  if (last == 0)
+    return 0;
+  else
+    return cmd;
+}
+
+int main(void)
+{
+  if (execute (1) == 0)
+    abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
new file mode 100644
index 00000000000..12ea9f4e442
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "call	_setjmp" 1 } } */
+/* { dg-final { scan-assembler-times "call	longjmp" 1 } } */
+
+#include <stdio.h>
+#include <setjmp.h>
+
+jmp_buf buf;
+static int bar (int);
+
+__attribute__ ((noinline, noclone))
+static int
+foo (int i)
+{
+  int j = i * 11;
+
+  if (!setjmp (buf))
+    {
+      j += 33;
+      printf ("After setjmp: j = %d\n", j);
+      bar (j);
+    }
+
+  return j + i;
+}
+
+__attribute__ ((noinline, noclone))
+static int
+bar (int i)
+{
+ int j = i;
+
+  j -= 111;
+  printf ("In longjmp: j = %d\n", j);
+  longjmp (buf, 1);
+
+  return j;
+}
+
+int
+main ()
+{
+  foo (10);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-switch-3.c b/gcc/testsuite/gcc.target/i386/cet-switch-3.c
new file mode 100644
index 00000000000..9b1b4369582
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-switch-3.c
@@ -0,0 +1,34 @@
+/* Verify that CET works.  */
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet -mcet-switch" } */
+/* { dg-final { scan-assembler-times "endbr32" 12 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 12 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "\[ \t]+jmp\[ \t]+\[*]" 1 } } */
+
+void func2 (int);
+
+__attribute__ ((noinline, noclone))
+static int
+func1 (int arg)
+{
+  switch (arg)
+  {
+    case 1: func2 (arg*100);
+    case 2: func2 (arg*300);
+    case 5: func2 (arg*500);
+    case 8: func2 (arg*700);
+    case 7: func2 (arg*900);
+    case -1: func2 (arg*-100);
+    case -2: func2 (arg*-300);
+    case -5: func2 (arg*-500);
+    case -7: func2 (arg*-700);
+    case -9: func2 (arg*-900);
+  }
+  return 0;
+}
+
+int
+foo (int arg)
+{
+  return func1 (arg);
+}
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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-23 23:09             ` H.J. Lu
@ 2017-10-24  8:50               ` Tsimbalist, Igor V
  2017-10-24  9:40                 ` Uros Bizjak
  0 siblings, 1 reply; 12+ messages in thread
From: Tsimbalist, Igor V @ 2017-10-24  8:50 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Tsimbalist, Igor V

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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-24  8:50               ` Tsimbalist, Igor V
@ 2017-10-24  9:40                 ` Uros Bizjak
  2017-10-24 11:06                   ` H.J. Lu
  0 siblings, 1 reply; 12+ messages in thread
From: Uros Bizjak @ 2017-10-24  9:40 UTC (permalink / raw)
  To: Tsimbalist, Igor V; +Cc: H.J. Lu, gcc-patches

On Tue, Oct 24, 2017 at 10:39 AM, Tsimbalist, Igor V
<igor.v.tsimbalist@intel.com> wrote:
> OK.

+/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */

I think we can only check for {\mendbr} in the testcases. There are
already plenty of testcases that check for the correct instruction.

Otherwise, LGTM

Thanks,
Uros.

> 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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-24  9:40                 ` Uros Bizjak
@ 2017-10-24 11:06                   ` H.J. Lu
  2017-10-25  9:59                     ` Rainer Orth
  0 siblings, 1 reply; 12+ messages in thread
From: H.J. Lu @ 2017-10-24 11:06 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Tsimbalist, Igor V, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

On Tue, Oct 24, 2017 at 2:33 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Tue, Oct 24, 2017 at 10:39 AM, Tsimbalist, Igor V
> <igor.v.tsimbalist@intel.com> wrote:
>> OK.
>
> +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
> +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
>
> I think we can only check for {\mendbr} in the testcases. There are
> already plenty of testcases that check for the correct instruction.
>
> Otherwise, LGTM
>

This is what I checked in.

Thanks.

-- 
H.J.

[-- Attachment #2: 0001-i386-Don-t-insert-ENDBR-at-function-entrance-when-ca.patch --]
[-- Type: text/x-patch, Size: 9777 bytes --]

From 81893b3645f6f0771771690e58629d3d233fcdb2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 22 Oct 2017 06:09:27 -0700
Subject: [PATCH] i386: Don't insert ENDBR at function entrance when called
 directly

There is no need to insert ENDBR instruction at function entrance if
function is only called directly.

gcc/

	PR target/82659
	* config/i386/i386.c (rest_of_insert_endbranch): Don't insert
	ENDBR instruction at function entrance if function is only
	called directly.

gcc/testsuite/

	PR target/82659
	* gcc.target/i386/cet-label-2.c: New test.
	* gcc.target/i386/cet-sjlj-4.c: Likewise.
	* gcc.target/i386/cet-sjlj-5.c: Likewise.
	* gcc.target/i386/cet-switch-3.c: Likewise.
	* gcc.target/i386/pr82659-1.c: Likewise.
	* 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                       |  3 +-
 gcc/testsuite/gcc.target/i386/cet-label-2.c  | 24 ++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-sjlj-4.c   | 45 ++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-sjlj-5.c   | 48 ++++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/cet-switch-3.c | 34 ++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr82659-1.c    | 18 +++++++++++
 gcc/testsuite/gcc.target/i386/pr82659-2.c    | 17 ++++++++++
 gcc/testsuite/gcc.target/i386/pr82659-3.c    | 20 ++++++++++++
 gcc/testsuite/gcc.target/i386/pr82659-4.c    | 14 ++++++++
 gcc/testsuite/gcc.target/i386/pr82659-5.c    | 10 ++++++
 gcc/testsuite/gcc.target/i386/pr82659-6.c    | 18 +++++++++++
 11 files changed, 250 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-label-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cet-switch-3.c
 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 72caf62bbf8..c75c8ef95e5 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2587,7 +2587,8 @@ rest_of_insert_endbranch (void)
      nocf_check attribute.  This will allow to reduce the number of EB.  */
 
   if (!lookup_attribute ("nocf_check",
-			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl))))
+			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+      && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
     {
       cet_eb = gen_nop_endbr ();
 
diff --git a/gcc/testsuite/gcc.target/i386/cet-label-2.c b/gcc/testsuite/gcc.target/i386/cet-label-2.c
new file mode 100644
index 00000000000..c7f79819079
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-label-2.c
@@ -0,0 +1,24 @@
+/* Verify that CET works.  */
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */
+
+__attribute__ ((noinline, noclone))
+static int
+func (int arg)
+{
+  static void *array[] = { &&foo, &&bar };
+
+  goto *array[arg];
+foo:
+  return arg*111;
+bar:
+  return arg*777;
+}
+
+int
+foo (int arg)
+{
+  return func (arg);
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
new file mode 100644
index 00000000000..d41406fde1f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-4.c
@@ -0,0 +1,45 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 3 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 3 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "rdssp\[dq]" 2 } } */
+/* { dg-final { scan-assembler-times "incssp\[dq]" 1 } } */
+
+/* Based on gcc.dg/setjmp-3.c.  */
+
+void *buf[5];
+
+extern void abort (void);
+
+void
+raise0 (void)
+{
+  __builtin_longjmp (buf, 1);
+}
+
+__attribute__ ((noinline, noclone))
+static int
+execute (int cmd)
+{
+  int last = 0;
+
+  if (__builtin_setjmp (buf) == 0)
+    while (1)
+      {
+	last = 1;
+	raise0 ();
+      }
+
+  if (last == 0)
+    return 0;
+  else
+    return cmd;
+}
+
+int main(void)
+{
+  if (execute (1) == 0)
+    abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
new file mode 100644
index 00000000000..12ea9f4e442
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "call	_setjmp" 1 } } */
+/* { dg-final { scan-assembler-times "call	longjmp" 1 } } */
+
+#include <stdio.h>
+#include <setjmp.h>
+
+jmp_buf buf;
+static int bar (int);
+
+__attribute__ ((noinline, noclone))
+static int
+foo (int i)
+{
+  int j = i * 11;
+
+  if (!setjmp (buf))
+    {
+      j += 33;
+      printf ("After setjmp: j = %d\n", j);
+      bar (j);
+    }
+
+  return j + i;
+}
+
+__attribute__ ((noinline, noclone))
+static int
+bar (int i)
+{
+ int j = i;
+
+  j -= 111;
+  printf ("In longjmp: j = %d\n", j);
+  longjmp (buf, 1);
+
+  return j;
+}
+
+int
+main ()
+{
+  foo (10);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/i386/cet-switch-3.c b/gcc/testsuite/gcc.target/i386/cet-switch-3.c
new file mode 100644
index 00000000000..9b1b4369582
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cet-switch-3.c
@@ -0,0 +1,34 @@
+/* Verify that CET works.  */
+/* { dg-do compile } */
+/* { dg-options "-O -fcf-protection -mcet -mcet-switch" } */
+/* { dg-final { scan-assembler-times "endbr32" 12 { target ia32 } } } */
+/* { dg-final { scan-assembler-times "endbr64" 12 { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler-times "\[ \t]+jmp\[ \t]+\[*]" 1 } } */
+
+void func2 (int);
+
+__attribute__ ((noinline, noclone))
+static int
+func1 (int arg)
+{
+  switch (arg)
+  {
+    case 1: func2 (arg*100);
+    case 2: func2 (arg*300);
+    case 5: func2 (arg*500);
+    case 8: func2 (arg*700);
+    case 7: func2 (arg*900);
+    case -1: func2 (arg*-100);
+    case -2: func2 (arg*-300);
+    case -5: func2 (arg*-500);
+    case -7: func2 (arg*-700);
+    case -9: func2 (arg*-900);
+  }
+  return 0;
+}
+
+int
+foo (int arg)
+{
+  return func1 (arg);
+}
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..485771d0f38
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82659-1.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 1 } } */
+
+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..7afffa440aa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82659-2.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+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..5f97b314092
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82659-3.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+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..c3cacaccbef
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82659-4.c
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+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..95413671d5c
--- /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 {\mendbr} 1 } } */
+
+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..51fc1a9f5c9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82659-6.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mcet" } */
+/* { dg-final { scan-assembler-times {\mendbr} 2 } } */
+
+extern int x;
+
+ __attribute__ ((visibility ("hidden")))
+void
+test (int i)
+{
+  x = i;
+}
+
+void
+bar (int i)
+{
+  test (i);
+}
-- 
2.13.6


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] i386: Don't generate ENDBR if function is only called directly
  2017-10-24 11:06                   ` H.J. Lu
@ 2017-10-25  9:59                     ` Rainer Orth
  0 siblings, 0 replies; 12+ messages in thread
From: Rainer Orth @ 2017-10-25  9:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, Tsimbalist, Igor V, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Hi H.J.,

> On Tue, Oct 24, 2017 at 2:33 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Tue, Oct 24, 2017 at 10:39 AM, Tsimbalist, Igor V
>> <igor.v.tsimbalist@intel.com> wrote:
>>> OK.
>>
>> +/* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
>> +/* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
>>
>> I think we can only check for {\mendbr} in the testcases. There are
>> already plenty of testcases that check for the correct instruction.
>>
>> Otherwise, LGTM
>>
>
> This is what I checked in.

the cet-sjlj-5.c test, like cet-sjlj-3.c, didn't account for an empty
USER_LABEL_PREFIX.  Fixed as the previous one, installed.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


2017-10-25  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* gcc.target/i386/cet-sjlj-5.c: Allow for emtpy user label prefix
	in setjmp call.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: testsuite-i386-cet-sjlj-5.patch --]
[-- Type: text/x-patch, Size: 742 bytes --]

# HG changeset patch
# Parent  bca0754daa2001dcf85a01cd8be2aad3cbd48fcd
Fix gcc.target/i386/cet-sjlj-5.c on Solaris

diff --git a/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
--- a/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
+++ b/gcc/testsuite/gcc.target/i386/cet-sjlj-5.c
@@ -2,7 +2,7 @@
 /* { dg-options "-O -fcf-protection -mcet" } */
 /* { dg-final { scan-assembler-times "endbr32" 2 { target ia32 } } } */
 /* { dg-final { scan-assembler-times "endbr64" 2 { target { ! ia32 } } } } */
-/* { dg-final { scan-assembler-times "call	_setjmp" 1 } } */
+/* { dg-final { scan-assembler-times "call	_?setjmp" 1 } } */
 /* { dg-final { scan-assembler-times "call	longjmp" 1 } } */
 
 #include <stdio.h>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-25  9:19 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-22 16:44 [PATCH] i386: Don't generate ENDBR if function is only called directly H.J. Lu
2017-10-23 19:43 ` Uros Bizjak
2017-10-23 21:50   ` Tsimbalist, Igor V
2017-10-23 21:56     ` H.J. Lu
2017-10-23 22:02       ` Tsimbalist, Igor V
2017-10-23 22:07         ` H.J. Lu
2017-10-23 23:00           ` Tsimbalist, Igor V
2017-10-23 23:09             ` H.J. Lu
2017-10-24  8:50               ` Tsimbalist, Igor V
2017-10-24  9:40                 ` Uros Bizjak
2017-10-24 11:06                   ` H.J. Lu
2017-10-25  9:59                     ` Rainer Orth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).