On Fri, Dec 14, 2018 at 1:28 PM Jeff Law <l...@redhat.com> wrote:
>
> On 12/11/18 9:03 AM, H.J. Lu wrote:
> > On Mon, Dec 3, 2018 at 5:45 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> >> On Mon, Jun 18, 2018 at 2:20 AM Richard Biener
> >> <richard.guent...@gmail.com> wrote:
> >>> On Fri, Jun 15, 2018 at 2:59 PM H.J. Lu <hongjiu...@intel.com> 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" <hjl.to...@gmail.com>
> > 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" <hjl.to...@gmail.com>
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_protection & CF_BRANCH));
+ return ((flag_cf_protection & CF_BRANCH)
+ && (!flag_manual_endbr
+ || lookup_attribute ("cf_check",
+ DECL_ATTRIBUTES (fun->decl))));
}
virtual unsigned int execute (function *)
diff --git a/gcc/testsuite/gcc.target/i386/cf_check-6.c b/gcc/testsuite/gcc.target/i386/cf_check-6.c
new file mode 100644
index 00000000000..7a62f8413d7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/cf_check-6.c
@@ -0,0 +1,23 @@
+/* PR target/89353 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcf-protection -mmanual-endbr" } */
+/* { dg-final { scan-assembler-not {\mendbr} } } */
+
+int
+bar (int* val)
+{
+ int status = 99;
+
+ if((val == 0))
+ {
+ status = 22;
+ goto end;
+ }
+
+ extern int x;
+ *val = x;
+
+ status = 0;
+end:
+ return status;
+}
--
2.20.1
