Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] asan + arm64 + shared libc++.so + -fuse_ld=lld + exceptions #988

Closed
arturbac opened this issue May 21, 2019 · 26 comments
Closed

[bug] asan + arm64 + shared libc++.so + -fuse_ld=lld + exceptions #988

arturbac opened this issue May 21, 2019 · 26 comments

Comments

@arturbac
Copy link

arturbac commented May 21, 2019

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

llvm-symbolizer -demangle -addresses -inlining -pretty-print -functions=short -color -e aAM-libs/asan_dev/arm64-v8a/lc++_shared.so 0xb8f7c
0xb8f7c: _Unwind_SetGR at /usr/local/google/buildbot/src/android/gcc/toolchain/build/../gcc/gcc-4.9/libgcc/unwind-dw2.c:275:0

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

 *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
 LineageOS Version: '15.1-20190412-NIGHTLY-santoni'
 Build fingerprint: 'Xiaomi/santoni/santoni:7.1.2/N2G47H/V9.2.1.0.NAMCNEK:user/release-keys'
 Revision: '0'
 ABI: 'arm64'
 pid: 27755, tid: 27857, name: Map  >>> pl.aqurat.automapa.dev <<<
 signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
     x0   0000000000000000  x1   0000000000006cd1  x2   0000000000000006  x3   0000000000000008
     x4   feff026ef0949b0a  x5   feff026ef0949b0a  x6   feff026ef0949b0a  x7   7f7f7f7fffffff7f
     x8   0000000000000083  x9   2adfd3b160362759  x10  0000001000000000  x11  0000000000000001
     x12  ffffffffffffffff  x13  000000000000000e  x14  0000000000000700  x15  0000000000000000
     x16  0000005943c72fa8  x17  000000791f0eb00c  x18  00000000787a5158  x19  0000000000006c6b
     x20  0000000000006cd1  x21  434c4e47432b2b00  x22  0000000000000000  x23  00000078443fc588
     x24  0000000000000000  x25  0000001f0875d2c0  x26  0000007843ae96a0  x27  0000007843ae9600
     x28  0000007843ae9660  x29  00000078443f8700  x30  000000791f09fa34
     sp   00000078443f86c0  pc   000000791f09fa50  pstate 0000000060000000
derer: Initialized EGL, version 1.4
derer: Swap behavior 2
 
 backtrace:

     #00 pc 000000000001da50  /system/lib64/libc.so (abort+104)
     #01 pc 00000000000b8f7c  /data/app/pl.aqurat.automapa.dev-VjYgEvoSwPZYI_d1drJDww==/lib/arm64/libc++_shared.so
     #02 pc 00000000000b5324  /data/app/pl.aqurat.automapa.dev-VjYgEvoSwPZYI_d1drJDww==/lib/arm64/libc++_shared.so (__gxx_pers
     #03 pc 00000000000da550  /data/app/pl.aqurat.automapa.dev-VjYgEvoSwPZYI_d1drJDww==/lib/arm64/libclang_rt.asan-aarch64-and
     #04 pc 00000000000da874  /data/app/pl.aqurat.automapa.dev-VjYgEvoSwPZYI_d1drJDww==/lib/arm64/libclang_rt.asan-aarch64-and
     #05 pc 0000000000069ad4  /system/lib64/libc++.so (__cxa_throw+112)
......

stripped important flags from final linking

--target=aarch64-none-linux-android27 --gcc-toolchain=/opt/android-ndk-r20/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/opt/android-ndk-r20/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fPIC -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fno-stack-protector -no-canonical-prefixes -fno-addrsig  -fPIC -std=gnu++17  -ggdb  -Ofast3  -stdlib=libc++  -Wl,--no-undefined  -fvisibility=default  -fsigned-char  -fintegrated-as  -ftemplate-depth=1024  -fno-data-sections  -fno-function-sections -fno-omit-frame-pointer  -fsanitize=address -fsanitize-address-use-after-scope -fno-optimize-sibling-calls  -fsanitize=alignment,bool,builtin,bounds,enum,float-cast-overflow,float-divide-by-zero,implicit-unsigned-integer-truncation,implicit-signed-integer-truncation,implicit-integer-sign-change,integer-divide-by-zero,nonnull-attribute,null,nullability-arg,nullability-assign,nullability-return,object-size,pointer-overflow,return,returns-nonnull-attribute,shift,signed-integer-overflow,unreachable,unsigned-integer-overflow,vla-bound  -Wl,--exclude-libs,libgcc.a -Wl,--exclude-libs,libatomic.a -Wl,--build-id -Wl,--warn-shared-textrel -Wl,--fatal-warnings -Wl,--no-undefined -Qunused-arguments -Wl,-z,noexecstack  -fuse-ld=lld -Wl,--build-id -shared -Wl,-soname,lib_mylib****_.so

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.

