[bug] asan + arm64 + shared libc++.so + -fuse_ld=lld + exceptions #988
Comments
|
Something here is very broken. I don't suppose you've been able to reproduce this on device not running Lineage OS? I have no idea if those devices pass CTS. |
|
I have only this xiaomi with android 8.1 capable of runing wrap.sh and emulators, other devices have to old androids ie 7.x, 5.x. Btw this device works very stable , battery last for 7 days and everything works perfectly, apps build with ld-ldd to, exceptions works perfectly when i link staticaly but with ASAN i cant. |
|
I have figure out how my library becomes poisoned (if it becomes) with libc++.so with standard android.toolchain.cmake and -DANDROID_STL=c++_shared |
|
What's the rest of the stack trace? Yours appears to be missing some lines at the bottom (and even some columns on the right) |
|
Rest is my code and is valid, exception is throw as normal situation that some data cannot be downloaded which is correct. I just dont want to put publicitly full backtrace of comercial app that is often cracked. |
|
W8 why libc++_shared.so from android-ndk-r20-beta1 and 3 have libgcc inside ? beta1 beta3 |
Because they're built against libgcc. Everything is. That's not the problem here.
What happened to this path? It's not valid. This should be impossible. Your app cannot use the system's libc++. I strongly suspect this is a bug in LineageOS because we've never seen an app accidentally call through to the platform's libc++ anywhere else. |
|
W8 backtraces are against libc++_shared.so from beta-1 ( i forget to update libc++_shared.so in my up from beta-3) |
|
How this could be possible with wrap.sh ? |
|
https://github.com/DanAlbert/asan-wrap-sh-exceptions-crash-repro crashes on Pixel as well (Q beta 1 on a marlin). Next time please provide a repro case like the bug template asks. It would have saved us both a lot of time and confusion. |
|
Thx. |
|
Fixing LD_PRELOAD solves ability to sanitize app for me. |
|
Good to know that there's a workaround. Was about to try that myself. I filed http://b/133258130 because this does appear to be a platform bug (non-public atm because I can't find the appropriate component in the public tracker; I'll fix that once I find it, but it's just a reference back to my comment with the repro case above triaged for R). For some reason this also seems to be specific to libc++: if I try something similar to load a symbol from libcutils it fails as expected. @dimitry- and @rprichard might have ideas since they know the most about the loader. |
arturbac
changed the title
|
I'm playing with malloc_hooks and i found one thing that is propably connected to this. |
|
Just for the record, asan from latest Before I found this bugreport I reported the problem here google/sanitizers#1101 |
|
I rechecked again and it seems google/sanitizers#1101 is not related to this one. |
|
Just for the record, |
|
Did the workaround mentioned in #988 (comment) work for you? Since this is a platform bug, that work around will be necessary whenever using asan devices running O through Q (R is the first release that could possibly have the fix). We'll get a doc published for this soon. I've been putting it off until we have a better understanding of what we need to do to fix it so the doc can include an estimate of the time frame, but until then I'll put something in the KIs. |
|
https://android-review.googlesource.com/c/platform/ndk/+/1008442 updates the KI list. |
Test: None Bug: android/ndk#988 Change-Id: I2946e2190c95e4b0088d8db9025d803ab6f84ed9
|
It's slightly worse than that KI explains (I'll update it, but the explanation is getting way too long for the changelog so I'm getting on pushing and actual doc to reference) in that it affects libc++_static as well, and since it's static you don't have any libc++_shared to preload. You can workaround this by building your library with |
|
ASan intercepts __cxa_throw in order to clean up all the redzones in the stack memory that is about to become unallocated. This is important because later, when a new function is using that memory for its stack frame, it expects it to be unpoisoned. Without this you are very likely to see false positive error reports from ASan. |
|
Okay, so inhibiting interposition here would break things, so that workaround isn't viable. The only other thing I can think to do here would be to pack libc++_shared.so into the APK even when using libc++_static and preloading that. That also sounds like a bad idea. Are we just out of luck for using ASan with libc++_static until the platform is fixed? |
|
I don't think this is entirely a platform bug. I think the ASan library needs to intercept certain APIs (e.g. pthread_create) from any linker namespace--platform or app--so it's LD_PRELOAD'ed, which adds it to every linker namespace. This technique also intercepts __cxa_throw from both platform and app code. There are two copies of exception-handling state (__cxa_get_globals, in the platform and app copies of libc++abi), and the ASan library redirects some of the app's libc++abi calls (e.g. __cxa_throw) but not others (e.g. __cxa_begin_catch). I believe this inconsistency explains the crash here. In general, I think we consider the libc++ ABI unstable, so we don't want to redirect app calls to the platform. I think that's less of a concern here because (a) maybe the EH ABI is more stable, and (b) apps can't ship with ASan enabled (no wrap.sh in release builds). Throwing out some possible fixes:
|
It is and it isn't. The API surface is the same across implementations, but that's about all the stability there is. There have been plenty of cases where the expected internal state at a given point varies from unwinder to unwinder, which is why we often see crashes when LLVM's unwinder accidentally calls into libgcc or vice versa. I suspect libunwindstack will have the same issues. The app delegating to the platform and the platform delegating to the app both make me nervous. The C++ ABI isn't a part of CTS, so there's no guarantee that it will work on every device (or even be present, but actually if it weren't present we wouldn't have this problem). I also worry about how that will behave if there's new ABI added (like there was to support I can't poke many holes in delegating from the platform to the app at the moment other than that it feels wrong. The issues mentioned above don't apply in the other direction because if the app has a broken C++ ABI the fix is to use a version of the NDK that doesn't have the bug.
This one would be my choice. |
|
On Mon, Jul 1, 2019 at 11:27 PM Dan Albert ***@***.***> wrote:
(a) maybe the EH ABI is more stable
It is and it isn't. The API surface is the same across implementations,
but that's about all the stability there is. There have been plenty of
cases where the expected internal state at a given point varies from
unwinder to unwinder, which is why we often see crashes when LLVM's
unwinder accidentally calls into libgcc or vice versa. I suspect
libunwindstack will have the same issues.
The app delegating to the platform and the platform delegating to the app
both make me nervous. The C++ ABI isn't a part of CTS, so there's no
guarantee that it will work on every device (or even be present, but
actually if it weren't present we wouldn't have this problem). I also worry
about how that will behave if there's new ABI added (like there was to
support thread_local) and then the app is run on a device that is older
than that change. Getting part of the ABI from the platform and part from
the app sounds fragile.
I can't poke many holes in delegating from the platform to the app at the
moment other than that it *feels* wrong. The issues mentioned above don't
apply in the other direction because if the app has a broken C++ ABI the
fix is to use a version of the NDK that doesn't have the bug.
Make the interceptor smarter about finding the real __cxa_throw. e.g. Get
the __cxa_throw caller's address, then somehow figure out what the next
__cxa_throw should be, given the caller's namespace. (New dynamic linker
API?)
This one would be my choice.
Wow, I never expected to run into this. It sounds like asan interceptors
are fundamentally incompatible with linker namespaces. There are two
separate worlds inside one process, and asan breaks this separation by
injecting into both.
The cleanest solution would be separating interception from asan core, and
having two of it, one per linker namespace.
In practice though, we only have this problem with libc++ symbols. Libc++
is the only non-LLNDK library that ASan intercepts. We only need two
functions out of it: __cxa_throw and __cxa_rethrow_primary_exception. I
also like the idea of figuring out the next function based on the caller
address.
How can ASan find the address of __cxa_throw in the other libc++?
And how does it decide which namespace the caller belongs to? We could look
at the library path though dl_iterate_phdr if nothing else.
|
This can't help users of the static STL, but it does help those using the shared STL. Also clean up by merging these into a single wrap.sh. There should never be more than one ASan library in the directory, so we can just use a wildcard. Test: https://github.com/DanAlbert/asan-wrap-sh-exceptions-crash-repro Bug: android/ndk#988 Change-Id: I890ba18faaebcdafe61f2800cc062ec7b5d9502e
Test: None Bug: android/ndk#988 Change-Id: I9047240acedb677a3c44c93992cf6283a6475369
Summary: When using wrap.sh to preload ASAN, you must also preload the C++ standard library to avoid android/ndk#988, which implies that the library must be placed inside the primary APK (since that's where wrap.sh needs to exist). If you're building the C++ standard library from source, you need cxx_library to support this. This is a mostly mechanical change to add a new category of native linkables to the packager and update all the relevant places. A few potentially interesting points: * We ignore libraries used by wrap scripts for native library merging, and just return them unchanged. This is for simplicity, since these libraries will be used in very limited circumstances and are not expected to be merged. Note that a library used by a wrap script cannot be an asset, so it would be ignored by native_library_merge_sequence anyway. * If a library is used by a wrap script, it would likely also make sense for all its dependents to be marked as such. We do not enforce this in any way (either by automatically marking all dependents or by erroring out if they aren't), again for simplicity, with the idea that this feature will be used in very limited circumstances by people aware of this requirement. Reviewed By: IanChilds fbshipit-source-id: e8e2dc46f26a421937d2a9b5383dc7440d019ef4
|
We've been told that asan is no longer supported (because hwasan is better in pretty much every way), so this isn't likely something we'll ever get a fix for. |
NDK 20-beta3
ABI arm64
ANDROID 8.1
I got a crash duringsanitizing application on armv64 when exception is thrown
I have build with -fuse-ld=lld which works perfectly for standard release application as i link staticaly to libc++
But when building sanitized application with wrap.sh i have to use shared libc++ and i found that it is linked to libgcc. As I remeber this will not work and i wll have crash on parsing eh frames, as I have. This problem doesn't exists in x86 code.
The last address in libc++ points to unwind from libgcc
And now I'm wondering where is problem in libc++ linked against libgcc instead of llvm/libclang_rt or something or in my build configuration
stripped important flags from final linking
And i don't understand that __cxa_throw is called from system wide libc++.so as I linked library against libc++_shared.so and it is attached with appliation and is used on aprsing eh frames.
The text was updated successfully, but these errors were encountered: