[libc++][release] Do not force building the runtimes with -fPIC

[ldionne]

Summary

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.

[JamesNagurne]

Adding relevant people from my team.

I fear that while this change may make people from point 5 happy, it will add a point 6 to the summary where people are unhappy about the default ABI change.

At a glance, this does resolve our team's current (potentially artificial) issues with the PIC default. Our linker is very particular about what it allows to be interlinked. PIC vs non-PIC is one of the qualities tested.

[MaskRay]

(On ELF, shared objects (-shared) needs -fPIC code; PIE executable needs -fPIE or -fPIC code; position-dependent executable accepts any of -fno-PIC/-fPIE/-fPIC.)

LLVM_ENABLE_PIC (default) adds -fPIC. -DCMAKE_POSITION_INDEPENDENT_CODE=ON adds a duplicated -fPIC.

Someone complained that libc++.a couldn't be used in shared libraries built without -fPIC (http://llvm.org/PR43604) since the LLVM 9 release.

I believe this was due to they doing a standalone libc++ build (cmake -Hlibcxx -Bout instead of cmake -Hllvm -Bout), so they did not receive -fPIC from LLVM_ENABLE_PIC.
It was a user configuration problem. They should express the PIC intention through CMAKE_POSITION_INDEPENDENT_CODE=on in the absence of LLVM_ENABLE_PIC=on.

As-is, this patch will have 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.

I think there is no difference in the build output, just one -fPIC now becoming two -fPIC in the compiler options :)

---

Setting -DCMAKE_POSITION_INDEPENDENT_CODE=ON looks good to me.
We may be able to drop LLVM_ENABLE_PIC in favor of the standard CMake variable.

[ldionne]

In D110261#3015789, @JamesNagurne wrote:

Adding relevant people from my team.

I fear that while this change may make people from point 5 happy, it will add a point 6 to the summary where people are unhappy about the default ABI change.

FWIW, we've had that ABI (no -fPIC by default) since LLVM 9. We changed it back to having -fPIC by default in LLVM 13 (which is being released right now). If this goes through, I would cherry-pick this on the LLVM 13 release branch ASAP so that we don't flip-flop our users too much.

[ldionne]

@chandlerc Would you be able to provide a reproducer for the testing you were doing in http://llvm.org/PR43604? I tried naively installing LLVM on a Ubuntu Docker image to repro what you were seeing and confirm that this patch solved it, but I got some bogus test.cpp:1:10: fatal error: 'iostream' file not found error, so I'm doing something wrong.

[ldionne]

Rebase onto main. Gentle ping, I do not know how to verify that this is going to fix the
issues that people were seeing.

[JamesNagurne]

As-is, this patch will have 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.

Do you mean that specifying LLVM_ENABLE_RUNTIMES at configure-time causes both llvm/clang and the enabled runtimes (libc++, libc++abi, libunwind, etc) to use the same PIC selection?
The runtimes have a way to use their own configuration. Prefixing a variable with RUNTIMES_ or BUILTINS_ sends that variable to the runtime/builtins build, overwriting the one used to configure LLVM.

https://github.com/llvm/llvm-project/blob/7562c64197acbee60c4bb0d211eb699ad24f5bb8/llvm/runtimes/CMakeLists.txt#L308

[MaskRay]

Still LGTM. I am pretty sure some people found issues because they or their distributions incorrectly specified -DLLVM_ENABLE_PIC=off (default is on).

[ldionne]

Rebase onto main and update documentation for enabling PIC with the bootstrapping build.

[ldionne]

Hopefully fix the CI

[ldionne]

Rebase onto main -- this should fix CI.

[ldionne]

CI is green for libc++. I want to ship this but I'd like to have @tstellar's approval before I do because I am modifying test-release.sh.

[tstellar]

LGTM.

libcxx/docs/ReleaseNotes.rst

 - Support for building the runtimes using the GCC 32 bit multilib flag (``-m32``) has been removed. Support
   for this had been flaky for a while, and we didn't know of anyone depending on this. Instead, please perform
   a normal cross-compilation of the runtimes using the appropriate target, such as passing the following to
   your bootstrapping build:
 
   .. 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.

libcxx/src/CMakeLists.txt

