Handle no_sanitize attribute values in the right way (PR sanitizer/85556).

Message ID 9beeb4c0-d059-ac6f-83a6-2e3669f34746@suse.cz
State New
Headers show
Series
  • Handle no_sanitize attribute values in the right way (PR sanitizer/85556).
Related show

Commit Message

Martin Liška May 10, 2018, 9:28 a.m.
Hi.

Parsing of no_sanitize attribute now supports
__attribute__((no_sanitize("address,undefined")))
which is wrong. And on the other hand this is not recognized:
__attribute__((no_sanitize("address", "undefined")))

Patch can bootstrap on x86_64-linux-gnu and survives regression tests. Then I would like
to backport that to GCC 8 branch.

Ready to be installed?
Martin

gcc/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

        PR sanitizer/85556
	* opts.c (parse_no_sanitize_attribute): Handle only a sinle
	option value.

gcc/c-family/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

        PR sanitizer/85556
	* c-attribs.c (handle_no_sanitize_attribute): Iterate all
	TREE_LIST values.

gcc/testsuite/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

        PR sanitizer/85556
	* c-c++-common/ubsan/attrib-6.c: New test.
---
 gcc/c-family/c-attribs.c                    | 20 +++++++++++--------
 gcc/opts.c                                  | 30 +++++++++++------------------
 gcc/testsuite/c-c++-common/ubsan/attrib-6.c | 22 +++++++++++++++++++++
 3 files changed, 45 insertions(+), 27 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/ubsan/attrib-6.c

Comments

Jakub Jelinek May 10, 2018, 9:45 a.m. | #1
On Thu, May 10, 2018 at 11:28:15AM +0200, Martin Liška wrote:
> Parsing of no_sanitize attribute now supports
> __attribute__((no_sanitize("address,undefined")))

Why is that wrong?  I don't see why we shouldn't support it that way.
It matches how we handle other similar attributes, say target attribute.

> which is wrong. And on the other hand this is not recognized:
> __attribute__((no_sanitize("address", "undefined")))

But we can certainly add support for this too for compatibility with clang.

	Jakub
Martin Liška May 10, 2018, 10:59 a.m. | #2
On 05/10/2018 11:45 AM, Jakub Jelinek wrote:
> On Thu, May 10, 2018 at 11:28:15AM +0200, Martin Liška wrote:
>> Parsing of no_sanitize attribute now supports
>> __attribute__((no_sanitize("address,undefined")))
> 
> Why is that wrong?  I don't see why we shouldn't support it that way.
> It matches how we handle other similar attributes, say target attribute.

Good, let's support both formats.

> 
>> which is wrong. And on the other hand this is not recognized:
>> __attribute__((no_sanitize("address", "undefined")))
> 
> But we can certainly add support for this too for compatibility with clang.
> 
> 	Jakub
> 

Done that in updated version of the patch. I've been running bootstrap and
tests.

Ready after it finishes?

Martin
From 9e2570eee9bb160b58075f6802d6ac1bb7b77341 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 10 May 2018 10:27:02 +0200
Subject: [PATCH] Support LLVM style of no_sanitize attribute (PR
 sanitizer/85556).

gcc/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

	* doc/extend.texi: Document LLVM style format for no_sanitize
	attribute.

gcc/c-family/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

        PR sanitizer/85556
	* c-attribs.c (handle_no_sanitize_attribute): Iterate all
	TREE_LIST values.

gcc/testsuite/ChangeLog:

2018-05-10  Martin Liska  <mliska@suse.cz>

        PR sanitizer/85556
	* c-c++-common/ubsan/attrib-6.c: New test.
---
 gcc/c-family/c-attribs.c                    | 20 ++++++++++++--------
 gcc/doc/extend.texi                         |  2 ++
 gcc/testsuite/c-c++-common/ubsan/attrib-6.c | 26 ++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/ubsan/attrib-6.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index e0630885cca..744315eec86 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -403,7 +403,7 @@ const struct attribute_spec c_common_attribute_table[] =
 			      0, 0, true, false, false, false,
 			      handle_no_address_safety_analysis_attribute,
 			      NULL },
