[cmake] Split linked libraries into private & public, for linker script

[mgorny]

Summary

Introduce LIBCXX_LIBRARIES_PUBLIC in addition to LIBCXX_LIBRARIES that
holds 'public' interface libraries -- that is, libraries that both
libc++ links to and programs linked against it need to link to.

Currently this includes the ABI library and optionally -lunwind (when
LIBCXXABI_USE_LLVM_UNWINDER is on). The libraries are included in the
linker script, in order to make it possible to link C++ programs using
clang with compiler-rt runtime out-of-the-box.

[rmaprath]

The patch generally makes sense to me.

I wonder if the test configuration can also benefit from this; test/libcxx/test/target_info.py manually updates the link line to use -lgcc_s or -lunwind depending on how the build is configured. Presumably, this workaround will no longer be needed if the linker script does that for us?

(Note:- -lgcc_s is linked twice there to satisfy some weird dependency, it's not a mistake)

[mgorny]

In D25008#555041, @rmaprath wrote:

The patch generally makes sense to me.

I wonder if the test configuration can also benefit from this; test/libcxx/test/target_info.py manually updates the link line to use -lgcc_s or -lunwind depending on how the build is configured. Presumably, this workaround will no longer be needed if the linker script does that for us?

I was originally thinking about that but we don't use the linker script on APPLE and I have no clue how that system is handled or whether it suffers from the same issue at all.

(Note:- -lgcc_s is linked twice there to satisfy some weird dependency, it's not a mistake)

Thanks for letting me know. I almost prepared a patch for that ;-D.

[mgorny]

Side note: I see that gcc is solving similar problem via .spec files for its libraries, e.g.:

$ cat /usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libgomp.spec 
# This spec file is read by gcc when linking.  It is used to specify the
# standard libraries we need in order to link with libgomp.
*link_gomp: -lgomp %{static: -ldl }

If we wanted to be more portable, I guess we could establish something similar for clang, and install such a file along with libc++. However, I don't know if that's a problem.

[mgorny]

I will be reworking this to use CMake's PRIVATE and PUBLIC linked libs consistently across the build, linker scripts and possibly tests.

@EricWF, would you prefer if I killed the add_library_flags* macros or extended them to support specifying PRIVATE and PUBLIC libs separately?

[mgorny]

Here's my patch v2. I have to admit it's not very different from the original one but the goal is to provide a more generic interface for future improvement.

[EricWF]

lib/CMakeLists.txt

41 ↗

(On Diff #73840)

Please handle the special case for Apple here, where it manually re-exports the ABI lib.

86 ↗

(On Diff #73840)

I don't think libgcc_s should be considered a public library like libunwind because both Clang and GCC link it as a default system library.

179 ↗

(On Diff #73840)

Should the libraries be added using target_link_libraries(foo PRIVATE ...) and target_link_libraries(foo PUBLIC ...) now?

[mgorny]

lib/CMakeLists.txt

86 ↗

(On Diff #73840)

However, clang doesn't add it if you use compiler-rt. In which case you are left without an unwinder.

[mgorny]

Apple re-export and PRIVATE/PUBLIC covered.

[mgorny]

As discussed on IRC, limited the unwinder deps to -lunwind, and I'll work on making clang include -lunwind by default.

[EricWF]

One concern I have is being able to specify libraries in the correct order when we have two different lists. However I'm not concerned enough not to try it.

LGTM.

lib/CMakeLists.txt

43 ↗

(On Diff #74020)

Urg. I think this whole setup is borked on Apple. This was an existing issue though, so you don't have to fix it. this LGTM for now.

[Hahnfeld]

Hi Michal,

this currently breaks for me: -lunwind will find the system default libunwind.so.8 instead of LLVMs libunwind.so.1 which breaks some tests.
This has worked magically before because libc++ and libc++abi where explicitely linked against LLVMs libunwind.so.1. Could we force that in the linker script as well?

Thanks,
Jonas

[mgorny]

I think it'd be technically possible to force a specific SONAME in the linker script but I don't think that's a good idea. Clang should be able to use any libunwind implementation, and libc++ shouldn't really be encoding internal implementation details to the point of specific SONAME.

On the other hand, if we are doing a complete in-tree build of LLVM with libunwind, I guess it'd be reasonable to avoid combining the system library with the just-built library. Assuming we want to force 'our' library, and considering the fact that the tests are specifying all dependencies explicitly anyway, I think the most correct solution for that would be to skip the linker script for tests.

I see two possible solutions for that: either we pass full SONAME of the library when linking tests, or we provide an additional symlink to skip linker script. Gentoo already does the latter, and I wanted to add support for that into libcxx anyway, so that may be the solution. The idea is that besides libc++.so (the linker script), additional libc++_shared.so symlink is installed that references the library directly.

[Hahnfeld]

LIBCXXABI_USE_LLVM_UNWINDER implies to me: "Use LLVM's libunwind whenever you use libc++abi". This has worked until now and I would vote for this to be the right thing to do.

[joerg]

So the historical reason for the libgcc_s dance for glibc was the use of unwinding for thread cancellation. With all the glibc versions I can find, going back to SLES11 SP1, this is no longer the case. To check for yourself, look for _Unwind* symbols in libc.so and libc.a. Only findfde seems to be exposed now.

As such, I think we can avoid all this complexity.

[Hahnfeld]

What are the possibilities to proceed here?

[mgorny]

In D25008#581889, @Hahnfeld wrote:

What are the possibilities to proceed here?

To be honest, I don't have any good idea besides renaming the library. Keeping two non-interchangeable libraries under the same name is bound to fail.

[Hahnfeld]

Please disregard the noise, I think the error lays in the testing of compiler-rt which clears LIBRARY_PATH and therefore doesn't find the right libunwind!

libcxx/trunk/CMakeLists.txt

 set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})
 set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBCXX_LIBRARY_DIR})
 
 # Declare libc++ configuration variables.
 # They are intended for use as follows:
 # LIBCXX_CXX_FLAGS: General flags for both the compiler and linker.
 # LIBCXX_COMPILE_FLAGS: Compile only flags.
 # LIBCXX_LINK_FLAGS: Linker only flags.
+# LIBCXX_LIBRARIES: Private libraries libc++ is linked to.
+# LIBCXX_LIBRARIES_PUBLIC: Public libraries libc++ is linked to,
+#                          also exposed in the linker script.
 set(LIBCXX_COMPILE_FLAGS "")
 set(LIBCXX_LINK_FLAGS "")
 set(LIBCXX_LIBRARIES "")
+set(LIBCXX_LIBRARIES_PUBLIC "")
 
 # Include macros for adding and removing libc++ flags.
 include(HandleLibcxxFlags)
 
 # Target flags ================================================================
 # These flags get added to CMAKE_CXX_FLAGS and CMAKE_C_FLAGS so that
 # 'config-ix' use them during feature checks. It also adds them to both
 # 'LIBCXX_COMPILE_FLAGS' and 'LIBCXX_LINK_FLAGS'

libcxx/trunk/lib/CMakeLists.txt

if(NOT LIBCXX_INSTALL_LIBRARY)
   set(exclude_from_all EXCLUDE_FROM_ALL)
 endif()
 
 #if LIBCXX_CXX_ABI_LIBRARY_PATH is defined we want to add it to the search path.
 add_link_flags_if(LIBCXX_CXX_ABI_LIBRARY_PATH "-L${LIBCXX_CXX_ABI_LIBRARY_PATH}")
 
 add_library_flags_if(LIBCXX_COVERAGE_LIBRARY "${LIBCXX_COVERAGE_LIBRARY}")
 
-add_library_flags_if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "-Wl,--whole-archive" "-Wl,-Bstatic")
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  add_library_flags("-Wl,--whole-archive" "-Wl,-Bstatic")
   add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
-add_library_flags_if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "-Wl,-Bdynamic" "-Wl,--no-whole-archive")
+  add_library_flags("-Wl,-Bdynamic" "-Wl,--no-whole-archive")
+elseif (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
+                   LIBCXX_CXX_ABI_LIBNAME STREQUAL "none"))
+  # Apple re-exports libc++abi in libc++, so don't make it public
+  add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
+else()
+  list(APPEND LIBCXX_LIBRARIES_PUBLIC "${LIBCXX_CXX_ABI_LIBRARY}")
+endif()
 
 if (APPLE AND LLVM_USE_SANITIZER)
   if (("${LLVM_USE_SANITIZER}" STREQUAL "Address") OR
       ("${LLVM_USE_SANITIZER}" STREQUAL "Address;Undefined") OR
       ("${LLVM_USE_SANITIZER}" STREQUAL "Undefined;Address"))
     set(LIBFILE "libclang_rt.asan_osx_dynamic.dylib")
   elseif("${LLVM_USE_SANITIZER}" STREQUAL "Undefined")
     set(LIBFILE "libclang_rt.ubsan_osx_dynamic.dylib")
