Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Constantine]

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

See this commit in cgit:

http://git.musl-libc.org/cgit/musl/commit/?id=01d4274

"make dlerror state and message thread-local and dynamically-allocated"


With this commit any application compiled with lsan yelds this error which
stops the application from running:

==15531==Sanitizer CHECK failed:
> /mnt/space/llvm/projects/compiler-rt/lib/lsan/lsan_interceptors.cc:58
> ((!lsan_init_is_running)) != (0) (0, 0)


But if I revert 01d4274
<https://github.com/procedural/musl-patches/blob/d9fab50/revert_01d4274.patch>
on musl 1.1.9 or higher versions, the error goes away and lsan works as
expected.

Just letting you guys know that this commit breaks external tools like lsan.

[-- Attachment #2: Type: text/html, Size: 1264 bytes --]

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Rich Felker]

On Thu, Jul 02, 2015 at 09:22:28PM +0300, Constantine wrote:
> See this commit in cgit:
> 
> http://git.musl-libc.org/cgit/musl/commit/?id=01d4274
> 
> "make dlerror state and message thread-local and dynamically-allocated"
> 
> 
> With this commit any application compiled with lsan yelds this error which
> stops the application from running:
> 
> ==15531==Sanitizer CHECK failed:
> > /mnt/space/llvm/projects/compiler-rt/lib/lsan/lsan_interceptors.cc:58
> > ((!lsan_init_is_running)) != (0) (0, 0)
> 
> 
> But if I revert 01d4274
> <https://github.com/procedural/musl-patches/blob/d9fab50/revert_01d4274.patch>
> on musl 1.1.9 or higher versions, the error goes away and lsan works as
> expected.
> 
> Just letting you guys know that this commit breaks external tools like lsan.

This happens for all programs, even when there are no dlopen/dlsym
errors? That seems really odd, since the changed code should be
unreachable in the absence of such errors. Does the error happen at
start, at exit, or some other time while the program is running?

Whatever the cause is, I'm pretty sure it's on the lsan side, and not
musl's fault. But it would be nice to get some more information so we
can check and maybe send a useful report to lsan for fixing whatever
is broken.

Rich

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2015-07-02 15:00:37 -0400]:
> On Thu, Jul 02, 2015 at 09:22:28PM +0300, Constantine wrote:
> > See this commit in cgit:
> > 
> > http://git.musl-libc.org/cgit/musl/commit/?id=01d4274
> > 
> > "make dlerror state and message thread-local and dynamically-allocated"
> > 
> > 
> > With this commit any application compiled with lsan yelds this error which
> > stops the application from running:
> > 
> > ==15531==Sanitizer CHECK failed:
> > > /mnt/space/llvm/projects/compiler-rt/lib/lsan/lsan_interceptors.cc:58
> > > ((!lsan_init_is_running)) != (0) (0, 0)
> > 
> > 
> > But if I revert 01d4274
> > <https://github.com/procedural/musl-patches/blob/d9fab50/revert_01d4274.patch>
> > on musl 1.1.9 or higher versions, the error goes away and lsan works as
> > expected.
> > 
> > Just letting you guys know that this commit breaks external tools like lsan.
> 
> This happens for all programs, even when there are no dlopen/dlsym
> errors? That seems really odd, since the changed code should be
> unreachable in the absence of such errors. Does the error happen at
> start, at exit, or some other time while the program is running?
> 
> Whatever the cause is, I'm pretty sure it's on the lsan side, and not
> musl's fault. But it would be nice to get some more information so we
> can check and maybe send a useful report to lsan for fixing whatever
> is broken.
> 

i think the cause is that they intercept all allocation functions
and if the act of interception calls into the intercepted function
itself then they have a problem

this happens on glibc with calloc and i think on musl with free
(at least that line is the interceptor for free and they do enough
shady things in their free wrapper to cause a libc internal free call)

of course all this is completely broken and not fixable on the musl side..

for glibc calloc they have some ugly workaround, maybe similar hack
is needed for free in musl, but none of this is future proof so
they will just keep piling hacks..

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Constantine]

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

> This happens for all programs, even when there are no dlopen/dlsym
errors?
> Does the error happen at
start, at exit, or some other time while the program is running?

At start, only for those programs which are compiled with Clang and
-fsanitize=leak flag.

> (at least that line is the interceptor for free and they do enough
shady things in their free wrapper to cause a libc internal free call)

Ah, you're right, they intercept free function there... But how
introduced change in that commit may affect it? I see only two `free` calls
in __dl_thread_cleanup...

> of course all this is completely broken and not fixable on the musl side..

Not completely! :) At least it works if that commit is reversed.

On Thu, Jul 2, 2015 at 10:29 PM, Szabolcs Nagy <nsz@port70.net> wrote:

