[PATCH][AArch64] Properly reject invalid attribute strings

James Greenhalgh james.greenhalgh@arm.com
Fri Jan 15 17:20:00 GMT 2016


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.tkachov@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.tkachov@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



More information about the Gcc-patches mailing list