| Message ID | e133bfeb-e307-426e-6918-115a40da1ad3@suse.cz |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 8, 2017 at 12:59 AM, Martin Liška <mliska@suse.cz> wrote: > On 08/02/2017 09:56 PM, Martin Sebor wrote: >> On 08/02/2017 01:04 PM, Jeff Law wrote: >>> On 07/28/2017 05:13 AM, Martin Liška wrote: >>>> Hello. >>>> >>>> Following patch skips empty strings in 'target' attribute. >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>> Ready to be installed? >>>> Martin >>>> >>>> gcc/ChangeLog: >>>> >>>> 2017-07-13 Martin Liska <mliska@suse.cz> >>>> >>>> PR c++/81355 >>>> * attribs.c (sorted_attr_string): Skip empty strings. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2017-07-13 Martin Liska <mliska@suse.cz> >>>> >>>> PR c++/81355 >>>> * g++.dg/other/pr81355.C: New test. >>> OK. THough one could legitimately ask if this ought to be a parsing error. >> >> I would suggest to at least issue a warning. It seems that >> the empty string must almost certainly be a bug here, say due >> to unintended macro expansion. >> >> Otherwise, if it should remain silently (and intentionally) >> accepted, I recommend to document it. As it is, the manual >> says that the "string" argument is equivalent to compiling >> with -mstring which for "" would be rejected by the driver. >> >> Martin > > Thanks you both for feedback. I decided to come up with an error message for that. > > Feedback appreciated. I'd think to check it in handle_target_argument rather than require all targets to check it. Jason
On 08/07/2017 10:59 PM, Martin Liška wrote: > On 08/02/2017 09:56 PM, Martin Sebor wrote: >> On 08/02/2017 01:04 PM, Jeff Law wrote: >>> On 07/28/2017 05:13 AM, Martin Liška wrote: >>>> Hello. >>>> >>>> Following patch skips empty strings in 'target' attribute. >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >>>> >>>> Ready to be installed? >>>> Martin >>>> >>>> gcc/ChangeLog: >>>> >>>> 2017-07-13 Martin Liska <mliska@suse.cz> >>>> >>>> PR c++/81355 >>>> * attribs.c (sorted_attr_string): Skip empty strings. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2017-07-13 Martin Liska <mliska@suse.cz> >>>> >>>> PR c++/81355 >>>> * g++.dg/other/pr81355.C: New test. >>> OK. THough one could legitimately ask if this ought to be a parsing error. >> >> I would suggest to at least issue a warning. It seems that >> the empty string must almost certainly be a bug here, say due >> to unintended macro expansion. >> >> Otherwise, if it should remain silently (and intentionally) >> accepted, I recommend to document it. As it is, the manual >> says that the "string" argument is equivalent to compiling >> with -mstring which for "" would be rejected by the driver. >> >> Martin > > Thanks you both for feedback. I decided to come up with an error message for that. > > Feedback appreciated. My only comment is on the text of the error message. I think the %' directive is supposed to be used instead of a bare apostrophe. But rather than using the somewhat informal "can't" I would suggest to follow other similar diagnostics that might print something like: empty string in attribute %<target%> (analogous to "empty precision in %s format" or "empty scalar initializer" etc. in gcc.pot) or attribute %<target%> argument may not be empty (similar to "output filename may not be empty"). Martin
From 7fd264acf60829b8ed4921c6659f01a87c944d44 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Wed, 12 Jul 2017 15:51:45 +0200 Subject: [PATCH] Fix target attribute handling (PR c++/81355). gcc/ChangeLog: 2017-07-13 Martin Liska <mliska@suse.cz> PR c++/81355 * config/i386/i386.c (ix86_valid_target_attribute_inner_p): Report error for an empty string argument of target attribute. gcc/testsuite/ChangeLog: 2017-07-13 Martin Liska <mliska@suse.cz> PR c++/81355 * g++.dg/other/pr81355.C: New test. --- gcc/config/i386/i386.c | 6 ++++++ gcc/testsuite/g++.dg/other/pr81355.C | 14 ++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 gcc/testsuite/g++.dg/other/pr81355.C diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 42e0ddaca56..62f08f9a80d 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -7140,6 +7140,12 @@ ix86_valid_target_attribute_inner_p (tree args, char *p_strings[], /* Handle multiple arguments separated by commas. */ next_optstr = ASTRDUP (TREE_STRING_POINTER (args)); + if (next_optstr && *next_optstr == '\0') + { + error ("attribute %<target%> argument can't be an empty string"); + return false; + } + while (next_optstr && *next_optstr != '\0') { char *p = next_optstr; diff --git a/gcc/testsuite/g++.dg/other/pr81355.C b/gcc/testsuite/g++.dg/other/pr81355.C new file mode 100644 index 00000000000..42b25b72611 --- /dev/null +++ b/gcc/testsuite/g++.dg/other/pr81355.C @@ -0,0 +1,14 @@ +/* { dg-do compile { target x86_64-*-* } } */ + +__attribute__((target("default"))) +int foo() {return 1;} + +__attribute__((target("arch=core2", ""))) +int foo2() {return 2;} /* { dg-error "attribute .target. argument can't be an empty string" } */ + +__attribute__((target("sse4.2", "", ""))) +int foo3() {return 2;} /* { dg-error "attribute .target. argument can't be an empty string" } */ + +int main() { + return foo() + foo2() + foo3(); +} -- 2.13.3