[libc++] Correct libcxx default linker script behavior #74130
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Patch 4b1fe09 introduced a bug when building libc++ for Fuchsia, it disabled the libc++.so linker script by default. This patch restores its original behavior.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Haowei (zeroomega) ChangesPatch 4b1fe09 introduced a bug when building libc++ for Fuchsia, it disabled the libc++.so linker script by default. This patch restores its original behavior. Full diff: https://github.com/llvm/llvm-project/pull/74130.diff 1 Files Affected:
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 8572e7530fd363e..31fc9cec1c57f25 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -251,14 +251,14 @@ option(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
# Generate and install a linker script inplace of libc++.so. The linker script
# will link libc++ to the correct ABI library. This option is on by default
-# on UNIX platforms other than Apple unless we statically link libc++abi
-# inside libc++.so, we don't build libc++.so at all or we don't have any
-# ABI library.
+# on UNIX platforms other than Apple, and on the Fuchsia platform unless we
+# statically link libc++abi inside libc++.so, we don't build libc++.so at all
+# or we don't have any ABI library.
if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
OR NOT LIBCXX_ENABLE_SHARED
OR LIBCXX_CXX_ABI STREQUAL "none")
set(ENABLE_LINKER_SCRIPT_DEFAULT_VALUE OFF)
-elseif(UNIX AND NOT APPLE)
+elseif((UNIX OR FUCHSIA) AND NOT APPLE)
set(ENABLE_LINKER_SCRIPT_DEFAULT_VALUE ON)
else()
set(ENABLE_LINKER_SCRIPT_DEFAULT_VALUE OFF)
|
zeroomega
referenced
this pull request
Dec 1, 2023
We really shouldn't be depending on far away configuration options like LLVM_HAVE_LINK_VERSION_SCRIPT here. This patch simplifies the enablement of the linker scripts and as a result gets rid of an undesirable dependency on HandleLLVMOptions.cmake. As a drive-by, the patch also stops taking into account whether Python3 is available. This should have no bearing on whether we generate a linker script or not, which is required for correctness. If someone tries to build libc++ and generate a linker script but Python3 is not available, they should get an error instead of silently getting an incorrect installation of the library.
petrhosek
approved these changes
Dec 1, 2023
philnik777
approved these changes
Dec 1, 2023
ldionne
approved these changes
Dec 1, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Patch 4b1fe09 introduced a bug when building libc++ for Fuchsia, it disabled the libc++.so linker script by default. This patch restores its original behavior.