Enable LSAN for Android

[oontvoo]

Summary

Make use of the newly added thread-properties API (available since 31)

Side-effect change: set -fno-emulated-tls for lsan/asan libraries and skip their tests on pre-S Android.

[eugenis]

compiler-rt/lib/lsan/lsan_common.cpp

36

I think this needs runtime detection (dlsym or a weak symbol, there are precedents in sanitizer_common), otherwise the NDK build of the runtime library will never enable this.

38

typo: ANDROILD

[eugenis]

FYI https://reviews.llvm.org/D85930 moves standalone LSan on Android/aarch64 to the 64-bit allocator, I don't know if you care about this.
LSan-in-ASan is already there.

[oontvoo]

Updated diff

[oontvoo]

compiler-rt/lib/lsan/lsan_common.cpp

36

Sorry, runtime detection for the _iterate_ function (used below)? Or do you mean the new header?

[eugenis]

I think there needs to be something to disable automatic leak detection on exit on older platform, otherwise existing users of ASan will start getting false leak reports.

compiler-rt/lib/lsan/lsan_common.cpp

36

For the function, like you've done already.

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

94

It seems easier to make this declaration unconditional (under SANITIZER_ANDROID of course) and get rid of the header include.

444

nit: I'd prefer start_addr and end_addr to be void*, and the casts moved to the assignments below.

[oontvoo]

In D85927#2216843, @eugenis wrote:

FYI https://reviews.llvm.org/D85930 moves standalone LSan on Android/aarch64 to the 64-bit allocator, I don't know if you care about this.
LSan-in-ASan is already there.

I'm not familiar with this. How would it change the way static and dynamic TLS are allocated on android? Specifically, would it still eventually go to the bionic allocator?

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

94

It seems easier to make this declaration unconditional (under SANITIZER_ANDROID of course) and get rid of the header include.

How would it know where to look for the function in the case where it does exist ?

I'm guessing somebody needs to include the new header?

[oontvoo]

Updated diff

[oontvoo]

Dont run leakcheck for android if the thread-properties API is not present

[oontvoo]

updated

[eugenis]

This change does not enable standalone LSan. It probably should, as a test vehicle if nothing else. See COMPILER_RT_HAS_LSAN.

It would be great to have some testing, too. Unfortunately with the pandemic our testing rig is locked in the office and most of the devices fell off over time... There used to be aarch64 and arm testing steps here:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/34766

It's pretty hard to setup, but could you check that check-asan check-lsan check-sanitizer pass both with old and new aosp images? I'm concerned about __lsan_disable which involves a thread-local variable built with -fno-emulated-tls and it's probably going to crash before Android Q.

You can peek at the buildbot setup: https://github.com/llvm/llvm-zorg/blob/master/zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh The interesting parts are configure_android and test_arch_on_device.

Any plans to add hwasan+lsan support? Android is generally moving in that direction, and both standalone lsan and asan+lsan are unlikely to see a lot of use.

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

67

why assert.h?

94

the header does not really add anything useful if you already have the declaration :)

446

Why +1? Looking at the implementation, end seems to be one-past-the-end, no adjustment should be needed.

[oontvoo]

updated diff

[oontvoo]

In D85927#2218749, @eugenis wrote:

This change does not enable standalone LSan. It probably should, as a test vehicle if nothing else. See COMPILER_RT_HAS_LSAN.

Done

It would be great to have some testing, too. Unfortunately with the pandemic our testing rig is locked in the office and most of the devices fell off over time... There used to be aarch64 and arm testing steps here:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/34766

It's pretty hard to setup, but could you check that check-asan check-lsan check-sanitizer pass both with old and new aosp images? I'm concerned about __lsan_disable which involves a thread-local variable built with -fno-emulated-tls and it's probably going to crash before Android Q.

