[PATCH] Optionally sanitize globals in user-defined sections

[PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

Hi all,

This patch adds an optional support for sanitizing user-defined 
sections.  Usually this is a bad idea because ASan changes relative 
position of variables in section thus breaking user assumptions.  But 
this is a desired feature for kernel which (ab)uses sections for various 
reasons (cache optimizations, etc.).

Bootstrapped and reg-tested on x64. Ok for trunk?

Best regards,
Yury

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

[-- Attachment #1: Type: text/plain, Size: 453 bytes --]

On 04/17/2015 10:33 AM, Yury Gribov wrote:
> Hi all,
>
> This patch adds an optional support for sanitizing user-defined
> sections.  Usually this is a bad idea because ASan changes relative
> position of variables in section thus breaking user assumptions.  But
> this is a desired feature for kernel which (ab)uses sections for various
> reasons (cache optimizations, etc.).
>
> Bootstrapped and reg-tested on x64. Ok for trunk?

The patch attached.


[-- Attachment #2: user-section-1.diff --]
[-- Type: text/x-patch, Size: 5543 bytes --]

commit 97c141d9be45b29fb7e281dc2b7cd904e93c2813
Author: Yury Gribov <y.gribov@samsung.com>
Date:   Mon Feb 2 14:33:17 2015 +0300

    2015-04-17  Yury Gribov  <y.gribov@samsung.com>
    
    gcc/
    	* asan.c (set_sanitized_sections): New function.
    	(section_sanitized_p): Ditto.
    	(asan_protect_global): Optionally sanitize user-defined
    	sections.
    	* asan.h (set_sanitized_sections): Declare new function.
    	* common.opt (fsanitize-sections): New option.
    	* doc/invoke.texi (-fsanitize-sections): Document new option.
    	* opts-global.c (handle_common_deferred_options): Handle new
    	option.
    
    gcc/testsuite/
    	* c-c++-common/asan/user-section-1.c: New test.

diff --git a/gcc/asan.c b/gcc/asan.c
index 9e4a629..6706af7 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -272,6 +272,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;
 
 /* Sets shadow offset to value in string VAL.  */
 
@@ -294,6 +295,33 @@ set_asan_shadow_offset (const char *val)
   return true;
 }
 
+/* Set list of user-defined sections that need to be sanitized.  */
+
+void
+set_sanitized_sections (const char *secs)
+{
+  sanitized_sections = secs;
+}
+
+/* Checks whether section SEC should be sanitized.  */
+
+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;
+    }
+  return false;
+}
+
 /* Returns Asan shadow offset.  */
 
 static unsigned HOST_WIDE_INT
@@ -1374,7 +1402,8 @@ asan_protect_global (tree decl)
 	 to be an array of such vars, putting padding in there
 	 breaks this assumption.  */
       || (DECL_SECTION_NAME (decl) != NULL
-	  && !symtab_node::get (decl)->implicit_section)
+	  && !symtab_node::get (decl)->implicit_section
+	  && !section_sanitized_p (DECL_SECTION_NAME (decl)))
       || DECL_SIZE (decl) == 0
       || ASAN_RED_ZONE_SIZE * BITS_PER_UNIT > MAX_OFILE_ALIGNMENT
       || !valid_constant_size_p (DECL_SIZE_UNIT (decl))
diff --git a/gcc/asan.h b/gcc/asan.h
index 51fd9cc..10ffaca 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -79,6 +79,8 @@ asan_red_zone_size (unsigned int size)
 
 extern bool set_asan_shadow_offset (const char *);
 
+extern void set_sanitized_sections (const char *);
+
 /* Return TRUE if builtin with given FCODE will be intercepted by
    libasan.  */
 
diff --git a/gcc/common.opt b/gcc/common.opt
index b49ac46..380848c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -897,6 +897,11 @@ fasan-shadow-offset=
 Common Joined RejectNegative Var(common_deferred_options) Defer
 -fasan-shadow-offset=<number>	Use custom shadow memory offset.
 
