* [PATCH] Optionally sanitize globals in user-defined sections @ 2015-04-17 7:32 Yury Gribov 2015-04-17 7:37 ` Yury Gribov 0 siblings, 1 reply; 11+ messages in thread From: Yury Gribov @ 2015-04-17 7:32 UTC (permalink / raw) To: GCC Patches; +Cc: Andrey Ryabinin, Jakub Jelinek, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-17 7:32 [PATCH] Optionally sanitize globals in user-defined sections Yury Gribov @ 2015-04-17 7:37 ` Yury Gribov 2015-04-17 7:41 ` Jakub Jelinek 2015-04-17 17:29 ` Andi Kleen 0 siblings, 2 replies; 11+ messages in thread From: Yury Gribov @ 2015-04-17 7:37 UTC (permalink / raw) To: GCC Patches; +Cc: Andrey Ryabinin, Jakub Jelinek, Dmitry Vyukov [-- 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" } } */ + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-17 7:37 ` Yury Gribov @ 2015-04-17 7:41 ` Jakub Jelinek 2015-04-17 17:29 ` Andi Kleen 1 sibling, 0 replies; 11+ messages in thread From: Jakub Jelinek @ 2015-04-17 7:41 UTC (permalink / raw) To: Yury Gribov; +Cc: GCC Patches, Andrey Ryabinin, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-17 7:37 ` Yury Gribov 2015-04-17 7:41 ` Jakub Jelinek @ 2015-04-17 17:29 ` Andi Kleen 2015-04-19 7:55 ` Yury Gribov 1 sibling, 1 reply; 11+ messages in thread From: Andi Kleen @ 2015-04-17 17:29 UTC (permalink / raw) To: Yury Gribov; +Cc: GCC Patches, Andrey Ryabinin, Jakub Jelinek, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-17 17:29 ` Andi Kleen @ 2015-04-19 7:55 ` Yury Gribov 2015-04-19 15:48 ` Jakub Jelinek 0 siblings, 1 reply; 11+ messages in thread From: Yury Gribov @ 2015-04-19 7:55 UTC (permalink / raw) To: Andi Kleen; +Cc: GCC Patches, Andrey Ryabinin, Jakub Jelinek, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-19 7:55 ` Yury Gribov @ 2015-04-19 15:48 ` Jakub Jelinek 2015-04-22 8:31 ` Yury Gribov 0 siblings, 1 reply; 11+ messages in thread From: Jakub Jelinek @ 2015-04-19 15:48 UTC (permalink / raw) To: Yury Gribov; +Cc: Andi Kleen, GCC Patches, Andrey Ryabinin, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-19 15:48 ` Jakub Jelinek @ 2015-04-22 8:31 ` Yury Gribov 2015-04-22 8:43 ` Yury Gribov 0 siblings, 1 reply; 11+ messages in thread From: Yury Gribov @ 2015-04-22 8:31 UTC (permalink / raw) To: GCC Patches Cc: Jakub Jelinek, Andi Kleen, Andrey Ryabinin, Dmitry Vyukov, Andi Kleen 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-22 8:31 ` Yury Gribov @ 2015-04-22 8:43 ` Yury Gribov 2015-04-22 9:00 ` Jakub Jelinek 0 siblings, 1 reply; 11+ messages in thread From: Yury Gribov @ 2015-04-22 8:43 UTC (permalink / raw) To: GCC Patches; +Cc: Jakub Jelinek, Andi Kleen, Andrey Ryabinin, Dmitry Vyukov [-- 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" } } */ + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-22 8:43 ` Yury Gribov @ 2015-04-22 9:00 ` Jakub Jelinek 2015-04-22 9:26 ` Yury Gribov 0 siblings, 1 reply; 11+ messages in thread From: Jakub Jelinek @ 2015-04-22 9:00 UTC (permalink / raw) To: Yury Gribov; +Cc: GCC Patches, Andi Kleen, Andrey Ryabinin, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-22 9:00 ` Jakub Jelinek @ 2015-04-22 9:26 ` Yury Gribov 2015-04-22 9:37 ` Jakub Jelinek 0 siblings, 1 reply; 11+ messages in thread From: Yury Gribov @ 2015-04-22 9:26 UTC (permalink / raw) To: Jakub Jelinek; +Cc: GCC Patches, Andi Kleen, Andrey Ryabinin, Dmitry Vyukov [-- 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" } } */ + ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Optionally sanitize globals in user-defined sections 2015-04-22 9:26 ` Yury Gribov @ 2015-04-22 9:37 ` Jakub Jelinek 0 siblings, 0 replies; 11+ messages in thread From: Jakub Jelinek @ 2015-04-22 9:37 UTC (permalink / raw) To: Yury Gribov; +Cc: GCC Patches, Andi Kleen, Andrey Ryabinin, Dmitry Vyukov 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-04-22 9:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-17 7:32 [PATCH] Optionally sanitize globals in user-defined sections Yury Gribov 2015-04-17 7:37 ` Yury Gribov 2015-04-17 7:41 ` Jakub Jelinek 2015-04-17 17:29 ` Andi Kleen 2015-04-19 7:55 ` Yury Gribov 2015-04-19 15:48 ` Jakub Jelinek 2015-04-22 8:31 ` Yury Gribov 2015-04-22 8:43 ` Yury Gribov 2015-04-22 9:00 ` Jakub Jelinek 2015-04-22 9:26 ` Yury Gribov 2015-04-22 9:37 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).