i386: Insert ENDBR to trampoline for -fcf-protection=branch -mibt

Message ID 20180324234948.GA2698@intel.com
State New
Headers show
Series
  • i386: Insert ENDBR to trampoline for -fcf-protection=branch -mibt
Related show

Commit Message

H.J. Lu March 24, 2018, 11:49 p.m.
When -fcf-protection=branch -mibt are used, we need to insert ENDBR
to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate
4-byte ENDBR instruction.

OK for trunk?

H.J.
----
gcc/

	PR target/85044
	* config/i386/i386.c (ix86_trampoline_init): Insert ENDBR for
	-fcf-protection=branch -mibt.
	* config/i386/i386.h (TRAMPOLINE_SIZE): Increased by 4 bytes.

gcc/testsuite/

	PR target/85044
	* gcc.target/i386/pr85044.c: New test.
---
 gcc/config/i386/i386.c                  | 17 +++++++++++++++++
 gcc/config/i386/i386.h                  |  2 +-
 gcc/testsuite/gcc.target/i386/pr85044.c | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr85044.c

-- 
2.14.3

Comments

Tsimbalist, Igor V March 26, 2018, 3:23 p.m. | #1
> -----Original Message-----

> From: Lu, Hongjiu

> Sent: Sunday, March 25, 2018 12:50 AM

> To: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>; Tsimbalist,

> Igor V <igor.v.tsimbalist@intel.com>

> Subject: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

> protection=branch -mibt

> 

> When -fcf-protection=branch -mibt are used, we need to insert ENDBR

> to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate

> 4-byte ENDBR instruction.

> 

> OK for trunk?


Regarding the test. Is it possible to check what is generated in a trampoline? In particular, that endbr is generated.

Igor

> H.J.

> ----

> gcc/

> 

> 	PR target/85044

> 	* config/i386/i386.c (ix86_trampoline_init): Insert ENDBR for

> 	-fcf-protection=branch -mibt.

> 	* config/i386/i386.h (TRAMPOLINE_SIZE): Increased by 4 bytes.

> 

> gcc/testsuite/

> 

> 	PR target/85044

> 	* gcc.target/i386/pr85044.c: New test.

> ---

>  gcc/config/i386/i386.c                  | 17 +++++++++++++++++

>  gcc/config/i386/i386.h                  |  2 +-

>  gcc/testsuite/gcc.target/i386/pr85044.c | 24

> ++++++++++++++++++++++++

>  3 files changed, 42 insertions(+), 1 deletion(-)

>  create mode 100644 gcc/testsuite/gcc.target/i386/pr85044.c

> 

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c

> index 3b264318f50..b4f6aec1434 100644

> --- a/gcc/config/i386/i386.c

> +++ b/gcc/config/i386/i386.c

> @@ -30411,6 +30411,7 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl,

> rtx chain_value)

>    rtx mem, fnaddr;

>    int opcode;

>    int offset = 0;

> +  bool need_endbr = (flag_cf_protection & CF_BRANCH) && TARGET_IBT;

> 

>    fnaddr = XEXP (DECL_RTL (fndecl), 0);

> 

> @@ -30418,6 +30419,14 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl,

> rtx chain_value)

>      {

>        int size;

> 

> +      if (need_endbr)

> +	{

> +	  /* Insert ENDBR64.  */

> +	  mem = adjust_address (m_tramp, SImode, offset);

> +	  emit_move_insn (mem, gen_int_mode (0xfa1e0ff3, SImode));

> +	  offset += 4;

> +	}

> +

>        /* Load the function address to r11.  Try to load address using

>  	 the shorter movl instead of movabs.  We may want to support

>  	 movq for kernel mode, but kernel does not use trampolines at

> @@ -30495,6 +30504,14 @@ ix86_trampoline_init (rtx m_tramp, tree fndecl,

> rtx chain_value)

>        else

>  	opcode = 0x68;

> 

> +      if (need_endbr)

> +	{

> +	  /* Insert ENDBR32.  */

> +	  mem = adjust_address (m_tramp, SImode, offset);

> +	  emit_move_insn (mem, gen_int_mode (0xfb1e0ff3, SImode));

> +	  offset += 4;

> +	}

> +

>        mem = adjust_address (m_tramp, QImode, offset);

>        emit_move_insn (mem, gen_int_mode (opcode, QImode));

> 

> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h

> index 7f4b04f421d..c7f9b4551b3 100644

> --- a/gcc/config/i386/i386.h

> +++ b/gcc/config/i386/i386.h

