android / ndk

The Android Native Development Kit
1.99k stars 257 forks source link

[FR] use `-Bsymbolic-non-weak-functions` by default if it lands? #1552

Open DanAlbert opened 3 years ago

DanAlbert commented 3 years ago

https://reviews.llvm.org/D102570

This sounds like our ideal behavior:

If and when that lands we should probably make that the default for Android.

eugenis commented 3 years ago

Please at least make sure it does not apply to malloc() and friends - that would break LD_PRELOAD-style interposition of the allocator. ASan on non-arm64 is still a thing (HWASan would not care).

MaskRay commented 3 years ago

The -Bsymbolic family only applies to relocatable object file definitions. malloc and friends are in libc.so and are therefore not affected.

There are several types of LD_PRELOAD usage.

First, use LD_PRELOAD=same_soname.so to replace a DT_NEEDED entry with the same SONAME. Both -fno-semantic-interposition and -Bsymbolic are compatible with such usage.

Second, use LD_PRELOAD=malloc.so to intercept some functions not defined in the application or any of its shared object dependencies. Both -fno-semantic-interposition and -Bsymbolic are compatible. Common examples include malloc replacement and fakeroot, both interposing some libc.so functions.

void *f() { return malloc(0xb612); }

Third, use LD_PRELOAD=different_soname.so to replace a non-vague-linkage function defined in a shared object dependency and the SONAME is different. (This usage is unlikely compatible with C++'s one definition rule.) Such usage is incompatible with -Bsymbolic, -Bsymbolic-functions, and (if non-weak) -Bsymbolic-non-weak-functions. If -fno-semantic-interposition causes an inlining and the call site is not intercepted, there is an incompatibility issue.

Note: it is unfair and usually incorrect to just state "it breaks LD_PRELOAD".

DanAlbert commented 3 years ago

I think he's worried about ASan breaking if we build libc.so with this flag. ASan relies on being able to replace the program's malloc (among other things), including the malloc calls within libc. Pre-L Android built with -Bsymbolic and it prevented ASan from working.

So, yes, I think we we do need to be careful and make sure that anything ASan needs to hook is marked weak for this. Thanks for remembering that @eugenis :+1: