| Message ID | e7055dbf-47ac-71fb-e30b-6721711d280b@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Do not error for no_sanitize attributes (PR sanitizer/82490). | expand |
On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote: > Hi. > > This changes error to a warning: > warning: ‘foobar’ attribute directive ignored [-Wattributes] > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. What is the rationale for not warning? LLVM compatibility? I think in some cases it would be nice to find out that I wrote a typo... > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c > @@ -0,0 +1,11 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=undefined" } */ > + > +__attribute__((no_sanitize(("foobar")))) Too many ()s around the no_sanitize argument. Jakub
On 2017.10.11 at 09:39 +0200, Jakub Jelinek wrote: > On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote: > > Hi. > > > > This changes error to a warning: > > warning: ‘foobar’ attribute directive ignored [-Wattributes] > > > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > What is the rationale for not warning? LLVM compatibility? > I think in some cases it would be nice to find out that I wrote a typo... The patch does warn. But yes, not erroring out improves LLVM compatibility. Chromium uses __attribute__(no_sanitize("function")) for example.
On Wed, Oct 11, 2017 at 09:45:27AM +0200, Markus Trippelsdorf wrote: > On 2017.10.11 at 09:39 +0200, Jakub Jelinek wrote: > > On Wed, Oct 11, 2017 at 08:24:28AM +0200, Martin Liška wrote: > > > Hi. > > > > > > This changes error to a warning: > > > warning: ‘foobar’ attribute directive ignored [-Wattributes] > > > > > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > > > What is the rationale for not warning? LLVM compatibility? > > I think in some cases it would be nice to find out that I wrote a typo... > > The patch does warn. But yes, not erroring out improves LLVM > compatibility. Chromium uses __attribute__(no_sanitize("function")) for > example. Ah, ok. The patch is ok with the nit testcase fixed. Jakub
diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c index 4e6754fba20..bd8ca306c2d 100644 --- a/gcc/c-family/c-attribs.c +++ b/gcc/c-family/c-attribs.c @@ -613,15 +613,8 @@ handle_no_sanitize_attribute (tree *node, tree name, tree args, int, return NULL_TREE; } - char *error_value = NULL; char *string = ASTRDUP (TREE_STRING_POINTER (id)); - unsigned int flags = parse_no_sanitize_attribute (string, &error_value); - - if (error_value) - { - error ("wrong argument: \"%s\"", error_value); - return NULL_TREE; - } + unsigned int flags = parse_no_sanitize_attribute (string); add_no_sanitize_value (*node, flags); diff --git a/gcc/opts.c b/gcc/opts.c index 5aa5d066dbe..adf3d89851d 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1700,11 +1700,10 @@ parse_sanitizer_options (const char *p, location_t loc, int scode, } /* Parse string values of no_sanitize attribute passed in VALUE. - Values are separated with comma. Wrong argument is stored to - WRONG_ARGUMENT variable. */ + Values are separated with comma. */ unsigned int -parse_no_sanitize_attribute (char *value, char **wrong_argument) +parse_no_sanitize_attribute (char *value) { unsigned int flags = 0; unsigned int i; @@ -1722,7 +1721,8 @@ parse_no_sanitize_attribute (char *value, char **wrong_argument) } if (sanitizer_opts[i].name == NULL) - *wrong_argument = q; + warning (OPT_Wattributes, + "%<%s%> attribute directive ignored", q); q = strtok (NULL, ","); } diff --git a/gcc/opts.h b/gcc/opts.h index 2774e2c8b40..10938615725 100644 --- a/gcc/opts.h +++ b/gcc/opts.h @@ -390,7 +390,7 @@ extern void handle_common_deferred_options (void); unsigned int parse_sanitizer_options (const char *, location_t, int, unsigned int, int, bool); -unsigned int parse_no_sanitize_attribute (char *value, char **wrong_argument); +unsigned int parse_no_sanitize_attribute (char *value); extern bool common_handle_option (struct gcc_options *opts, struct gcc_options *opts_set, const struct cl_decoded_option *decoded, diff --git a/gcc/testsuite/c-c++-common/ubsan/attrib-5.c b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c new file mode 100644 index 00000000000..1dfe50dd0b4 --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/attrib-5.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined" } */ + +__attribute__((no_sanitize(("foobar")))) +static void +float_cast2 (void) +{ /* { dg-warning "attribute directive ignored" } */ + volatile double d = 300; + volatile signed char c; + c = d; +}