diff mbox

Optionally sanitize globals in user-defined sections

Message ID 55376961.609@samsung.com
State New
Headers show

Commit Message

Yury Gribov April 22, 2015, 9:26 a.m. UTC 
On 04/22/2015 12:00 PM, Jakub Jelinek wrote:
> On Wed, Apr 22, 2015 at 11:43:53AM +0300, Yury Gribov wrote:
>> @@ -272,7 +273,7 @@ along with GCC; see the file COPYING3.  If not see
>>
>>   static unsigned HOST_WIDE_INT asan_shadow_offset_value;
>>   static bool asan_shadow_offset_computed;
>> -static const char *sanitized_sections;
>> +static vec<char *, va_gc> *sanitized_sections;
>
> Why don't you use static vec<char *> sanitized_section instead?

Fixed.  I thought we try to avoid creating unnecessary vectors but 
probably not that important.

>> -set_sanitized_sections (const char *secs)
>> +set_sanitized_sections (const char *sections)
>>   {
>> -  sanitized_sections = secs;
>> +  char *pat;
>> +  for (unsigned i = 0;
>> +       sanitized_sections && sanitized_sections->iterate (i, &pat);
>> +       ++i)
>
> This really should be FOR_EACH_VEC_SAFE_ELT (if you keep using va_gc
> vec *) or FOR_EACH_VEC_ELT.

Done.

>> +    {
>> +      free (pat);
>> +    }
>
> No {}s around single line body.

Done.

>> @@ -308,16 +325,13 @@ set_sanitized_sections (const char *secs)
>>   static bool
>>   section_sanitized_p (const char *sec)
>>   {
>> -  if (!sanitized_sections)
>> -    return false;
>> -  size_t len = strlen (sec);
>> -  const char *p = sanitized_sections;
>> -  while ((p = strstr (p, sec)))
>> +  char *pat;
>> +  for (unsigned i = 0;
>> +       sanitized_sections && sanitized_sections->iterate (i, &pat);
>> +       ++i)
>
> Similarly.

Ok.

> Also, wonder if won't be too expensive if people use too long
> list of sections.  Perhaps we could cache positive as well as negative
> answers in a hash table?  Though, perhaps it is worth that only if this
> shows up to be a bottleneck.

Yeah, I thought about throwing in a hashtable but wasn't sure that added 
complexity would be justified.  So I'd rather wait and see whether this 
causes a noticeable slowdown.

-Y

Comments

Jakub Jelinek April 22, 2015, 9:37 a.m. UTC | #1 
On Wed, Apr 22, 2015 at 12:26:57PM +0300, Yury Gribov wrote:
> On 04/22/2015 12:00 PM, Jakub Jelinek wrote:
> >On Wed, Apr 22, 2015 at 11:43:53AM +0300, Yury Gribov wrote:
> >>@@ -272,7 +273,7 @@ along with GCC; see the file COPYING3.  If not see
> >>
> >>  static unsigned HOST_WIDE_INT asan_shadow_offset_value;
> >>  static bool asan_shadow_offset_computed;
> >>-static const char *sanitized_sections;
> >>+static vec<char *, va_gc> *sanitized_sections;
> >
> >Why don't you use static vec<char *> sanitized_section instead?
> 
> Fixed.  I thought we try to avoid creating unnecessary vectors but probably
> not that important.

Sure, we try to avoid global var ctors/dtors and unnecessary unconditional
allocations.  But AFAIK vec<char *> (unlike auto_vec) doesn't have any user
ctor/dtor, it is a POD, and it is basically just
a struct containing that vec<char *, va_heap, vl_embed> * as a sole member,
and the zero initialization of global POD vars arranges for it to be
properly initialized.

>     2015-04-22  Yury Gribov  <y.gribov@samsung.com>
>     
>     	gcc/
>     	* asan.c (set_sanitized_sections): Parse incoming arg.
>     	(section_sanitized_p): Support wildcards.
>     	* doc/invoke.texi (-fsanitize-sections): Update description.
>     
>     	gcc/testsuite/
>     	* c-c++-common/asan/user-section-1.c: New test.
>     	* c-c++-common/asan/user-section-2.c: New test.
>     	* c-c++-common/asan/user-section-3.c: New test.

Ok, with minor nit.

> +      for (end = s; *end && *end != ','; ++end);

Please put ; on the following line, properly indented, so that
it is clear the for body is empty intentionally.

	Jakub
diff mbox

Patch

commit bc33a73d9406abf5209d98aba79eee33b14aadc6
Author: Yury Gribov <y.gribov@samsung.com>
Date:   Tue Apr 21 20:47:04 2015 +0300

    2015-04-22  Yury Gribov  <y.gribov@samsung.com>
    
    	gcc/
    	* asan.c (set_sanitized_sections): Parse incoming arg.
    	(section_sanitized_p): Support wildcards.
    	* doc/invoke.texi (-fsanitize-sections): Update description.
    
    	gcc/testsuite/
    	* c-c++-common/asan/user-section-1.c: New test.
    	* c-c++-common/asan/user-section-2.c: New test.
    	* c-c++-common/asan/user-section-3.c: New test.

diff --git a/gcc/asan.c b/gcc/asan.c
index cd6ccdc..479301a 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -88,6 +88,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "ubsan.h"
 #include "params.h"
 #include "builtins.h"
+#include "fnmatch.h"
 
 /* AddressSanitizer finds out-of-bounds and use-after-free bugs
    with <2x slowdown on average.
@@ -272,7 +273,7 @@  along with GCC; see the file COPYING3.  If not see
 
 static unsigned HOST_WIDE_INT asan_shadow_offset_value;
 static bool asan_shadow_offset_computed;
-static const char *sanitized_sections;
+static vec<char *> sanitized_sections;
 
 /* Sets shadow offset to value in string VAL.  */
 
@@ -298,9 +299,22 @@  set_asan_shadow_offset (const char *val)
 /* Set list of user-defined sections that need to be sanitized.  */
 
 void
-set_sanitized_sections (const char *secs)
+set_sanitized_sections (const char *sections)
 {
-  sanitized_sections = secs;
+  char *pat;
+  unsigned i;
+  FOR_EACH_VEC_ELT (sanitized_sections, i, pat)
+    free (pat);
+  sanitized_sections.truncate (0);
+
+  for (const char *s = sections; *s; )
+    {
+      const char *end;
+      for (end = s; *end && *end != ','; ++end);
+      size_t len = end - s;
+      sanitized_sections.safe_push (xstrndup (s, len));
+      s = *end ? end + 1 : end;
+    }
 }
 
 /* Checks whether section SEC should be sanitized.  */
@@ -308,17 +322,11 @@  set_sanitized_sections (const char *secs)
 static bool
 section_sanitized_p (const char *sec)
 {
-  if (!sanitized_sections)
-    return false;
-  size_t len = strlen (sec);
-  const char *p = sanitized_sections;
-  while ((p = strstr (p, sec)))
-    {
-      if ((p == sanitized_sections || p[-1] == ',')
-	  && (p[len] == 0 || p[len] == ','))
-	return true;
-      ++p;
-    }
+  char *pat;
+  unsigned i;
+  FOR_EACH_VEC_ELT (sanitized_sections, i, pat)
+    if (fnmatch (pat, sec, FNM_PERIOD) == 0)
+      return true;
   return false;
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c20dd4d..a939ff7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -5806,7 +5806,8 @@  Kernel AddressSanitizer.
 
 @item -fsanitize-sections=@var{s1,s2,...}
 @opindex fsanitize-sections
-Sanitize global variables in selected user-defined sections.
+Sanitize global variables in selected user-defined sections.  @var{si} may
+contain wildcards.
 
 @item -fsanitize-recover@r{[}=@var{opts}@r{]}
 @opindex fsanitize-recover
diff --git a/gcc/testsuite/c-c++-common/asan/user-section-1.c b/gcc/testsuite/c-c++-common/asan/user-section-1.c
new file mode 100644
index 0000000..51e2b99
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/user-section-1.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address -fsanitize-sections=.xxx,.yyy -fdump-tree-sanopt" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
+
+int x __attribute__((section(".xxx"))) = 1;
+int y __attribute__((section(".yyy"))) = 1;
+int z __attribute__((section(".zzz"))) = 1;
+
+/* { dg-final { scan-tree-dump "__builtin___asan_unregister_globals \\(.*, 2\\);" "sanopt" } } */
+/* { dg-final { cleanup-tree-dump "sanopt" } } */
+
diff --git a/gcc/testsuite/c-c++-common/asan/user-section-2.c b/gcc/testsuite/c-c++-common/asan/user-section-2.c
new file mode 100644
index 0000000..f602116
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/user-section-2.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address -fsanitize-sections=.x* -fdump-tree-sanopt" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
+
+int x __attribute__((section(".x1"))) = 1;
+int y __attribute__((section(".x2"))) = 1;
+int z __attribute__((section(".x3"))) = 1;
+
+/* { dg-final { scan-tree-dump "__builtin___asan_unregister_globals \\(.*, 3\\);" "sanopt" } } */
+/* { dg-final { cleanup-tree-dump "sanopt" } } */
+
diff --git a/gcc/testsuite/c-c++-common/asan/user-section-3.c b/gcc/testsuite/c-c++-common/asan/user-section-3.c
new file mode 100644
index 0000000..66e5f9a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/asan/user-section-3.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address -fsanitize-sections=.x* -fsanitize-sections=.y* -fdump-tree-sanopt" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } { "" } } */
+
+int x __attribute__((section(".x1"))) = 1;
+int y __attribute__((section(".x2"))) = 1;
+int z __attribute__((section(".y1"))) = 1;
+
+/* { dg-final { scan-tree-dump "__builtin___asan_unregister_globals \\(.*, 1\\);" "sanopt" } } */
+/* { dg-final { cleanup-tree-dump "sanopt" } } */
+