#02 pc .../lib/arm64/libc++_shared.so (__gxx_pers
#05   /system/lib64/libc++.so (__cxa_throw+112)
@DanAlbert
Copy link
Member

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.

@arturbac
Copy link
Author

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.
On emulators ASAN broke running with abort because of heap buffer overflow in libglesv2
https://issuetracker.google.com/issues/130707992
and this still is not solved so I don't have other option as this device.

Btw this device works very stable , battery last for 7 days
https://www.reddit.com/r/LineageOS/comments/bdeymh/redmi_4x_outstanding_power_saving

and everything works perfectly, apps build with ld-ldd to, exceptions works perfectly when i link staticaly but with ASAN i cant.

@arturbac
Copy link
Author

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

@DanAlbert
Copy link
Member

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)

@arturbac
Copy link
Author

arturbac commented May 21, 2019

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.

 *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
 LineageOS Version: '15.1-20190412-NIGHTLY-santoni'
 Build fingerprint: 'Xiaomi/santoni/santoni:7.1.2/N2G47H/V9.2.1.0.NAMCNEK:user/release-keys'
 Revision: '0'
 ABI: 'arm64'
 pid: 32420, tid: 32549, name: GUI$EXCLUSIVE  >>> pl.aqurat.automapa.dev <<<
 signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
     x0   0000000000000000  x1   0000000000007f25  x2   0000000000000006  x3   0000000000000008
     x4   feff02729ee49b0a  x5   feff02729ee49b0a  x6   feff02729ee49b0a  x7   7f7f7f7fffffff7f
     x8   0000000000000083  x9   620dd464f3c6e560  x10  0000001000000000  x11  0000000000000001
     x12  ffffffffffffffff  x13  000000000000000e  x14  0000000000000700  x15  0000000000000000
     x16  0000005f6387ffa8  x17  0000007c0577900c  x18  0000000000000020  x19  0000000000007ea4
     x20  0000000000007f25  x21  434c4e47432b2b00  x22  0000000000000000  x23  0000007b2c255588
     x24  0000000000000000  x25  0000001f65713b80  x26  0000007b2b89dca0  x27  0000007b2b89dc00
     x28  0000007b2b89dc60  x29  0000007b2c251b40  x30  0000007c0572da34
     sp   0000007b2c251b00  pc   0000007c0572da50  pstate 0000000060000000
 
 backtrace:
     #00 pc 000000000001da50  /system/lib64/libc.so (abort+104)
     #01 pc 00000000000b8f7c  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libc++_shared.so
     #02 pc 00000000000b5324  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libc++_shared.so (__gxx_personality_v0+244)
     #03 pc 00000000000da550  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libclang_rt.asan-aarch64-android.so (offset 0x50000)
     #04 pc 00000000000da874  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libclang_rt.asan-aarch64-android.so (offset 0x50000) (_Unwind_RaiseException+252)
     #05 pc 0000000000069ad4  /system/lib64/libc++.so (__cxa_throw+112)
     #06 pc 0000000009571424  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libautomapa.e063.so (offset 0x5ff0000) (Aqurat::admin::administrative_online_fmt_t::request_file_info(Aqurat::admin::administrative_file_name_t const&, Aqurat::admin::io_online_options_t) const+1780)
     [...]
    #15 pc 0000000006305cd4  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libautomapa.e063.so (offset 0x5ff0000) (Java_pl_aqurat_common_jni [...]
     #16 pc 00000000000789c0  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/oat/arm64/base.odex (offset 0x5f000)

@arturbac
Copy link
Author

W8 why libc++_shared.so from android-ndk-r20-beta1 and 3 have libgcc inside ?

beta1
llvm-symbolizer -demangle -addresses -inlining -pretty-print -functions=short -color -e aAM-libs/asan_dev/arm64-v8a/lc++_shared.so 0xb8f7c
0xb8f7c: _Unwind_SetGR at /usr/local/google/buildbot/src/android/gcc/toolchain/build/../gcc/gcc-4.9/libgcc/unwind-dw2.c:275:0

beta3
llvm-symbolizer -demangle -addresses -inlining -pretty-print -functions=short -color -e /opt/android-ndk-r20/sources/cxx-stl/llvm-libc++/libs/arm64-v8a/libc++_shared.so -print-source-context-lines=8 0xb8f7c
0xb8f7c: execute_stack_op at /usr/local/google/buildbot/src/android/gcc/toolchain/build/../gcc/gcc-4.9/libgcc/unwind-dw2.c:773:0

@DanAlbert
Copy link
Member

W8 why libc++_shared.so from android-ndk-r20-beta1 and 3 have libgcc inside ?

Because they're built against libgcc. Everything is. That's not the problem here.

llvm-symbolizer -demangle -addresses -inlining -pretty-print -functions=short -color -e aAM-libs/asan_dev/arm64-v8a/lc++_shared.so

What happened to this path? It's not valid.

     #05 pc 0000000000069ad4  /system/lib64/libc++.so (__cxa_throw+112)
     #06 pc 0000000009571424  /data/app/pl.aqurat.automapa.dev-h7M4kfR8o0ThVb1guiJ21Q==/lib/arm64/libautomapa.e063.so (offset 0x5ff0000) (Aqurat::admin::administrative_online_fmt_t::request_file_info(Aqurat::admin::administrative_file_name_t const&, Aqurat::admin::io_online_options_t) const+1780)

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.

@arturbac
Copy link
Author

arturbac commented May 21, 2019

W8 backtraces are against libc++_shared.so from beta-1 ( i forget to update libc++_shared.so in my up from beta-3)
Ill prepare backtrace with beta-3 libc++_shared.so

@arturbac
Copy link
Author

How this could be possible with wrap.sh ?
Why only cxa_throw is resolved against system libc++ ?
Ill try to add libc++_shared.so to wrap.sh LD_PRELOAD, we will see if this solves problem.

#!/system/bin/sh
HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1,detect_stack_use_after_return=1,check_initialization_order=true,quarantine_size_mb=64,color=never,new_delete_type_mismatch=0
export UBSAN_OPTIONS=print_stacktrace=1,log_to_syslog=false,color=never
export LD_PRELOAD="$HERE/libclang_rt.asan-aarch64-android.so"
exec "$@"

DanAlbert added a commit to DanAlbert/asan-wrap-sh-exceptions-crash-repro that referenced this issue May 21, 2019
@DanAlbert
Copy link
Member

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.

