| Message ID | 20220510162954.2755700-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add -fcf-check-attribute=[yes|no|none] for Linux kernel | expand |
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? > 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; >
On Tue, May 10, 2022 at 11:39 PM Richard Biener <rguenther@suse.de> 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? 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 <rguenther@suse.de> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
On Wed, 11 May 2022, H.J. Lu wrote: > On Tue, May 10, 2022 at 11:39 PM Richard Biener <rguenther@suse.de> 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 <rguenther@suse.de> > > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > > Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg) > > > >
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;