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?
> 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?
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