05-21 13:54:38.743  8597  8597 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
05-21 13:54:38.743  8597  8597 F DEBUG   : Build fingerprint: 'google/marlin/marlin:Q/QPP1.190205.017/5326479:user/release-keys'
05-21 13:54:38.743  8597  8597 F DEBUG   : Revision: '0'
05-21 13:54:38.743  8597  8597 F DEBUG   : ABI: 'arm64'
05-21 13:54:38.744  8597  8597 F DEBUG   : Timestamp: 2019-05-21 13:54:38-0700
05-21 13:54:38.744  8597  8597 F DEBUG   : pid: 8569, tid: 8569, name: eloper.asantest  >>> com.android.developer.asantest <<<
05-21 13:54:38.744  8597  8597 F DEBUG   : signal 6 (SIGABRT), code -1 (SI_QUEUE), fault addr --------
05-21 13:54:38.744  8597  8597 F DEBUG   :     x0  0000000000000000  x1  0000000000002179  x2  0000000000000006  x3  0000007fca8af840
05-21 13:54:38.744  8597  8597 F DEBUG   :     x4  fefeff00aec49b0a  x5  fefeff00aec49b0a  x6  fefeff00aec49b0a  x7  7f7f7f7fffffff7f
05-21 13:54:38.744  8597  8597 F DEBUG   :     x8  00000000000000f0  x9  bdca73d956a6680b  x10 fffffff8fffffbdf  x11 0000000000000000
05-21 13:54:38.744  8597  8597 F DEBUG   :     x12 0000000000000001  x13 0000000000000000  x14 ffffffffffffffff  x15 0000000000000000
05-21 13:54:38.744  8597  8597 F DEBUG   :     x16 0000006fc29b67b0  x17 0000006fc29939b0  x18 0000007fca8b10a0  x19 00000000000000ac
05-21 13:54:38.744  8597  8597 F DEBUG   :     x20 0000000000002179  x21 00000000000000b2  x22 0000000000002179  x23 00000000ffffffff
05-21 13:54:38.744  8597  8597 F DEBUG   :     x24 0000000000000000  x25 0000000000000037  x26 0000006fc7252020  x27 0000000000000001
05-21 13:54:38.744  8597  8597 F DEBUG   :     x28 0000007fca8b1ac0  x29 0000007fca8af8f0
05-21 13:54:38.744  8597  8597 F DEBUG   :     sp  0000007fca8af820  lr  0000006fc2946778  pc  0000006fc2946798
05-21 13:54:38.948  4006  4006 D zz      : UtWallpaperService$UtEngine.onVisibilityChanged() false
05-21 13:54:38.953  4006  4006 D zz      : UtRenderer.onNotVisible() 
05-21 13:54:38.956  8597  8597 F DEBUG   : 
05-21 13:54:38.956  8597  8597 F DEBUG   : backtrace:
05-21 13:54:38.956  8597  8597 F DEBUG   :       #00 pc 0000000000084798  /bionic/lib64/libc.so (abort+160)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #01 pc 000000000003b52c  /data/app/com.android.developer.asantest-BkImvykWr4FskWS0s1F5Cg==/lib/arm64/libnative-lib.so (_Unwind_SetGR+16)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #02 pc 00000000000245f8  /data/app/com.android.developer.asantest-BkImvykWr4FskWS0s1F5Cg==/lib/arm64/libnative-lib.so (__gxx_personality_v0+364)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #03 pc 00000000000bebfc  /system/lib64/libc++.so (_Unwind_RaiseException_Phase2+124)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #04 pc 00000000000bef20  /system/lib64/libc++.so (_Unwind_RaiseException+252)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #05 pc 0000000000063da4  /system/lib64/libc++.so (__cxa_throw+104)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #06 pc 00000000000135b0  /data/app/com.android.developer.asantest-BkImvykWr4FskWS0s1F5Cg==/lib/arm64/libnative-lib.so (foo()+212)
05-21 13:54:38.956  8597  8597 F DEBUG   :       #07 pc 0000000000013864  /data/app/com.android.developer.asantest-BkImvykWr4FskWS0s1F5Cg==/lib/arm64/libnative-lib.so (Java_com_android_developer_asantest_MainActivity_stringFromJNI+688)
[trimmed Java portion of the backtrace]

@arturbac
Copy link
Author

Thx.
Sure but I wasn't sure if this is a problem with toolchain or a bug in my configuration so i decided to satrt with Question.

@arturbac
Copy link
Author

Fixing LD_PRELOAD solves ability to sanitize app for me.

#!/system/bin/sh
HERE="$(cd "$(dirname "$0")" && pwd)"
export ASAN_OPTIONS=log_to_syslog=false,allow_user_segv_handler=1,detect_stack_use_after_return=1,check_initialization_order=true,quarantine_size_mb=64,color=never,new_delete_type_mismatch=0
export UBSAN_OPTIONS=print_stacktrace=1,log_to_syslog=false,color=never
export LD_PRELOAD="$HERE/libclang_rt.asan-aarch64-android.so $HERE/libc++_shared.so"
exec "$@"

@DanAlbert
Copy link
Member

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 arturbac changed the title [question] asan + arm64 + shared libc++.so + -fuse_ld=lld + exceptions [bug] asan + arm64 + shared libc++.so + -fuse_ld=lld + exceptions May 22, 2019
@arturbac
Copy link
Author

