PING^2: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

2018-12-11 Thread H.J. Lu
On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu  wrote:
>
> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
>  wrote:
> >
> > On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu  wrote:
> > >
> > > Currently GCC inserts ENDBR instruction at entries of all non-static
> > > functions, unless LTO compilation is used.  Marking all functions,
> > > which are not called indirectly with nocf_check attribute, is not
> > > ideal since 99% of functions in a program may be of this kind.
> > >
> > > This patch adds -mmanual-endbr and cf_check function attribute.  They
> > > can be used together with -fcf-protection such that ENDBR instruction
> > > is inserted only at entries of functions with cf_check attribute.  It
> > > can limit number of ENDBR instructions to reduce program size.
> > >
> > > OK for trubk?
> >
> > I wonder if the linker could assist with ENDBR creation by
> > redirecting all non-direct call relocs to a linker-generated
> > stub with ENBR and a direct branch?
> >
>
> The goal of this patch is to add as few as ENDBR as possible
> to reduce program size as much as possible.   Also there is no
> relocation for indirect branch via register.
>

Hi Honza, Jakub, Jeff, Richard,

Here is the rebased patch.  Can you guys take a look?

Thanks.


-- 
H.J.
From 5934c6be6495b2d6f278646e25f9e684f6610e2b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Thu, 14 Jun 2018 09:19:27 -0700
Subject: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

Currently GCC inserts ENDBR instruction at entries of all non-static
functions, unless LTO compilation is used.  Marking all functions,
which are not called indirectly with nocf_check attribute, is not
ideal since 99% of functions in a program may be of this kind.

This patch adds -mmanual-endbr and cf_check function attribute.  They
can be used together with -fcf-protection such that ENDBR instruction
is inserted only at entries of functions with cf_check attribute.  It
can limit number of ENDBR instructions to reduce program size.

gcc/

	* config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR
	at the function entry only when -mmanual-endbr isn't used or
	there is cf_check function attribute.
	(ix86_attribute_table): Add cf_check.
	* config/i386/i386.opt: Add -mmanual-endbr.
	* doc/extend.texi: Document cf_check attribute.
	* doc/invoke.texi: Document -mmanual-endbr.

gcc/testsuite/

	* gcc.target/i386/cf_check-1.c: New test.
	* gcc.target/i386/cf_check-2.c: Likewise.
	* gcc.target/i386/cf_check-3.c: Likewise.
	* gcc.target/i386/cf_check-4.c: Likewise.
	* gcc.target/i386/cf_check-5.c: Likewise.