+fsanitize-sections=
+Common Joined RejectNegative Var(common_deferred_options) Defer
+-fsanitize-sections=<sec1,sec2,...>	Sanitize global variables
+in user-defined sections.
+
 fsanitize-recover=
 Common Report Joined
 After diagnosing undefined behavior attempt to continue execution
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bb17385..f5f79b8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -301,7 +301,8 @@ Objective-C and Objective-C++ Dialects}.
 @xref{Debugging Options,,Options for Debugging Your Program or GCC}.
 @gccoptlist{-d@var{letters}  -dumpspecs  -dumpmachine  -dumpversion @gol
 -fsanitize=@var{style} -fsanitize-recover -fsanitize-recover=@var{style} @gol
--fasan-shadow-offset=@var{number} -fsanitize-undefined-trap-on-error @gol
+-fasan-shadow-offset=@var{number} -fsanitize-sections=@var{s1,s2,...} @gol
+-fsanitize-undefined-trap-on-error @gol
 -fcheck-pointer-bounds -fchkp-check-incomplete-type @gol
 -fchkp-first-field-has-own-bounds -fchkp-narrow-bounds @gol
 -fchkp-narrow-to-innermost-array -fchkp-optimize @gol
@@ -5803,6 +5804,10 @@ This option forces GCC to use custom shadow offset in AddressSanitizer checks.
 It is useful for experimenting with different shadow memory layouts in
 Kernel AddressSanitizer.
 
+@item -fsanitize-sections=@var{s1,s2,...}
+@opindex fsanitize-sections
+Sanitize global variables in selected user-defined sections.
+
 @item -fsanitize-recover@r{[}=@var{opts}@r{]}
 @opindex fsanitize-recover
 @opindex fno-sanitize-recover
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index b61bdcf..1035b8d 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -458,6 +458,10 @@ handle_common_deferred_options (void)
 	     error ("unrecognized shadow offset %qs", opt->arg);
 	  break;
 
+	case OPT_fsanitize_sections_:
+	  set_sanitized_sections (opt->arg);
+	  break;
+
 	default:
 	  gcc_unreachable ();
 	}
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" } } */
+

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Jakub Jelinek]

On Fri, Apr 17, 2015 at 10:37:50AM +0300, Yury Gribov wrote:
> commit 97c141d9be45b29fb7e281dc2b7cd904e93c2813
> Author: Yury Gribov <y.gribov@samsung.com>
> Date:   Mon Feb 2 14:33:17 2015 +0300
> 
>     2015-04-17  Yury Gribov  <y.gribov@samsung.com>
>     
>     gcc/
>     	* asan.c (set_sanitized_sections): New function.
>     	(section_sanitized_p): Ditto.
>     	(asan_protect_global): Optionally sanitize user-defined
>     	sections.
>     	* asan.h (set_sanitized_sections): Declare new function.
>     	* common.opt (fsanitize-sections): New option.
>     	* doc/invoke.texi (-fsanitize-sections): Document new option.
>     	* opts-global.c (handle_common_deferred_options): Handle new
>     	option.
>     
>     gcc/testsuite/
>     	* c-c++-common/asan/user-section-1.c: New test.

Ok for trunk.

	Jakub

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Andi Kleen]

Yury Gribov <y.gribov@samsung.com> writes:
> +
> +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;

No wildcard support? That may be a long option in some cases.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

On 04/17/2015 08:29 PM, Andi Kleen wrote:
> Yury Gribov <y.gribov@samsung.com> writes:
>> +
>> +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;
>
> No wildcard support? That may be a long option in some cases.

Right. Do you think * will be enough or we also need ? and [a-f] syntax?

-Y

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Jakub Jelinek]

On Sun, Apr 19, 2015 at 10:54:57AM +0300, Yury Gribov wrote:
> On 04/17/2015 08:29 PM, Andi Kleen wrote:
> >Yury Gribov <y.gribov@samsung.com> writes:
> >>+
> >>+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;
> >
> >No wildcard support? That may be a long option in some cases.
> 
> Right. Do you think * will be enough or we also need ? and [a-f] syntax?