> * Rich Felker <dalias@libc.org> [2015-07-02 15:00:37 -0400]:
> > On Thu, Jul 02, 2015 at 09:22:28PM +0300, Constantine wrote:
> > > See this commit in cgit:
> > >
> > > http://git.musl-libc.org/cgit/musl/commit/?id=01d4274
> > >
> > > "make dlerror state and message thread-local and dynamically-allocated"
> > >
> > >
> > > With this commit any application compiled with lsan yelds this error
> which
> > > stops the application from running:
> > >
> > > ==15531==Sanitizer CHECK failed:
> > > > /mnt/space/llvm/projects/compiler-rt/lib/lsan/lsan_interceptors.cc:58
> > > > ((!lsan_init_is_running)) != (0) (0, 0)
> > >
> > >
> > > But if I revert 01d4274
> > > <
> https://github.com/procedural/musl-patches/blob/d9fab50/revert_01d4274.patch
> >
> > > on musl 1.1.9 or higher versions, the error goes away and lsan works as
> > > expected.
> > >
> > > Just letting you guys know that this commit breaks external tools like
> lsan.
> >
> > This happens for all programs, even when there are no dlopen/dlsym
> > errors? That seems really odd, since the changed code should be
> > unreachable in the absence of such errors. Does the error happen at
> > start, at exit, or some other time while the program is running?
> >
> > Whatever the cause is, I'm pretty sure it's on the lsan side, and not
> > musl's fault. But it would be nice to get some more information so we
> > can check and maybe send a useful report to lsan for fixing whatever
> > is broken.
> >
>
> i think the cause is that they intercept all allocation functions
> and if the act of interception calls into the intercepted function
> itself then they have a problem
>
> this happens on glibc with calloc and i think on musl with free
> (at least that line is the interceptor for free and they do enough
> shady things in their free wrapper to cause a libc internal free call)
>
> of course all this is completely broken and not fixable on the musl side..
>
> for glibc calloc they have some ugly workaround, maybe similar hack
> is needed for free in musl, but none of this is future proof so
> they will just keep piling hacks..
>
>


-- 
*コンスタンティン*
*・タラセンコフ*https://procedural.github.io

[-- Attachment #2: Type: text/html, Size: 5411 bytes --]

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Szabolcs Nagy]

* Constantine <iamvfx@gmail.com> [2015-07-02 22:45:11 +0300]:
> > This happens for all programs, even when there are no dlopen/dlsym
> errors?
> > Does the error happen at
> start, at exit, or some other time while the program is running?
> 
> At start, only for those programs which are compiled with Clang and
> -fsanitize=leak flag.
> 
> > (at least that line is the interceptor for free and they do enough
> shady things in their free wrapper to cause a libc internal free call)
> 
> Ah, you're right, they intercept free function there... But how
> introduced change in that commit may affect it? I see only two `free` calls
> in __dl_thread_cleanup...
> 

the lsan alloc symbols are statically linked into the application so
they override the extern calls in shared objects eg in libstdc++.so

if you linked a c++ application and if the lsan initialization somehow
called into libstdc++.so (it is not supposed to, but lsan is c++ code
so you can never know) and free is called there then you'd get the
observed behaviour. (this scenario has a few ifs and does not explain
why the specific commit broke it, but i dont see other way now)

it would be easier to know what happens if you put a breakpoint
on __sanitizer::CheckFailed and printed a backtrace

(or replace that code with __builtin_trap and rebuid compiler_rt
that way to get a proper core dump)

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2015-07-02 21:29:16 +0200]:
> * Rich Felker <dalias@libc.org> [2015-07-02 15:00:37 -0400]:
> > On Thu, Jul 02, 2015 at 09:22:28PM +0300, Constantine wrote:
> > > See this commit in cgit:
> > > 
> > > http://git.musl-libc.org/cgit/musl/commit/?id=01d4274
> > > 
> > > "make dlerror state and message thread-local and dynamically-allocated"
> > > 
> > > 
> > > With this commit any application compiled with lsan yelds this error which
> > > stops the application from running:
> > > 
> > > ==15531==Sanitizer CHECK failed:
> > > > /mnt/space/llvm/projects/compiler-rt/lib/lsan/lsan_interceptors.cc:58
> > > > ((!lsan_init_is_running)) != (0) (0, 0)
> > > 
> > > 
> > > But if I revert 01d4274
> > > <https://github.com/procedural/musl-patches/blob/d9fab50/revert_01d4274.patch>
> > > on musl 1.1.9 or higher versions, the error goes away and lsan works as
> > > expected.
> > > 
> > > Just letting you guys know that this commit breaks external tools like lsan.
> > 
> > This happens for all programs, even when there are no dlopen/dlsym
> > errors? That seems really odd, since the changed code should be
> > unreachable in the absence of such errors. Does the error happen at
> > start, at exit, or some other time while the program is running?
> > 
> > Whatever the cause is, I'm pretty sure it's on the lsan side, and not
> > musl's fault. But it would be nice to get some more information so we
> > can check and maybe send a useful report to lsan for fixing whatever
> > is broken.
> > 
> 
> i think the cause is that they intercept all allocation functions
> and if the act of interception calls into the intercepted function
> itself then they have a problem
> 
> this happens on glibc with calloc and i think on musl with free
> (at least that line is the interceptor for free and they do enough
> shady things in their free wrapper to cause a libc internal free call)
> 
> of course all this is completely broken and not fixable on the musl side..
> 
> for glibc calloc they have some ugly workaround, maybe similar hack
> is needed for free in musl, but none of this is future proof so
> they will just keep piling hacks..

