[runtimes] Support ELF dependent libraries feature

[phosek]

Summary

As of r360984, LLD supports dependent libraries feature for ELF.
libunwind and libc++ have library dependencies: libdl and libpthread
respectively, which means that when libunwind and libc++ are being
statically linked (using -static-libstdc++ flag), user has to manually
specify -ldl -lpthread which is onerous. This change includes the
lib pragma to specify the library dependencies directly in the source
that uses those libraries. This doesn't make any difference when using
linkers that don't support dependent libraries. However, when using
LLD that has dependent libraries feature, users no longer have to
manually specifying library dependencies when using static linking,
linker will pick them up automatically. The use of this feature is
conditionalized on CMake build option and can be enabled by vendors.

[phosek]

Would it be possible to take a look? I believe this is a better alternative to both D54724 and D54726 which doesn't require any changes to the driver or special linker scripts.

[ldionne]

I guess I don't have any strong objection, but I think I had a preference for handling it in the build system per https://reviews.llvm.org/D54726. This would conceivably work on other platforms that support linker scripts and it's less intrusive to the source code. But if you feel strongly that this is the right way to go, I'm OK with it.

[bd1976llvm]

Great to see someone wanting to use this. During the RFC we decided not to set SHF_EXCLUDE on the .deplibs ELF section that is used to store the strings from these pragmas. This means that other linkers that don't support dependent libraries will not remove the sections and they will end up in the output. This shouldn't cause any functional issues; however, it might be noticed by developers and commented on. Perhaps we should change this decision? Also, have you considered the other option of having the build system invoke the clang with the --dependent-library=<specifier> switch. This is an alternative approach where modifying the source code is not appropriate, an example use is: https://reviews.llvm.org/D61742.

[phosek]

In D62090#1510537, @ldionne wrote:

I guess I don't have any strong objection, but I think I had a preference for handling it in the build system per https://reviews.llvm.org/D54726. This would conceivably work on other platforms that support linker scripts and it's less intrusive to the source code. But if you feel strongly that this is the right way to go, I'm OK with it.

I have a slight preference for this approach for two reasons: (1) with linker scripts we have to rename the static library which may lead to some confusion, e.g. simply running readelf -sW libc++.a will fail because libc++.a is now a linker script; (2) it localizes the dependency information to where it's needed, e.g. the pthread library dependency is only included if _LIBCPP_HAS_THREAD_API_PTHREAD is defined.

In D62090#1510816, @bd1976llvm wrote:

Great to see someone wanting to use this. During the RFC we decided not to set SHF_EXCLUDE on the .deplibs ELF section that is used to store the strings from these pragmas. This means that other linkers that don't support dependent libraries will not remove the sections and they will end up in the output. This shouldn't cause any functional issues; however, it might be noticed by developers and commented on. Perhaps we should change this decision?

I think we should consider that.

In D62090#1510816, @bd1976llvm wrote:

Also, have you considered the other option of having the build system invoke the clang with the --dependent-library=<specifier> switch. This is an alternative approach where modifying the source code is not appropriate, an example use is: https://reviews.llvm.org/D61742.

That might be an option as well and is something that can be done from CMake invocation via CFLAGS. The downside is AFAICT that the information will no longer be localized to where it's needed and instead applied to all translation units inside the archive e.g. even if you use part of libc++ that doesn't need pthread, you'll still get the dependency. An alternative would be to set that flag on a per translation unit basis but I'm not sure whether that's better than having the pragma directly in the source.

[ldionne]

I don't have any objections to this (but it doesn't impact my platform).

[phosek]

I've conditionalized the use of this feature on CMake build option which defaults to OFF and can be turned on by vendors as needed. Hopefully that's cleaner than the previous version where it was enabled unconditionally.

[ldionne]

It's not entirely clear to me that adding an option to control this is better than just doing it unconditionally. Have you run into a scenario where you didn't want to enable it (and if so, why?).

If that's "just in case", I'd suggest doing it unconditionally and adding an option later if a vendor wants to disable it.

libcxx/CMakeLists.txt

547 ↗

(On Diff #201812)

Please use the per-target form. We've done a lot of work to remove global definitions and flags, let's keep working in that direction.

[phosek]

In D62090#1521187, @ldionne wrote:

It's not entirely clear to me that adding an option to control this is better than just doing it unconditionally. Have you run into a scenario where you didn't want to enable it (and if so, why?).

If that's "just in case", I'd suggest doing it unconditionally and adding an option later if a vendor wants to disable it.

That makes sense, I've removed the option but I've moved the pragma from headers into source files to avoid it being injected into users code through includes.

libcxx/include/__threading_support

 
 #if defined(_LIBCPP_HAS_THREAD_API_EXTERNAL)
 # include <__external_threading>
 #elif !defined(_LIBCPP_HAS_NO_THREADS)
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD)
 # include <pthread.h>
 # include <sched.h>
+#if defined(__clang__) && defined(__ELF__) && defined(__linux__)
+#pragma comment(lib, "pthread")
+#endif
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 
 #if defined(_LIBCPP_HAS_THREAD_LIBRARY_EXTERNAL) || \
     defined(_LIBCPP_BUILDING_THREAD_LIBRARY_EXTERNAL) || \
     defined(_LIBCPP_HAS_THREAD_API_WIN32)

libunwind/src/AddressSpace.hpp

#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32)
     #define _LIBUNWIND_USE_DLADDR 1
   #else
     #define _LIBUNWIND_USE_DLADDR 0
   #endif
 #endif
 
 #if _LIBUNWIND_USE_DLADDR
 #include <dlfcn.h>
+#if defined(__clang__) && defined(__ELF__) && defined(__linux__)
+#pragma comment(lib, "dl")
+#endif
 #endif
 
 #ifdef __APPLE__
 #include <mach-o/getsect.h>
 namespace libunwind {
    bool checkKeyMgrRegisteredFDEs(uintptr_t targetAddr, void *&fde);
 }
 #endif