[libcxx-commits] [PATCH] D78787: [libcxx][libcxxabi][libunwind] Use libgcc on Android

[Shoaib Meenai via Phabricator via libcxx-commits]

smeenai added a comment.

In D78787#2003189 <https://reviews.llvm.org/D78787#2003189>, @rprichard wrote:

> I suppose this change is probably OK. I don't think it will affect the Android organization, because I don't think we're using the CMake files for libc++/libc++abi/libunwind yet. I think we'll start using libunwind's CMake files soon.


Note that you can still specify `LIBUNWIND_USE_COMPILER_RT` to use compiler-rt, as before.

> Android doesn't ever use libgcc_s. We're trying to remove our use of libgcc and replace it with compiler-rt and libunwind.

Neat! If you can say, are you planning for a static or shared libunwind? The libunwind and libgcc mixup causes a bunch of issues on 32-bit ARM (https://android.googlesource.com/platform/ndk/+/refs/heads/master/docs/BuildSystemMaintainers.md#Unwinding); do you anticipate similar issues on other architectures?

> It seems that these macros are used to determine which libraries to link into libc++/libc++abi/libunwind, which only matters when the output is a shared library? libc++ is the only one that the Android platform or NDK builds as a shared library. Our libc++[_shared].so always has libc++abi linked statically. For the default platform builds, we currently link against the unwinder from libc.so, but otherwise (for the NDK and for some APEXs), we link an unwinder into libc++[_shared].so statically. (If we move to using libc++[_shared].so from the Android toolchain build, we'll need three different outputs?)
> 
> Aside: On my gLinux/Debian system, it appears that libgcc.a doesn't have an unwinder in it. Instead, the unwinder is in libgcc_eh.a or libgcc_s.so.1. libgcc_s.so is a text file that pulls in libgcc_s.so.1 and libgcc.a.
> 
> Also, libcxxabi is detecting libgcc by looking for an ARM EABI symbol. Should that be something architecture-independent instead?
> 
>   check_library_exists(gcc __aeabi_uldivmod "" LIBCXXABI_HAS_GCC_LIB)

Yup, this really only matters for libc++, but I figured I'd make all the runtimes consistent. (The sanitizers already have this logic.)

Note that, in this change, I'm adding `ANDROID` conditionals, so the behavior on other platforms will be unchanged. For the libc++abi detection, I'm assuming the intent of the current logic was to use libgcc in addition to libgcc_s on certain platforms. I'm changing it so that on Android you only look for libgcc, since that's covers everything (and libgcc_s doesn't exist). In the `ANDROID` case, I'm checking for `__gcc_personality_v0` in libgcc, so that's architecture-agnostic. I'm intentionally leaving the existing logic alone for other platforms, because I don't want to modify the behavior for them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78787/new/

https://reviews.llvm.org/D78787