Page MenuHomePhabricator

[libcxx][libcxxabi] Link against libdl on Android
Needs ReviewPublic

Authored by smeenai on Wed, May 13, 1:11 PM.

Details

Reviewers
danalbert
enh
ldionne
phosek
rprichard
srhines
Group Reviewers
Restricted Project
Restricted Project
Summary

libgcc's DWARF unwinder (used for all Android targets except armv7) uses
dl_iterate_phdr, so we need to link against libdl to make the link
succeed. The driver ordinarily injects libdl for you, but we prevent the
driver's default library additions so we have to add it ourselves.
Ideally the libgcc linker script in the NDK would be adjusted to add the
libdl link, but until we have an NDK with that change, work around it in
the libc++ and libc++abi build systems.

Note that LIBCXXABI_HAS_DL_LIB was defined but previously unused, so I'm
just repurposing it. libunwind already links against libdl so no changes
are needed there.

Diff Detail

Event Timeline

smeenai created this revision.Wed, May 13, 1:11 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptWed, May 13, 1:11 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
smeenai added a comment.EditedWed, May 13, 1:12 PM

This is a follow-up to D78787. I'd only tested that one with armv7, so I didn't notice this earlier.

(To be more precise, I'd tested the other architectures, but with an inadvertent config change which meant this issue wasn't exposed.)

Why can we not use ${CMAKE_DL_LIBS} instead of explicitly detecting libdl and linking against it?

Why can we not use ${CMAKE_DL_LIBS} instead of explicitly detecting libdl and linking against it?

Because I wasn't aware of its existence :D Happy to switch to that.

Just to clarify, LIBCXX[ABI]_HAS_DL_LIB doesn't just mean "we can link against libdl.so", but more specifically, "do link libc++[abi].so against libdl.so"? i.e. On GNU/Linux, we could link libc++[abi].so against libdl.so, but we don't, because we don't need to, because the unwinder always comes from libgcc_s.so.

libgcc's DWARF unwinder (used for all Android targets except armv7) uses
dl_iterate_phdr, so we need to link against libdl to make the link
succeed. The driver ordinarily injects libdl for you, but we prevent the
driver's default library additions so we have to add it ourselves.
Ideally the libgcc linker script in the NDK would be adjusted to add the
libdl link, but until we have an NDK with that change, work around it in
the libc++ and libc++abi build systems.

The logic here also applies to arm32 too, I think, where the arm32 libunwind needs dl_unwind_find_exidx from libdl.so (or libc.a). It looks like the NDK's arm32 linker script already includes -ldl, though, so maybe we can add it to other architectures.

It might do the wrong thing for static executables, though. For Android static executables, libdl.a has dummy implementations of dl* APIs that do nothing and sometimes report failure. dl_iterate_phdr (and dl_unwind_find_exidx), on the other hand, now live in libc.a and do something reasonable for unwinding when there is only one ELF binary. We had a complaint about the platform build system when libdl.a was automatically linked into static executables -- someone had code that called dlopen+dlsym, and they preferred a link-time error rather than a run-time error.

So, maybe the arm32 libgcc.a script shouldn't link against -ldl, and we should generally rely on the driver inserting -ldl for dynamic exes/libs.

I don't think we should make either change to libgcc.a, though, given that we want to migrate to compiler-rt + libunwind. e.g. If Android starts defaulting to -rtlib=compiler-rt, then there is no file named libgcc.a, and the driver implicitly links against libclang_rt.builtins.*.a instead. It's not obvious to me then whether the driver would implicitly link against libunwind.a, or whether it would simply omit the unwinder. LLVM's -rtlib=compiler-rt behavior seems to prefer omitting the unwinder.

e.g. I looked at the Fuchsia LLVM configuration, and it uses a libc++.so linker script that has -lunwind. There is also a libc++.a that subsumes the object files from libc++abi.a and libunwind.a. IIUC, a C program (or C++ program with no STL) isn't automatically linked against an unwinder. A C program that calls _Unwind_Backtrace must explicitly link -lunwind. Maybe the Android NDK would work the same way, maybe the driver would differ (e.g. have Linux::GetUnwindLibType() return ToolChain::UNW_CompilerRT). Or, we could even match FreeBSD 12.1, which seems to use compiler-rt + libunwind, but access them using the libgcc{.a,_eh.a,_s.so} names.

Why can we not use ${CMAKE_DL_LIBS} instead of explicitly detecting libdl and linking against it?

Because I wasn't aware of its existence :D Happy to switch to that.