if (LIBFILE)
     set(LIBCXX_SANITIZER_LIBRARY "${LIBDIR}/${LIBFILE}")
     set(LIBCXX_SANITIZER_LIBRARY "${LIBCXX_SANITIZER_LIBRARY}" PARENT_SCOPE)
     message(STATUS "Manually linking compiler-rt library: ${LIBCXX_SANITIZER_LIBRARY}")
     add_library_flags("${LIBCXX_SANITIZER_LIBRARY}")
     add_link_flags("-Wl,-rpath,${LIBDIR}")
   endif()
 endif()
 
-# Generate library list.
+# Generate private library list.
 add_library_flags_if(LIBCXX_HAS_PTHREAD_LIB pthread)
 add_library_flags_if(LIBCXX_HAS_C_LIB c)
 add_library_flags_if(LIBCXX_HAS_M_LIB m)
 add_library_flags_if(LIBCXX_HAS_RT_LIB rt)
 add_library_flags_if(LIBCXX_HAS_GCC_S_LIB gcc_s)
 add_library_flags_if(LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB atomic)
 
+# Add the unwinder library.
+if (LIBCXXABI_USE_LLVM_UNWINDER)
+  list(APPEND LIBCXX_LIBRARIES_PUBLIC unwind)
+endif()
+
 # Setup flags.
 if (NOT WIN32)
   add_flags_if_supported(-fPIC)
 endif()
 add_link_flags_if_supported(-nodefaultlibs)
 
 if ( APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
                 LIBCXX_CXX_ABI_LIBNAME STREQUAL "none"))
