[libcxx-commits] [libcxx] a6e5563 - [libc++][release] Do not force building the runtimes with -fPIC
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Dec 8 08:35:23 PST 2021
Author: Louis Dionne
Date: 2021-12-08T11:34:35-05:00
New Revision: a6e5563dfaff0cb7147058b9c49e38b611a28fb1
URL: https://github.com/llvm/llvm-project/commit/a6e5563dfaff0cb7147058b9c49e38b611a28fb1
DIFF: https://github.com/llvm/llvm-project/commit/a6e5563dfaff0cb7147058b9c49e38b611a28fb1.diff
LOG: [libc++][release] Do not force building the runtimes with -fPIC
There's a lot of history behind this, so here's a summary:
1. I stopped forcing -fPIC when building the runtimes in 30f305efe279,
before the LLVM 9 release back in 2019.
2. Someone complained that libc++.a couldn't be used in shared libraries
built without -fPIC (http://llvm.org/PR43604) since the LLVM 9 release.
This had been caused by my removal of -fPIC when building libc++.a in (1).
3. I suggested two ways of fixing the issue, the first being to force
-fPIC back unconditionally (http://llvm.org/D104328), and the second
being to specify that option explicitly when building the LLVM release
(http://llvm.org/D104327). We converged on the first solution.
4. I landed D104328, which forced building the runtimes with -fPIC.
This was included in the LLVM 13.0 release.
5. People complained about that and requested that we be able to
customize this setting (basically we should have done the second
solution).
This patch makes it such that the LLVM release script will specifically
ask for building with -fPIC using CMAKE_POSITION_INDEPENDENT_CODE,
however by default the runtimes will not force that option onto users.
This patch has the unintended effect that Clang and the LLVM libraries
(not only the runtime ones like libc++) will also be built with -fPIC
in the release. It would be better if we could specify that -fPIC is to
be used only when building the runtimes, however this is left as a
future improvement. The release should probably be using a bootstrapping
build and passing those options to the stage that builds the runtimes
only, see https://reviews.llvm.org/D112748 for that change.
Differential Revision: https://reviews.llvm.org/D110261
Added:
Modified:
libcxx/docs/ReleaseNotes.rst
libcxx/src/CMakeLists.txt
libcxx/test/libcxx/debug/extern-templates.sh.cpp
libcxx/utils/libcxx/test/dsl.py
libcxx/utils/libcxx/test/features.py
libcxxabi/src/CMakeLists.txt
libunwind/src/CMakeLists.txt
llvm/utils/release/test-release.sh
Removed:
################################################################################
diff --git a/libcxx/docs/ReleaseNotes.rst b/libcxx/docs/ReleaseNotes.rst
index 42e85f8cc8935..e789ba0f15b03 100644
--- a/libcxx/docs/ReleaseNotes.rst
+++ b/libcxx/docs/ReleaseNotes.rst
@@ -157,3 +157,9 @@ Build System Changes
.. code-block:: bash
-DLLVM_RUNTIME_TARGETS=i386-unknown-linux
+
+- Libc++, libc++abi and libunwind will not be built with ``-fPIC`` by default anymore.
+ If you want to build those runtimes with position independent code, please specify
+ ``-DCMAKE_POSITION_INDEPENDENT_CODE=ON`` explicitly when configuring the build, or
+ ``-DRUNTIMES_<target-name>_CMAKE_POSITION_INDEPENDENT_CODE=ON`` if using the
+ bootstrapping build.
diff --git a/libcxx/src/CMakeLists.txt b/libcxx/src/CMakeLists.txt
index c0e87bbcf342c..999f97b8aaa86 100644
--- a/libcxx/src/CMakeLists.txt
+++ b/libcxx/src/CMakeLists.txt
@@ -208,7 +208,6 @@ if (LIBCXX_ENABLE_SHARED)
VERSION "${LIBCXX_ABI_VERSION}.0"
SOVERSION "${LIBCXX_ABI_VERSION}"
DEFINE_SYMBOL ""
- POSITION_INDEPENDENT_CODE ON
)
cxx_add_common_build_flags(cxx_shared)
cxx_set_common_defines(cxx_shared)
@@ -284,7 +283,6 @@ if (LIBCXX_ENABLE_STATIC)
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS "${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
- POSITION_INDEPENDENT_CODE ON
)
cxx_add_common_build_flags(cxx_static)
cxx_set_common_defines(cxx_static)
diff --git a/libcxx/test/libcxx/debug/extern-templates.sh.cpp b/libcxx/test/libcxx/debug/extern-templates.sh.cpp
index 40142931c6e5f..0d1b052f07867 100644
--- a/libcxx/test/libcxx/debug/extern-templates.sh.cpp
+++ b/libcxx/test/libcxx/debug/extern-templates.sh.cpp
@@ -14,6 +14,7 @@
// UNSUPPORTED: libcxx-no-debug-mode
// UNSUPPORTED: libcpp-has-no-localization
+// UNSUPPORTED: cant-build-shared-library
// This test relies on linking a shared library and then passing that shared
// library as input when linking an executable; this is generally not supported
@@ -23,7 +24,7 @@
// UNSUPPORTED: windows
// RUN: %{cxx} %{flags} %{compile_flags} %s %{link_flags} -fPIC -DTU1 -D_LIBCPP_DEBUG=1 -fvisibility=hidden -shared -o %t.lib
-// RUN: cd %T && %{cxx} %{flags} %{compile_flags} %s ./%basename_t.tmp.lib %{link_flags} -fPIC -DTU2 -D_LIBCPP_DEBUG=1 -fvisibility=hidden -o %t.exe
+// RUN: cd %T && %{cxx} %{flags} %{compile_flags} %s ./%basename_t.tmp.lib %{link_flags} -DTU2 -D_LIBCPP_DEBUG=1 -fvisibility=hidden -o %t.exe
// RUN: %{exec} %t.exe
#include <cassert>
diff --git a/libcxx/utils/libcxx/test/dsl.py b/libcxx/utils/libcxx/test/dsl.py
index 9042e7c21a0a4..c9e0056725351 100644
--- a/libcxx/utils/libcxx/test/dsl.py
+++ b/libcxx/utils/libcxx/test/dsl.py
@@ -112,18 +112,19 @@ def __exit__(self, *args):
os.remove(tmp.name)
return TestWrapper(suite, pathInSuite, config)
- at _memoizeExpensiveOperation(lambda c, s: (c.substitutions, c.environment, s))
-def sourceBuilds(config, source):
+ at _memoizeExpensiveOperation(lambda c, s, f=[]: (c.substitutions, c.environment, s, f))
+def sourceBuilds(config, source, additionalFlags=[]):
"""
Return whether the program in the given string builds successfully.
This is done by compiling and linking a program that consists of the given
- source with the %{cxx} substitution, and seeing whether that succeeds.
+ source with the %{cxx} substitution, and seeing whether that succeeds. If
+ any additional flags are passed, they are appended to the compiler invocation.
"""
with _makeConfigTest(config) as test:
with open(test.getSourcePath(), 'w') as sourceFile:
sourceFile.write(source)
- _, _, exitCode, _ = _executeScriptInternal(test, ['%{build}'])
+ _, _, exitCode, _ = _executeScriptInternal(test, ['%{{build}} {}'.format(' '.join(additionalFlags))])
return exitCode == 0
@_memoizeExpensiveOperation(lambda c, p, args=None: (c.substitutions, c.environment, p, args))
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 26ee17ecc77e0..f70a8613e1d5d 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -59,6 +59,19 @@
int main(int, char**) { return x.is_lock_free(); }
""")),
+ # Some tests rely on creating shared libraries which link in the C++ Standard Library. In some
+ # cases, this doesn't work (e.g. if the library was built as a static archive and wasn't compiled
+ # as position independent). This feature informs the test suite of whether it's possible to create
+ # a shared library in a shell test by using the '-shared' compiler flag.
+ #
+ # Note: To implement this check properly, we need to make sure that we use something inside the
+ # compiled library, not only in the headers. It should be safe to assume that all implementations
+ # define `operator new` in the compiled library.
+ Feature(name='cant-build-shared-library',
+ when=lambda cfg: not sourceBuilds(cfg, """
+ void f() { new int(3); }
+ """, ['-shared'])),
+
Feature(name='apple-clang', when=_isAppleClang),
Feature(name=lambda cfg: 'apple-clang-{__clang_major__}'.format(**compilerMacros(cfg)), when=_isAppleClang),
Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)), when=_isAppleClang),
diff --git a/libcxxabi/src/CMakeLists.txt b/libcxxabi/src/CMakeLists.txt
index c237fb19f84f9..762649a965ca2 100644
--- a/libcxxabi/src/CMakeLists.txt
+++ b/libcxxabi/src/CMakeLists.txt
@@ -192,7 +192,6 @@ if (LIBCXXABI_ENABLE_SHARED)
SOVERSION "1"
VERSION "${LIBCXXABI_LIBRARY_VERSION}"
DEFINE_SYMBOL ""
- POSITION_INDEPENDENT_CODE ON
)
list(APPEND LIBCXXABI_BUILD_TARGETS "cxxabi_shared")
@@ -245,7 +244,6 @@ if (LIBCXXABI_ENABLE_STATIC)
COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
LINK_FLAGS "${LIBCXXABI_LINK_FLAGS}"
OUTPUT_NAME "c++abi"
- POSITION_INDEPENDENT_CODE ON
)
if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
diff --git a/libunwind/src/CMakeLists.txt b/libunwind/src/CMakeLists.txt
index 3b20e97c856ac..41513ddfff1f5 100644
--- a/libunwind/src/CMakeLists.txt
+++ b/libunwind/src/CMakeLists.txt
@@ -145,7 +145,6 @@ if (LIBUNWIND_ENABLE_SHARED)
OUTPUT_NAME "unwind"
VERSION "1.0"
SOVERSION "1"
- POSITION_INDEPENDENT_CODE ON
)
list(APPEND LIBUNWIND_BUILD_TARGETS "unwind_shared")
if (LIBUNWIND_INSTALL_SHARED_LIBRARY)
@@ -171,7 +170,6 @@ if (LIBUNWIND_ENABLE_STATIC)
LINK_FLAGS "${LIBUNWIND_LINK_FLAGS}"
LINKER_LANGUAGE C
OUTPUT_NAME "unwind"
- POSITION_INDEPENDENT_CODE ON
)
if(LIBUNWIND_HIDE_SYMBOLS)
diff --git a/llvm/utils/release/test-release.sh b/llvm/utils/release/test-release.sh
index 3609557a3bdca..ad1166f5086b4 100755
--- a/llvm/utils/release/test-release.sh
+++ b/llvm/utils/release/test-release.sh
@@ -386,12 +386,14 @@ function configure_llvmCore() {
echo "#" env CC="$c_compiler" CXX="$cxx_compiler" \
cmake -G "$generator" \
-DCMAKE_BUILD_TYPE=$BuildType -DLLVM_ENABLE_ASSERTIONS=$Assertions \
+ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
-DLLVM_ENABLE_PROJECTS="$project_list" \
$ExtraConfigureFlags $BuildDir/llvm-project/llvm \
2>&1 | tee $LogDir/llvm.configure-Phase$Phase-$Flavor.log
env CC="$c_compiler" CXX="$cxx_compiler" \
cmake -G "$generator" \
-DCMAKE_BUILD_TYPE=$BuildType -DLLVM_ENABLE_ASSERTIONS=$Assertions \
+ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
-DLLVM_ENABLE_PROJECTS="$project_list" \
$ExtraConfigureFlags $BuildDir/llvm-project/llvm \
2>&1 | tee $LogDir/llvm.configure-Phase$Phase-$Flavor.log
More information about the libcxx-commits
mailing list