| Message ID | 5698F6AA.3060202@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
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
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; } /* Count how many times the character C appears in diff --git a/gcc/testsuite/gcc.target/aarch64/target_attr_17.c b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c new file mode 100644 index 0000000000000000000000000000000000000000..483cc6d4a1d0b78ac404ae903c7e98a6ad288d17 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/target_attr_17.c @@ -0,0 +1,8 @@ +__attribute__((target("invalid-attr-string"))) +int +foo (int a) +{ + return a + 5; +} + +/* { dg-error "target attribute.*is invalid" "" { target *-*-* } 0 } */ \ No newline at end of file
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. Bootstrapped and tested on aarch64. Ok for trunk? Thanks, Kyrill 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.