[Bug sanitizer/59061] New: Port leaksanitizer

[Bug sanitizer/59061] New: Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

            Bug ID: 59061
           Summary: Port leaksanitizer
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Joost.VandeVondele at mat dot ethz.ch
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org

Clearly an enhancement request, but it would be great to have leaksanitizer
ported to gcc. 

Will this make the 4.9 release ?

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Joost.VandeVondele at mat dot ethz
                   |                            |.ch
           Severity|normal                      |enhancement

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #1 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #2 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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 ?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #3 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
https://code.google.com/p/address-sanitizer/wiki/LeakSanitizer

I agree, we need to update the gcc doc to mention the URL above.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #4 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
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 ?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #5 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #6 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #7 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
> > 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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #8 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #9 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(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.?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #11 from Kostya Serebryany <kcc at gcc dot gnu.org> ---

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

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

Kostya Serebryany <kcc at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |samsonov at google dot com

--- Comment #13 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
> 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.

[Bug sanitizer/59061] Port leaksanitizer

[samsonov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #14 from Alexey Samsonov <samsonov at google dot com> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[earthdok at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #16 from Sergey Matveev <earthdok at google dot com> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #17 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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 ?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #18 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #19 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #20 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
> 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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #21 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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).

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #22 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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)

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 31221
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31221&action=edit
gcc49-pr59061.patch

Untested patch.

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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)?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #25 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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)

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug sanitizer/59061] Port leaksanitizer

[samsonov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #27 from Alexey Samsonov <samsonov at google dot com> ---
(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).

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #29 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
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
-------------------------------------------------------------------------------

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #30 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #32 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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)

[Bug sanitizer/59061] Port leaksanitizer

[earthdok at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #33 from Sergey Matveev <earthdok at google dot com> ---
(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?

[Bug sanitizer/59061] Port leaksanitizer

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #34 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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?

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #36 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(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?

[Bug sanitizer/59061] Port leaksanitizer

[earthdok at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #37 from Sergey Matveev <earthdok at google dot com> ---
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.

[Bug sanitizer/59061] Port leaksanitizer

[earthdok at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #38 from Sergey Matveev <earthdok at google dot com> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #39 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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!

[Bug sanitizer/59061] Port leaksanitizer

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #40 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
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)

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

--- Comment #41 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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.

[Bug sanitizer/59061] Port leaksanitizer

[Joost.VandeVondele at mat dot ethz.ch]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59061

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #42 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
(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!