On Wed, 11 May 2022, H.J. Lu wrote:
> On Tue, May 10, 2022 at 11:39 PM Richard Biener <[email protected]> wrote:
> >
> > On Tue, 10 May 2022, H.J. Lu wrote:
> >
> > > When compiling Linux kernel with -fcf-protection=branch to enable x86
> > > Indiret Branch Tracking (IBT), ENDBR is added to all global functions.
> > > This creates more "legal" forward edges than necessary. -mmanual-endbr
> > > provides a way to insert ENDBR instruction at function entry only via
> > > the 'cf_check' function attribute and programmers can add the 'cf_check'
> > > function attribute to functions which can be reached by indirect branch.
> > >
> > > Add -fcf-check-attribute=[yes|no|none] to imply "cf_check" or "nocf_check"
> > > function attributes so that GCC can produce a diagnostic when there is a
> > > mismatch in cf_check or nocf_check function attributes.
> > >
> > > This has been tested on Linux kernel.
> >
> > I don't quite understand the purpose but wouldn't
> > -fcf-check-attribute=[cf|nocf|off] be better than having no and none?
> > Since 'off' is the default you need 'off' only to be able to negate
> > an earlier cf/nocf?
>
> This option adds implied "cf_check" or "nocf_check" attributes if there
> is none. I need an option not to add them by default. I can use
> -fcf-check-attribute=none or -fcf-check-attribute=off. Which one is
> better?
I guess 'none' is then better, but still -fcf-check-attribute=[cf|nocf]
is then better matching than -fcf-check-attribute=[yes|no].
Richard.
> Thanks.
>
> > > gcc/
> > >
> > > PR target/102953
> > > * attribs.cc (decl_attributes): Add implied cf_check or nocf_check
> > > function attributes.
> > > * common.opt: Add -fcf-check-attribute=.
> > > * flag-types.h (cf_check_attribute): New.
> > > * doc/invoke.texi: Document -fcf-check-attribute=.
> > >
> > > gcc/c/
> > >
> > > PR target/102953
> > > * c-decl.cc (diagnose_mismatched_decls): Check implied cf_check
> > > and nocf_check function attributes.
> > >
> > > gcc/testsuite/
> > >
> > > PR target/102953
> > > * gcc.target/i386/pr102953-3.c: New test.
> > > * gcc.target/i386/pr102953-4.c: Likewise.
> > > * gcc.target/i386/pr102953-5.c: Likewise.
> > > * gcc.target/i386/pr102953-6.c: Likewise.
> > > ---
> > > gcc/attribs.cc | 19 +++++++++++++++++++
> > > gcc/c/c-decl.cc | 22 +++++++++++++++++++++-
> > > gcc/common.opt | 16 ++++++++++++++++
> > > gcc/doc/invoke.texi | 12 ++++++++++++
> > > gcc/flag-types.h | 8 ++++++++
> > > gcc/testsuite/gcc.target/i386/pr102953-3.c | 8 ++++++++
> > > gcc/testsuite/gcc.target/i386/pr102953-4.c | 7 +++++++
> > > gcc/testsuite/gcc.target/i386/pr102953-5.c | 7 +++++++
> > > gcc/testsuite/gcc.target/i386/pr102953-6.c | 8 ++++++++
> > > 9 files changed, 106 insertions(+), 1 deletion(-)
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-3.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-4.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-5.c
> > > create mode 100644 gcc/testsuite/gcc.target/i386/pr102953-6.c
> > >
> > > diff --git a/gcc/attribs.cc b/gcc/attribs.cc
> > > index b219f878042..34e8707eac1 100644
> > > --- a/gcc/attribs.cc
> > > +++ b/gcc/attribs.cc
> > > @@ -694,6 +694,25 @@ decl_attributes (tree *node, tree attributes, int
> > > flags,
> > > attributes = tree_cons (get_identifier ("no_icf"), NULL,
> > > attributes);
> > > }
> > >
> > > + /* -fcf-check-attribute=[yes|no] implies "cf_check" or "nocf_check"
> > > + function attribute. */
> > > + if (TREE_CODE (*node) == FUNCTION_DECL
> > > + && flag_cf_check_attribute != CF_CHECK_ATTRIBUTE_NONE
> > > + && !fndecl_built_in_p (*node)
> > > + && lookup_attribute ("nocf_check",
> > > + DECL_ATTRIBUTES (*node)) == nullptr
> > > + && lookup_attribute ("cf_check",
> > > + DECL_ATTRIBUTES (*node)) == nullptr
> > > + && (!attributes
> > > + || (lookup_attribute ("nocf_check", attributes) == nullptr
> > > + && lookup_attribute ("cf_check", attributes) == nullptr)))
> > > + {
> > > + const char *attr = (flag_cf_check_attribute ==
> > > CF_CHECK_ATTRIBUTE_YES
> > > + ? "cf_check" : "nocf_check");
> > > + attributes = tree_cons (get_identifier (attr), nullptr,
> > > + attributes);
> > > + }
> > > +
> > > targetm.insert_attributes (*node, &attributes);
> > >
> > > /* Note that attributes on the same declaration are not necessarily
> > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > index c701f07befe..787c39dc0fe 100644
> > > --- a/gcc/c/c-decl.cc
> > > +++ b/gcc/c/c-decl.cc
> > > @@ -2133,7 +2133,27 @@ diagnose_mismatched_decls (tree newdecl, tree
> > > olddecl,
> > > error ("conflicting type qualifiers for %q+D", newdecl);
> > > }
> > > else
> > > - error ("conflicting types for %q+D; have %qT", newdecl,
> > > newtype);
> > > + {
> > > + if (flag_cf_check_attribute == CF_CHECK_ATTRIBUTE_NO
> > > + && (!lookup_attribute ("nocf_check",
> > > + TYPE_ATTRIBUTES (oldtype))
> > > + != !lookup_attribute ("nocf_check",
> > > + TYPE_ATTRIBUTES (newtype))))
> > > + error ("conflicting types for %q+D; have %qT with "
> > > + "implied %<nocf_check%> attribute",
> > > + newdecl, newtype);
> > > + else if (flag_cf_check_attribute == CF_CHECK_ATTRIBUTE_YES
> > > + && (!lookup_attribute ("cf_check",
> > > + TYPE_ATTRIBUTES (oldtype))
> > > + != !lookup_attribute ("cf_check",
> > > + TYPE_ATTRIBUTES (newtype))))
> > > + error ("conflicting types for %q+D; have %qT with "
> > > + "implied %<cf_check%> attribute",
> > > + newdecl, newtype);
> > > + else
> > > + error ("conflicting types for %q+D; have %qT",
> > > + newdecl, newtype);
> > > + }
> > > diagnose_arglist_conflict (newdecl, olddecl, newtype, oldtype);
> > > locate_old_decl (olddecl);
> > > return false;
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index 8a0dafc522d..d5952a44765 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -1877,6 +1877,22 @@ Enum(cf_protection_level) String(check)
> > > Value(CF_CHECK)
> > > EnumValue
> > > Enum(cf_protection_level) String(none) Value(CF_NONE)
> > >
> > > +fcf-check-attribute=
> > > +Target RejectNegative Joined Enum(cf_check_attribute)
> > > Var(flag_cf_check_attribute) Init(CF_CHECK_ATTRIBUTE_NONE)
> > > +-fcf-check-attribute=[none|yes|no] Select the implied function attribute.
> > > +
> > > +Enum
> > > +Name(cf_check_attribute) Type(enum cf_check_attribute)
> > > UnknownError(unknown default Control-Flow attribute %qs)
> > > +
> > > +EnumValue
> > > +Enum(cf_check_attribute) String(none) Value(CF_CHECK_ATTRIBUTE_NONE)
> > > +
> > > +EnumValue
> > > +Enum(cf_check_attribute) String(yes) Value(CF_CHECK_ATTRIBUTE_YES)
> > > +
> > > +EnumValue
> > > +Enum(cf_check_attribute) String(no) Value(CF_CHECK_ATTRIBUTE_NO)
> > > +
> > > finstrument-functions
> > > Common Var(flag_instrument_function_entry_exit)
> > > Instrument function entry and exit with profiling calls.
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index 7a35d9613a4..22e02b8be7e 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -606,6 +606,7 @@ Objective-C and Objective-C++ Dialects}.
> > > -fasan-shadow-offset=@var{number}
> > > -fsanitize-sections=@var{s1},@var{s2},... @gol
> > > -fsanitize-undefined-trap-on-error -fbounds-check @gol
> > > -fcf-protection=@r{[}full@r{|}branch@r{|}return@r{|}none@r{|}check@r{]}
> > > @gol
> > > +-fcf-check-attribute=@r{[}none@r{|}yes@r{|}no@r{]} @gol
> > > -fharden-compares -fharden-conditional-branches @gol
> > > -fstack-protector -fstack-protector-all -fstack-protector-strong @gol
> > > -fstack-protector-explicit -fstack-check @gol
> > > @@ -16070,6 +16071,17 @@ Currently the x86 GNU/Linux target provides an
> > > implementation based
> > > on Intel Control-flow Enforcement Technology (CET) which works for
> > > i686 processor or newer.
> > >
> > > +@item -fcf-check-attribute=@r{[}none@r{|}yes@r{|}no@r{]}
> > > +@opindex fcf-check-attribute
> > > +Select the implied function attribute for code instrumentation of
> > > +control-flow transfers.
> > > +
> > > +The value @code{yes} makes the @code{cf_check} attribute implied on
> > > +functions. The value @code{no} makes the @code{nocf_check} attribute
> > > +implied on functions. The value @code{none}, which is the default,
> > > +doesn't imply the @code{cf_check} nor @code{nocf_check} attributes on
> > > +functions.
> > > +
> > > @item -fharden-compares
> > > @opindex fharden-compares
> > > For every logical test that survives gimple optimizations and is
> > > diff --git a/gcc/flag-types.h b/gcc/flag-types.h
> > > index 2c8498169e0..c84d67b1ac8 100644
> > > --- a/gcc/flag-types.h
> > > +++ b/gcc/flag-types.h
> > > @@ -449,6 +449,14 @@ enum cf_protection_level
> > > CF_CHECK = 1 << 3
> > > };
> > >
> > > +/* Default Control-Flow Protection attribute. */
> > > +enum cf_check_attribute
> > > +{
> > > + CF_CHECK_ATTRIBUTE_NONE = 0,
> > > + CF_CHECK_ATTRIBUTE_YES,
> > > + CF_CHECK_ATTRIBUTE_NO
> > > +};
> > > +
> > > /* Parloops schedule type. */
> > > enum parloops_schedule_type
> > > {
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr102953-3.c
> > > b/gcc/testsuite/gcc.target/i386/pr102953-3.c
> > > new file mode 100644
> > > index 00000000000..fd92c9c577e
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr102953-3.c
> > > @@ -0,0 +1,8 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-Wall -fcf-protection -mmanual-endbr
> > > -fcf-check-attribute=no" } */
> > > +
> > > +static void __attribute__((cf_check)) foo(void);
> > > +static void foo(void) /* { dg-error "implied 'nocf_check' attribute" } */
> > > +{
> > > +}
> > > +void (*ptr)(void) = foo; /* { dg-warning "incompatible pointer type" } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr102953-4.c
> > > b/gcc/testsuite/gcc.target/i386/pr102953-4.c
> > > new file mode 100644
> > > index 00000000000..cd8f4279180
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr102953-4.c
> > > @@ -0,0 +1,7 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-Wall -fcf-protection -mmanual-endbr
> > > -fcf-check-attribute=no" } */
> > > +
> > > +static void foo(void)
> > > +{
> > > +}
> > > +void (*ptr)(void) = foo; /* { dg-warning "incompatible pointer type" } */
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr102953-5.c
> > > b/gcc/testsuite/gcc.target/i386/pr102953-5.c
> > > new file mode 100644
> > > index 00000000000..b1bd4afe85f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr102953-5.c
> > > @@ -0,0 +1,7 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-Wall -fcf-protection -mmanual-endbr
> > > -fcf-check-attribute=no" } */
> > > +
> > > +extern void foo(void);
> > > +void __attribute__((cf_check)) foo(void) /* { dg-error "implied
> > > 'nocf_check' attribute" } */
> > > +{
> > > +}
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr102953-6.c
> > > b/gcc/testsuite/gcc.target/i386/pr102953-6.c
> > > new file mode 100644
> > > index 00000000000..ee0c66f94cb
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr102953-6.c
> > > @@ -0,0 +1,8 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-Wall -fcf-protection -mmanual-endbr
> > > -fcf-check-attribute=no" } */
> > > +
> > > +static int __attribute__((cf_check)) foo(char a[], int b)
> > > +{
> > > + return 0;
> > > +}
> > > +int (*ptr)(char[], int) = foo;
> > >
> >
> > --
> > Richard Biener <[email protected]>
> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> > Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
>
>
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