---
 gcc/config/i386/i386.c |  6 ++
 gcc/config/i386/i386.opt   |  5 +
 gcc/doc/extend.texi|  7 +++
 gcc/doc/invoke.texi|  9 -
 gcc/testsuite/gcc.target/i386/cf_check-1.c | 11 +++
 gcc/testsuite/gcc.target/i386/cf_check-2.c | 11 +++
 gcc/testsuite/gcc.target/i386/cf_check-3.c | 11 +++
 gcc/testsuite/gcc.target/i386/cf_check-4.c | 10 ++
 gcc/testsuite/gcc.target/i386/cf_check-5.c |  9 +
 9 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-4.c
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-5.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3e2fdfa86ff..b05c538c097 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2638,6 +2638,9 @@ rest_of_insert_endbranch (void)
 
   if (!lookup_attribute ("nocf_check",
 			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
+  && (!flag_manual_endbr
+	  || lookup_attribute ("cf_check",
+			   DECL_ATTRIBUTES (cfun->decl)))
   && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
 {
   /* Queue ENDBR insertion to x86_function_profiler.  */
@@ -45246,6 +45249,9 @@ static const struct attribute_spec ix86_attribute_table[] =
 ix86_handle_fentry_name, NULL },
   { "fentry_section", 1, 1, true, false, false, false,
 ix86_handle_fentry_name, NULL },
+  { "cf_check", 0, 0, true, false, false, false,
+ix86_handle_fndecl_attribute, NULL },
+
   /* End element.  */
   { NULL, 0, 0, false, false, false, false, NULL, NULL }
 };
diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index b30b55b7826..007e88b57f9 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -1028,6 +1028,11 @@ Target Report Undocumented Var(flag_cet_switch) Init(0)
 Turn on CET instrumentation for switch statements that use a jump table and
 an indirect jump.
 
+mmanual-endbr
+Target Report Var(flag_manual_endbr) Init(0)
+Insert ENDBR instruction at function entry only via cf_check attribute
+for CET instrumentation.
+
 mforce-indirect-call
 Target Report Var(flag_force_indirect_call) Init(0)
 Make a

Re: PING^2: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

2019-02-22 Thread H.J. Lu
On Fri, Dec 14, 2018 at 1:28 PM Jeff Law  wrote:
>
> On 12/11/18 9:03 AM, H.J. Lu wrote:
> > On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu  wrote:
> >> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
> >>  wrote:
> >>> On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu  wrote:
>  Currently GCC inserts ENDBR instruction at entries of all non-static
>  functions, unless LTO compilation is used.  Marking all functions,
>  which are not called indirectly with nocf_check attribute, is not
>  ideal since 99% of functions in a program may be of this kind.
> 
>  This patch adds -mmanual-endbr and cf_check function attribute.  They
>  can be used together with -fcf-protection such that ENDBR instruction
>  is inserted only at entries of functions with cf_check attribute.  It
>  can limit number of ENDBR instructions to reduce program size.
> 
>  OK for trubk?
> >>> I wonder if the linker could assist with ENDBR creation by
> >>> redirecting all non-direct call relocs to a linker-generated
> >>> stub with ENBR and a direct branch?
> >>>
> >> The goal of this patch is to add as few as ENDBR as possible
> >> to reduce program size as much as possible.   Also there is no
> >> relocation for indirect branch via register.
> >>
> > Hi Honza, Jakub, Jeff, Richard,
> >
> > Here is the rebased patch.  Can you guys take a look?
> >
> > Thanks.
> >
> >
> > -- H.J.
> >
> >
> > 0001-i386-Add-mmanual-endbr-and-cf_check-function-attribu.patch
> >
> > From 5934c6be6495b2d6f278646e25f9e684f6610e2b Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" 
> > Date: Thu, 14 Jun 2018 09:19:27 -0700
> > Subject: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute
> >
> > Currently GCC inserts ENDBR instruction at entries of all non-static
> > functions, unless LTO compilation is used.  Marking all functions,
> > which are not called indirectly with nocf_check attribute, is not
> > ideal since 99% of functions in a program may be of this kind.
> >
> > This patch adds -mmanual-endbr and cf_check function attribute.  They
> > can be used together with -fcf-protection such that ENDBR instruction
> > is inserted only at entries of functions with cf_check attribute.  It
> > can limit number of ENDBR instructions to reduce program size.
> >
> > gcc/
> >
> >   * config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR
> >   at the function entry only when -mmanual-endbr isn't used or
> >   there is cf_check function attribute.
> >   (ix86_attribute_table): Add cf_check.
> >   * config/i386/i386.opt: Add -mmanual-endbr.
> >   * doc/extend.texi: Document cf_check attribute.
> >   * doc/invoke.texi: Document -mmanual-endbr.
> >
> > gcc/testsuite/
> >
> >   * gcc.target/i386/cf_check-1.c: New test.
> >   * gcc.target/i386/cf_check-2.c: Likewise.
> >   * gcc.target/i386/cf_check-3.c: Likewise.
> >   * gcc.target/i386/cf_check-4.c: Likewise.
> >   * gcc.target/i386/cf_check-5.c: Likewise.
> OK.
>
> Though I'm not sure how valuable this is in practice.  Yea, it saves
> some space at the start of functions, but I find myself wondering more
> and more if we should be pushing folks towards LTO for a variety of reasons.
>

Hi Jeff,

Here is an update for this new option to move  -mmanual-endbr check to
pass_insert_endbranch::gate.  I'd like to get it into GCC 9.


-- 
H.J.
From c1a25d170a942410eeb7803d61e6cac29678c9bd Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Thu, 14 Feb 2019 06:23:06 -0800
Subject: [PATCH] i386: Check -mmanual-endbr in pass_insert_endbranch::gate

When -mmanual-endbr is used with -fcf-protection, only functions marked
with cf_check attribute should be instrumented with ENDBR.  We should
skip rest_of_insert_endbranch on functions without cf_check attribute.

gcc/

	PR target/89353
	* config/i386/i386.c (rest_of_insert_endbranch): Move the
	-mmanual-endbr and cf_check attribute check to ..
	(pass_insert_endbranch::gate): Here.

gcc/testsuite/

	PR target/89353
	* gcc.target/i386/cf_check-6.c: New test.
---
 gcc/config/i386/i386.c | 10 +-
 gcc/testsuite/gcc.target/i386/cf_check-6.c | 23 ++
 2 files changed, 28 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/cf_check-6.c

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index fd05873ba39..a99ca23fffa 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -2640,9 +2640,6 @@ rest_of_insert_endbranch (void)
 
   if (!lookup_attribute ("nocf_check",
 			 TYPE_ATTRIBUTES (TREE_TYPE (cfun->decl)))
-  && (!flag_manual_endbr
-	  || lookup_attribute ("cf_check",
-			   DECL_ATTRIBUTES (cfun->decl)))
   && !cgraph_node::get (cfun->decl)->only_called_directly_p ())
 {
   /* Queue ENDBR insertion to x86_function_profiler.  */
@@ -2773,9 +2770,12 @@ public:
   {}
 
   /* opt_pass methods: */
-  virtual bool gate (function *)
+  virtual bool gate (function *fun)
 {
-  return ((flag_cf_protecti

Re: PING^2: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

2019-02-22 Thread H.J. Lu
On Fri, Feb 22, 2019 at 11:18 AM H.J. Lu  wrote:
>
> On Fri, Dec 14, 2018 at 1:28 PM Jeff Law  wrote:
> >
> > On 12/11/18 9:03 AM, H.J. Lu wrote:
> > > On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu  wrote:
> > >> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
> > >>  wrote:
> > >>> On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu  wrote:
> >  Currently GCC inserts ENDBR instruction at entries of all non-static
> >  functions, unless LTO compilation is used.  Marking all functions,
> >  which are not called indirectly with nocf_check attribute, is not
> >  ideal since 99% of functions in a program may be of this kind.
> > 
> >  This patch adds -mmanual-endbr and cf_check function attribute.  They
> >  can be used together with -fcf-protection such that ENDBR instruction
> >  is inserted only at entries of functions with cf_check attribute.  It
> >  can limit number of ENDBR instructions to reduce program size.
> > 
> >  OK for trubk?
> > >>> I wonder if the linker could assist with ENDBR creation by
> > >>> redirecting all non-direct call relocs to a linker-generated
> > >>> stub with ENBR and a direct branch?
> > >>>
> > >> The goal of this patch is to add as few as ENDBR as possible
> > >> to reduce program size as much as possible.   Also there is no
> > >> relocation for indirect branch via register.
> > >>
> > > Hi Honza, Jakub, Jeff, Richard,
> > >
> > > Here is the rebased patch.  Can you guys take a look?
> > >
> > > Thanks.
> > >
> > >
> > > -- H.J.
> > >
> > >
> > > 0001-i386-Add-mmanual-endbr-and-cf_check-function-attribu.patch
> > >
> > > From 5934c6be6495b2d6f278646e25f9e684f6610e2b Mon Sep 17 00:00:00 2001
> > > From: "H.J. Lu" 
> > > Date: Thu, 14 Jun 2018 09:19:27 -0700
> > > Subject: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute
> > >
> > > Currently GCC inserts ENDBR instruction at entries of all non-static
> > > functions, unless LTO compilation is used.  Marking all functions,
> > > which are not called indirectly with nocf_check attribute, is not
> > > ideal since 99% of functions in a program may be of this kind.
> > >
> > > This patch adds -mmanual-endbr and cf_check function attribute.  They
> > > can be used together with -fcf-protection such that ENDBR instruction
> > > is inserted only at entries of functions with cf_check attribute.  It
> > > can limit number of ENDBR instructions to reduce program size.
> > >
> > > gcc/
> > >
> > >   * config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR
> > >   at the function entry only when -mmanual-endbr isn't used or
> > >   there is cf_check function attribute.
> > >   (ix86_attribute_table): Add cf_check.
> > >   * config/i386/i386.opt: Add -mmanual-endbr.
> > >   * doc/extend.texi: Document cf_check attribute.
> > >   * doc/invoke.texi: Document -mmanual-endbr.
> > >
> > > gcc/testsuite/
> > >
> > >   * gcc.target/i386/cf_check-1.c: New test.
> > >   * gcc.target/i386/cf_check-2.c: Likewise.
> > >   * gcc.target/i386/cf_check-3.c: Likewise.
> > >   * gcc.target/i386/cf_check-4.c: Likewise.
> > >   * gcc.target/i386/cf_check-5.c: Likewise.
> > OK.
> >
> > Though I'm not sure how valuable this is in practice.  Yea, it saves
> > some space at the start of functions, but I find myself wondering more
> > and more if we should be pushing folks towards LTO for a variety of reasons.
> >
>
> Hi Jeff,
>
> Here is an update for this new option to move  -mmanual-endbr check to
> pass_insert_endbranch::gate.  I'd like to get it into GCC 9.
>
>

I withdraw this patch.  It is covered by

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89355


-- 
H.J.


Re: PING^2: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

2018-12-14 Thread Jeff Law
On 12/11/18 9:03 AM, H.J. Lu wrote:
> On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu  wrote:
>> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
>>  wrote:
>>> On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu  wrote:
 Currently GCC inserts ENDBR instruction at entries of all non-static
 functions, unless LTO compilation is used.  Marking all functions,
 which are not called indirectly with nocf_check attribute, is not
 ideal since 99% of functions in a program may be of this kind.

 This patch adds -mmanual-endbr and cf_check function attribute.  They
 can be used together with -fcf-protection such that ENDBR instruction
 is inserted only at entries of functions with cf_check attribute.  It
 can limit number of ENDBR instructions to reduce program size.

 OK for trubk?
>>> I wonder if the linker could assist with ENDBR creation by
>>> redirecting all non-direct call relocs to a linker-generated
>>> stub with ENBR and a direct branch?
>>>
>> The goal of this patch is to add as few as ENDBR as possible
>> to reduce program size as much as possible.   Also there is no
>> relocation for indirect branch via register.
>>
> Hi Honza, Jakub, Jeff, Richard,
> 
> Here is the rebased patch.  Can you guys take a look?
> 
> Thanks.
> 
> 
> -- H.J.
> 
> 
> 0001-i386-Add-mmanual-endbr-and-cf_check-function-attribu.patch
> 
> From 5934c6be6495b2d6f278646e25f9e684f6610e2b Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" 
> Date: Thu, 14 Jun 2018 09:19:27 -0700
> Subject: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute
> 
> Currently GCC inserts ENDBR instruction at entries of all non-static
> functions, unless LTO compilation is used.  Marking all functions,
> which are not called indirectly with nocf_check attribute, is not
> ideal since 99% of functions in a program may be of this kind.
> 
> This patch adds -mmanual-endbr and cf_check function attribute.  They
> can be used together with -fcf-protection such that ENDBR instruction
> is inserted only at entries of functions with cf_check attribute.  It
> can limit number of ENDBR instructions to reduce program size.
> 
> gcc/
> 
>   * config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR
>   at the function entry only when -mmanual-endbr isn't used or
>   there is cf_check function attribute.
>   (ix86_attribute_table): Add cf_check.
>   * config/i386/i386.opt: Add -mmanual-endbr.
>   * doc/extend.texi: Document cf_check attribute.
>   * doc/invoke.texi: Document -mmanual-endbr.
> 
> gcc/testsuite/
> 
>   * gcc.target/i386/cf_check-1.c: New test.
>   * gcc.target/i386/cf_check-2.c: Likewise.
>   * gcc.target/i386/cf_check-3.c: Likewise.
>   * gcc.target/i386/cf_check-4.c: Likewise.
>   * gcc.target/i386/cf_check-5.c: Likewise.
OK.

Though I'm not sure how valuable this is in practice.  Yea, it saves
some space at the start of functions, but I find myself wondering more
and more if we should be pushing folks towards LTO for a variety of reasons.

jeff



Re: PING^2: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute

2018-12-14 Thread H.J. Lu
On Fri, Dec 14, 2018 at 1:28 PM Jeff Law  wrote:
>
> On 12/11/18 9:03 AM, H.J. Lu wrote:
> > On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu  wrote:
> >> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
> >>  wrote:
> >>> On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu  wrote:
>  Currently GCC inserts ENDBR instruction at entries of all non-static
>  functions, unless LTO compilation is used.  Marking all functions,
>  which are not called indirectly with nocf_check attribute, is not
>  ideal since 99% of functions in a program may be of this kind.
> 
>  This patch adds -mmanual-endbr and cf_check function attribute.  They
>  can be used together with -fcf-protection such that ENDBR instruction
>  is inserted only at entries of functions with cf_check attribute.  It
>  can limit number of ENDBR instructions to reduce program size.
> 
>  OK for trubk?
> >>> I wonder if the linker could assist with ENDBR creation by
> >>> redirecting all non-direct call relocs to a linker-generated
> >>> stub with ENBR and a direct branch?
> >>>
> >> The goal of this patch is to add as few as ENDBR as possible
> >> to reduce program size as much as possible.   Also there is no
> >> relocation for indirect branch via register.
> >>
> > Hi Honza, Jakub, Jeff, Richard,
> >
> > Here is the rebased patch.  Can you guys take a look?
> >
> > Thanks.
> >
> >
> > -- H.J.
> >
> >
> > 0001-i386-Add-mmanual-endbr-and-cf_check-function-attribu.patch
> >
> > From 5934c6be6495b2d6f278646e25f9e684f6610e2b Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" 
> > Date: Thu, 14 Jun 2018 09:19:27 -0700
> > Subject: [PATCH] i386; Add -mmanual-endbr and cf_check function attribute
> >
> > Currently GCC inserts ENDBR instruction at entries of all non-static
> > functions, unless LTO compilation is used.  Marking all functions,
> > which are not called indirectly with nocf_check attribute, is not
> > ideal since 99% of functions in a program may be of this kind.
> >
> > This patch adds -mmanual-endbr and cf_check function attribute.  They
> > can be used together with -fcf-protection such that ENDBR instruction
> > is inserted only at entries of functions with cf_check attribute.  It
> > can limit number of ENDBR instructions to reduce program size.
> >
> > gcc/
> >
> >   * config/i386/i386.c (rest_of_insert_endbranch): Insert ENDBR
> >   at the function entry only when -mmanual-endbr isn't used or
> >   there is cf_check function attribute.
> >   (ix86_attribute_table): Add cf_check.
> >   * config/i386/i386.opt: Add -mmanual-endbr.
> >   * doc/extend.texi: Document cf_check attribute.
> >   * doc/invoke.texi: Document -mmanual-endbr.
> >
> > gcc/testsuite/
> >
> >   * gcc.target/i386/cf_check-1.c: New test.
> >   * gcc.target/i386/cf_check-2.c: Likewise.
> >   * gcc.target/i386/cf_check-3.c: Likewise.
> >   * gcc.target/i386/cf_check-4.c: Likewise.
> >   * gcc.target/i386/cf_check-5.c: Likewise.
> OK.
>
> Though I'm not sure how valuable this is in practice.  Yea, it saves
> some space at the start of functions, but I find myself wondering more
> and more if we should be pushing folks towards LTO for a variety of reasons.
>

LTO can solve many issues in addition to unnecessary ENDBR.
I have been fixing LTO bugs in bfd linker uncovered by people
who are using LTO :-).

-- 
H.J.