This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][AArch64] Properly reject invalid attribute strings
- From: James Greenhalgh <james dot greenhalgh at arm dot com>
- To: Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <marcus dot shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>
- Date: Fri, 15 Jan 2016 17:20:28 +0000
- Subject: Re: [PATCH][AArch64] Properly reject invalid attribute strings
- Authentication-results: sourceware.org; auth=none
- References: <5698F6AA dot 3060202 at foss dot arm dot com>
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