libiberty contains and gcc build utilities already use fnmatch, so you
should just use that (with carefully chosen FNM_* options).

	Jakub

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

On 04/19/2015 06:11 PM, Jakub Jelinek wrote:
> On Sun, Apr 19, 2015 at 10:54:57AM +0300, Yury Gribov wrote:
>> On 04/17/2015 08:29 PM, Andi Kleen wrote:
>>> Yury Gribov <y.gribov@samsung.com> writes:
>>>> +
>>>> +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;
>>>
>>> No wildcard support? That may be a long option in some cases.
>>
>> Right. Do you think * will be enough or we also need ? and [a-f] syntax?
>
> libiberty contains and gcc build utilities already use fnmatch, so you
> should just use that (with carefully chosen FNM_* options).

Hi all,

Here is an new patch which adds support for wildcards in 
-fsanitize-file:///home/ygribov/user-section-2.diff
sections.  This also adds a test which I forgot to svn-add last time 
(shame on me).

Bootstrapped and regtested on x64.  Ok to commit?

-Y

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]

On 04/22/2015 11:31 AM, Yury Gribov wrote:
> On 04/19/2015 06:11 PM, Jakub Jelinek wrote:
>> On Sun, Apr 19, 2015 at 10:54:57AM +0300, Yury Gribov wrote:
>>> On 04/17/2015 08:29 PM, Andi Kleen wrote:
>>>> Yury Gribov <y.gribov@samsung.com> writes:
>>>>> +
>>>>> +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;
>>>>
>>>> No wildcard support? That may be a long option in some cases.
>>>
>>> Right. Do you think * will be enough or we also need ? and [a-f] syntax?
>>
>> libiberty contains and gcc build utilities already use fnmatch, so you
>> should just use that (with carefully chosen FNM_* options).
>
> Hi all,
>
> Here is an new patch which adds support for wildcards in
> -fsanitize-sections.  This also adds a test which I forgot to svn-add last time
> (shame on me).
>
> Bootstrapped and regtested on x64.  Ok to commit?

Attached the patch.


[-- Attachment #2: user-section-2.diff --]
[-- Type: text/x-patch, Size: 5207 bytes --]

commit 0438afd83e555d1d484d2bef899125a8b9b4f10a
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..f7c595c 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 *, va_gc> *sanitized_sections;
 
 /* Sets shadow offset to value in string VAL.  */
 
@@ -298,9 +299,25 @@ 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;
+  for (unsigned i = 0;
+       sanitized_sections && sanitized_sections->iterate (i, &pat);
+       ++i)
+    {
+      free (pat);
+    }
+  vec_safe_truncate (sanitized_sections, 0);
+
+  for (const char *s = sections; *s; )
+    {
+      const char *end;
+      for (end = s; *end && *end != ','; ++end);
+      size_t len = end - s;
+      vec_safe_push (sanitized_sections, xstrndup (s, len));
+      s = *end ? end + 1 : end;
+    }
 }
 
 /* Checks whether section SEC should be sanitized.  */
@@ -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)
     {
-      if ((p == sanitized_sections || p[-1] == ',')
-	  && (p[len] == 0 || p[len] == ','))
+      if (fnmatch (pat, sec, FNM_PERIOD) == 0)
 	return true;
-      ++p;
     }
   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" } } */
+

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Jakub Jelinek]

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?

> -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.

> +    {
> +      free (pat);
> +    }

No {}s around single line body.

> @@ -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.  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.

	Jakub

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Yury Gribov]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

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

[-- Attachment #2: user-section-3.diff --]
[-- Type: text/x-patch, Size: 5104 bytes --]

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" } } */
+

Re: [PATCH] Optionally sanitize globals in user-defined sections

[Jakub Jelinek]

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