Updated patch, version #3. Igor
> -----Original Message----- > From: Tsimbalist, Igor V > Sent: Friday, September 29, 2017 4:32 PM > To: Jeff Law <l...@redhat.com>; gcc-patches@gcc.gnu.org > Cc: richard.guent...@gmail.com; Tsimbalist, Igor V > <igor.v.tsimbal...@intel.com> > Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling > > > -----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 > > > > > >
0001-Add-generic-part-for-Intel-CET-enabling-fcf-protecti.patch
Description: 0001-Add-generic-part-for-Intel-CET-enabling-fcf-protecti.patch