PROPERTIES
     COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
 )
 
 set(LIBCXX_TARGETS)
 
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $<TARGET_OBJECTS:cxx_objects>)
-  target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
+  target_link_libraries(cxx_shared
+    PRIVATE ${LIBCXX_LIBRARIES}
+    PUBLIC ${LIBCXX_LIBRARIES_PUBLIC})
   set_target_properties(cxx_shared
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
       VERSION       "${LIBCXX_ABI_VERSION}.0"
       SOVERSION     "${LIBCXX_ABI_VERSION}"
   )
   list(APPEND LIBCXX_TARGETS "cxx_shared")
 endif()
 
 # Build the static library.
 if (LIBCXX_ENABLE_STATIC)
   add_library(cxx_static STATIC $<TARGET_OBJECTS:cxx_objects>)
-  target_link_libraries(cxx_static ${LIBCXX_LIBRARIES})
+  target_link_libraries(cxx_static
+    PRIVATE ${LIBCXX_LIBRARIES}
+    PUBLIC ${LIBCXX_LIBRARIES_PUBLIC})
   set_target_properties(cxx_static
     PROPERTIES
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
   )
   list(APPEND LIBCXX_TARGETS "cxx_static")
 endif()
 
if (LIBCXX_ENABLE_SHARED AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
   endif()
   # Generate a linker script inplace of a libc++.so symlink. Rerun this command
   # after cxx builds.
   add_custom_command(TARGET cxx_shared POST_BUILD
     COMMAND
       ${PYTHON_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/gen_link_script/gen_link_script.py
     ARGS
       "$<TARGET_LINKER_FILE:cxx_shared>"
-      "${SCRIPT_ABI_LIBNAME}"
+      "\"${LIBCXX_LIBRARIES_PUBLIC}\""
     WORKING_DIRECTORY ${LIBCXX_BUILD_DIR}
   )
 endif()
 
 if (LIBCXX_INSTALL_LIBRARY)
   if (LIBCXX_INSTALL_EXPERIMENTAL_LIBRARY)
     set(experimental_lib cxx_experimental)
   endif()

libcxx/trunk/utils/gen_link_script/gen_link_script.py

 
 def usage_and_exit():
     print_and_exit("Usage: ./gen_link_script.py [--help] [--dryrun] <path/to/libcxx.so> <abi_libname>")
 
 def help_and_exit():
     help_msg = \
 """Usage
 
-  gen_link_script.py [--help] [--dryrun] <path/to/libcxx.so> <abi_libname>
+  gen_link_script.py [--help] [--dryrun] <path/to/libcxx.so> <public_libs>
 
   Generate a linker script that links libc++ to the proper ABI library.
   The script replaces the specified libc++ symlink.
   An example script for c++abi would look like "INPUT(libc++.so.1 -lc++abi)".
 
 Arguments
   <path/to/libcxx.so> - The top level symlink to the versioned libc++ shared
                         library. This file is replaced with a linker script.
-  <abi_libname>       - The name of the ABI library to use in the linker script.
-                        The name must be one of [c++abi, stdc++, supc++, cxxrt].
+  <public_libs>       - List of library names to include in linker script.
 
 Exit Status:
   0 if OK,
   1 if the action failed.
 """
     print_and_exit(help_msg)
 
 def parse_args():
     args = list(sys.argv)
     del args[0]
     if len(args) == 0:
         usage_and_exit()
     if args[0] == '--help':
         help_and_exit()
     dryrun = '--dryrun' == args[0]
     if dryrun:
         del args[0]
     if len(args) != 2:
         usage_and_exit()
     symlink_file = args[0]
-    abi_libname = args[1]
-    return dryrun, symlink_file, abi_libname
+    public_libs = args[1].split(';')
+    return dryrun, symlink_file, public_libs
 
 def main():
-    dryrun, symlink_file, abi_libname = parse_args()
+    dryrun, symlink_file, public_libs = parse_args()
 
     # Check that the given libc++.so file is a valid symlink.
     if not os.path.islink(symlink_file):
         print_and_exit("symlink file %s is not a symlink" % symlink_file)
 
     # Read the symlink so we know what libc++ to link to in the linker script.
     linked_libcxx = os.readlink(symlink_file)
 
-    # Check that the abi_libname is one of the supported values.
-    supported_abi_list = ['c++abi', 'stdc++', 'supc++', 'cxxrt']
-    if abi_libname not in supported_abi_list:
-        print_and_exit("abi name '%s' is not supported: Use one of %r" %
-                        (abi_libname, supported_abi_list))
+    # Prepare the list of public libraries to link.
+    public_libs = ['-l%s' % l for l in public_libs]
 
     # Generate the linker script contents and print the script and destination
     # information.
-    contents = "INPUT(%s -l%s)" % (linked_libcxx, abi_libname)
+    contents = "INPUT(%s %s)" % (linked_libcxx, ' '.join(public_libs))
     print("GENERATING SCRIPT: '%s' as file %s" % (contents, symlink_file))
 
     # Remove the existing libc++ symlink and replace it with the script.
     if not dryrun:
         os.unlink(symlink_file)
         with open(symlink_file, 'w') as f:
             f.write(contents + "\n")
 
 
 if __name__ == '__main__':
     main()