-  { "no_sanitize",	      1, 1, true, false, false, false,
+  { "no_sanitize",	      1, -1, true, false, false, false,
 			      handle_no_sanitize_attribute, NULL },
   { "no_sanitize_address",    0, 0, true, false, false, false,
 			      handle_no_sanitize_address_attribute, NULL },
@@ -683,22 +683,26 @@ static tree
 handle_no_sanitize_attribute (tree *node, tree name, tree args, int,
 			      bool *no_add_attrs)
 {
+  unsigned int flags = 0;
   *no_add_attrs = true;
-  tree id = TREE_VALUE (args);
   if (TREE_CODE (*node) != FUNCTION_DECL)
     {
       warning (OPT_Wattributes, "%qE attribute ignored", name);
       return NULL_TREE;
     }
 
-  if (TREE_CODE (id) != STRING_CST)
+  for (; args; args = TREE_CHAIN (args))
     {
-      error ("no_sanitize argument not a string");
-      return NULL_TREE;
-    }
+      tree id = TREE_VALUE (args);
+      if (TREE_CODE (id) != STRING_CST)
+	{
+	  error ("no_sanitize argument not a string");
+	  return NULL_TREE;
+	}
 
-  char *string = ASTRDUP (TREE_STRING_POINTER (id));
-  unsigned int flags = parse_no_sanitize_attribute (string);
+      char *string = ASTRDUP (TREE_STRING_POINTER (id));
+      flags |= parse_no_sanitize_attribute (string);
+    }
 
   add_no_sanitize_value (*node, flags);
 
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 9d085844cfd..a4664cad819 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2977,6 +2977,8 @@ mentioned in @var{sanitize_option}.  A list of values acceptable by
 @smallexample
 void __attribute__ ((no_sanitize ("alignment", "object-size")))
 f () @{ /* @r{Do something.} */; @}
+void __attribute__ ((no_sanitize ("alignment,object-size")))
+g () @{ /* @r{Do something.} */; @}
 @end smallexample
 
 @item no_sanitize_address
diff --git a/gcc/testsuite/c-c++-common/ubsan/attrib-6.c b/gcc/testsuite/c-c++-common/ubsan/attrib-6.c
new file mode 100644
index 00000000000..2af70c8c2cf
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/attrib-6.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined" } */
+
+static void __attribute__((no_sanitize("foobar")))
+foo (void) { /* { dg-warning "attribute directive ignored" } */
+}
+
+static void __attribute__((no_sanitize("address,undefined")))
+foo2 (void) {
+}
+
+static void __attribute__((no_sanitize("address", "undefined")))
+foo3 (void) {
+}
+
+static void __attribute__((no_sanitize("address", "address", "")))
+foo4 (void) {
+}
+
+static void __attribute__((no_sanitize("address", "address", "address,address")))
+foo5 (void) {
+}
+
+static void __attribute__((no_sanitize("address", "address,kernel-address,thread,leak,undefined,vptr,shift,integer-divide-by-zero,unreachable,vla-bound,null,return,signed-integer-overflow,bounds,bounds-strict,alignment,object-size,float-divide-by-zero,float-cast-overflow,nonnull-attribute,returns-nonnull-attribute,bool,enum")))
+foo6 (void) {
+}
Jakub Jelinek May 10, 2018, 11:39 a.m. | #3
On Thu, May 10, 2018 at 12:59:39PM +0200, Martin Liška wrote:
> >From 9e2570eee9bb160b58075f6802d6ac1bb7b77341 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 10 May 2018 10:27:02 +0200
> Subject: [PATCH] Support LLVM style of no_sanitize attribute (PR
>  sanitizer/85556).
> 
> gcc/ChangeLog:
> 
> 2018-05-10  Martin Liska  <mliska@suse.cz>
> 
> 	* doc/extend.texi: Document LLVM style format for no_sanitize
> 	attribute.
> 
> gcc/c-family/ChangeLog:
> 
> 2018-05-10  Martin Liska  <mliska@suse.cz>
> 
>         PR sanitizer/85556
> 	* c-attribs.c (handle_no_sanitize_attribute): Iterate all
> 	TREE_LIST values.
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-05-10  Martin Liska  <mliska@suse.cz>
> 
>         PR sanitizer/85556
> 	* c-c++-common/ubsan/attrib-6.c: New test.

Ok, thanks.

	Jakub

Patch

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index e0630885cca..744315eec86 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -403,7 +403,7 @@  const struct attribute_spec c_common_attribute_table[] =
 			      0, 0, true, false, false, false,
 			      handle_no_address_safety_analysis_attribute,
 			      NULL },