> @@ -1716,7 +1716,7 @@ typedef struct ix86_args {

> 

>  /* Length in units of the trampoline for entering a nested function.  */

> 

> -#define TRAMPOLINE_SIZE (TARGET_64BIT ? 24 : 10)

> +#define TRAMPOLINE_SIZE (TARGET_64BIT ? 28 : 14)

>  


>  /* Definitions for register eliminations.

> 

> diff --git a/gcc/testsuite/gcc.target/i386/pr85044.c

> b/gcc/testsuite/gcc.target/i386/pr85044.c

> new file mode 100644

> index 00000000000..332f582d79b

> --- /dev/null

> +++ b/gcc/testsuite/gcc.target/i386/pr85044.c

> @@ -0,0 +1,24 @@

> +/* { dg-do run { target cet } } */

> +/* { dg-options "-O2 -fcf-protection=branch -mibt" } */

> +

> +void callme (void (*callback) (void));

> +

> +int

> +main (void)

> +{

> +  int ok = 0;

> +  void callback (void) { ok = 1; }

> +

> +  callme (&callback);

> +

> +  if (!ok)

> +   __builtin_abort ();

> +  return 0;

> +}

> +

> +__attribute__((noinline, noclone))

> +void

> +callme (void (*callback) (void))

> +{

> +  (*callback) ();

> +}

> --

> 2.14.3
H.J. Lu March 26, 2018, 3:59 p.m. | #2
On Mon, Mar 26, 2018 at 8:23 AM, Tsimbalist, Igor V
<igor.v.tsimbalist@intel.com> wrote:
>> -----Original Message-----

>> From: Lu, Hongjiu

>> Sent: Sunday, March 25, 2018 12:50 AM

>> To: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>; Tsimbalist,

>> Igor V <igor.v.tsimbalist@intel.com>

>> Subject: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

>> protection=branch -mibt

>>

>> When -fcf-protection=branch -mibt are used, we need to insert ENDBR

>> to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate

>> 4-byte ENDBR instruction.

>>

>> OK for trunk?

>

> Regarding the test. Is it possible to check what is generated in a trampoline? In particular, that endbr is generated.

>


I think run-time test is sufficient.

-- 
H.J.
Tsimbalist, Igor V March 26, 2018, 8:42 p.m. | #3
> -----Original Message-----

> From: H.J. Lu [mailto:hjl.tools@gmail.com]

> Sent: Monday, March 26, 2018 5:59 PM

> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>

> Cc: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>

> Subject: Re: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

> protection=branch -mibt

> 

> On Mon, Mar 26, 2018 at 8:23 AM, Tsimbalist, Igor V

> <igor.v.tsimbalist@intel.com> wrote:

> >> -----Original Message-----

> >> From: Lu, Hongjiu

> >> Sent: Sunday, March 25, 2018 12:50 AM

> >> To: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>;

> Tsimbalist,

> >> Igor V <igor.v.tsimbalist@intel.com>

> >> Subject: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

> >> protection=branch -mibt

> >>

> >> When -fcf-protection=branch -mibt are used, we need to insert ENDBR

> >> to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate

> >> 4-byte ENDBR instruction.

> >>

> >> OK for trunk?

> >

> > Regarding the test. Is it possible to check what is generated in a

> trampoline? In particular, that endbr is generated.

> >

> 

> I think run-time test is sufficient.


Ok then.

> --

> H.J.
Uros Bizjak March 27, 2018, 5:08 p.m. | #4
On Mon, Mar 26, 2018 at 10:42 PM, Tsimbalist, Igor V
<igor.v.tsimbalist@intel.com> wrote:
>> -----Original Message-----

>> From: H.J. Lu [mailto:hjl.tools@gmail.com]

>> Sent: Monday, March 26, 2018 5:59 PM

>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>

>> Cc: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>

>> Subject: Re: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

>> protection=branch -mibt

>>

>> On Mon, Mar 26, 2018 at 8:23 AM, Tsimbalist, Igor V

>> <igor.v.tsimbalist@intel.com> wrote:

>> >> -----Original Message-----

>> >> From: Lu, Hongjiu

>> >> Sent: Sunday, March 25, 2018 12:50 AM

>> >> To: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>;

>> Tsimbalist,

>> >> Igor V <igor.v.tsimbalist@intel.com>

>> >> Subject: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

>> >> protection=branch -mibt

>> >>

>> >> When -fcf-protection=branch -mibt are used, we need to insert ENDBR

>> >> to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate

>> >> 4-byte ENDBR instruction.

>> >>

>> >> OK for trunk?

>> >

>> > Regarding the test. Is it possible to check what is generated in a

>> trampoline? In particular, that endbr is generated.

>> >

>>

>> I think run-time test is sufficient.

>

> Ok then.


Rubber-stamp OK.

Uros.
H.J. Lu March 27, 2018, 5:19 p.m. | #5
On Tue, Mar 27, 2018 at 10:08 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Mar 26, 2018 at 10:42 PM, Tsimbalist, Igor V

> <igor.v.tsimbalist@intel.com> wrote:

>>> -----Original Message-----

>>> From: H.J. Lu [mailto:hjl.tools@gmail.com]

>>> Sent: Monday, March 26, 2018 5:59 PM

>>> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>

>>> Cc: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>

>>> Subject: Re: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

>>> protection=branch -mibt

>>>

>>> On Mon, Mar 26, 2018 at 8:23 AM, Tsimbalist, Igor V

>>> <igor.v.tsimbalist@intel.com> wrote:

>>> >> -----Original Message-----

>>> >> From: Lu, Hongjiu

>>> >> Sent: Sunday, March 25, 2018 12:50 AM

>>> >> To: gcc-patches@gcc.gnu.org; Uros Bizjak <ubizjak@gmail.com>;

>>> Tsimbalist,

>>> >> Igor V <igor.v.tsimbalist@intel.com>

>>> >> Subject: [PATCH] i386: Insert ENDBR to trampoline for -fcf-

>>> >> protection=branch -mibt

>>> >>

>>> >> When -fcf-protection=branch -mibt are used, we need to insert ENDBR

>>> >> to trampoline.  TRAMPOLINE_SIZE is creased by 4 bytes to accommodate

>>> >> 4-byte ENDBR instruction.

>>> >>

>>> >> OK for trunk?

>>> >

>>> > Regarding the test. Is it possible to check what is generated in a

>>> trampoline? In particular, that endbr is generated.

>>> >

>>>

>>> I think run-time test is sufficient.

>>

>> Ok then.

>

> Rubber-stamp OK.

>


Done.

Thanks.

-- 
H.J.

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3b264318f50..b4f6aec1434 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -30411,6 +30411,7 @@  ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
   rtx mem, fnaddr;
   int opcode;
   int offset = 0;
+  bool need_endbr = (flag_cf_protection & CF_BRANCH) && TARGET_IBT;
 
   fnaddr = XEXP (DECL_RTL (fndecl), 0);
 
@@ -30418,6 +30419,14 @@  ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
     {
       int size;
 
+      if (need_endbr)
+	{
+	  /* Insert ENDBR64.  */
+	  mem = adjust_address (m_tramp, SImode, offset);
+	  emit_move_insn (mem, gen_int_mode (0xfa1e0ff3, SImode));
+	  offset += 4;
+	}
+
       /* Load the function address to r11.  Try to load address using
 	 the shorter movl instead of movabs.  We may want to support
 	 movq for kernel mode, but kernel does not use trampolines at
@@ -30495,6 +30504,14 @@  ix86_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
       else
 	opcode = 0x68;
 
+      if (need_endbr)
+	{
+	  /* Insert ENDBR32.  */
+	  mem = adjust_address (m_tramp, SImode, offset);
+	  emit_move_insn (mem, gen_int_mode (0xfb1e0ff3, SImode));
+	  offset += 4;
+	}
+
       mem = adjust_address (m_tramp, QImode, offset);
       emit_move_insn (mem, gen_int_mode (opcode, QImode));
 
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index 7f4b04f421d..c7f9b4551b3 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1716,7 +1716,7 @@  typedef struct ix86_args {
 
 /* Length in units of the trampoline for entering a nested function.  */
 
-#define TRAMPOLINE_SIZE (TARGET_64BIT ? 24 : 10)
+#define TRAMPOLINE_SIZE (TARGET_64BIT ? 28 : 14)
 
 /* Definitions for register eliminations.
 
diff --git a/gcc/testsuite/gcc.target/i386/pr85044.c b/gcc/testsuite/gcc.target/i386/pr85044.c
new file mode 100644
index 00000000000..332f582d79b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr85044.c
@@ -0,0 +1,24 @@ 
+/* { dg-do run { target cet } } */
+/* { dg-options "-O2 -fcf-protection=branch -mibt" } */
+
+void callme (void (*callback) (void));
+
+int
+main (void)
+{
+  int ok = 0;
+  void callback (void) { ok = 1; }
+
+  callme (&callback);
+
+  if (!ok)
+   __builtin_abort ();
+  return 0;
+}
+
+__attribute__((noinline, noclone))
+void
+callme (void (*callback) (void))
+{
+  (*callback) ();
+}