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

[Ryan Prichard via Phabricator via libcxx-commits]

rprichard added a subscriber: pcc.
rprichard added a comment.

> 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?

For R+, I think the situation for other architectures will be the same as the current arm32 situation. i.e. The platform libc.so exports _Unwind_* symbols from libgcc.a, but the app uses an incompatible unwinder from LLVM's libunwind.

To reduce the trouble, maybe we can:

- Only distribute one unwinder implementation with the NDK (e.g. just stop distributing the libgcc unwinder).
- Mark the libunwind.a symbols hidden. It looks like this is what the libgcc_eh.a / libgcc.a libraries do on my gLinux system. (Some symbols in libunwind.a are already hidden, e.g. __unw_*, but unw_* and _Unwind_* have default visibility.) The NDK's libgcc_real.a _Unwind_* symbols use default visibility instead.

We had been thinking of having a shared libunwind.so in the platform, but decided to export the symbols from libc.so instead. (I don't really remember the rationale. The internal notes are at http://b/144430859#comment24. Maybe it was something about forwarding _Unwind_* APIs from libc.so -> libunwind.so on arm32?)

Maybe the NDK could have a shared libunwind.so, but:

- I think we'd have trouble ensuring that all call sites consistently used an unwinder compatible with the current unwind context.
  - Even in theory, it'd be hard to prevent libc.so's unwinder from being used, in some situations. (e.g. If there are multiple app shared libraries, and only some of them throw exceptions, call _Unwind_Resume, etc. Maybe some libraries are built with -Wl,--as-needed. The dynamic linker might encounter libc.so before it encounters libunwind.so in a BFS walk of a dependency graph of an app library.)
  - If an app library links against another library exporting an incompatible unwinder, then things break.
- Presumably libc++_shared.so would have to use the NDK libunwind.so, and I'd guess the extra library would break the Android Gradle plugin? I'd guess the extra library would break other NDK users.

Maybe symbol versioning helps:

- Tag the public symbols in libunwind.so with a LIBUNWIND version.
- The _Unwind_* symbols in libc.so are tagged with LIBC_PRIVATE or (as of R) LIBC_R.
- For Android M and up, a relocation targeting _Unwind_*@@LIBUNWIND should never match an _Unwind_*@@LIBC_* in libc.so.
- Prior to Android M, the linker didn't implement symbol versioning, but symbol lookup ignores transitive DT_NEEDED dependencies <https://github.com/android/ndk/issues/1244#issuecomment-620974993>, so as long as libunwind.so appears before libc.so, the linker should find the right symbol.
- There will be warnings on L-MR1, but that's a known issue <https://github.com/android/ndk/issues/622> and maybe it can't be fixed.

My current plan is to continue linking a non-exported unwinder into every shared library that needs the unwinder. To do this, I'm assuming that it's safe to have duplicate copies of the same version of libunwind operate on the same unwind context. I think that's currently OK for libunwind and mostly OK for libgcc. For libgcc, there was a problem where something called _Unwind_GetGR without first calling uw_init_context to initialize dwarf_reg_size_table. I think(?) that was only a problem with HWASan, maybe? (internal link: http://b/144430859#comment8)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78787