hm wait a minute, interception was not supposed to work with musl
it still uses -Bsymbolic-functions so my analysis cant be correct

i dont know why it worked before and why that check fails now

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Alexander Monakov]

> i think the cause is that they intercept all allocation functions
> and if the act of interception calls into the intercepted function
> itself then they have a problem

This should never happen on musl since it links with -Bsymbolic-functions, and
even uses protected visibility now.
 
> for glibc calloc they have some ugly workaround, maybe similar hack
> is needed for free in musl, but none of this is future proof so
> they will just keep piling hacks..

Re: hacks, I think it doesn't have to be like that.  There should be a
reasonable mechanism to wrap library functions in the context of dynamic
linking; static linking is slightly ahead in that game if you count ld
--wrap=symbol functionality.

For dynamic linking it should be doable with a new type of ELF tag and a bit
of support from the dynamic linker.  Would you be interested?

(at the moment I don't have an answer why lsan started failing like that; and
even if, hypothetically, there would be a good library wrapping mechanism,
sanitizers would likely need other support from libc, like dynamic tls layout
discovery for memory sanitizer; I hope it can be resolved to everyone's
satisfaction eventually, improving sanitizers usability, rather than ignoring
it and perpetually calling sanitizers a pile of hacks)

Alexander

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Rich Felker]

On Thu, Jul 02, 2015 at 10:57:58PM +0300, Alexander Monakov wrote:
> > for glibc calloc they have some ugly workaround, maybe similar hack
> > is needed for free in musl, but none of this is future proof so
> > they will just keep piling hacks..
> 
> Re: hacks, I think it doesn't have to be like that.  There should be a
> reasonable mechanism to wrap library functions in the context of dynamic
> linking; static linking is slightly ahead in that game if you count ld
> --wrap=symbol functionality.
> 
> For dynamic linking it should be doable with a new type of ELF tag and a bit
> of support from the dynamic linker.  Would you be interested?
> 
> (at the moment I don't have an answer why lsan started failing like that; and
> even if, hypothetically, there would be a good library wrapping mechanism,
> sanitizers would likely need other support from libc, like dynamic tls layout
> discovery for memory sanitizer; I hope it can be resolved to everyone's
> satisfaction eventually, improving sanitizers usability, rather than ignoring
> it and perpetually calling sanitizers a pile of hacks)

The problem is that we're still lacking any viable proposal for how
this should work. Some constraints I think belong on a proposal for it
to be viable include:

- It should not place new constraints on the implementation in terms
  of requiring it to track information that would otherwise not be
  required and that may be costly, or to use particular internal
  representations of data/state, even when there's presently only one
  dominant representation in use. Any complex ot costly representation
  tracking should happen in the sanitizer layer, not libc.

- It should be free of initialization order issues and thread-safety
  issues. This may mean that the sanitizer implementation has to use
  something like call_once all over the place.

- It should work correctly with both static and dynamic linking, and
  should not depend on particulars of libc's dynamic linking -- in
  particular it should not depend on intra-libc allocator calls going
  through a PLT or via a symbolic GOT entry.

Can you think of any others?

Rich

Re: Commit 01d4274 breaks Clang's LeakSanitizer when used with musl

[Alexander Monakov]

> The problem is that we're still lacking any viable proposal for how
> this should work. Some constraints I think belong on a proposal for it
> to be viable include:
> 
> - It should not place new constraints on the implementation in terms
>   of requiring it to track information that would otherwise not be
>   required and that may be costly, or to use particular internal
>   representations of data/state, even when there's presently only one
>   dominant representation in use. Any complex ot costly representation
>   tracking should happen in the sanitizer layer, not libc.
> 
> - It should be free of initialization order issues and thread-safety
>   issues. This may mean that the sanitizer implementation has to use
>   something like call_once all over the place.
> 
> - It should work correctly with both static and dynamic linking, and
>   should not depend on particulars of libc's dynamic linking -- in
>   particular it should not depend on intra-libc allocator calls going
>   through a PLT or via a symbolic GOT entry.
> 
> Can you think of any others?

Perhaps this one?

- Natural composability, or nesting: a mechanism that allows sanitizers to
  observe or interpose on some libc activity should naturally allow multiple
  such interposers to co-exist.


For a write up on requirements from a sanitizer author's perspective, see
https://sourceware.org/glibc/wiki/ThreadPropertiesAPI

Alexander