-  { "no_sanitize",	      1, 1, true, false, false, false,
+  { "no_sanitize",	      1, -1, true, false, false, false,
 			      handle_no_sanitize_attribute, NULL },
   { "no_sanitize_address",    0, 0, true, false, false, false,
 			      handle_no_sanitize_address_attribute, NULL },
@@ -683,22 +683,26 @@  static tree
 handle_no_sanitize_attribute (tree *node, tree name, tree args, int,
 			      bool *no_add_attrs)
 {
+  unsigned int flags = 0;
   *no_add_attrs = true;
-  tree id = TREE_VALUE (args);
   if (TREE_CODE (*node) != FUNCTION_DECL)
     {
       warning (OPT_Wattributes, "%qE attribute ignored", name);
       return NULL_TREE;
     }
 
-  if (TREE_CODE (id) != STRING_CST)
+  for (; args; args = TREE_CHAIN (args))
     {
-      error ("no_sanitize argument not a string");
-      return NULL_TREE;
-    }
+      tree id = TREE_VALUE (args);
+      if (TREE_CODE (id) != STRING_CST)
+	{
+	  error ("no_sanitize argument not a string");
+	  return NULL_TREE;
+	}
 
-  char *string = ASTRDUP (TREE_STRING_POINTER (id));
-  unsigned int flags = parse_no_sanitize_attribute (string);
+      char *string = ASTRDUP (TREE_STRING_POINTER (id));
+      flags |= parse_no_sanitize_attribute (string);
+    }
 
   add_no_sanitize_value (*node, flags);
 
diff --git a/gcc/opts.c b/gcc/opts.c
index 33efcc0d6e7..f999dccd009 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -1748,33 +1748,25 @@  parse_sanitizer_options (const char *p, location_t loc, int scode,
   return flags;
 }
 
-/* Parse string values of no_sanitize attribute passed in VALUE.
-   Values are separated with comma.  */
+/* Parse string value of no_sanitize attribute passed in VALUE.  */
 
 unsigned int
 parse_no_sanitize_attribute (char *value)
 {
   unsigned int flags = 0;
   unsigned int i;
-  char *q = strtok (value, ",");
 
-  while (q != NULL)
-    {
-      for (i = 0; sanitizer_opts[i].name != NULL; ++i)
-	if (strcmp (sanitizer_opts[i].name, q) == 0)
-	  {
-	    flags |= sanitizer_opts[i].flag;
-	    if (sanitizer_opts[i].flag == SANITIZE_UNDEFINED)
-	      flags |= SANITIZE_UNDEFINED_NONDEFAULT;
-	    break;
-	  }
-
-      if (sanitizer_opts[i].name == NULL)
-	warning (OPT_Wattributes,
-		 "%<%s%> attribute directive ignored", q);
+  for (i = 0; sanitizer_opts[i].name != NULL; ++i)
+    if (strcmp (sanitizer_opts[i].name, value) == 0)
+      {
+	flags |= sanitizer_opts[i].flag;
+	if (sanitizer_opts[i].flag == SANITIZE_UNDEFINED)
+	  flags |= SANITIZE_UNDEFINED_NONDEFAULT;
+	break;
+      }
 
-      q = strtok (NULL, ",");
-    }
+  if (sanitizer_opts[i].name == NULL)
+    warning (OPT_Wattributes, "%<%s%> attribute directive ignored", value);
 
   return flags;
 }
diff --git a/gcc/testsuite/c-c++-common/ubsan/attrib-6.c b/gcc/testsuite/c-c++-common/ubsan/attrib-6.c
new file mode 100644
index 00000000000..ac08dc219ec
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ubsan/attrib-6.c
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined" } */
+
+static void __attribute__((no_sanitize("foobar")))
+foo (void) { /* { dg-warning "attribute directive ignored" } */
+}
+
+static void __attribute__((no_sanitize("address,undefined")))
+foo2 (void) { /* { dg-warning ".address,undefined. attribute directive ignored" } */
+}
+
+static void __attribute__((no_sanitize("address", "undefined")))
+foo3 (void) {
+}
+
+static void __attribute__((no_sanitize("address", "address", "")))
+foo4 (void) { /* { dg-warning ".. attribute directive ignored" } */
+}
+
+static void __attribute__((no_sanitize("address", "address", "address,address")))
+foo5 (void) { /* { dg-warning ".address,address. attribute directive ignored" } */
+}