Do not error for no_sanitize attributes (PR sanitizer/82490).

Message ID e7055dbf-47ac-71fb-e30b-6721711d280b@suse.cz
State New
Headers show
Series
  • Do not error for no_sanitize attributes (PR sanitizer/82490).
Related show

Commit Message

Martin Liška Oct. 11, 2017, 6:24 a.m.
Hi.

This changes error to a warning:
warning: ‘foobar’ attribute directive ignored [-Wattributes]

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-10-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/82490
	* opts.c (parse_no_sanitize_attribute): Do not use error_value
	variable.
	* opts.h (parse_no_sanitize_attribute): Remove last argument.

gcc/c-family/ChangeLog:

2017-10-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/82490
	* c-attribs.c (handle_no_sanitize_attribute): Report directly
	Wattributes warning.

gcc/testsuite/ChangeLog:

2017-10-10  Martin Liska  <mliska@suse.cz>

	PR sanitizer/82490
	* c-c++-common/ubsan/attrib-5.c: New test.
---
 gcc/c-family/c-attribs.c                    |  9 +--------
 gcc/opts.c                                  |  8 ++++----
 gcc/opts.h                                  |  2 +-
 gcc/testsuite/c-c++-common/ubsan/attrib-5.c | 11 +++++++++++
 4 files changed, 17 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/ubsan/attrib-5.c

Comments

Jakub Jelinek Oct. 11, 2017, 7:39 a.m. | #1
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
Markus Trippelsdorf Oct. 11, 2017, 7:45 a.m. | #2
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.
Jakub Jelinek Oct. 11, 2017, 7:54 a.m. | #3
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

Patch

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;
+}