> -----Original Message----- > From: Jeff Law [mailto:l...@redhat.com] > Sent: Friday, September 29, 2017 12:44 AM > To: Tsimbalist, Igor V <igor.v.tsimbal...@intel.com>; gcc- > patc...@gcc.gnu.org > Cc: richard.guent...@gmail.com > Subject: Re: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling > > On 09/19/2017 07:39 AM, Tsimbalist, Igor V wrote: > > Here is an updated patch (version #2). The main differences are: > > > > - Change attribute and option names; > > - Add additional parameter to gimple_build_call_from_tree by adding a > type parameter and > > use it 'nocf_check' attribute propagation; > > - Reimplement fixes in expand_call_stmt to propagate 'nocf_check' > > attribute; > > - Consider 'nocf_check' attribute in Identical Code Folding (ICF) > > optimization; > > - Add warning for type inconsistency regarding 'nocf_check' attribute; > > - Many small fixes; > > > > gcc/c-family/ > > * c-attribs.c (handle_nocf_check_attribute): New function. > > (c_common_attribute_table): Add 'nocf_check' handling. > > * c-common.c (check_missing_format_attribute): New function. > > * c-common.h: Likewise. > > > > gcc/c/ > > * c-typeck.c (convert_for_assignment): Add check for nocf_check > > attribute. > > * gimple-parser.c: Add second argument NULL to > > gimple_build_call_from_tree. > > > > gcc/cp/ > > * typeck.c (convert_for_assignment): Add check for nocf_check > > attribute. > > > > gcc/ > > * cfgexpand.c (expand_call_stmt): Set REG_CALL_NOCF_CHECK for > > call insn. > > * combine.c (distribute_notes): Add REG_CALL_NOCF_CHECK > handling. > > * common.opt: Add fcf-protection flag. > > * emit-rtl.c (try_split): Add REG_CALL_NOCF_CHECK handling. > > * flag-types.h: Add enum cf_protection_level. > > * gimple.c (gimple_build_call_from_tree): Add second parameter. > > Add 'nocf_check' attribute propagation to gimple call. > > * gimple.h (gf_mask): Add GF_CALL_NOCF_CHECK. > > (gimple_call_nocf_check_p): New function. > > (gimple_call_set_nocf_check): Likewise. > > * gimplify.c: Add second argument to gimple_build_call_from_tree. > > * ipa-icf.c: Add nocf_check attribute in statement hash. > > * recog.c (peep2_attempt): Add REG_CALL_NOCF_CHECK handling. > > * reg-notes.def: Add REG_NOTE (CALL_NOCF_CHECK). > > * toplev.c (process_options): Add flag_cf_protection handling. > > > > Is it ok for trunk? > > > > Thanks, > > Igor > > > > > > > > > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index > > 0337537..77d1909 100644 > > --- a/gcc/c-family/c-attribs.c > > +++ b/gcc/c-family/c-attribs.c > > @@ -65,6 +65,7 @@ static tree handle_asan_odr_indicator_attribute > > (tree *, tree, tree, int, static tree handle_stack_protect_attribute > > (tree *, tree, tree, int, bool *); static tree > > handle_noinline_attribute (tree *, tree, tree, int, bool *); static > > tree handle_noclone_attribute (tree *, tree, tree, int, bool *); > > +static tree handle_nocf_check_attribute (tree *, tree, tree, int, > > +bool *); > > static tree handle_noicf_attribute (tree *, tree, tree, int, bool *); > > static tree handle_noipa_attribute (tree *, tree, tree, int, bool *); > > static tree handle_leaf_attribute (tree *, tree, tree, int, bool *); > > @@ -367,6 +368,8 @@ const struct attribute_spec > c_common_attribute_table[] = > > { "patchable_function_entry", 1, 2, true, false, false, > > handle_patchable_function_entry_attribute, > > false }, > > + { "nocf_check", 0, 0, false, true, true, > > + handle_nocf_check_attribute, false }, > > { NULL, 0, 0, false, false, false, NULL, false } > > }; > > > > @@ -783,6 +786,26 @@ handle_noclone_attribute (tree *node, tree > name, > > return NULL_TREE; > > } > > > > +/* Handle a "nocf_check" attribute; arguments as in > > + struct attribute_spec.handler. */ > > + > > +static tree > > +handle_nocf_check_attribute (tree *node, tree name, > > + tree ARG_UNUSED (args), > > + int ARG_UNUSED (flags), bool *no_add_attrs) { > > + if (TREE_CODE (*node) != FUNCTION_TYPE > > + && TREE_CODE (*node) != METHOD_TYPE > > + && TREE_CODE (*node) != FIELD_DECL > > + && TREE_CODE (*node) != TYPE_DECL) > So curious, is this needed for FIELD_DECL and TYPE_DECL? ISTM the attribute > is applied to function/method types. > > If we do need to handle FIELD_DECL and TYPE_DECL here, can you add a > quick comment why?
You are right. Probably it was left from the attribute transition from decl to type. I removed these two lines. All CET tests passed. > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index > > b3ec3a0..78a730e 100644 > > --- a/gcc/c-family/c-common.c > > +++ b/gcc/c-family/c-common.c > > @@ -7253,6 +7253,26 @@ check_missing_format_attribute (tree ltype, > tree rtype) > > return false; > > } > > > > +/* Check for missing nocf_check attributes on function pointers. LTYPE is > > + the new type or left-hand side type. RTYPE is the old type or > > + right-hand side type. Returns TRUE if LTYPE is missing the desired > > + attribute. */ > > + > > +bool > > +check_missing_nocf_check_attribute (tree ltype, tree rtype) { > > + tree const ttr = TREE_TYPE (rtype), ttl = TREE_TYPE (ltype); > > + tree ra, la; > > + > > + for (ra = TYPE_ATTRIBUTES (ttr); ra; ra = TREE_CHAIN (ra)) > > + if (is_attribute_p ("nocf_check", TREE_PURPOSE (ra))) > > + break; > > + for (la = TYPE_ATTRIBUTES (ttl); la; la = TREE_CHAIN (la)) > > + if (is_attribute_p ("nocf_check", TREE_PURPOSE (la))) > > + break; > > + return la != ra; > ? ISTM the only time la == ra here is when they're both NULL. Aren't the > TYPE_ATTRIBUTE chain members unique and thus pointer equality isn't the > right check? > > Shouldn't you be looking at the TREE_PURPOSE of ra and la and comparing > those? It looks I was lucky :). I see the point and re-wrote the return statement as if ((la && ra) /* Both types have the attribute. */ || (la == ra)) /* Both types do not have the attribute. */ return false; else return true; /* One of the types has the attribute. */ Igor > Not accepting or rejecting at this point as I could mis-understand how how > this is supposed to work in my two comments above. > > jeff > > >