if (LIBCXX_ENABLE_SHARED)
   set_target_properties(cxx_shared
     PROPERTIES
       COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
       LINK_FLAGS    "${LIBCXX_LINK_FLAGS}"
       OUTPUT_NAME   "c++"
       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)
 
   # Link against LLVM libunwind
   if (LIBCXXABI_USE_LLVM_UNWINDER)
     if (NOT LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
       target_link_libraries(cxx_shared PUBLIC unwind_shared)
if (LIBCXX_ENABLE_STATIC)
   target_include_directories(cxx_static PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
   target_link_libraries(cxx_static PUBLIC cxx-headers
                                    PRIVATE ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_static
     PROPERTIES
       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)
 
   if (LIBCXXABI_USE_LLVM_UNWINDER)
     # If we intend to use the just-built unwinder, add a dependency so that it
     # gets built, even if we technically aren't going to link it in at this
     # stage.

libcxx/test/libcxx/debug/extern-templates.sh.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // XFAIL: LIBCXX-AIX-FIXME
 
 // This test checks that we retain extern template instantiation declarations
 // for members of <locale> even when the debug mode is enabled, which is
 // necessary for correctness. See https://llvm.org/D94718 for details.
 
 // 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
 // on Windows (GNU ld supports it, but MS link.exe and LLD don't) - one has to
 // use an import library instead. (Additionally, the test uses the -fPIC
 // option which clang doesn't accept on Windows.)
 // 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>
 #include <cstdio>
 #include <sstream>
 #include <string>
 
 std::string __attribute__((visibility("default"))) numToString(int x);

libcxx/utils/libcxx/test/dsl.py

def __enter__(self):
       os.makedirs(testDir)
       return self
     def __exit__(self, *args):
       testDir, _ = libcxx.test.format._getTempPaths(self)
       shutil.rmtree(testDir)
       os.remove(tmp.name)
   return TestWrapper(suite, pathInSuite, config)
 
-@_memoizeExpensiveOperation(lambda c, s: (c.substitutions, c.environment, s))
-def sourceBuilds(config, source):
+@_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))
 def programOutput(config, program, args=None):
   """
   Compiles a program for the test target, run it on the test target and return
   the output.
 

libcxx/utils/libcxx/test/features.py

DEFAULT_FEATURES = [
   Feature(name='is-lockfree-runtime-function',
           when=lambda cfg: sourceBuilds(cfg, """
             #include <atomic>
             struct Large { int storage[100]; };
             std::atomic<Large> x;
             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),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)), when=_isAppleClang),
 
   Feature(name='clang',                                                                                                            when=_isClang,
           actions=[AddCompileFlag('-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER')]),
   Feature(name=lambda cfg: 'clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                                when=_isClang),

libcxxabi/src/CMakeLists.txt

PROPERTIES
       CXX_STANDARD 20
       CXX_STANDARD_REQUIRED OFF
       COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
       LINK_FLAGS "${LIBCXXABI_LINK_FLAGS}"
       OUTPUT_NAME "c++abi"
       SOVERSION "1"
       VERSION "${LIBCXXABI_LIBRARY_VERSION}"
       DEFINE_SYMBOL ""
-      POSITION_INDEPENDENT_CODE ON
   )
 
   list(APPEND LIBCXXABI_BUILD_TARGETS "cxxabi_shared")
   if (LIBCXXABI_INSTALL_SHARED_LIBRARY)
     list(APPEND LIBCXXABI_INSTALL_TARGETS "cxxabi_shared")
   endif()
 
   add_library(cxxabi-reexports INTERFACE)
if (LIBCXXABI_ENABLE_STATIC)
   set_target_properties(cxxabi_static
     PROPERTIES
       CXX_EXTENSIONS OFF
       CXX_STANDARD 20
       CXX_STANDARD_REQUIRED OFF
       COMPILE_FLAGS "${LIBCXXABI_COMPILE_FLAGS}"
       LINK_FLAGS "${LIBCXXABI_LINK_FLAGS}"
       OUTPUT_NAME "c++abi"
-      POSITION_INDEPENDENT_CODE ON
     )
 
   if(LIBCXXABI_HERMETIC_STATIC_LIBRARY)
     append_flags_if_supported(CXXABI_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
     # If the hermetic library doesn't define the operator new/delete functions
     # then its code shouldn't declare them with hidden visibility.  They might
     # actually be provided by a shared library at link time.
     if (LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS)

libunwind/src/CMakeLists.txt

PROPERTIES
       CXX_STANDARD 11
       CXX_STANDARD_REQUIRED ON
       COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
       LINK_FLAGS "${LIBUNWIND_LINK_FLAGS}"
       LINKER_LANGUAGE C
       OUTPUT_NAME "unwind"
       VERSION "1.0"
       SOVERSION "1"
-      POSITION_INDEPENDENT_CODE ON
   )
   list(APPEND LIBUNWIND_BUILD_TARGETS "unwind_shared")
   if (LIBUNWIND_INSTALL_SHARED_LIBRARY)
     list(APPEND LIBUNWIND_INSTALL_TARGETS "unwind_shared")
   endif()
 endif()
 
 # Build the static library.
set_target_properties(unwind_static
     PROPERTIES
       CXX_EXTENSIONS OFF
       CXX_STANDARD 11
       CXX_STANDARD_REQUIRED ON
       COMPILE_FLAGS "${LIBUNWIND_COMPILE_FLAGS}"
       LINK_FLAGS "${LIBUNWIND_LINK_FLAGS}"
       LINKER_LANGUAGE C
       OUTPUT_NAME "unwind"
-      POSITION_INDEPENDENT_CODE ON
   )
 
   if(LIBUNWIND_HIDE_SYMBOLS)
     append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility=hidden)
     append_flags_if_supported(UNWIND_STATIC_LIBRARY_FLAGS -fvisibility-global-new-delete-hidden)
     target_compile_options(unwind_static PRIVATE ${UNWIND_STATIC_LIBRARY_FLAGS})
     target_compile_definitions(unwind_static PRIVATE _LIBUNWIND_HIDE_SYMBOLS)
   endif()

llvm/utils/release/test-release.sh

function configure_llvmCore() {
     echo "# Using C++ compiler: $cxx_compiler"
 
     cd $ObjDir
     echo "# Configuring llvm $Release-$RC $Flavor"
 
     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
 
     cd $BuildDir
 }
 
 function build_llvmCore() {