[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
Hahnfeld added a comment. 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`! Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
mgorny added a comment. In https://reviews.llvm.org/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. Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
Hahnfeld added a comment. What are the possibilities to proceed here? Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
joerg added a comment. 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. Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
Hahnfeld added a comment. `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. Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
mgorny added a comment. 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. Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
Hahnfeld added a comment. 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 Repository: rL LLVM https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
This revision was automatically updated to reflect the committed changes.
Closed by commit rL283659: [cmake] Split linked libraries into private &
public, for linker script (authored by mgorny).
Changed prior to commit:
https://reviews.llvm.org/D25008?vs=74020=74034#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25008
Files:
libcxx/trunk/CMakeLists.txt
libcxx/trunk/lib/CMakeLists.txt
libcxx/trunk/utils/gen_link_script/gen_link_script.py
Index: libcxx/trunk/utils/gen_link_script/gen_link_script.py
===
--- libcxx/trunk/utils/gen_link_script/gen_link_script.py
+++ libcxx/trunk/utils/gen_link_script/gen_link_script.py
@@ -22,17 +22,16 @@
help_msg = \
"""Usage
- gen_link_script.py [--help] [--dryrun]
+ gen_link_script.py [--help] [--dryrun]
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
- The top level symlink to the versioned libc++ shared
library. This file is replaced with a linker script.
- - The name of the ABI library to use in the linker script.
-The name must be one of [c++abi, stdc++, supc++, cxxrt].
+ - List of library names to include in linker script.
Exit Status:
0 if OK,
@@ -53,28 +52,25 @@
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.
Index: libcxx/trunk/CMakeLists.txt
===
--- libcxx/trunk/CMakeLists.txt
+++ libcxx/trunk/CMakeLists.txt
@@ -270,9 +270,13 @@
# 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)
Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -33,9 +33,17 @@
add_library_flags_if(LIBCXX_COVERAGE_LIBRARY "${LIBCXX_COVERAGE_LIBRARY}")
-add_library_flags_if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "-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")
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+ add_library_flags("-Wl,--whole-archive" "-Wl,-Bstatic")
+ add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
+ 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
@@ -67,14 +75,19 @@
endif()
endif()
-# Generate library list.
+# Generate private library list.
add_library_flags_if(LIBCXX_HAS_PTHREAD_LIB pthread)
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.
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.
Comment at: lib/CMakeLists.txt:43
+ # Apple re-exports libc++abi in libc++, so don't make it public
+ add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
+else()
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.
https://reviews.llvm.org/D25008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
mgorny marked 2 inline comments as done. mgorny added a comment. As discussed on IRC, limited the unwinder deps to -lunwind, and I'll work on making clang include -lunwind by default. https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
mgorny updated the summary for this revision.
mgorny updated this revision to Diff 74020.
https://reviews.llvm.org/D25008
Files:
CMakeLists.txt
lib/CMakeLists.txt
utils/gen_link_script/gen_link_script.py
Index: utils/gen_link_script/gen_link_script.py
===
--- utils/gen_link_script/gen_link_script.py
+++ utils/gen_link_script/gen_link_script.py
@@ -22,17 +22,16 @@
help_msg = \
"""Usage
- gen_link_script.py [--help] [--dryrun]
+ gen_link_script.py [--help] [--dryrun]
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
- The top level symlink to the versioned libc++ shared
library. This file is replaced with a linker script.
- - The name of the ABI library to use in the linker script.
-The name must be one of [c++abi, stdc++, supc++, cxxrt].
+ - List of library names to include in linker script.
Exit Status:
0 if OK,
@@ -53,28 +52,25 @@
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.
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -33,9 +33,17 @@
add_library_flags_if(LIBCXX_COVERAGE_LIBRARY "${LIBCXX_COVERAGE_LIBRARY}")
-add_library_flags_if(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "-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")
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+ add_library_flags("-Wl,--whole-archive" "-Wl,-Bstatic")
+ add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
+ 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
@@ -67,14 +75,19 @@
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)
@@ -151,7 +164,9 @@
# Build the shared library.
if (LIBCXX_ENABLE_SHARED)
add_library(cxx_shared SHARED $)
- 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}"
@@ -165,7 +180,9 @@
# Build the static library.
if (LIBCXX_ENABLE_STATIC)
add_library(cxx_static STATIC $)
- target_link_libraries(cxx_static ${LIBCXX_LIBRARIES})
+ target_link_libraries(cxx_static
+PRIVATE ${LIBCXX_LIBRARIES}
+PUBLIC
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
mgorny marked 2 inline comments as done. mgorny added inline comments. Comment at: lib/CMakeLists.txt:86 +elseif (LIBCXX_HAS_GCC_S_LIB) + list(APPEND LIBCXX_LIBRARIES_PUBLIC gcc_s) +endif() EricWF wrote: > 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. However, clang doesn't add it if you use compiler-rt. In which case you are left without an unwinder. https://reviews.llvm.org/D25008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25008: [cmake] Split linked libraries into private & public, for linker script
EricWF added inline comments.
Comment at: lib/CMakeLists.txt:41
+else()
+ list(APPEND LIBCXX_LIBRARIES_PUBLIC "${LIBCXX_CXX_ABI_LIBRARY}")
+endif()
Please handle the special case for Apple here, where it manually re-exports the
ABI lib.
Comment at: lib/CMakeLists.txt:86
+elseif (LIBCXX_HAS_GCC_S_LIB)
+ list(APPEND LIBCXX_LIBRARIES_PUBLIC gcc_s)
+endif()
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.
Comment at: lib/CMakeLists.txt:179
add_library(cxx_static STATIC $)
- target_link_libraries(cxx_static ${LIBCXX_LIBRARIES})
+ target_link_libraries(cxx_static ${LIBCXX_LIBRARIES}
${LIBCXX_LIBRARIES_PUBLIC})
set_target_properties(cxx_static
Should the libraries be added using `target_link_libraries(foo PRIVATE ...)`
and `target_link_libraries(foo PUBLIC ...)` now?
https://reviews.llvm.org/D25008
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
