From: "Tsimbalist, Igor V" <igor.v.tsimbalist@intel.com>
To: Jeff Law <law@redhat.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Cc: "richard.guenther@gmail.com" <richard.guenther@gmail.com>,
"Tsimbalist, Igor V" <igor.v.tsimbalist@intel.com>
Subject: RE: 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling
Date: Fri, 29 Sep 2017 14:31:00 -0000 [thread overview]
Message-ID: <D511F25789BA7F4EBA64C8A63891A00291F396FA@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <8ae1434e-5c90-b129-1968-e2fe325d9005@redhat.com>
> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Friday, September 29, 2017 12:44 AM
> To: Tsimbalist, Igor V <igor.v.tsimbalist@intel.com>; gcc-
> patches@gcc.gnu.org
> Cc: richard.guenther@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
>
>
>
next prev parent reply other threads:[~2017-09-29 14:31 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-01 8:56 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-08-15 14:08 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Richard Biener
2017-08-18 14:01 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-08-18 14:06 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Richard Biener
2017-08-18 14:58 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-12 15:34 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-15 11:12 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-15 12:14 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Richard Biener
2017-09-19 13:39 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-28 22:44 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-09-29 14:31 ` Tsimbalist, Igor V [this message]
2017-09-29 16:04 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-10-05 10:20 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-10-12 6:26 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-10-12 8:33 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-10-12 15:15 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-08-25 21:03 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-09-12 15:40 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-13 19:05 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-08-25 21:05 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-09-12 15:59 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-13 18:56 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
2017-09-13 17:08 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Tsimbalist, Igor V
2017-09-13 19:01 ` 0001-Part-1.-Add-generic-part-for-Intel-CET-enabling Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D511F25789BA7F4EBA64C8A63891A00291F396FA@IRSMSX102.ger.corp.intel.com \
--to=igor.v.tsimbalist@intel.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=law@redhat.com \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).