Maybe moving the check for existence of the new API before the call to __lsan_disable would prevent the crash? (because it wouldn't to that) ?

You can peek at the buildbot setup: https://github.com/llvm/llvm-zorg/blob/master/zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh The interesting parts are configure_android and test_arch_on_device.

I've grabbed a pixel3 test phone from the office. Is there a one-step setup script that I could run and point it at the device? Or would I have to set it up manually ?
I could also do the manual tests where I copy the new bionic libc and this patch's asan-aarch64-android.so. to the phone and run some binaries ... see if they work

Any plans to add hwasan+lsan support?

Yes, not sure where to start. :-) Any pointers? (I was under the impression it'd "just work" ... what's still missing? )

[MaskRay]

compiler-rt/lib/lsan/lsan_common.cpp

315

The convention is just size_t /*dso_id*/, no unused.

[eugenis]

I've grabbed a pixel3 test phone from the office. Is there a one-step setup script that I could run and point it at the device? Or would I have to set it up manually ?
I could also do the manual tests where I copy the new bionic libc and this patch's asan-aarch64-android.so. to the phone and run some binaries ... see if they work

Try this one, it's the same script that runs on the buildbot. Be careful, it can mess up anything in the current directory (it expect source/build trees there for a possible incremental run).
Uncomment the line at the bottom to enable arm/aarch64 testing.
https://github.com/llvm/llvm-zorg/blob/master/zorg/buildbot/builders/sanitizers/buildbot_android.sh

Any plans to add hwasan+lsan support?

Yes, not sure where to start. :-) Any pointers? (I was under the impression it'd "just work" ... what's still missing? )

There are a few small things, search for "lsan" in compiler-rt/lib/asan. The main one is an allocator interface for scanning, and a few hooks for initializing lsan and passing the runtime flags.

[oontvoo]

upated diff

[oontvoo]

Avoid accessing TLS variables for android when the thread-properties API is not present.

[oontvoo]

updated diff: skipped lsan/asan tests on pre-S images

[oontvoo]

Updated a few tests to work on Android.

@eugenis: I think this is ready to be reviewed now.
Here are the check-lsan and check-asan result on aarch64:

• check-lsan: https://pastebin.ubuntu.com/p/xRbgqckXPJ/
• check-asan: https://pastebin.ubuntu.com/p/YJ3Hw95SdP/

i686:

• check-lsan: https://pastebin.ubuntu.com/p/kDCy9j3gnh/
• check-asan: https://pastebin.ubuntu.com/p/ryrTHrWNmX/

