Clearly an enhancement request, but it would be great to have leaksanitizer ported to gcc. Will this make the 4.9 release ?
It should be there already: http://gcc.gnu.org/viewcvs?rev=204368&root=gcc&view=rev Please check. admittedly, the gcc tree lacks the tests for lsan.
(In reply to Kostya Serebryany from comment #1) > It should be there already: triggering my report was indeed some vague memory that the recent merge would bring leaksanitizer... I didn't see it mentioned on the list or doc changes or so. So, how does it work, is it part of -fsanitize=address (doesn't seem to do that) or is there a new option -fsanitize=leaks ?
https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer I agree, we need to update the gcc doc to mention the URL above.
That's great... works even for Fortran code :-) One more question the docs mention: In performance-critical scenarios, LSan can also be used without ASan instrumentation. But it is unclear from that description how that would work.. any hints ?
(In reply to Joost VandeVondele from comment #4) > That's great... works even for Fortran code :-) Wow! > > One more question the docs mention: > > In performance-critical scenarios, LSan can also be used without ASan > instrumentation. > > But it is unclear from that description how that would work.. any hints ? Clang supports -fsanitize=leak which simply links a standalone lsan library (no instrumentation at compile time required). Perhaps gcc can add such option too. But: we do not test -fsanitize=leak on anything other than tiny tests, so we truly support only the lsan+asan use case. We'll need to update the wiki page to have this all explained in detail, thanks for raising these questions.
(In reply to Kostya Serebryany from comment #5) > > That's great... works even for Fortran code :-) > Wow! well, many thanks to those people making sanitizer happen, also in gcc. > > In performance-critical scenarios, LSan can also be used without ASan > > instrumentation. > > > > But it is unclear from that description how that would work.. any hints ? > > Clang supports -fsanitize=leak which simply links a standalone lsan library > (no instrumentation at compile time required). > Perhaps gcc can add such option too. I agree that such an option would be useful (see also below). For now, I put '-fsanitize=address' for the link step only, which has the same effect. Additionally, it seems important to have -g -fno-omit-frame-pointer in the options. Maybe gcc should add these options for best 'user experience', for example as part of -fsanitize=address/leak. Power users can always override with -fsanitize=leak -fomit-frame-pointer. > But: we do not test -fsanitize=leak on anything other than tiny tests, > so we truly support only the lsan+asan use case. It would be great to have -fsanitize=leak robust. For our testing purposes we can not have the overhead of -fsanitize=address (which for Fortran is largely, but not completely, covered by -fcheck=bounds). Anyway, first test on our codebase (~1Mloc) suggests it works well (modulo PR59063). I'll add -fsanitize=x to my nightly gcc trunk tester. > We'll need to update the wiki page to have this all explained in detail, > thanks for raising these questions. One more example for the wiki, that's how we can run this with cp2k ;-) time ASAN_OPTIONS="detect_leaks=1" ../../exe/Linux-x86-64-leaks/cp2k.sdbg C.inp 2>&1 | python /data/vjoost/gnu/bugs/asan_symbolize.py
> > Clang supports -fsanitize=leak which simply links a standalone lsan library > > (no instrumentation at compile time required). > > Perhaps gcc can add such option too. > > I agree that such an option would be useful (see also below). I am not an expert in the gcc build system so this will have to be done by someone else. Also, I am heavily frightened by the amount of differences between the clang and gcc builds of libsanitizer. On my... The only good solution I see will hardly be accepted by the gcc community (which is: export compiler-rt as is, with all of its build and test system, anything else is going to increase the maintenance cost). > For now, I put '-fsanitize=address' for the link step only, which has the > same effect. It has a bit different effect because asan's allocator has greater memory footprint compared to a plain lsan's allocator. But -fsanitize=leak as a separate entity is not on top of our priorities. It works, but we don't test it on anything large. > > Additionally, it seems important to have -g -fno-omit-frame-pointer in the > options. Maybe gcc should add these options for best 'user experience', for > example as part of -fsanitize=address/leak. Power users can always override > with -fsanitize=leak -fomit-frame-pointer. -g: please no. -g incurs very large compile-time and binary size overhead. we need something like -gmlt (in google's gcc branch, not sure if it's available in trunk) or clang's -gline-tables-only. -fno-omit-frame-pointer: that may or may not be a good idea, I don't know. If we do any of these flags as default for -fsanitize=x, it would be nice to have the same defaults in clang.
(In reply to Kostya Serebryany from comment #7) > -fno-omit-frame-pointer: that may or may not be a good idea, I don't know. I seem to need it to get good backtraces (i.e. full stack, line numbers etc.). Might be something I'm doing wrong, it is hard to figure out what the best practice is. I agree that -g incurs some overhead, but indeed, some line number information is most helpful to debug the issues flagged by sanitizer. If there are better approaches, they could indeed substitute the plain '-g'. I hardly dare to mention it, but would be nice have these symbolic stacks printed without the symbolize python script that seems needed now (and I believe is not part of gcc). I believe in this context libbacktrace has been metioned.
(In reply to Joost VandeVondele from comment #8) > (In reply to Kostya Serebryany from comment #7) > > -fno-omit-frame-pointer: that may or may not be a good idea, I don't know. > > I seem to need it to get good backtraces (i.e. full stack, line numbers > etc.). Might be something I'm doing wrong, it is hard to figure out what the > best practice is. You do need -fno-omit-frame-pointer to get stack traces using our fast fp-based unwinder. The question is whether we want -fsanitize=x to enable -fno-omit-frame-pointer under the hood. My not-very-strong opinion is "no, make users read the documentation instead -- they have to do it anyway". > > I agree that -g incurs some overhead, but indeed, some line number > information is most helpful to debug the issues flagged by sanitizer. If > there are better approaches, they could indeed substitute the plain '-g'. Need to port -gmlt to gcc trunk. > > I hardly dare to mention it, but would be nice have these symbolic stacks > printed without the symbolize python script that seems needed now (and I > believe is not part of gcc). I believe in this context libbacktrace has been > metioned. We have a solution in the llvm-land: llvm-symbolizer https://code.google.com/p/address-sanitizer/wiki/CallStack You can build it (it's just a separate binary) and use it with gcc's asan. We have work-in-progress to make the symbolizer in-process as opposed to the current one that is out-of-process (uses pipes): https://code.google.com/p/address-sanitizer/source/browse/#svn%2Ftrunk%2Finternal_symbolizer
(In reply to Kostya Serebryany from comment #7) > > > Clang supports -fsanitize=leak which simply links a standalone lsan library > > > (no instrumentation at compile time required). > > > Perhaps gcc can add such option too. > > > > I agree that such an option would be useful (see also below). > > I am not an expert in the gcc build system so this will have to be done by > someone else. Also, I am heavily frightened by the amount of differences > between the clang and gcc builds of libsanitizer. On my... > The only good solution I see will hardly be accepted by the gcc community > (which is: export compiler-rt as is, with all of its build and test system, > anything else is going to increase the maintenance cost). No way, we really don't want to bring all those extra dependencies on prerequisites, completely different (and IMNSHO very ugly) build system etc. > > For now, I put '-fsanitize=address' for the link step only, which has the > > same effect. > > It has a bit different effect because asan's allocator has greater memory > footprint compared to a plain lsan's allocator. > But -fsanitize=leak as a separate entity is not on top of our priorities. > It works, but we don't test it on anything large. So, do you link for -fsanitize=address -lasan and for -fsanitize=leak -llsan instead? Easily doable of course, but we should create liblsan as shared library in that case too. What combination of those do you allow? I mean, is -fsanitize=address,leak allowed (and only links -lasan?), etc.?
> > I am not an expert in the gcc build system so this will have to be done by > > someone else. Also, I am heavily frightened by the amount of differences > > between the clang and gcc builds of libsanitizer. On my... > > The only good solution I see will hardly be accepted by the gcc community > > (which is: export compiler-rt as is, with all of its build and test system, > > anything else is going to increase the maintenance cost). > > No way, we really don't want to bring all those extra dependencies on > prerequisites, completely different (and IMNSHO very ugly) build system etc. I did not expect you to say "yes" :) Build system is a mess. But having two build systems is more mess. Also, the test system for libsanitizer in gcc is much weaker than in clang. We have no tests for tsan/lsan in gcc and don't have newer lit-style tests for asan (we tend to use more lit-style tests nowadays). This is not a huge issue since the sources are the same, but hidden build system differences may actually break things w/o us noticing. > > > > For now, I put '-fsanitize=address' for the link step only, which has the > > > same effect. > > > > It has a bit different effect because asan's allocator has greater memory > > footprint compared to a plain lsan's allocator. > > But -fsanitize=leak as a separate entity is not on top of our priorities. > > It works, but we don't test it on anything large. > > So, do you link for -fsanitize=address -lasan and for -fsanitize=leak -llsan > instead? Yes. > Easily doable of course, but we should create liblsan as shared > library in that case too. What combination of those do you allow? I mean, > is > -fsanitize=address,leak allowed (and only links -lasan?), etc.? Sergey, please answer here (and make sure this is documented)
(In reply to Kostya Serebryany from comment #9) > We have work-in-progress to make the symbolizer in-process as opposed to > the current one that is out-of-process (uses pipes): > https://code.google.com/p/address-sanitizer/source/browse/ > #svn%2Ftrunk%2Finternal_symbolizer Why don't you use libbacktrace for that? It is not GPL, so Apple and other GPL haters can't complain, maintained by Google, and IMHO it is far better to just use existing code base for that rather than writing yet another DWARF parser. Especially if you are writing it as part of llvm, it will unlikely handle all the DWARF GNU extensions needed to symbolize GCC code. Sure, there is work to be done on libbacktrace to handle some still unhandled extensions (e.g. DWZ produced extensions), but if you use libbacktrace, that can be done just in one spot, otherwise it will need to be written two times.
> Why don't you use libbacktrace for that? It is not GPL, so Apple and other I *think* we evaluated libbacktrace over 2 years ago and discarded for some technical reason. Or was this something else? Alexey? We may want to re-evaluate it, but OTOH llvm-symbolizer works fine for us already. The symbolizer is pluggable so we may use another one in gcc. > GPL haters can't complain, maintained by Google, and IMHO it is far better > to just use existing code base for that rather than writing yet another > DWARF parser. *We* are not writing yet another parser, we are reusing the code used by lldb. > Especially if you are writing it as part of llvm, it will unlikely handle > all the DWARF GNU extensions needed to symbolize GCC code. > Sure, there is work to be done on libbacktrace to handle some still > unhandled extensions (e.g. DWZ produced extensions), but if you use > libbacktrace, that can be done just in one spot, otherwise it will need to > be written two times.
(In reply to Kostya Serebryany from comment #13) > > Why don't you use libbacktrace for that? It is not GPL, so Apple and other > > I *think* we evaluated libbacktrace over 2 years ago and > discarded for some technical reason. Or was this something else? > Alexey? > We may want to re-evaluate it, but OTOH llvm-symbolizer works fine for us > already. I don't remember that, probably I should take a closer look at it. On a first glance, we'll still have to spend some effort to make it work fine with sanitizers - e.g. backtrace calls malloc, and likely other library functions. We must also ensure that it's able to unwind inlined frames, and debug info produced by -gline-tables-only/-gmlt. It works for ELF binaries only, while llvm-symbolizer supports Mach-O (and we use it on Mac). > > The symbolizer is pluggable so we may use another one in gcc. > > > > GPL haters can't complain, maintained by Google, and IMHO it is far better > > to just use existing code base for that rather than writing yet another > > DWARF parser. > > *We* are not writing yet another parser, we are reusing the code used by > lldb. Not really, we use a DWARF parser in LLVM, it's a fork of the one used in lldb. I think we'll be able to add support for certain DWARF GNU extensions to make it work with GCC-produced binaries. E.g. llvm-symbolizer already have basic support for DWAARF-5 Fission proposal. > > > Especially if you are writing it as part of llvm, it will unlikely handle > > all the DWARF GNU extensions needed to symbolize GCC code. > > Sure, there is work to be done on libbacktrace to handle some still > > unhandled extensions (e.g. DWZ produced extensions), but if you use > > libbacktrace, that can be done just in one spot, otherwise it will need to > > be written two times.
(In reply to Kostya Serebryany from comment #7) > > > > Additionally, it seems important to have -g -fno-omit-frame-pointer in the > > options. Maybe gcc should add these options for best 'user experience', for > > example as part of -fsanitize=address/leak. Power users can always override > > with -fsanitize=leak -fomit-frame-pointer. > > -g: please no. -g incurs very large compile-time and binary size overhead. > we need something like -gmlt (in google's gcc branch, not sure if it's > available in trunk) or clang's -gline-tables-only. -g1 should be the same as clang's -gline-tables-only now. no need for -gmlt.
(In reply to Kostya Serebryany from comment #11) > > Easily doable of course, but we should create liblsan as shared > > library in that case too. What combination of those do you allow? I mean, > > is > > -fsanitize=address,leak allowed (and only links -lasan?), etc.? > > Sergey, please answer here (and make sure this is documented) Under the current behavior -fsanitize=address,leak is equivalent to -fsanitize=address. We've considered other options, such as making -fsanitize=leak change the default run-time behavior (currently the ASan runtime always has leak detection runtime-disabled by default, whereas the standalone LSan runtime always has it enabled). But we never arrived at anything sensible.
(In reply to Sergey Matveev from comment #16) > > Under the current behavior -fsanitize=address,leak is equivalent to > -fsanitize=address. > > We've considered other options, such as making -fsanitize=leak change the > default run-time behavior (currently the ASan runtime always has leak > detection runtime-disabled by default, whereas the standalone LSan runtime > always has it enabled). But we never arrived at anything sensible. From my 'user perspective' it would be great if -fsanitize=leak would perform leak checking by default (i.e. remove the requirement to export ASAN_OPTIONS="detect_leaks=1"). At that point it would be natural to expect that -fsanitize=address,leak just enables the leak checking by default, while -fsanitize=address might not. Out of curiosity, is there a runtime performance difference in using -llsan or -lasan for leak checking only ?
(In reply to Joost VandeVondele from comment #17) > (In reply to Sergey Matveev from comment #16) > > > > Under the current behavior -fsanitize=address,leak is equivalent to > > -fsanitize=address. > > > > We've considered other options, such as making -fsanitize=leak change the > > default run-time behavior (currently the ASan runtime always has leak > > detection runtime-disabled by default, whereas the standalone LSan runtime > > always has it enabled). But we never arrived at anything sensible. > > From my 'user perspective' it would be great if -fsanitize=leak would > perform leak checking by default (i.e. remove the requirement to export > ASAN_OPTIONS="detect_leaks=1"). That's what -fsanitize=leak does in clang. > At that point it would be natural to expect that -fsanitize=address,leak > just enables the leak checking by default, while -fsanitize=address might > not. That's not so trivial to implement and I don't like anything non-trivial :) The trivial solution is to have lsan on by default in asan -- we are working on that. > > Out of curiosity, is there a runtime performance difference in using -llsan > or -lasan for leak checking only ? I don't think we've measured pure-lsan slowdown, but I expect it to be small. asan/lsan bring in a different allocator (malloc/free). We tried to make it very fast and our measurements show that's it is close to tcmalloc performance (but a bit more greedy in memory). It also performs stack unwind on every malloc, so on malloc-intensive apps you may see some small slowdown.
(In reply to Kostya Serebryany from comment #18) > I don't think we've measured pure-lsan slowdown, but I expect it to be small. > asan/lsan bring in a different allocator (malloc/free). > We tried to make it very fast and our measurements show that's it is close > to > tcmalloc performance (but a bit more greedy in memory). > It also performs stack unwind on every malloc, so on malloc-intensive apps > you may see some small slowdown. I our simulation code, it looks like the overhead for leak checking is about 20%. I haven't done very careful measurements yet, since this is more or less what we're willing to pay to integrate the (very useful) feature in our testing setup.
> I our simulation code, it looks like the overhead for leak checking is about > 20%. I haven't done very careful measurements yet, since this is more or > less what we're willing to pay to integrate the (very useful) feature in our > testing setup. that's with -fsanitize=address? as I said, asan allocator uses more memory (redzones, quarantine) and has extra overhead ([un]poisoning redzones, etc) compared to plain lsan. So 20% would be quite expected. Pure lsan should have smaller overhead. We are not actively testing pure lsan on large things because we are already testing all our large things with asan and we don't want yet another build config.
(In reply to Kostya Serebryany from comment #20) > > I our simulation code, it looks like the overhead for leak checking is about > > 20%. I haven't done very careful measurements yet, since this is more or > > less what we're willing to pay to integrate the (very useful) feature in our > > testing setup. > > that's with -fsanitize=address? No, full asan is about 100% (or more) overhead for us (I guess the overhead depends on the optimization level, but roughly speaking). This is only the leak checking (as obtained by only linking with -fsanitize=address and exporting the flag).
(In reply to Joost VandeVondele from comment #21) > (In reply to Kostya Serebryany from comment #20) > > > I our simulation code, it looks like the overhead for leak checking is about > > > 20%. I haven't done very careful measurements yet, since this is more or > > > less what we're willing to pay to integrate the (very useful) feature in our > > > testing setup. > > > > that's with -fsanitize=address? > > No, full asan is about 100% (or more) overhead for us (I guess the overhead > depends on the optimization level, but roughly speaking). This is only the > leak checking (as obtained by only linking with -fsanitize=address and > exporting the flag). That's what I meant. -fsanitize=address applied at compile time adds expensive instrumentation. -fsanitize=address applied at link time adds only expensive asan's allocator. -fsanitize=leak will add a cheaper allocator (but less tested)
Created attachment 31221 [details] gcc49-pr59061.patch Untested patch.
Just tried to bootstrap/regtest that patch, unfortunately it doesn't build at all on i686. Is it meant to work on x86_64 only (or only for SANITIZER_WORDSIZE == 64)?
(In reply to Jakub Jelinek from comment #24) > Just tried to bootstrap/regtest that patch, unfortunately it doesn't build > at all on i686. Is it meant to work on x86_64 only (or only for > SANITIZER_WORDSIZE == 64)? Yes, as of now lsan is supported only on x86_64. (heh, this time it is even properly documented. Good! https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer)
libbacktrace doesn't use malloc (unless mmap isn't supported), handles inline frames just fine and Ian has posted today a patch to support also data symbol lookups. I think I'll post a sanitizer_symbolizer_posix_libcdep.cc alternative on Monday that will use libbacktrace, then users can at least choose what they prefer, say at configure time.
(In reply to Jakub Jelinek from comment #26) > libbacktrace doesn't use malloc (unless mmap isn't supported), handles > inline frames just fine and Ian has posted today a patch to support also > data symbol lookups. I think I'll post a > sanitizer_symbolizer_posix_libcdep.cc alternative > on Monday that will use libbacktrace, then users can at least choose what > they prefer, say at configure time. Awesome if you want to give it a try :) Note that support for in-process symbolization is already there. You may simply link in the library with functions __sanitizer_symbolize_code and __sanitizer_symbolize_data, and these functions will be picked up by ASan runtime and used instead of forking llvm-symbolizer. See InternalSymbolizer class in sanitizer_symbolizer_posix_libcdep.cc. We use this behavior in our private setup (i.e. we link with hermetic library version of llvm-symbolizer tool, built by a huge and ugly script).
Author: jakub Date: Fri Nov 22 21:13:08 2013 New Revision: 205290 URL: http://gcc.gnu.org/viewcvs?rev=205290&root=gcc&view=rev Log: PR sanitizer/59061 * common.opt (static-liblsan): Add. * config/gnu-user.h (STATIC_LIBLSAN_LIBS, STATIC_LIBUBSAN_LIBS): Define. * flag-types.h (enum sanitize_code): Add SANITIZE_LEAK. Renumber SANITIZE_SHIFT, SANITIZE_DIVIDE, SANITIZE_UNREACHABLE, SANITIZE_VLA, SANITIZE_RETURN. * opts.c (common_handle_option): Handle -fsanitize=leak. * gcc.c (ADD_STATIC_LIBLSAN_LIBS, LIBLSAN_SPEC): Define. (LIBUBSAN_SPEC): Don't test LIBUBSAN_EARLY_SPEC. (LIBUBSAN_EARLY_SPEC): Remove. (SANITIZER_EARLY_SPEC): Don't do anything for libubsan. (SANITIZER_SPEC): Add -fsanitize=leak handling. (sanitize_spec_function): Handle %sanitize(leak). * doc/invoke.texi (-static-liblsan, -fsanitize=leak): Document. * c-c++-common/asan/no-redundant-instrumentation-7.c: Fix cleanup-tree-dump directive. * configure.tgt: Set LSAN_SUPPORTED=yes for x86_64-linux. * configure.ac (LSAN_SUPPORTED): New AM_CONDITIONAL. * configure: Regenerated. * lsan/Makefile.am (toolexeclib_LTLIBRARIES, lsan_files, liblsan_la_SOURCES, liblsan_la_LIBADD, liblsan_la_LDFLAGS): Add. * lsan/Makefile.in: Regenerated. Modified: trunk/gcc/ChangeLog trunk/gcc/common.opt trunk/gcc/config/gnu-user.h trunk/gcc/doc/invoke.texi trunk/gcc/flag-types.h trunk/gcc/gcc.c trunk/gcc/opts.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c trunk/libsanitizer/ChangeLog trunk/libsanitizer/configure trunk/libsanitizer/configure.ac trunk/libsanitizer/configure.tgt trunk/libsanitizer/lsan/Makefile.am trunk/libsanitizer/lsan/Makefile.in
I've tried -fsanitize=leak and it works well, thanks! Concerning the speed, I'm still seeing about 20% slowdown (again, this is acceptable from my point of view). Under oprofile, the __sanitizer::internal_memset is near the top (see below), it this expected for -fsanitize=leak ? Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (No unit mask) count 90000 samples % linenr info image name symbol name ------------------------------------------------------------------------------- 1 8.1e-04 (no location information) cp2k.sopt __input_cp2k_poisson_MOD_create_multipole_section 74 0.0596 (no location information) libc-2.12.so register_printf_type 124031 99.9396 crtstuff.c:0 liblsan.so.0.0.0 __sanitizer::internal_memset(void*, int, unsigned long) 124031 9.6999 crtstuff.c:0 liblsan.so.0.0.0 __sanitizer::internal_memset(void*, int, unsigned long) 124031 46.7440 crtstuff.c:0 liblsan.so.0.0.0 __sanitizer::internal_memset(void*, int, unsigned long) 124031 46.7440 crtstuff.c:0 liblsan.so.0.0.0 __sanitizer::internal_memset(void*, int, unsigned long) [self] 17279 6.5120 (no location information) no-vmlinux /no-vmlinux -------------------------------------------------------------------------------
(In reply to Joost VandeVondele from comment #29) > I've tried -fsanitize=leak and it works well, thanks! > > Concerning the speed, I'm still seeing about 20% slowdown (again, this is > acceptable from my point of view). Under oprofile, the > __sanitizer::internal_memset is near the top (see below), it this expected > for -fsanitize=leak ? > Good catch. That's what I meant when I said that we don't test lsan outside of asan. lsan's allocator clears all memory using internal_memset, which is damn slow. (sets on byte at a time) asan's allocator doesn't do that (it sets first 4K bytes of allocated region with garbage using the REAL fast memset) I think the right solution is to finally implement *fast* internal_memset. We'll do that. Sergey, can this difference between asan and lsan allocators cause false negatives/positives in lsan?
(In reply to Kostya Serebryany from comment #30) > lsan's allocator clears all memory using internal_memset, which is damn > slow. (sets on byte at a time) > > asan's allocator doesn't do that (it sets first 4K bytes of allocated region > with garbage using the REAL fast memset) > > I think the right solution is to finally implement *fast* internal_memset. > We'll do that. > > Sergey, can this difference between asan and lsan allocators cause > false negatives/positives in lsan? It can cause a false negative if there's a stale pointer outside of those 4k. But in practice 4k ought to be enough for anybody. I think standalone LSan should support the max_alloc_fill_size flag. I'll also change it to use real memset.
(In reply to Sergey Matveev from comment #31) > (In reply to Kostya Serebryany from comment #30) > > lsan's allocator clears all memory using internal_memset, which is damn > > slow. (sets on byte at a time) > > > > asan's allocator doesn't do that (it sets first 4K bytes of allocated region > > with garbage using the REAL fast memset) > > > > I think the right solution is to finally implement *fast* internal_memset. > > We'll do that. Some improvement made in http://llvm.org/viewvc/llvm-project?view=revision&revision=195549 Will reach gcc with the next merge. > > > > Sergey, can this difference between asan and lsan allocators cause > > false negatives/positives in lsan? > > It can cause a false negative if there's a stale pointer outside of those > 4k. But in practice 4k ought to be enough for anybody. lol :) > > I think standalone LSan should support the max_alloc_fill_size flag. Mmm. Maybe... max_alloc_fill_size in asan is there primarily to protect from filling huge allocations coming from LargeMmapAllocator >> I'll also change it to use real memset. Using REAL(memset) in sanitizer_common may not be a great idea. (first of all, you can't do it easily) Anyway, my change above improved the situation quite a bit (2x on a synthetic test)
(In reply to Kostya Serebryany from comment #32) > > I think standalone LSan should support the max_alloc_fill_size flag. > > Mmm. Maybe... > max_alloc_fill_size in asan is there primarily to protect from filling huge > allocations coming from LargeMmapAllocator Ok, if we can afford to clear all small chunks entirely, we don't need that flag since the large chunks are already cleared by mmap. Apropos, the common allocator currently clears memory even for large chunks, which needs to be fixed. > >> I'll also change it to use real memset. > > Using REAL(memset) in sanitizer_common may not be a great idea. > (first of all, you can't do it easily) What I meant is that LSan could clear the memory instead of relying on the sanitizer allocator to do it. Then we'd only have to call memset() from the LSan runtime. How soon will internal_memset() be on par with the library implementation?
And what is the reason why you want to duplicate the library optimization (which for memset/memcpy etc. is highly optimized, in glibc these days depends on the CPUs through ifunc etc.)? I mean, for small sizes you probably don't care and then doing it inline is fine, for larger sizes I think the interceptor has dlsym (RTLD_NEXT, "memcpy") saved somewhere anyway, so why don't you call that function pointer for say copying/clearing of say more than 64 bytes? lsan doesn't even intercept memset (or does it?), thus is there a reason why it just can't call memset directly?
> What I meant is that LSan could clear the memory instead of relying on the > sanitizer allocator to do it. Then we'd only have to call memset() from the > LSan runtime. Right, that's an option. (to call REAL(memset)) > How soon will internal_memset() be on par with the library > implementation? Dunno. maybe never.
(In reply to Jakub Jelinek from comment #34) > And what is the reason why you want to duplicate the library optimization > (which for memset/memcpy etc. is highly optimized, in glibc these days > depends on the CPUs through ifunc etc.)? We do use REAL(memset) (the one that comes from dlsym) in all performance-critical places in asan/tsan/msan. Can do it lsan too if there is still a need. Due to our tangled dependencies we can not easily call REAL(memset) from sanitizer_common (see my ugly workaround that allows to call REAL(pthread_attr_getstack) from sanitizer_common: http://llvm.org/viewvc/llvm-project?rev=195441&view=rev) However, we do need fast memset/memcpy of our own in one case: hybrid instrumentation (DRASan, MSanDR). When we combine static compiler instrumentation with DynamoRIO instrumentation to handle third-party libs we need to instrument libc's memset/memmove and so we need to use internal_mem* inside our run-time. This is hairy... > I mean, for small sizes you probably don't care and then doing it inline is > fine, for larger sizes I think the interceptor has dlsym (RTLD_NEXT, > "memcpy") saved somewhere anyway, so why don't you call that function > pointer for say copying/clearing of say more than 64 bytes? lsan doesn't > even intercept memset (or does it?), thus is there a reason why it just > can't call memset directly?
I've patched LSan to use the real memset(). At least on my machine this brought no performance improvement compared to kcc's latest change (just FYI - not trying to make any point). As of now, LSan will still clear out the entire small chunk (which can be up to 128KB in size), compared to a default of just the first 4KB in ASan. I'm undecided as to whether this is worth changing.
(In reply to Jakub Jelinek from comment #28) > Author: jakub > Date: Fri Nov 22 21:13:08 2013 > New Revision: 205290 It looks like you use dynamic linking by default. The last time I checked, leak detection (in both ASan and standalone LSan) didn't work 100% correctly with dynamic linking. Should be easy to fix by giving one of the interceptors a stronger visibility.
(In reply to Sergey Matveev from comment #37) > I've patched LSan to use the real memset(). At least on my machine this > brought no performance improvement compared to kcc's latest change (just FYI > - not trying to make any point). After the latest sanitizer merge, I see a significant reduction in the overhead of -fsanitize=leak (overhead went down from ~20% to <5%). Nice!
Thanks for the feedback! Is there anything else left in this bug? If not, please close this one and open another for the next problem(s)
(In reply to Kostya Serebryany from comment #40) > Is there anything else left in this bug? > If not, please close this one and open another for the next problem(s) I'll close it as soon as libbacktrace is used with sanitizer in trunk (see comment #12), but if Jakob feels this is covered by e.g. PR59136, this one can be closed, indeed.
(In reply to Joost VandeVondele from comment #41) > I'll close it as soon as libbacktrace is used with sanitizer in trunk (see > comment #12), but if Jakob feels this is covered by e.g. PR59136, this one > can be closed, indeed. I believe this now works nicely: > gfortran -g1 -fsanitize=leak test.f90 ; ./a.out ================================================================= ==55523==ERROR: LeakSanitizer: detected memory leaks Direct leak of 40 byte(s) in 1 object(s) allocated from: #0 0x7f5e57c63348 in __interceptor_malloc ../../../../gcc/libsanitizer/lsan/lsan_interceptors.cc:66 #1 0x400862 in s1 /data/vjoost/gnu/bugs/test.f90:3 #2 0x4008be in MAIN__ /data/vjoost/gnu/bugs/test.f90:6 #3 0x4008f4 in main /data/vjoost/gnu/bugs/test.f90:7 #4 0x305321ecdc in __libc_start_main (/lib64/libc.so.6+0x305321ecdc) SUMMARY: LeakSanitizer: 40 byte(s) leaked in 1 allocation(s) thanks!