Message ID | 572C7A3E.4000905@suse.cz |
---|---|
State | New |
Headers | show |
Hello. One more issue I forgot to mention in the previous email: e) As one can come up with a source code which jumps to a label within a block scope (use-after-scope-goto-1.c): // { dg-do run } // { dg-additional-options "-fsanitize=use-after-scope -fstack-reuse=none" } int main(int argc, char **argv) { int a = 123; if (argc == 0) { int *ptr; label: { ptr = &a; *ptr = 1; return 0; } } else goto label; return 0; } It's necessary to record all local variables in gimplifier and possibly emit unpoisoning code when a LABEL_EXPR is seen. That results in following gimple output: label: _20 = (unsigned long) &a; _21 = (unsigned long) 4; __builtin___asan_unpoison_stack_memory (_20, _21); _22 = (unsigned long) &ptr; _23 = (unsigned long) 8; __builtin___asan_unpoison_stack_memory (_22, _23); ptr = &a; ptr.0_10 = ptr; _24 = (unsigned long) ptr.0_10; _25 = _24 >> 3; _26 = _25 + 2147450880; _27 = (signed char *) _26; _28 = *_27; _29 = _28 != 0; _30 = _24 & 7; _31 = (signed char) _30; _32 = _31 + 3; _33 = _32 >= _28; _34 = _29 & _33; if (_34 != 0) goto <bb 5>; else goto <bb 6>; I know that the solution is a big hammer, but it works. Martin
On 05/06/2016 02:04 PM, Martin Liška wrote: > Hello. > > I've started working on the patch couple of month go, basically after > a brief discussion with Jakub on IRC. > > I'm sending the initial version which can successfully run instrumented > tramp3d, postgresql server and Inkscape. It catches the basic set of > examples which are added in following patch. > > The implementation is quite straightforward as works in following steps: > > 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission) > 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin) > and the variable is marked as addressable > 3) Similarly, BIND_EXPR is the place where we poison the variable (scope exit) > 4) At the very end of the function, we clean up the poisoned memory > 5) The builtins are expanded to call to libsanitizer run-time library (__asan_poison_stack_memory, __asan_unpoison_stack_memory) Can we inline these? > 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library Note that upstream seems to use a different cmdline interface. They don't have a dedicated -fsanitize=use-after-scope and instead consider it to be a part of -fsanitize=address (disabled by default, enabled via -mllvm -asan-use-after-scope=1). I'd suggest to keep this interface (or at least discuss with them) and use GCC's --param. FTR here's the upstream work on this: http://reviews.llvm.org/D19347 > Example: > > int > main (void) > { > char *ptr; > { > char my_char[9]; > ptr = &my_char[0]; > } > > *(ptr+9) = 'c'; > } > > ./a.out > ================================================================= > ==12811==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffec9bcff69 at pc 0x000000400a73 bp 0x7ffec9bcfef0 sp 0x7ffec9bcfee8 > WRITE of size 1 at 0x7ffec9bcff69 thread T0 > #0 0x400a72 in main (/tmp/a.out+0x400a72) > #1 0x7f100824860f in __libc_start_main (/lib64/libc.so.6+0x2060f) > #2 0x400868 in _start (/tmp/a.out+0x400868) > > Address 0x7ffec9bcff69 is located in stack of thread T0 at offset 105 in frame > #0 0x400945 in main (/tmp/a.out+0x400945) > > This frame has 2 object(s): > [32, 40) 'ptr' > [96, 105) 'my_char' <== Memory access at offset 105 overflows this variable > HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext > (longjmp and C++ exceptions *are* supported) > SUMMARY: AddressSanitizer: stack-use-after-scope (/tmp/a.out+0x400a72) in main > Shadow bytes around the buggy address: > 0x100059371f90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059371fa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059371fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059371fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059371fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > =>0x100059371fe0: f1 f1 f1 f1 00 f4 f4 f4 f2 f2 f2 f2 f8[f8]f4 f4 > 0x100059371ff0: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059372000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059372010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059372020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x100059372030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Heap right redzone: fb > Freed heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack partial redzone: f4 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user: f7 > Container overflow: fc > Array cookie: ac > Intra object redzone: bb > ASan internal: fe > Left alloca redzone: ca > Right alloca redzone: cb > ==12811==ABORTING > > As mentioned, it's request for comment as it still has couple of limitations: > a) VLA are not supported, which should make sense as we are unable to allocate a stack slot for that Note that we plan some work on VLA sanitization later this year (upstream ASan now sanitizes dynamic allocas and VLAs). > b) we can possibly strip some instrumentation in situations where a variable is introduced in a very first BB (RTL poisoning is superfluous). > Similarly for a very last BB of a function, we can strip end of scope poisoning (and RTL unpoisoning). I'll do that incrementally. > c) We require -fstack-reuse=none option, maybe it worth to warn a user if -fsanitize=use-after-scope is provided without the option? As a user, I'd prefer it to be automatically disabled when use-after-scope is on (unless it has been set explicitly in cmdline in which case we should probably issue error). > d) An instrumented binary is quite slow (~20x for tramp3d) as every function call produces many memory read/writes. I'm wondering whether > we should provide a faster alternative (like instrument just variables that have address taken) ? Can this due to -fstack-reuse or to outline poisoning? The latter sounds particularly heavy. > Patch can survive bootstrap and regression tests on x86_64-linux-gnu. That's bootstrap-asan? > Thanks for feedback. > Martin >
On Fri, May 06, 2016 at 01:04:30PM +0200, Martin Liška wrote: > I've started working on the patch couple of month go, basically after > a brief discussion with Jakub on IRC. > > I'm sending the initial version which can successfully run instrumented > tramp3d, postgresql server and Inkscape. It catches the basic set of > examples which are added in following patch. > > The implementation is quite straightforward as works in following steps: > > 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission) > 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin) > and the variable is marked as addressable Not all vars have DECL_EXPRs though. > 3) Similarly, BIND_EXPR is the place where we poison the variable (scope exit) > 4) At the very end of the function, we clean up the poisoned memory > 5) The builtins are expanded to call to libsanitizer run-time library (__asan_poison_stack_memory, __asan_unpoison_stack_memory) > 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library > As mentioned, it's request for comment as it still has couple of limitations: > a) VLA are not supported, which should make sense as we are unable to allocate a stack slot for that > b) we can possibly strip some instrumentation in situations where a variable is introduced in a very first BB (RTL poisoning is superfluous). > Similarly for a very last BB of a function, we can strip end of scope poisoning (and RTL unpoisoning). I'll do that incrementally. Yeah. > c) We require -fstack-reuse=none option, maybe it worth to warn a user if -fsanitize=use-after-scope is provided without the option? This should be implicitly set by -fsanitize=use-after-scope. Only if some other -fstack-reuse= option is explicitly set together with -fsanitize=use-after-scope, we should warn and reset it anyway. > d) An instrumented binary is quite slow (~20x for tramp3d) as every function call produces many memory read/writes. I'm wondering whether > we should provide a faster alternative (like instrument just variables that have address taken) ? I don't see any point in instrumenting !needs_to_live_in_memory vars, at least not those that don't need to live in memory at gimplification time. How could one use those after scope? They can't be accessed by dereferencing some pointer, and the symbol itself should be unavailable for symbol lookup after the symbol goes out of scope. Plus obviously ~20x slowdown isn't acceptable. Another thing is what to do with variables that are addressable at gimplification time, but generally are made non-addressable afterwards, such as due to optimizing away the taking of their address, inlining, etc. Perhaps depending on how big slowdown you get after just instrumenting needs_to_live_in_memory vars from ~ gimplification time and/or with the possible inlining of the poisoning/unpoisoning (again, should be another knob), at least with small sized vars, there might be another knob, which would tell if vars that are made non-addressable during optimizations later on should be instrumented or not. E.g. if you ASAN_MARK all needs_to_live_in_memory vars early, you could during the addressable determination when the knob says stuff should be faster, but less precise, ignore the vars that are addressable just because of the ASAN_MARK calls, and if they'd then turn to be non-addressable, remove the corresponding ASAN_MARK calls. > 2016-05-04 Martin Liska <mliska@suse.cz> > > * asan/asan_poisoning.cc: Do not call PoisonShadow in case > of zero size of aligned size. Generally, libsanitizer changes would need to go through upstream. > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see > #include "varasm.h" > #include "stor-layout.h" > #include "tree-iterator.h" > +#include "params.h" > #include "asan.h" > #include "dojump.h" > #include "explow.h" > @@ -54,7 +55,6 @@ along with GCC; see the file COPYING3. If not see > #include "cfgloop.h" > #include "gimple-builder.h" > #include "ubsan.h" > -#include "params.h" > #include "builtins.h" > #include "fnmatch.h" Why do you need to move params.h around? Does asan.h now depend on params.h? > + gimplify_seq_add_stmt > + (seq_p, gimple_build_call_internal (IFN_ASAN_MARK, 3, > + build_int_cst (integer_type_node, > + flags), > + base, unit_size)); Formatting, better introduce some temporary variables, like gimple *g = gimple_build_call_internal (...); gimplify_seq_add_stmt (seq_p, g); > --- a/gcc/tree-vect-patterns.c > +++ b/gcc/tree-vect-patterns.c > @@ -3570,7 +3570,8 @@ vect_recog_mask_conversion_pattern (vec<gimple *> *stmts, tree *type_in, > { > gimple *last_stmt = stmts->pop (); > enum tree_code rhs_code; > - tree lhs, rhs1, rhs2, tmp, rhs1_type, rhs2_type, vectype1, vectype2; > + tree lhs = NULL_TREE, rhs1, rhs2, tmp, rhs1_type, rhs2_type; > + tree vectype1, vectype2; > stmt_vec_info stmt_vinfo = vinfo_for_stmt (last_stmt); > stmt_vec_info pattern_stmt_info; > vec_info *vinfo = stmt_vinfo->vinfo; How is the above related to this patch? > +/* Return TRUE if we should instrument for use-after-scope sanity checking. */ > + > +static inline bool > +asan_sanitize_use_after_scope (void) > +{ > + return (flag_sanitize & SANITIZE_ADDRESS_USE_AFTER_SCOPE) > + == SANITIZE_ADDRESS_USE_AFTER_SCOPE > + && flag_stack_reuse == SR_NONE > + && ASAN_STACK; > +} Formatting, there should be ()s around the whole return expression as it spans multiple lines, and it should be indented properly. Plus IMHO flag_stack_reuse should be dealt with during option handling. Jakub
On Fri, May 06, 2016 at 02:48:30PM +0300, Yury Gribov wrote: > >6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library > > Note that upstream seems to use a different cmdline interface. They don't > have a dedicated -fsanitize=use-after-scope and instead consider it to be a > part of -fsanitize=address (disabled by default, enabled via -mllvm > -asan-use-after-scope=1). I'd suggest to keep this interface (or at least > discuss with them) and use GCC's --param. I personally think -fsanitize=use-after-scope (which implies address sanitization in it) is better, can upstream be convinved not to change it? > FTR here's the upstream work on this: http://reviews.llvm.org/D19347 > > >Example: > > > >int > >main (void) > >{ > > char *ptr; > > { > > char my_char[9]; > > ptr = &my_char[0]; > > } > > > > *(ptr+9) = 'c'; > >} Well, this testcase shows not just use after scope, but also out of bound access. Would be better not to combine it, at least in the majority of testcases. Jakub
On 05/06/2016 02:38 PM, Jakub Jelinek wrote: > On Fri, May 06, 2016 at 02:48:30PM +0300, Yury Gribov wrote: >>> 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library >> >> Note that upstream seems to use a different cmdline interface. They don't >> have a dedicated -fsanitize=use-after-scope and instead consider it to be a >> part of -fsanitize=address (disabled by default, enabled via -mllvm >> -asan-use-after-scope=1). I'd suggest to keep this interface (or at least >> discuss with them) and use GCC's --param. > > I personally think -fsanitize=use-after-scope (which implies address > sanitization in it) is better, can upstream be convinved not to change it? I also incline to the original -fsanitize=use-after-scope, which is compatible to remaining -fsanitize=... options we have in the GCC. > >> FTR here's the upstream work on this: http://reviews.llvm.org/D19347 >> >>> Example: >>> >>> int >>> main (void) >>> { >>> char *ptr; >>> { >>> char my_char[9]; >>> ptr = &my_char[0]; >>> } >>> >>> *(ptr+9) = 'c'; >>> } > > Well, this testcase shows not just use after scope, but also out of bound > access. Would be better not to combine it, at least in the majority of > testcases. Sure, that's a typo, should be: *(ptr+8) = 'c'; with: [96, 105) 'my_char' <== Memory access at offset 104 is inside this variable Intention was to touch the second shadow byte for the array. Martin > > Jakub >
On 05/06/2016 01:48 PM, Yury Gribov wrote: > On 05/06/2016 02:04 PM, Martin Liška wrote: >> Hello. >> >> I've started working on the patch couple of month go, basically after >> a brief discussion with Jakub on IRC. >> >> I'm sending the initial version which can successfully run instrumented >> tramp3d, postgresql server and Inkscape. It catches the basic set of >> examples which are added in following patch. >> >> The implementation is quite straightforward as works in following steps: >> >> 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission) >> 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin) >> and the variable is marked as addressable >> 3) Similarly, BIND_EXPR is the place where we poison the variable (scope exit) >> 4) At the very end of the function, we clean up the poisoned memory >> 5) The builtins are expanded to call to libsanitizer run-time library (__asan_poison_stack_memory, __asan_unpoison_stack_memory) > > Can we inline these? Currently not as libasan is a shared library that an instrumented executable is linked with. Possible solution would be to directly emit gimple instruction that would poison/unpoison the memory. But it's not a trivial job which is done in the poisoning code (ALWAYS_INLINE void FastPoisonShadow(uptr aligned_beg, uptr aligned_size, u8 value) > >> 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library > > Note that upstream seems to use a different cmdline interface. They don't have a dedicated -fsanitize=use-after-scope and instead consider it to be a part of -fsanitize=address (disabled by default, enabled via -mllvm -asan-use-after-scope=1). I'd suggest to keep this interface (or at least discuss with them) and use GCC's --param. > > FTR here's the upstream work on this: http://reviews.llvm.org/D19347 Thanks for the link, I will adapt part of the test to our test-suite. Some of them are really interesting. Martin > >> Example: >> >> int >> main (void) >> { >> char *ptr; >> { >> char my_char[9]; >> ptr = &my_char[0]; >> } >> >> *(ptr+9) = 'c'; >> } >> >> ./a.out >> ================================================================= >> ==12811==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffec9bcff69 at pc 0x000000400a73 bp 0x7ffec9bcfef0 sp 0x7ffec9bcfee8 >> WRITE of size 1 at 0x7ffec9bcff69 thread T0 >> #0 0x400a72 in main (/tmp/a.out+0x400a72) >> #1 0x7f100824860f in __libc_start_main (/lib64/libc.so.6+0x2060f) >> #2 0x400868 in _start (/tmp/a.out+0x400868) >> >> Address 0x7ffec9bcff69 is located in stack of thread T0 at offset 105 in frame >> #0 0x400945 in main (/tmp/a.out+0x400945) >> >> This frame has 2 object(s): >> [32, 40) 'ptr' >> [96, 105) 'my_char' <== Memory access at offset 105 overflows this variable >> HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext >> (longjmp and C++ exceptions *are* supported) >> SUMMARY: AddressSanitizer: stack-use-after-scope (/tmp/a.out+0x400a72) in main >> Shadow bytes around the buggy address: >> 0x100059371f90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059371fa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059371fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059371fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059371fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> =>0x100059371fe0: f1 f1 f1 f1 00 f4 f4 f4 f2 f2 f2 f2 f8[f8]f4 f4 >> 0x100059371ff0: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059372000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059372010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059372020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> 0x100059372030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> Shadow byte legend (one shadow byte represents 8 application bytes): >> Addressable: 00 >> Partially addressable: 01 02 03 04 05 06 07 >> Heap left redzone: fa >> Heap right redzone: fb >> Freed heap region: fd >> Stack left redzone: f1 >> Stack mid redzone: f2 >> Stack right redzone: f3 >> Stack partial redzone: f4 >> Stack after return: f5 >> Stack use after scope: f8 >> Global redzone: f9 >> Global init order: f6 >> Poisoned by user: f7 >> Container overflow: fc >> Array cookie: ac >> Intra object redzone: bb >> ASan internal: fe >> Left alloca redzone: ca >> Right alloca redzone: cb >> ==12811==ABORTING >> >> As mentioned, it's request for comment as it still has couple of limitations: >> a) VLA are not supported, which should make sense as we are unable to allocate a stack slot for that > > Note that we plan some work on VLA sanitization later this year (upstream ASan now sanitizes dynamic allocas and VLAs). > >> b) we can possibly strip some instrumentation in situations where a variable is introduced in a very first BB (RTL poisoning is superfluous). >> Similarly for a very last BB of a function, we can strip end of scope poisoning (and RTL unpoisoning). I'll do that incrementally. >> c) We require -fstack-reuse=none option, maybe it worth to warn a user if -fsanitize=use-after-scope is provided without the option? > > As a user, I'd prefer it to be automatically disabled when use-after-scope is on (unless it has been set explicitly in cmdline in which case we should probably issue error). > >> d) An instrumented binary is quite slow (~20x for tramp3d) as every function call produces many memory read/writes. I'm wondering whether >> we should provide a faster alternative (like instrument just variables that have address taken) ? > > Can this due to -fstack-reuse or to outline poisoning? The latter sounds particularly heavy. > >> Patch can survive bootstrap and regression tests on x86_64-linux-gnu. > > That's bootstrap-asan? > >> Thanks for feedback. >> Martin >> >
On Fri, May 06, 2016 at 03:17:23PM +0200, Martin Liška wrote: > On 05/06/2016 01:48 PM, Yury Gribov wrote: > > On 05/06/2016 02:04 PM, Martin Liška wrote: > >> I've started working on the patch couple of month go, basically after > >> a brief discussion with Jakub on IRC. > >> > >> I'm sending the initial version which can successfully run instrumented > >> tramp3d, postgresql server and Inkscape. It catches the basic set of > >> examples which are added in following patch. > >> > >> The implementation is quite straightforward as works in following steps: > >> > >> 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission) > >> 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin) > >> and the variable is marked as addressable > >> 3) Similarly, BIND_EXPR is the place where we poison the variable (scope exit) > >> 4) At the very end of the function, we clean up the poisoned memory > >> 5) The builtins are expanded to call to libsanitizer run-time library (__asan_poison_stack_memory, __asan_unpoison_stack_memory) > > > > Can we inline these? > > Currently not as libasan is a shared library that an instrumented executable is linked with. > Possible solution would be to directly emit gimple instruction that would poison/unpoison the memory. > But it's not a trivial job which is done in the poisoning code (ALWAYS_INLINE void FastPoisonShadow(uptr aligned_beg, uptr aligned_size, u8 value) Well, we already have the gimple poisoning/unpoisoning code on RTL (emitted after the prologue and before the epilogue), so it shouldn't be that hard. I'd only do the most common/easy cases inline though, like 1/2/4/8/16/32 bytes long variables. Jakub
On 05/06/2016 03:38 PM, Jakub Jelinek wrote: > On Fri, May 06, 2016 at 02:48:30PM +0300, Yury Gribov wrote: >>> 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library >> >> Note that upstream seems to use a different cmdline interface. They don't >> have a dedicated -fsanitize=use-after-scope and instead consider it to be a >> part of -fsanitize=address (disabled by default, enabled via -mllvm >> -asan-use-after-scope=1). I'd suggest to keep this interface (or at least >> discuss with them) and use GCC's --param. > > I personally think -fsanitize=use-after-scope (which implies address > sanitization in it) is better, can upstream be convinved not to change it? Will that work with -fsanitize=kernel-address? > >> FTR here's the upstream work on this: http://reviews.llvm.org/D19347 >> >>> Example: >>> >>> int >>> main (void) >>> { >>> char *ptr; >>> { >>> char my_char[9]; >>> ptr = &my_char[0]; >>> } >>> >>> *(ptr+9) = 'c'; >>> } > > Well, this testcase shows not just use after scope, but also out of bound > access. Would be better not to combine it, at least in the majority of > testcases. > > Jakub > >
On Fri, May 06, 2016 at 05:22:46PM +0300, Yury Gribov wrote: > On 05/06/2016 03:38 PM, Jakub Jelinek wrote: > >On Fri, May 06, 2016 at 02:48:30PM +0300, Yury Gribov wrote: > >>>6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library > >> > >>Note that upstream seems to use a different cmdline interface. They don't > >>have a dedicated -fsanitize=use-after-scope and instead consider it to be a > >>part of -fsanitize=address (disabled by default, enabled via -mllvm > >>-asan-use-after-scope=1). I'd suggest to keep this interface (or at least > >>discuss with them) and use GCC's --param. > > > >I personally think -fsanitize=use-after-scope (which implies address > >sanitization in it) is better, can upstream be convinved not to change it? > > Will that work with -fsanitize=kernel-address? Depends on how exactly it is defined. It could be enabling just its own sanitizer bit and nothing else, then users would need to use -fsanitize=address,use-after-scope or -fsanitize=kernel-address,use-after-scope (order doesn't matter), or it could enable the SANITIZE_ADDRESS bit together with its own, and then we'd just post-option processing (where we e.g. reject address,kernel-address) default to SANITIZE_USER_ADDRESS if SANITIZE_ADDRESS is on together with SANITIZE_USE_AFTER_SCOPE, but neither SANITIZE_{USER,KERNEL}_ADDRESS is defined. -fsanitize=address -fno-sanitize=use-after-scope obviously shouldn't in any case disable SANITIZE_ADDRESS, similarly -fsanitize=kernel-address -fno-sanitize=use-after-scope Jakub
On 05/06/2016 03:25 PM, Jakub Jelinek wrote: > Well, we already have the gimple poisoning/unpoisoning code on RTL (emitted > after the prologue and before the epilogue), so it shouldn't be that hard. > I'd only do the most common/easy cases inline though, like 1/2/4/8/16/32 > bytes long variables. > > Jakub You are right, I didn't realize it earlier. As I've collected statistics for tramp3d, poisoning code has following distribution: 4:1.62% 8:3.53% 12:94.76% which is quite interesting that 12B are such a common size :) Probably due to a lot of time spent in ::evaluate (MultiArgEvaluator and MultiArgEvaluator). Considering just variables which needs_to_live_in_memory, tramp3d is still ~15x slower. Anyway profile report tells: 26.51% a.out libasan.so.3.0.0 [.] __asan::PoisonShadow 18.49% a.out libasan.so.3.0.0 [.] PoisonAlignedStackMemory 5.61% a.out libc-2.22.so [.] __memset_avx2 5.41% a.out a.out [.] MultiArgEvaluator<RemoteMultiPatchEvaluatorTag>::evaluate<MultiArg2<...> 3.56% a.out libasan.so.3.0.0 [.] __asan_unpoison_stack_memory 2.69% a.out libasan.so.3.0.0 [.] __asan_poison_stack_memory I'll continue working on that after weekend. Martin
On Fri, May 06, 2016 at 04:41:41PM +0200, Martin Liška wrote: > On 05/06/2016 03:25 PM, Jakub Jelinek wrote: > > Well, we already have the gimple poisoning/unpoisoning code on RTL (emitted > > after the prologue and before the epilogue), so it shouldn't be that hard. > > I'd only do the most common/easy cases inline though, like 1/2/4/8/16/32 > > bytes long variables. > > > > Jakub > > You are right, I didn't realize it earlier. > As I've collected statistics for tramp3d, poisoning code has following distribution: > > 4:1.62% > 8:3.53% > 12:94.76% > > which is quite interesting that 12B are such a common size :) > Probably due to a lot of time spent in ::evaluate (MultiArgEvaluator and MultiArgEvaluator). > Considering just variables which needs_to_live_in_memory, tramp3d is still ~15x slower. Please look at other testcases, not just tramp3d - we in the end don't want to tune it to just tramp3d. Pick up some 3-4 C/C++ benchmarks, tramp3d can be one of them ;) Jakub
On 05/06/2016 04:39 PM, Jakub Jelinek wrote: > Depends on how exactly it is defined. It could be enabling just its own > sanitizer bit and nothing else, then users would need to use > -fsanitize=address,use-after-scope > or > -fsanitize=kernel-address,use-after-scope I'm inclined to the second option, where the new option would be automatically added if a ADDRESS sanitizer is enabled (SANITIZE_{USER,KERNEL}_ADDRESS): Is it acceptable behavior? > (order doesn't matter), or it could enable the SANITIZE_ADDRESS > bit together with its own, and then we'd just post-option processing > (where we e.g. reject address,kernel-address) default to > SANITIZE_USER_ADDRESS if SANITIZE_ADDRESS is on together with > SANITIZE_USE_AFTER_SCOPE, but neither SANITIZE_{USER,KERNEL}_ADDRESS > is defined. > -fsanitize=address -fno-sanitize=use-after-scope > obviously shouldn't in any case disable SANITIZE_ADDRESS, similarly > -fsanitize=kernel-address -fno-sanitize=use-after-scope > > Jakub
On Tue, May 10, 2016 at 05:03:30PM +0200, Martin Liška wrote: > On 05/06/2016 04:39 PM, Jakub Jelinek wrote: > > Depends on how exactly it is defined. It could be enabling just its own > > sanitizer bit and nothing else, then users would need to use > > -fsanitize=address,use-after-scope > > or > > -fsanitize=kernel-address,use-after-scope > > I'm inclined to the second option, where the new option would be automatically > added if a ADDRESS sanitizer is enabled (SANITIZE_{USER,KERNEL}_ADDRESS): > > Is it acceptable behavior? To me, yes. But, the question is if it is acceptable to clang too. Limiting it to a param means it will be command line option incompatible between clang and gcc. Jakub
From 242bcaf2fa7777ded33291d05a5c4c5306f849de Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Tue, 3 May 2016 15:35:22 +0200 Subject: [PATCH 1/2] Introduce -fsanitize=use-after-scope gcc/ChangeLog: 2016-05-03 Martin Liska <mliska@suse.cz> * asan.c (enum asan_check_flags): Cut the enum from here. (asan_poison_stack_variables): New function. (asan_emit_stack_protection): Poison stack variables. (asan_expand_mark_ifn): New function. * asan.h (enum asan_mark_flags): Paste here the enum from source file. (asan_sanitize_stack_p): Move function from cfgexpand. (asan_sanitize_use_after_scope): New function. (asan_no_sanitize_address_p): New function. * flag-types.h (enum sanitize_code): Introduce new enum value. * gimplify.c (asan_poison_variable): New function. (gimplify_bind_expr): Poison all local variables that go out of a scope. (gimplify_decl_expr): Unpoison a local vairable and mark the variable to live on a stack. (gimplify_expr): Unclobber all seen variables that can be poisoned due to a jump to a label. (gimplify_function_tree): * internal-fn.c (expand_ASAN_MARK): New function. * internal-fn.def: Define ASAN_MARK. * opts.c: Introduce use-after-scope as a new option value for -fsanitize. * sanitizer.def: Declare two new sanitizer builtins (__asan_poison_stack_memory, __asan_unpoison_stack_memory). * sanopt.c (pass_sanopt::execute): Call expansion of ASAN_MARKs. * tree-inline.c (setup_one_parameter): Record local variables introduced by inlining. (copy_decl_no_change): Likewise. * builtins.c: Include params.h header file. * gimplify.c: Include params.h header file. * opts-global.c: Include params.h header file. * sancov.c: Include params.h header file. * sanopt.c: Include params.h header file. * tree-streamer-in.c: Include params.h header file. * tsan.c: Include params.h header file. * ubsan.c: Include params.h header file. * varasm.c: Include params.h header file. * cfgexpand.c (asan_sanitize_stack_p): Use asan_no_sanitize_address_p function. libsanitizer/ChangeLog: 2016-05-04 Martin Liska <mliska@suse.cz> * asan/asan_poisoning.cc: Do not call PoisonShadow in case of zero size of aligned size. gcc/ChangeLog: 2016-05-04 Martin Liska <mliska@suse.cz> gcc/c-family/ChangeLog: 2016-05-04 Martin Liska <mliska@suse.cz> * c-ubsan.c: Include params.h header file. gcc/cp/ChangeLog: 2016-05-04 Martin Liska <mliska@suse.cz> * decl2.c: Include params.h header file. Partial revert + fixed. --- gcc/asan.c | 100 ++++++++++++++++++++++++++++++++---- gcc/asan.h | 41 +++++++++++++++ gcc/builtins.c | 1 + gcc/c-family/c-ubsan.c | 1 + gcc/cfgexpand.c | 3 +- gcc/cp/decl2.c | 1 + gcc/flag-types.h | 5 +- gcc/gimplify.c | 87 +++++++++++++++++++++++++++---- gcc/internal-fn.c | 9 ++++ gcc/internal-fn.def | 1 + gcc/opts-global.c | 1 + gcc/opts.c | 2 + gcc/sancov.c | 1 + gcc/sanitizer.def | 4 ++ gcc/sanopt.c | 5 +- gcc/tree-inline.c | 15 +++++- gcc/tree-streamer-in.c | 1 + gcc/tree-vect-patterns.c | 3 +- gcc/tsan.c | 1 + gcc/ubsan.c | 1 + gcc/varasm.c | 1 + libsanitizer/asan/asan_poisoning.cc | 5 +- 22 files changed, 260 insertions(+), 29 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index 71095fb..0c1b92a 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see #include "varasm.h" #include "stor-layout.h" #include "tree-iterator.h" +#include "params.h" #include "asan.h" #include "dojump.h" #include "explow.h" @@ -54,7 +55,6 @@ along with GCC; see the file COPYING3. If not see #include "cfgloop.h" #include "gimple-builder.h" #include "ubsan.h" -#include "params.h" #include "builtins.h" #include "fnmatch.h" @@ -313,6 +313,8 @@ asan_shadow_offset () alias_set_type asan_shadow_set = -1; +hash_set <tree> asan_inlined_variables; + /* Pointer types to 1 resp. 2 byte integers in shadow memory. A separate alias set is used for all shadow memory accesses. */ static GTY(()) tree shadow_ptr_types[2]; @@ -320,15 +322,6 @@ static GTY(()) tree shadow_ptr_types[2]; /* Decl for __asan_option_detect_stack_use_after_return. */ static GTY(()) tree asan_detect_stack_use_after_return; -/* Various flags for Asan builtins. */ -enum asan_check_flags -{ - ASAN_CHECK_STORE = 1 << 0, - ASAN_CHECK_SCALAR_ACCESS = 1 << 1, - ASAN_CHECK_NON_ZERO_LEN = 1 << 2, - ASAN_CHECK_LAST = 1 << 3 -}; - /* Hashtable support for memory references used by gimple statements. */ @@ -1020,6 +1013,45 @@ asan_function_start (void) current_function_funcdef_no); } +/* Depending on POISON flag, emit a call to poison (or unpoison) stack memory + allocated for local variables, localted in OFFSETS. LENGTH is number + of OFFSETS, BASE is the register holding the stack base, + against which OFFSETS array offsets are relative to. BASE_OFFSET represents + an offset requested by alignment and similar stuff. */ + +static void +asan_poison_stack_variables (rtx base, HOST_WIDE_INT base_offset, + HOST_WIDE_INT *offsets, int length, + tree *decls, bool poison) +{ + if (asan_sanitize_use_after_scope ()) + for (int l = length - 2; l > 0; l -= 2) + { + tree decl = decls[l / 2 - 1]; + if (asan_inlined_variables.contains (decl)) + { + gcc_assert (DECL_ABSTRACT_ORIGIN (decl)); + continue; + } + + HOST_WIDE_INT var_offset = offsets[l]; + HOST_WIDE_INT var_end_offset = offsets[l - 1]; + + rtx source = expand_binop (Pmode, add_optab, base, + gen_int_mode + (var_offset - base_offset, Pmode), + NULL_RTX, 1, OPTAB_DIRECT); + + rtx size = GEN_INT (var_end_offset - var_offset); + const char *fname = poison ? "__asan_poison_stack_memory" + :"__asan_unpoison_stack_memory"; + rtx ret = init_one_libfunc (fname); + emit_library_call (ret, LCT_NORMAL, VOIDmode, 2, source, ptr_mode, + size, TYPE_MODE (pointer_sized_int_node)); + } +} + + /* Insert code to protect stack vars. The prologue sequence should be emitted directly, epilogue sequence returned. BASE is the register holding the stack base, against which OFFSETS array offsets are relative to, OFFSETS @@ -1228,6 +1260,11 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, } cur_shadow_byte = ASAN_STACK_MAGIC_MIDDLE; } + + /* Poison stack variables at the very beginning of a function. */ + asan_poison_stack_variables (base, base_offset, offsets, length, + decls, true); + do_pending_stack_adjust (); /* Construct epilogue sequence. */ @@ -1309,6 +1346,11 @@ asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb, asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT); } + /* Unpoison stack variables at the end of a function. As the former + stack memory can be reused, we have to unpoison the memory. */ + asan_poison_stack_variables (base, base_offset, offsets, length, decls, + false); + do_pending_stack_adjust (); if (lab) emit_label (lab); @@ -2573,6 +2615,44 @@ asan_finish_file (void) flag_sanitize |= SANITIZE_ADDRESS; } +/* Expand the ASAN_MARK builtins. */ + +bool +asan_expand_mark_ifn (gimple_stmt_iterator *iter) +{ + gimple *g = gsi_stmt (*iter); + location_t loc = gimple_location (g); + HOST_WIDE_INT flags = tree_to_shwi (gimple_call_arg (g, 0)); + gcc_assert (flags < ASAN_MARK_LAST); + bool is_clobber = (flags & ASAN_MARK_CLOBBER) != 0; + + tree base = gimple_call_arg (g, 1); + tree len = gimple_call_arg (g, 2); + + HOST_WIDE_INT size_in_bytes + = tree_fits_shwi_p (len) ? tree_to_shwi (len) : -1; + gcc_assert (size_in_bytes); + + g = gimple_build_assign (make_ssa_name (pointer_sized_int_node), + NOP_EXPR, base); + gimple_set_location (g, loc); + gsi_insert_before (iter, g, GSI_SAME_STMT); + tree base_addr = gimple_assign_lhs (g); + + g = gimple_build_assign (make_ssa_name (pointer_sized_int_node), + NOP_EXPR, len); + gimple_set_location (g, loc); + gsi_insert_before (iter, g, GSI_SAME_STMT); + tree sz_arg = gimple_assign_lhs (g); + + tree fun = builtin_decl_implicit (is_clobber ? BUILT_IN_ASAN_CLOBBER_N + : BUILT_IN_ASAN_UNCLOBBER_N); + g = gimple_build_call (fun, 2, base_addr, sz_arg); + gimple_set_location (g, loc); + gsi_replace (iter, g, false); + return false; +} + /* Expand the ASAN_{LOAD,STORE} builtins. */ bool diff --git a/gcc/asan.h b/gcc/asan.h index 7ec693f..573af2f 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -29,6 +29,7 @@ extern bool asan_protect_global (tree); extern void initialize_sanitizer_builtins (void); extern tree asan_dynamic_init_call (bool); extern bool asan_expand_check_ifn (gimple_stmt_iterator *, bool); +extern bool asan_expand_mark_ifn (gimple_stmt_iterator *); extern gimple_stmt_iterator create_cond_insert_point (gimple_stmt_iterator *, bool, bool, bool, basic_block *, basic_block *); @@ -36,6 +37,10 @@ extern gimple_stmt_iterator create_cond_insert_point /* Alias set for accessing the shadow memory. */ extern alias_set_type asan_shadow_set; +/* Hash set of inlined variables that cannot be poisoned by use-after-cope + sanitizer. */ +extern hash_set <tree> asan_inlined_variables; + /* Shadow memory is found at (address >> ASAN_SHADOW_SHIFT) + asan_shadow_offset (). */ #define ASAN_SHADOW_SHIFT 3 @@ -59,6 +64,23 @@ extern alias_set_type asan_shadow_set; #define ASAN_STACK_FRAME_MAGIC 0x41b58ab3 #define ASAN_STACK_RETIRED_MAGIC 0x45e0360e +/* Various flags for Asan builtins. */ +enum asan_check_flags +{ + ASAN_CHECK_STORE = 1 << 0, + ASAN_CHECK_SCALAR_ACCESS = 1 << 1, + ASAN_CHECK_NON_ZERO_LEN = 1 << 2, + ASAN_CHECK_LAST = 1 << 3 +}; + +/* Flags for Asan check builtins. */ +enum asan_mark_flags +{ + ASAN_MARK_CLOBBER = 1 << 0, + ASAN_MARK_UNCLOBBER = 1 << 1, + ASAN_MARK_LAST = 1 << 2 +}; + /* Return true if DECL should be guarded on the stack. */ static inline bool @@ -105,4 +127,23 @@ asan_intercepted_p (enum built_in_function fcode) || fcode == BUILT_IN_STRNCMP || fcode == BUILT_IN_STRNCPY; } + +/* Return TRUE if we should instrument for use-after-scope sanity checking. */ + +static inline bool +asan_sanitize_use_after_scope (void) +{ + return (flag_sanitize & SANITIZE_ADDRESS_USE_AFTER_SCOPE) + == SANITIZE_ADDRESS_USE_AFTER_SCOPE + && flag_stack_reuse == SR_NONE + && ASAN_STACK; +} + +static inline bool +asan_no_sanitize_address_p (void) +{ + return lookup_attribute ("no_sanitize_address", + DECL_ATTRIBUTES (current_function_decl)); +} + #endif /* TREE_ASAN */ diff --git a/gcc/builtins.c b/gcc/builtins.c index 3d89baf..15b5553 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -58,6 +58,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "value-prof.h" #include "builtins.h" +#include "params.h" #include "asan.h" #include "cilk.h" #include "tree-chkp.h" diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c index 4022bdf..1a49e58 100644 --- a/gcc/c-family/c-ubsan.c +++ b/gcc/c-family/c-ubsan.c @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see #include "c-family/c-common.h" #include "ubsan.h" #include "c-family/c-ubsan.h" +#include "params.h" #include "asan.h" #include "stor-layout.h" #include "builtins.h" diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 21f21c9..1f77cfb 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -876,8 +876,7 @@ asan_sanitize_stack_p (void) { return ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK - && !lookup_attribute ("no_sanitize_address", - DECL_ATTRIBUTES (current_function_decl))); + && !asan_no_sanitize_address_p ()); } /* A subroutine of expand_used_vars. Binpack the variables into diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index 0ea326d..d341478 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -46,6 +46,7 @@ along with GCC; see the file COPYING3. If not see #include "dumpfile.h" #include "intl.h" #include "c-family/c-ada-spec.h" +#include "params.h" #include "asan.h" extern cpp_reader *parse_in; diff --git a/gcc/flag-types.h b/gcc/flag-types.h index 8201676..c7c69ba 100644 --- a/gcc/flag-types.h +++ b/gcc/flag-types.h @@ -253,6 +253,7 @@ enum sanitize_code { SANITIZE_OBJECT_SIZE = 1UL << 20, SANITIZE_VPTR = 1UL << 21, SANITIZE_BOUNDS_STRICT = 1UL << 22, + SANITIZE_USE_AFTER_SCOPE = 1UL << 23, SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN | SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM @@ -261,7 +262,9 @@ enum sanitize_code { | SANITIZE_RETURNS_NONNULL_ATTRIBUTE | SANITIZE_OBJECT_SIZE | SANITIZE_VPTR, SANITIZE_NONDEFAULT = SANITIZE_FLOAT_DIVIDE | SANITIZE_FLOAT_CAST - | SANITIZE_BOUNDS_STRICT + | SANITIZE_BOUNDS_STRICT, + SANITIZE_ADDRESS_USE_AFTER_SCOPE = SANITIZE_ADDRESS + | SANITIZE_USE_AFTER_SCOPE }; /* flag_vtable_verify initialization levels. */ diff --git a/gcc/gimplify.c b/gcc/gimplify.c index e223e59..305faaa 100644 --- a/gcc/gimplify.c +++ b/gcc/gimplify.c @@ -59,6 +59,11 @@ along with GCC; see the file COPYING3. If not see #include "gimple-walk.h" #include "langhooks-def.h" /* FIXME: for lhd_set_decl_assembler_name */ #include "builtins.h" +#include "params.h" +#include "asan.h" + +/* Hash set of poisoned variables in a bind expr. */ +static hash_set <tree> asan_poisoned_variables (13); enum gimplify_omp_var_data { @@ -1075,6 +1080,29 @@ build_stack_save_restore (gcall **save, gcall **restore) 1, tmp_var); } +/* Generate IFN_ASAN_MARK internal call that depending on POISON flag + either poisons or unpoisons a DECL. Created statement is appended + to SEQ_P gimple sequence. */ + +static void +asan_poison_variable (tree decl, bool poison, gimple_seq *seq_p) +{ + /* When within an OMP context, do not emit ASAN_MARK internal fns. */ + if (gimplify_omp_ctxp) + return; + + tree unit_size = DECL_SIZE_UNIT (decl); + tree base = build_fold_addr_expr (decl); + + HOST_WIDE_INT flags = poison ? ASAN_MARK_CLOBBER : ASAN_MARK_UNCLOBBER; + + gimplify_seq_add_stmt + (seq_p, gimple_build_call_internal (IFN_ASAN_MARK, 3, + build_int_cst (integer_type_node, + flags), + base, unit_size)); +} + /* Gimplify a BIND_EXPR. Just voidify and recurse. */ static enum gimplify_status @@ -1177,6 +1205,10 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) /* Add clobbers for all variables that go out of scope. */ for (t = BIND_EXPR_VARS (bind_expr); t ; t = DECL_CHAIN (t)) { + if (asan_sanitize_use_after_scope () + && asan_poisoned_variables.contains (t)) + asan_poisoned_variables.remove (t); + if (TREE_CODE (t) == VAR_DECL && !is_global_var (t) && DECL_CONTEXT (t) == current_function_decl @@ -1185,17 +1217,25 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p) && !DECL_HAS_VALUE_EXPR_P (t) /* Only care for variables that have to be in memory. Others will be rewritten into SSA names, hence moved to the top-level. */ - && !is_gimple_reg (t) - && flag_stack_reuse != SR_NONE) + && !is_gimple_reg (t)) { - tree clobber = build_constructor (TREE_TYPE (t), NULL); - gimple *clobber_stmt; - TREE_THIS_VOLATILE (clobber) = 1; - clobber_stmt = gimple_build_assign (t, clobber); - gimple_set_location (clobber_stmt, end_locus); - gimplify_seq_add_stmt (&cleanup, clobber_stmt); - - if (flag_openacc && oacc_declare_returns != NULL) + if (flag_stack_reuse != SR_NONE) + { + tree clobber = build_constructor (TREE_TYPE (t), NULL); + gimple *clobber_stmt; + TREE_THIS_VOLATILE (clobber) = 1; + clobber_stmt = gimple_build_assign (t, clobber); + gimple_set_location (clobber_stmt, end_locus); + gimplify_seq_add_stmt (&cleanup, clobber_stmt); + } + + if (asan_sanitize_use_after_scope () + && !asan_no_sanitize_address_p ()) + asan_poison_variable (t, true, &cleanup); + + if (flag_stack_reuse != SR_NONE + && flag_openacc + && oacc_declare_returns != NULL) { tree *c = oacc_declare_returns->get (t); if (c != NULL) @@ -1467,6 +1507,21 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p) STACK_CHECK_MAX_VAR_SIZE) > 0)) gimplify_vla_decl (decl, seq_p); + tree unit_size = DECL_SIZE_UNIT (decl); + if (asan_sanitize_use_after_scope () + && !asan_no_sanitize_address_p () + && TREE_CODE (unit_size) == INTEGER_CST + && DECL_NAME (decl) != NULL + && IDENTIFIER_POINTER (DECL_NAME (decl)) != NULL + && !TREE_STATIC (decl)) + { + TREE_ADDRESSABLE (decl) = 1; + DECL_GIMPLE_REG_P (decl) = 0; + + asan_poisoned_variables.add (decl); + asan_poison_variable (decl, false, seq_p); + } + /* Some front ends do not explicitly declare all anonymous artificial variables. We compensate here by declaring the variables, though it would be better if the front ends would @@ -10526,6 +10581,14 @@ gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p, == current_function_decl); gimplify_seq_add_stmt (pre_p, gimple_build_label (LABEL_EXPR_LABEL (*expr_p))); + + /* TODO: make a stable sorting of the hash table. */ + if (asan_sanitize_use_after_scope () + && !asan_no_sanitize_address_p ()) + for (hash_set <tree>::iterator it + = asan_poisoned_variables.begin (); + it != asan_poisoned_variables.end (); ++it) + asan_poison_variable (*it, false, pre_p); break; case CASE_LABEL_EXPR: @@ -11586,7 +11649,11 @@ gimplify_function_tree (tree fndecl) && !needs_to_live_in_memory (ret)) DECL_GIMPLE_REG_P (ret) = 1; + gcc_checking_assert (!asan_sanitize_use_after_scope () + || asan_poisoned_variables.elements () == 0); bind = gimplify_body (fndecl, true); + gcc_checking_assert (!asan_sanitize_use_after_scope () + || asan_poisoned_variables.elements () == 0); /* The tree body of the function is no longer needed, replace it with the new GIMPLE body. */ diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c index e70c73a..14fd4b3 100644 --- a/gcc/internal-fn.c +++ b/gcc/internal-fn.c @@ -234,6 +234,15 @@ expand_ASAN_CHECK (internal_fn, gcall *) gcc_unreachable (); } +/* This should get expanded in the sanopt pass. */ + +static void +expand_ASAN_MARK (internal_fn, gcall *) +{ + gcc_unreachable (); +} + + /* This should get expanded in the tsan pass. */ static void diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def index a62f3e8..48b8700 100644 --- a/gcc/internal-fn.def +++ b/gcc/internal-fn.def @@ -158,6 +158,7 @@ DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL) DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...") +DEF_INTERNAL_FN (ASAN_MARK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R..") DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) diff --git a/gcc/opts-global.c b/gcc/opts-global.c index b7e5232..963b542 100644 --- a/gcc/opts-global.c +++ b/gcc/opts-global.c @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see #include "plugin.h" #include "toplev.h" #include "context.h" +#include "params.h" #include "asan.h" typedef const char *const_char_p; /* For DEF_VEC_P. */ diff --git a/gcc/opts.c b/gcc/opts.c index 649b84b..40dcf3d 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1444,6 +1444,8 @@ const struct sanitizer_opts_s sanitizer_opts[] = { #define SANITIZER_OPT(name, flags) { #name, flags, sizeof #name - 1 } SANITIZER_OPT (address, SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS), + SANITIZER_OPT (use-after-scope, SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS + | SANITIZE_USE_AFTER_SCOPE), SANITIZER_OPT (kernel-address, SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS), SANITIZER_OPT (thread, SANITIZE_THREAD), SANITIZER_OPT (leak, SANITIZE_LEAK), diff --git a/gcc/sancov.c b/gcc/sancov.c index f3211dd..655ff64 100644 --- a/gcc/sancov.c +++ b/gcc/sancov.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-cfg.h" #include "tree-pass.h" #include "tree-iterator.h" +#include "params.h" #include "asan.h" namespace { diff --git a/gcc/sanitizer.def b/gcc/sanitizer.def index 303c1e4..1c142e9 100644 --- a/gcc/sanitizer.def +++ b/gcc/sanitizer.def @@ -165,6 +165,10 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_BEFORE_DYNAMIC_INIT, DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_AFTER_DYNAMIC_INIT, "__asan_after_dynamic_init", BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_CLOBBER_N, "__asan_poison_stack_memory", + BT_FN_VOID_PTR_PTRMODE, 0) +DEF_SANITIZER_BUILTIN(BUILT_IN_ASAN_UNCLOBBER_N, "__asan_unpoison_stack_memory", + BT_FN_VOID_PTR_PTRMODE, 0) /* Thread Sanitizer */ DEF_SANITIZER_BUILTIN(BUILT_IN_TSAN_INIT, "__tsan_init", diff --git a/gcc/sanopt.c b/gcc/sanopt.c index 2660453..43488a9 100644 --- a/gcc/sanopt.c +++ b/gcc/sanopt.c @@ -29,9 +29,9 @@ along with GCC; see the file COPYING3. If not see #include "gimple-pretty-print.h" #include "fold-const.h" #include "gimple-iterator.h" +#include "params.h" #include "asan.h" #include "ubsan.h" -#include "params.h" #include "tree-hash-traits.h" @@ -704,6 +704,9 @@ pass_sanopt::execute (function *fun) case IFN_ASAN_CHECK: no_next = asan_expand_check_ifn (&gsi, use_calls); break; + case IFN_ASAN_MARK: + no_next = asan_expand_mark_ifn (&gsi); + break; default: break; } diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 015a907..4b6d6e2 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -57,6 +57,8 @@ along with GCC; see the file COPYING3. If not see #include "cfgloop.h" #include "builtins.h" #include "tree-chkp.h" +#include "params.h" +#include "asan.h" /* I'm not real happy about this, but we need to handle gimple and @@ -3214,10 +3216,14 @@ setup_one_parameter (copy_body_data *id, tree p, tree value, tree fn, } } else - init_stmt = gimple_build_assign (var, rhs); + init_stmt = gimple_build_assign (var, rhs); + + /* Record an inlined variable if we sanitize for use-after-scope. */ + if (asan_sanitize_use_after_scope ()) + asan_inlined_variables.add (var); if (bb && init_stmt) - insert_init_stmt (id, bb, init_stmt); + insert_init_stmt (id, bb, init_stmt); } return init_stmt; } @@ -5439,6 +5445,11 @@ copy_decl_no_change (tree decl, copy_body_data *id) copy = copy_node (decl); + if (TREE_CODE (decl) == VAR_DECL + && asan_inlined_variables.contains (decl) + && asan_sanitize_use_after_scope ()) + asan_inlined_variables.add (copy); + /* The COPY is not abstract; it will be generated in DST_FN. */ DECL_ABSTRACT_P (copy) = false; lang_hooks.dup_lang_specific_decl (copy); diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c index 2ad2f92..99397eb 100644 --- a/gcc/tree-streamer-in.c +++ b/gcc/tree-streamer-in.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "builtins.h" #include "ipa-chkp.h" #include "gomp-constants.h" +#include "params.h" #include "asan.h" diff --git a/gcc/tree-vect-patterns.c b/gcc/tree-vect-patterns.c index 27080d2..14a6081 100644 --- a/gcc/tree-vect-patterns.c +++ b/gcc/tree-vect-patterns.c @@ -3570,7 +3570,8 @@ vect_recog_mask_conversion_pattern (vec<gimple *> *stmts, tree *type_in, { gimple *last_stmt = stmts->pop (); enum tree_code rhs_code; - tree lhs, rhs1, rhs2, tmp, rhs1_type, rhs2_type, vectype1, vectype2; + tree lhs = NULL_TREE, rhs1, rhs2, tmp, rhs1_type, rhs2_type; + tree vectype1, vectype2; stmt_vec_info stmt_vinfo = vinfo_for_stmt (last_stmt); stmt_vec_info pattern_stmt_info; vec_info *vinfo = stmt_vinfo->vinfo; diff --git a/gcc/tsan.c b/gcc/tsan.c index 47764bc..bdd002b 100644 --- a/gcc/tsan.c +++ b/gcc/tsan.c @@ -37,6 +37,7 @@ along with GCC; see the file COPYING3. If not see #include "tree-iterator.h" #include "tree-ssa-propagate.h" #include "tree-ssa-loop-ivopts.h" +#include "params.h" #include "tsan.h" #include "asan.h" #include "builtins.h" diff --git a/gcc/ubsan.c b/gcc/ubsan.c index 802341e..cd4953a 100644 --- a/gcc/ubsan.c +++ b/gcc/ubsan.c @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3. If not see #include "cfgloop.h" #include "ubsan.h" #include "expr.h" +#include "params.h" #include "asan.h" #include "gimplify-me.h" #include "dfp.h" diff --git a/gcc/varasm.c b/gcc/varasm.c index 4a7124e..47e5215 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see #include "langhooks.h" #include "debug.h" #include "common/common-target.h" +#include "params.h" #include "asan.h" #include "rtl-iter.h" diff --git a/libsanitizer/asan/asan_poisoning.cc b/libsanitizer/asan/asan_poisoning.cc index 39f7487..bdadf37 100644 --- a/libsanitizer/asan/asan_poisoning.cc +++ b/libsanitizer/asan/asan_poisoning.cc @@ -292,8 +292,9 @@ uptr __asan_load_cxx_array_cookie(uptr *p) { static void PoisonAlignedStackMemory(uptr addr, uptr size, bool do_poison) { if (size == 0) return; uptr aligned_size = size & ~(SHADOW_GRANULARITY - 1); - PoisonShadow(addr, aligned_size, - do_poison ? kAsanStackUseAfterScopeMagic : 0); + if (aligned_size != 0) + PoisonShadow(addr, aligned_size, + do_poison ? kAsanStackUseAfterScopeMagic : 0); if (size == aligned_size) return; s8 end_offset = (s8)(size - aligned_size); -- 2.8.1