(Note: The i686 needs some fix in Bionic or it'd sigsev https://android-review.googlesource.com/c/platform/bionic/+/1419435)

[oontvoo]

(minor cleanup)

[eugenis]

compiler-rt/cmake/config-ix.cmake

167 ↗

(On Diff #295033)

Why is android special here?

295 ↗

(On Diff #295033)

Please remind me why i686? I don't like that this list contains both i386 and i686, it's hard to tell what can break.

Does it affect the library name? That would definitely break a number of things.

If this is about the test script matching build with a device, the fix should be limited to the the tests.

compiler-rt/lib/lsan/lsan_common.cpp

97

This looks like it would suppress everything in libc++. Could it be refined somehow?

compiler-rt/test/asan/lit.cfg.py

253 ↗

(On Diff #295033)

ASan is certainly supported before android-31.

compiler-rt/test/lit.common.cfg.py

370 ↗

(On Diff #295033)

should not this be android-30?

compiler-rt/test/lsan/TestCases/Linux/log-path_test.cpp

12 ↗

(On Diff #295033)

Do you mean it does not work with the android test harness?

compiler-rt/test/lsan/TestCases/Linux/use_tls_dynamic.cpp

6 ↗

(On Diff #295033)

why are you removing the glibc-2.27 check?

compiler-rt/test/lsan/lit.common.cfg.py

26 ↗

(On Diff #295033)

This is better done in cmake, otherwise you can end up with two ASan configs.

But I'd rather fix the segfault problem instead.

[oontvoo]

updated per review comments

compiler-rt/cmake/config-ix.cmake

295 ↗

(On Diff #295033)

Please remind me why i686? I don't like that this list contains both i386 and i686, it's hard to tell what can break.
If this is about the test script matching build with a device, the fix should be limited to the the tests.

The emulator is i686, so none of the tests would run if i686 is not in the list of supported archs.

Additionally from the thread I started awhile ago with you, enh@, et al, the options then were:

1. change x86 to mean i686
2. try to paint i686 as i386 in the test scripts
3. add i686 to the list of supported things

IIUC, the consensus was "no" to (1) because of a long tail of things.
And I think (2) would be misleading given bionic publicly state that they don't support i386 ABI. (And it'll make it's harder to fix (1) when we need to, which supposedly would happen at some point?)

That leaves us with (3)

Does it affect the library name? That would definitely break a number of things.

Yes, if the test script sees that it's building for i686, it'll change .so file names to have i686. The library itself is still called LeakSanitizer-AddressSanitizer-i386, though.

How do I produce the breakages? Is it better to fix those breakages?

compiler-rt/test/asan/lit.cfg.py

253 ↗

(On Diff #295033)

Done - removed

compiler-rt/test/lit.common.cfg.py

370 ↗

(On Diff #295033)

Technically, the thread_properties API is declared to be introduced in 31.

In practice, on master build (which is 30), it's already available ... and there's no 31 builds yet.

Should we change the introduction version to 30?

compiler-rt/test/lsan/TestCases/Linux/log-path_test.cpp

12 ↗

(On Diff #295033)

Do you mean it does not work with the android test harness?

Right, because we need to pull the file down to lit-check.
Anyway, updated the test so it will work.

compiler-rt/test/lsan/TestCases/Linux/use_tls_dynamic.cpp

6 ↗

(On Diff #295033)

Done - put it back.

compiler-rt/test/lsan/lit.common.cfg.py

26 ↗

(On Diff #295033)

It didn't produce any stacktrace - I was only able to track it to somewhere *before* __libc_init_main*

( I suspect it may have to do with emulated-tls )

Any pointer?

[eugenis]

compiler-rt/cmake/config-ix.cmake

295 ↗

(On Diff #295033)

The breakage I'd expect are in android platform build system, and in asan/scripts/asan_device_setup hardcoding library names. Probably something in chromium, too. I'd prefer not to change the existing library names.

But, looking at the prebuilt android toolchains, asan library names already use i686 for android targets (and i386 for the host). Not sure how that happens, could have something to do with the build flags. Could you check if what they do works for you, as well?

The reason I don't like this change is because you add a second alias for x86, and now, potentially, every place in cmake that references ${X86} needs to look for i686, too.

compiler-rt/test/lit.common.cfg.py

370 ↗

(On Diff #295033)

Yeah that's a problem.
API 30 does not have thread properties. We should assume that the test device is a release build by default.

• we could set an environment variable
• pull libc.so from the device and look at the symbols
• look for "master" in ro.build.fingerprint + api level 30

... then add a feature "thread-properties-api".

Even better, look at ro.build.version.codename.

compiler-rt/test/lsan/lit.common.cfg.py

26 ↗

(On Diff #295033)

try attaching a debugger, or running under strace. Try disabling the segv handler in lsan (if it is enabled).

[oontvoo]

Updated diff

compiler-rt/cmake/config-ix.cmake

295 ↗

(On Diff #295033)

But, looking at the prebuilt android toolchains, asan library names already use i686 for android targets (and i386 for the host).

Yes, even the buildbot already uses i686 (And all I was doing here was running the buildbot_android.sh script.)

So apparently it's already skipping all the tests on i686 right now.
But looks like we can change the bot script to use i386 and run these tests as usual.

Ok, I think I will stop poking at this can of worms for now. :-)

Reverted all the i686s.

compiler-rt/test/lit.common.cfg.py

370 ↗

(On Diff #295033)

Done: use codename when api level is lower than 31.

compiler-rt/test/lsan/lit.common.cfg.py

26 ↗

(On Diff #295033)

Done.

Issue was lsan not being initted

[eugenis]

compiler-rt/lib/asan/CMakeLists.txt

238 ↗

(On Diff #296361)

Same as before, why is Android special here? The cmake check should have found that the linker does not support this option. Is it because we are forcing LLD at line 90, and the check has run against a different linker?

I guess it's fine as a workaround, but mention the reason in the comment.

compiler-rt/lib/lsan/lsan_common.cpp

98

A bit strange that these suppression is needed on android, and not everywhere with libc++.
What does the leak look like?

compiler-rt/test/asan/lit.cfg.py

212 ↗

(On Diff #296361)

You are enabling the tests on non-android arm32/arm64.
They might just work there, but I would not do that in this change.
For simplicity, move the android conditions to leak_detection_android.

compiler-rt/test/lsan/TestCases/Linux/use_tls_dynamic.cpp

6 ↗

(On Diff #295033)

This condition looks weird, because glibc-2.27 refers to the host system, and android refers to the testing device.

It's probably better to make sure that glibc-2.27 is not set when config.android==true.

compiler-rt/test/lsan/TestCases/strace_test.cpp

6 ↗

(On Diff #296361)

What does this error look like, and why can't it be captured?
Is it something that can be fixed in the adb scripts?

[oontvoo]

updated diff

compiler-rt/lib/asan/CMakeLists.txt

238 ↗

(On Diff #296361)

Yeah, you'll get a build warning saying /usr/bin/ld: -Wl,-z,gnu-version-script-compat is ignored
Added the explanation.

compiler-rt/lib/lsan/lsan_common.cpp

98

There were leaks in ignore_object.c, cleanup_in_tsd_destructor.c, disabler_in_tsd_destructor.c, and disabler.c

Leak reports (for ignore_object.c):

Test alloc: 0xc5600750.

=================================================================
==31243==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 1337 byte(s) in 1 object(s) allocated from:
    #0 0xe7a116d5 in malloc /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x59a33cb2 in main /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/test/lsan/TestCases/ignore_object.c:17:22
    #2 0xe7df5e43 in __libc_init (/system/lib/libc.so+0x4fe43)
    #3 0x59a33ba8 in _start (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x2ba8)

Objects leaked above:
0xc5003680 (1337 bytes)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0xe7a116d5 in malloc /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0xe7e89183 in __register_atfork (/system/lib/libc.so+0xe3183)
    #2 0xe7de8c9a in pthread_atfork (/system/lib/libc.so+0x42c9a)
    #3 0xe7e2546c in __libc_init_fork_handler() (/system/lib/libc.so+0x7f46c)
    #4 0xe7df5d78 in __libc_preinit_impl() (/system/lib/libc.so+0x4fd78)
    #5 0xe7df5cdb in __libc_preinit() (/system/lib/libc.so+0x4fcdb)
    #6 0xe86d0728  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x60728)
    #7 0xe86d09cc  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x609cc)
    #8 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #9 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #10 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #11 0xe871cbf7  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xacbf7)
    #12 0xe871b582  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xab582)
    #13 0xe86d435a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6435a)

Objects leaked above:
0xc5a00b80 (24 bytes)

Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0xe7a116d5 in malloc /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0xe7e7bdc1 in operator new(unsigned int) (/system/lib/libc.so+0xd5dc1)
    #2 0xe7e25b92 in newlocale (/system/lib/libc.so+0x7fb92)
    #3 0xe7d0a1eb  (/system/lib/libc++.so+0x851eb)
    #4 0xe7d100fd  (/system/lib/libc++.so+0x8b0fd)
    #5 0xe7d101f3 in std::__1::locale::locale() (/system/lib/libc++.so+0x8b1f3)
    #6 0xe7ce077f in std::__1::basic_streambuf<char, std::__1::char_traits<char> >::basic_streambuf() (/system/lib/libc++.so+0x5b77f)
    #7 0xe7ced54b in std::__1::ios_base::Init::Init() (/system/lib/libc++.so+0x6854b)
    #8 0xe7cee9fa  (/system/lib/libc++.so+0x699fa)
    #9 0xe86d0728  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x60728)
    #10 0xe86d09cc  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x609cc)
    #11 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #12 0xe86b4303  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x44303)
    #13 0xe86ae912  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x3e912)
    #14 0xe86ae9f8  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x3e9f8)
    #15 0xe8320a76 in dlopen (/apex/com.android.runtime/lib/bionic/libdl.so+0x1a76)
    #16 0xe7ded93c in netdClientInitImpl() (/system/lib/libc.so+0x4793c)
    #17 0xe7e8c9c0 in pthread_once (/system/lib/libc.so+0xe69c0)
    #18 0xe7ded8ab in netdClientInit (/system/lib/libc.so+0x478ab)
    #19 0xe7df5d7d in __libc_preinit_impl() (/system/lib/libc.so+0x4fd7d)
    #20 0xe7df5cdb in __libc_preinit() (/system/lib/libc.so+0x4fcdb)
    #21 0xe86d0728  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x60728)
    #22 0xe86d09cc  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x609cc)
    #23 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #24 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #25 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #26 0xe871cbf7  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xacbf7)
    #27 0xe871b582  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xab582)
    #28 0xe86d435a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6435a)

Objects leaked above:
0xc56007b0 (4 bytes)

Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0xe7a1e189 in operator new(unsigned int) /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:99:3
    #1 0xe7c03462  (/data/local/tmp/Output/libc++_shared.so+0xd7462)
    #2 0xe7ba2aee  (/data/local/tmp/Output/libc++_shared.so+0x76aee)
    #3 0xe7baac6d in std::__ndk1::locale::__global() (/data/local/tmp/Output/libc++_shared.so+0x7ec6d)
    #4 0xe7baad0a in std::__ndk1::locale::locale() (/data/local/tmp/Output/libc++_shared.so+0x7ed0a)
    #5 0xe7b7eb74  (/data/local/tmp/Output/libc++_shared.so+0x52b74)
    #6 0xe7b7e7dd in std::__ndk1::ios_base::Init::Init() (/data/local/tmp/Output/libc++_shared.so+0x527dd)
    #7 0xe7b6008a  (/data/local/tmp/Output/libc++_shared.so+0x3408a)
    #8 0xe86d0728  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x60728)
    #9 0xe86d09cc  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x609cc)
    #10 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #11 0xe86d083a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6083a)
    #12 0xe871cbf7  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xacbf7)
    #13 0xe871b582  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0xab582)
    #14 0xe86d435a  (/data/local/tmp/Output/usr/local/google/home/vyng/repo/i686_buildbot/rundir/compiler_rt_build_android_i386/test/lsan/I386LsanConfig/TestCases/Output/ignore_object.c.tmp+0x6435a)

Objects leaked above:
0xc5600790 (4 bytes)

-----------------------------------------------------
Suppressions used:
  count      bytes template
      2        144 *tls_get_addr*
-----------------------------------------------------

SUMMARY: AddressSanitizer: 1369 byte(s) leaked in 4 allocation(s).

compiler-rt/test/lsan/TestCases/strace_test.cpp

6 ↗

(On Diff #296361)

Running adb shell "strace ..." on a terminal gets you the expected error ("leaksanitizer doesn't work under ptrace").

Unfortunately, my python-fu was limited and I couldn't figure out why the android_run.py wrapper(line 26) would not get that error into either device_stderr or its own stderr.

So this test would die (expectedly) but no error message to check against.

--
Exit Code: 2

Command Output (stderr):
--
FileCheck error: '<stdin>' is empty.
FileCheck command line:  FileCheck /usr/local/google/home/vyng/repo/i686_buildbot/rundir/llvm-project/compiler-rt/test/lsan/TestCases/strace_test.cpp

--

[oontvoo]

updated diff

[oontvoo]

rebase

[eugenis]

compiler-rt/lib/lsan/lsan_common.cpp

98

#1 0xe7e89183 in __register_atfork (/system/lib/libc.so+0xe3183)

This is a global in libc.so.

#3 0xe7baac6d in std::__ndk1::locale::__global()

This is a global (function-local static) in libc++.so

I don't know why they are not seen as reachable, but something must be wrong.
Try figuring it out? Those pointers should have been discovered when scanning the module mappings. LSAN_OPTIONS report_objects, log_pointers, etc could help.

[oontvoo]

compiler-rt/lib/lsan/lsan_common.cpp

98

Oh wait, these tests explicitly exclude globals (use_globals=0), so obviously they won't scan it.

I guess we'll just have to set use_globals=1 by default for android?

[oontvoo]

Make test set use_global=1 for Android and add warnings if use_global is turnt off.

[eugenis]

LGTM with 1 comment

compiler-rt/lib/lsan/lsan_common.cpp

98

Better to remove use_globals from the tests on all platforms. I don't see why its needed there, must be to narrow down the test scope as much as possible, but it is clearly not working on android.

[oontvoo]

remove use_globals=0 from tests

[dmajor]

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

The name is referenced from https://github.com/llvm/llvm-project/blob/85e578f53ad1ba21771470dc9516068a259d29cf/compiler-rt/lib/asan/asan_malloc_linux.cpp#L274 but the definition is no longer provided now that SANITIZER_INTERCEPT_MEMALIGN has excluded Android.

For anyone playing along at home: this landed in https://github.com/llvm/llvm-project/commit/a2291a58bf1c860d026581fee6fe96019dc25440, not sure why Phab didn't automatically mention that here.

[dmajor]

And later we also get use of undeclared identifier 'dl_iterate_phdr' in (EDIT:) lsan_common_linux.cpp. We compile for API 16, which doesn't implement that.

[oontvoo]

Thanks for the report!

In D85927#2325138, @dmajor wrote:

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

Are you also building with API 16? (From your second comment)

The name is referenced from https://github.com/llvm/llvm-project/blob/85e578f53ad1ba21771470dc9516068a259d29cf/compiler-rt/lib/asan/asan_malloc_linux.cpp#L274 but the definition is no longer provided now that SANITIZER_INTERCEPT_MEMALIGN has excluded Android.

For anyone playing along at home: this landed in https://github.com/llvm/llvm-project/commit/a2291a58bf1c860d026581fee6fe96019dc25440, not sure why Phab didn't automatically mention that here.

I landed it with git push instead of arc land ... maybe that's the issue?

[dmajor]

In D85927#2325276, @oontvoo wrote:

Thanks for the report!

In D85927#2325138, @dmajor wrote:

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

Are you also building with API 16? (From your second comment)

Yes, that's right. The memalign error is still present after D89251.

[eugenis]

Hmm, I did not notice the memalign chunk. Why was it removed?

[oontvoo]

In D85927#2325516, @dmajor wrote:

In D85927#2325276, @oontvoo wrote:

Thanks for the report!

In D85927#2325138, @dmajor wrote:

This caused a build break in our compiler-rt build for Android: use of undeclared identifier '__interceptor_memalign'

Are you also building with API 16? (From your second comment)

Yes, that's right. The memalign error is still present after D89251.

Included the fix now.

In D85927#2325570, @eugenis wrote:

Hmm, I did not notice the memalign chunk. Why was it removed?

memalign is callled during lsan init for Android, causing the "ENSURE_LSAN_INITED" check and the stack-trace to fail.
(See comment on the other patch for explanation)

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h

507–509 ↗

(On Diff #297306)

this breaks android bot http://lab.llvm.org:8014/#/builders/54/builds/9/steps/7/logs/stdio

[vitalybuka]

reverted with 25a8881b724abf7251a9278e72224af7e82cb9c2

[oontvoo]

(relanded)

compiler-rt/lib/lsan/lsan_common.h

 // LeakSanitizer relies on some Glibc's internals (e.g. TLS machinery) on Linux.
 // Also, LSan doesn't like 32 bit architectures
 // because of "small" (4 bytes) pointer size that leads to high false negative
 // ratio on large leaks. But we still want to have it for some 32 bit arches
 // (e.g. x86), see https://github.com/google/sanitizers/issues/403.
 // To enable LeakSanitizer on a new architecture, one needs to implement the
 // internal_clone function as well as (probably) adjust the TLS machinery for
 // the new architecture inside the sanitizer library.
-#if (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC) &&          \
-    (SANITIZER_WORDSIZE == 64) &&                                        \
+#if (SANITIZER_LINUX || SANITIZER_MAC) && (SANITIZER_WORDSIZE == 64) &&  \
     (defined(__x86_64__) || defined(__mips64) || defined(__aarch64__) || \
      defined(__powerpc64__) || defined(__s390x__))
 #define CAN_SANITIZE_LEAKS 1
-#elif defined(__i386__) && \
-    (SANITIZER_LINUX && !SANITIZER_ANDROID || SANITIZER_MAC)
+#elif defined(__i386__) && (SANITIZER_LINUX || SANITIZER_MAC)
 #define CAN_SANITIZE_LEAKS 1
-#elif defined(__arm__) && \
-    SANITIZER_LINUX && !SANITIZER_ANDROID
+#elif defined(__arm__) && SANITIZER_LINUX
 #define CAN_SANITIZE_LEAKS 1
 #elif SANITIZER_NETBSD || SANITIZER_FUCHSIA
 #define CAN_SANITIZE_LEAKS 1
 #else
 #define CAN_SANITIZE_LEAKS 0
 #endif
 
 namespace __sanitizer {

compiler-rt/lib/lsan/lsan_common.cpp

 #include "sanitizer_common/sanitizer_stacktrace.h"
 #include "sanitizer_common/sanitizer_suppressions.h"
 #include "sanitizer_common/sanitizer_thread_registry.h"
 #include "sanitizer_common/sanitizer_tls_get_addr.h"
 
 extern "C" const char *__lsan_current_stage = "unknown";
 
 #if CAN_SANITIZE_LEAKS
+
+#if SANITIZER_ANDROID
+
+// Bionic provides an API to iterate through DTLS chunk.
+SANITIZER_WEAK_ATTRIBUTE void __libc_iterate_dynamic_tls(
+    pid_t, void (*cb)(void *, void *, size_t, void *), void *);
+
+#endif
+
 namespace __lsan {
 
 // This mutex is used to prevent races between DoLeakCheck and IgnoreObject, and
 // also to protect the global list of root regions.
 BlockingMutex global_mutex(LINKER_INITIALIZED);
 
 Flags lsan_flags;
 

 #if SANITIZER_MAC
   // For Darwin and os_log/os_trace: https://reviews.llvm.org/D35173
   "leak:*_os_trace*\n"
 #endif
   // TLS leak in some glibc versions, described in
   // https://sourceware.org/bugzilla/show_bug.cgi?id=12650.
   "leak:*tls_get_addr*\n";
 
 void InitializeSuppressions() {
   CHECK_EQ(nullptr, suppression_ctx);
   suppression_ctx = new (suppression_placeholder)
       SuppressionContext(kSuppressionTypes, ARRAY_SIZE(kSuppressionTypes));
   suppression_ctx->ParseFromFile(flags()->suppressions);
   if (&__lsan_default_suppressions)
     suppression_ctx->Parse(__lsan_default_suppressions());
   suppression_ctx->Parse(kStdSuppressions);
 }
 
if (flags()->use_tls) {
           if (tls_begin < cache_begin)
             ScanRangeForPointers(tls_begin, cache_begin, frontier, "TLS",
                                  kReachable);
           if (tls_end > cache_end)
             ScanRangeForPointers(cache_end, tls_end, frontier, "TLS",
                                  kReachable);
         }
       }
+#if SANITIZER_ANDROID
+      auto dtls_visitor = [&](void* dtls_begin,
+                              void* dtls_end,
+                              size_t /*dso_id unused*/,
+                              void* /*arg unused*/))->void {
+        ScanRangeForPointers(dtls_begin, dtls_end, frontier, "DTLS",
+                             kReachable);
+      };
+      __libc_iterate_dynamic_tls(os_id, dtls_visitor, nullptr);
+#else
       if (dtls && !DTLSInDestruction(dtls)) {
         for (uptr j = 0; j < dtls->dtv_size; ++j) {
           uptr dtls_beg = dtls->dtv[j].beg;
           uptr dtls_end = dtls_beg + dtls->dtv[j].size;
           if (dtls_beg < dtls_end) {
             LOG_THREADS("DTLS %zu at %p-%p.\n", j, dtls_beg, dtls_end);
             ScanRangeForPointers(dtls_beg, dtls_end, frontier, "DTLS",
                                  kReachable);
           }
         }
       } else {
         // We are handling a thread with DTLS under destruction. Log about
         // this and continue.
         LOG_THREADS("Thread %d has DTLS under destruction.\n", os_id);
       }
+#endif
     }
   }
 }
 
 #endif  // SANITIZER_FUCHSIA
 
 void ScanRootRegion(Frontier *frontier, const RootRegion &root_region,
                     uptr region_begin, uptr region_end, bool is_readable) {

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

 
 #if SANITIZER_SOLARIS
 #include <stdlib.h>
 #include <thread.h>
 #endif
 
 #if SANITIZER_ANDROID
 #include <android/api-level.h>
+#include <assert.h>
 #if !defined(CPU_COUNT) && !defined(__aarch64__)
 #include <dirent.h>
 #include <fcntl.h>
 struct __sanitizer::linux_dirent {
   long           d_ino;
   off_t          d_off;
   unsigned short d_reclen;
   char           d_name[];
 };
 #endif
-#endif
+
+#endif  // SANITIZER_ANDROID
 
 #if !SANITIZER_ANDROID
 #include <elf.h>
 #include <unistd.h>
 #endif
 
 namespace __sanitizer {
 
 SANITIZER_WEAK_ATTRIBUTE int
 real_sigaction(int signum, const void *act, void *oldact);
 
+#if SANITIZER_ANDROID
+
+// Bionic provides an API for querying TLS bounds.
+SANITIZER_WEAK_ATTRIBUTE void __libc_get_static_tls_bounds(void **, void **);
+
+#endif  // SANITIZER_ANDROID
+
 int internal_sigaction(int signum, const void *act, void *oldact) {
 #if !SANITIZER_GO
   if (&real_sigaction)
     return real_sigaction(signum, act, oldact);
 #endif
   return sigaction(signum, (const struct sigaction *)act,
                    (struct sigaction *)oldact);
 }
for (; hdr != last_hdr; ++hdr) {
     }
   }
   return 0;
 }
 #endif  // SANITIZER_NETBSD
 
 #if !SANITIZER_GO
 static void GetTls(uptr *addr, uptr *size) {
-#if SANITIZER_LINUX && !SANITIZER_ANDROID
+#if SANITIZER_ANDROID
+  *addr = 0;
+  *size = 0;
+
+  void *start_addr;
+  void *end_addr;
+  __libc_get_static_tls_bounds(&start_addr, &end_addr);
+  *addr = reinterpret_cast<uptr>(start_addr);
+  *size =
+      reinterpret_cast<uptr>(end_addr) - reinterpret_cast<uptr>(start_addr) + 1;
+#elif SANITIZER_LINUX && !SANITIZER_ANDROID
 # if defined(__x86_64__) || defined(__i386__) || defined(__s390__)
   *addr = ThreadSelf();
   *size = GetTlsSize();
   *addr -= *size;
   *addr += ThreadDescriptorSize();
 # elif defined(__mips__) || defined(__aarch64__) || defined(__powerpc64__) \
     || defined(__arm__)
   *addr = ThreadSelf();
if (tcb != 0) {
     if (*size != 0) {
       // The block has been found and tcb_dtv[1] contains the base address
       *addr = (uptr)tcb->tcb_dtv[1];
     }
   }
 #elif SANITIZER_OPENBSD
   *addr = 0;
   *size = 0;
-#elif SANITIZER_ANDROID
-  *addr = 0;
-  *size = 0;
 #elif SANITIZER_SOLARIS
   // FIXME
   *addr = 0;
   *size = 0;
 #else
 # error "Unknown OS"
 #endif
 }