arturbac commented May 28, 2019

I'm playing with malloc_hooks and i found one thing that is propably connected to this.
My appliation is build with c++_static, but during runtime some of android native libraries uses operator new from system shared libc++
3e769e0 _Znwm libc++.so
and I'm wondering how this could work when someone uses c++_shared
does this use on android rpath and per library resolves symbols agains different address of _Znwm in memory , are 2 equal symbols from 2 diffrent libraries allowed to be in memory at once ? or all allocations from libc++_shared are resolved to system libc++ or vice versa ?

@bog-dan-ro
Copy link

bog-dan-ro commented May 30, 2019

Just for the record, asan from latest ndk-bundle (19.2.5345600) is crashing on all platforms (including on x86_64 emulator).

Before I found this bugreport I reported the problem here google/sanitizers#1101

@bog-dan-ro
Copy link

I rechecked again and it seems google/sanitizers#1101 is not related to this one.
On x86_64 asan is crashing no matter what and it has nothing to do with exceptions.

@andreya108
Copy link

andreya108 commented Jul 1, 2019

Just for the record,
played with asan recently, and found it is crashing on the first encountered c++ exception (while unwinding like on the above traces).
It's true for ndk r19c and r21-canary 28 jul nightly.
libc++_shared.
arm64-v8a.
Target API level 23.
default linker (-fuse_ld=lld not used).
Both
Android 9 (not CTS rooted device)
and
Q beta QPP4.190502.09 on Pixel XL (marlin)

@DanAlbert
Copy link
Member

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.

@DanAlbert
Copy link
Member

disigma pushed a commit to wimal-build/ndk that referenced this issue Jul 1, 2019
Test: None
Bug: android/ndk#988
Change-Id: I2946e2190c95e4b0088d8db9025d803ab6f84ed9
@DanAlbert
Copy link
Member

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 -Wl,--exclude-libs,libc++_static.a -Wl,--exclude-libs,libc++abi.a -Wl,--exclude-libs,libgcc_real.a to enforce that __cxa_throw and all of the unwind symbols are resolved locally, but I don't know if that will negatively impact ASan since I don't know why ASan is interposing that in the first place. @eugenis, any idea?

@eugenis
Copy link
Collaborator

eugenis commented Jul 1, 2019

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.

@DanAlbert
Copy link
Member

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?

@rprichard
Copy link
Collaborator

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:

  • Intercept all of the libc++abi EH APIs in the ASan lib. The other interceptors merely forward calls to the platform libc++.so.
  • Like @DanAlbert suggested, pack libc++_shared.so and preload it after the ASan lib. (We only need libc++abi, I guess, and really only the EH APIs.) This change redirects platform calls to the app.
  • Intercept the EH APIs for the app, not the platform. Break the EH interceptors out of the ASan lib into a new ASan-EH lib that isn't preloaded, but still takes precedence over the app's libc++abi. http://b/133258130#comment4, http://b/67755729, and -Wl,--as-needed all look like problems with this idea. Marking the ASan-EH lib with DF_1_GLOBAL would ensure that it overrode libc++_static.a, as long as the NDK solibs need ASan-EH.
  • 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?)
  • Call __asan_handle_no_return directly from the app's libc++abi __cxa_throw.

@DanAlbert
Copy link
Member

(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.

@eugenis
Copy link
Collaborator

eugenis commented Jul 2, 2019 via email

DanAlbert added a commit to DanAlbert/asan-wrap-sh-exceptions-crash-repro that referenced this issue Jul 9, 2019
disigma pushed a commit to wimal-build/ndk that referenced this issue Jul 9, 2019
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
disigma pushed a commit to wimal-build/ndk that referenced this issue Jul 10, 2019
Test: None
Bug: android/ndk#988
Change-Id: I9047240acedb677a3c44c93992cf6283a6475369
@DanAlbert DanAlbert added this to the external dependency milestone Apr 13, 2020
facebook-github-bot pushed a commit to facebook/buck that referenced this issue Jul 26, 2022
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
@DanAlbert
Copy link
Member

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.

@DanAlbert DanAlbert closed this as not planned Won't fix, can't repro, duplicate, stale Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants