On Fri, Jan 15, 2016 at 01:39:54PM +0000, Kyrill Tkachov wrote:
> Hi all,
>
> A bug in the target attribute parsing logic led to us silently accepting
> attribute strings that did not appear in the attributes table i.e invalid
> attributes.
>
> This patch fixes that oversight so we now error out on obviously bogus
> strings.
This is ok.
> 2016-01-15 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>
> * config/aarch64/aarch64.c (aarch64_process_one_target_attr): Return
> false when argument string is not found in the attributes table
> at all.
>
> 2016-01-15 Kyrylo Tkachov <kyrylo.tkac...@arm.com>
>
> * gcc.target/aarch64/target_attr_17.c: New test.
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> e54ce7985f52c6a61b2ef1e3d7f847f22b1a959f..f2e4b45ac0ad1223e8149d1a35782c13f493a740
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -8938,6 +8938,7 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> arg++;
> }
> const struct aarch64_attribute_info *p_attr;
> + bool found = false;
> for (p_attr = aarch64_attributes; p_attr->name; p_attr++)
> {
> /* If the names don't match up, or the user has given an argument
> @@ -8946,6 +8947,7 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> if (strcmp (str_to_check, p_attr->name) != 0)
> continue;
>
> + found = true;
> bool attr_need_arg_p = p_attr->attr_type == aarch64_attr_custom
> || p_attr->attr_type == aarch64_attr_enum;
>
> @@ -9025,7 +9027,10 @@ aarch64_process_one_target_attr (char *arg_str, const
> char* pragma_or_attr)
> }
> }
>
> - return true;
> + /* If we reached here we either have found an attribute and validated
> + it or didn't match any. If we matched an attribute but its arguments
> + were malformed we will have returned false already. */
> + return found;
I don't like this "found" variable, it normally smells of a refactoring
opportunity. I wonder whether you could clean the function up a bit by
restructuring the logic.
Regardless, this is OK for trunk.
Thanks,
James