It looks like glibc/musl's libc.so exports dl_iterate_phdr, so even libgcc_s.so doesn't depend on libdl.so. Bionic's libc.so mostly doesn't expose the unwinding dl* APIs, but its libc.a does. (exception: arm32 libc.so exports __gnu_Unwind_Find_exidx)

I would guess that using ${CMAKE_DL_LIBS} would link libc++abi.so against libdl.so on GNU/Linux systems, which is unnecessary and would make any C++ program implicitly link against libdl.so?

Given that ${CMAKE_DL_LIBS} is specified by CMake, I'm wondering how well it works with cross-compilation. I'd guess that it ought to work.

Just to clarify, LIBCXX[ABI]_HAS_DL_LIB doesn't just mean "we can link against libdl.so", but more specifically, "do link libc++[abi].so against libdl.so"? i.e. On GNU/Linux, we could link libc++[abi].so against libdl.so, but we don't, because we don't need to, because the unwinder always comes from libgcc_s.so.

Right. I can come up with a better name.

libgcc's DWARF unwinder (used for all Android targets except armv7) uses
dl_iterate_phdr, so we need to link against libdl to make the link
succeed. The driver ordinarily injects libdl for you, but we prevent the
driver's default library additions so we have to add it ourselves.
Ideally the libgcc linker script in the NDK would be adjusted to add the
libdl link, but until we have an NDK with that change, work around it in
the libc++ and libc++abi build systems.

The logic here also applies to arm32 too, I think, where the arm32 libunwind needs dl_unwind_find_exidx from libdl.so (or libc.a). It looks like the NDK's arm32 linker script already includes -ldl, though, so maybe we can add it to other architectures.

It might do the wrong thing for static executables, though. For Android static executables, libdl.a has dummy implementations of dl* APIs that do nothing and sometimes report failure. dl_iterate_phdr (and dl_unwind_find_exidx), on the other hand, now live in libc.a and do something reasonable for unwinding when there is only one ELF binary. We had a complaint about the platform build system when libdl.a was automatically linked into static executables -- someone had code that called dlopen+dlsym, and they preferred a link-time error rather than a run-time error.

So, maybe the arm32 libgcc.a script shouldn't link against -ldl, and we should generally rely on the driver inserting -ldl for dynamic exes/libs.

I don't think we should make either change to libgcc.a, though, given that we want to migrate to compiler-rt + libunwind. e.g. If Android starts defaulting to -rtlib=compiler-rt, then there is no file named libgcc.a, and the driver implicitly links against libclang_rt.builtins.*.a instead. It's not obvious to me then whether the driver would implicitly link against libunwind.a, or whether it would simply omit the unwinder. LLVM's -rtlib=compiler-rt behavior seems to prefer omitting the unwinder.

e.g. I looked at the Fuchsia LLVM configuration, and it uses a libc++.so linker script that has -lunwind. There is also a libc++.a that subsumes the object files from libc++abi.a and libunwind.a. IIUC, a C program (or C++ program with no STL) isn't automatically linked against an unwinder. A C program that calls _Unwind_Backtrace must explicitly link -lunwind. Maybe the Android NDK would work the same way, maybe the driver would differ (e.g. have Linux::GetUnwindLibType() return ToolChain::UNW_CompilerRT). Or, we could even match FreeBSD 12.1, which seems to use compiler-rt + libunwind, but access them using the libgcc{.a,_eh.a,_s.so} names.

Good points. I would imagine linking against the unwinder explicitly when you use it is okay, though I haven't thought about it too hard.

The current driver logic to add -ldl to all non-static Android links is specific to libgcc right now though (it lives in AddLibgcc), so it'd need some adjustment either way (unless you want anyone linking against libunwind manually to also link against libdl manually, and assuming a static libunwind, which is what I understood from D78787).

Why can we not use ${CMAKE_DL_LIBS} instead of explicitly detecting libdl and linking against it?

Because I wasn't aware of its existence :D Happy to switch to that.

It looks like glibc/musl's libc.so exports dl_iterate_phdr, so even libgcc_s.so doesn't depend on libdl.so. Bionic's libc.so mostly doesn't expose the unwinding dl* APIs, but its libc.a does. (exception: arm32 libc.so exports __gnu_Unwind_Find_exidx)

I would guess that using ${CMAKE_DL_LIBS} would link libc++abi.so against libdl.so on GNU/Linux systems, which is unnecessary and would make any C++ program implicitly link against libdl.so?

Given that ${CMAKE_DL_LIBS} is specified by CMake, I'm wondering how well it works with cross-compilation. I'd guess that it ought to work.

Yeah, I'll think about how to make the logic here better (specific to Android, and ideally not breaking static builds).