[runtimes] Build with the install RPATH #67835
Closed
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
Make sure that we build libraries in the runtimes using the final RPATH they will use after installation. This ensures that when we link against these libraries in our tests, they use the same RPATH that they will use after being installed, which increases reproducibility. As a side effect, it also solves the issue that libraries built from different build directories would have different binary signatures since they would have different offsets based on the length of the RPATH used during the build. This was undesirable since it made builds less reproducible. Fixes llvm#67375
llvmbot
added
cmake
|
@llvm/pr-subscribers-libcxxabi @llvm/pr-subscribers-libcxx ChangesMake sure that we build libraries in the runtimes using the final RPATH they will use after installation. This ensures that when we link against these libraries in our tests, they use the same RPATH that they will use after being installed, which increases reproducibility. As a side effect, it also solves the issue that libraries built from different build directories would have different binary signatures since they would have different offsets based on the length of the RPATH used during the build. This was undesirable since it made builds less reproducible. Fixes #67375 Full diff: https://github.com/llvm/llvm-project/pull/67835.diff 6 Files Affected:
diff --git a/libcxx/cmake/caches/AIX.cmake b/libcxx/cmake/caches/AIX.cmake
index 31d9a2d4b72751d..142399e404c9d96 100644
--- a/libcxx/cmake/caches/AIX.cmake
+++ b/libcxx/cmake/caches/AIX.cmake
@@ -1,5 +1,4 @@
set(CMAKE_BUILD_TYPE Release CACHE STRING "")
-set(CMAKE_BUILD_WITH_INSTALL_RPATH ON CACHE BOOL "")
set(CMAKE_C_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
set(CMAKE_CXX_FLAGS "-D__LIBC_NO_CPP_MATH_OVERLOADS__" CACHE STRING "")
set(CMAKE_SHARED_LINKER_FLAGS "-Wl,-G -Wl,-bcdtors:all:-2147483548:s" CACHE STRING "")
diff --git a/libcxx/test/libcxx/vendor/apple/system-install-properties.sh.cpp b/libcxx/test/libcxx/vendor/apple/system-install-properties.sh.cpp
index 6c84e0dfff2bab0..a97adcd7c50c644 100644
--- a/libcxx/test/libcxx/vendor/apple/system-install-properties.sh.cpp
+++ b/libcxx/test/libcxx/vendor/apple/system-install-properties.sh.cpp
@@ -27,15 +27,21 @@
// Make sure the install_name is /usr/lib.
//
-// In particular, make sure we don't use any @rpath in the load commands. When building as
-// a system library, it is important to hardcode the installation paths in the dylib, because
-// various tools like dyld and ld64 will treat us specially if they recognize us as being a
-// system library.
+// In particular, make sure we don't use any @rpath-relative paths in the load commands.
+// When building as a system library, it is important to hardcode the installation paths
+// in the dylib, because various tools like dyld and ld64 will treat us specially if they
+// recognize us as being a system library.
//
// TODO: We currently don't do that correctly in the CMake build.
//
// XRUNX: otool -L "%{lib}/libc++.1.dylib" | grep '/usr/lib/libc++.1.dylib'
-// XRUNX: ! otool -l "%{lib}/libc++.1.dylib" | grep -E "LC_RPATH|@loader_path|@rpath"
+// XRUNX: otool -l "%{lib}/libc++.1.dylib" | grep --invert-match -E "@loader_path|@rpath"
+
+// Make sure we don't set a RPATH when we build the library. Since we are building a system
+// library, we are supposed to find our dependencies at the usual system-provided locations,
+// which doesn't require setting a RPATH in the library itself.
+//
+// RUN: otool -l "%{lib}/libc++.1.dylib" | grep --invert-match -e "LC_RPATH"
// Make sure the compatibility_version of libc++ is 1.0.0.
// Failure to respect this can result in applications not being able to find libc++
diff --git a/libcxx/test/libcxx/vendor/llvm/apple-install-properties.sh.cpp b/libcxx/test/libcxx/vendor/llvm/apple-install-properties.sh.cpp
new file mode 100644
index 000000000000000..e89cefe252ef610
--- /dev/null
+++ b/libcxx/test/libcxx/vendor/llvm/apple-install-properties.sh.cpp
@@ -0,0 +1,38 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: stdlib=libc++ && target={{.+}}-apple-{{.+}}
+
+// This file checks various properties of the installation of libc++ when built as
+// part of the LLVM releases, on Apple platforms.
+
+// Make sure we install the libc++ headers in the right location.
+//
+// RUN: stat "%{include}/__config"
+
+// Make sure we install libc++.1.dylib and libc++experimental.a in the right location.
+//
+// RUN: stat "%{lib}/libc++.1.dylib"
+// RUN: stat "%{lib}/libc++experimental.a"
+
+// Make sure we install a symlink from libc++.dylib to libc++.1.dylib.
+//
+// RUN: stat "%{lib}/libc++.dylib"
+// RUN: readlink "%{lib}/libc++.dylib" | grep "libc++.1.dylib"
+
+// Make sure we don't set a RPATH when we build the library. Since we are building a system
+// library, we are supposed to find our dependencies at the usual system-provided locations,
+// which doesn't require setting a RPATH in the library itself.
+//
+// RUN: otool -l "%{lib}/libc++.1.dylib" | grep --invert-match -e "LC_RPATH"
+
+// Make sure the compatibility_version of libc++ is 1.0.0.
+// Failure to respect this can result in applications not being able to find libc++
+// when they are loaded by dyld, if the compatibility version was bumped.
+//
+// RUN: otool -L "%{lib}/libc++.1.dylib" | grep "libc++.1.dylib" | grep "compatibility version 1.0.0"
diff --git a/libcxxabi/test/vendor/apple/system-install-properties.sh.cpp b/libcxxabi/test/vendor/apple/system-install-properties.sh.cpp
index d4df7b3dff6923d..8a72cc648ecd058 100644
--- a/libcxxabi/test/vendor/apple/system-install-properties.sh.cpp
+++ b/libcxxabi/test/vendor/apple/system-install-properties.sh.cpp
@@ -37,7 +37,13 @@
// TODO: We currently don't do that correctly in the CMake build.
//
// XRUNX: otool -L "%{lib}/libc++abi.dylib" | grep '/usr/lib/libc++abi.dylib'
-// XRUNX: ! otool -l "%{lib}/libc++abi.dylib" | grep -E "LC_RPATH|@loader_path|@rpath"
+// XRUNX: otool -l "%{lib}/libc++abi.dylib" | grep --invert-match -E "LC_RPATH|@loader_path|@rpath"
+
+// Make sure we don't set a RPATH when we build the library. Since we are building a system
+// library, we are supposed to find our dependencies at the usual system-provided locations,
+// which doesn't require setting a RPATH in the library itself.
+//
+// RUN: otool -l "%{lib}/libc++abi.dylib" | grep --invert-match -e "LC_RPATH"
// Make sure the compatibility_version of libc++abi is 1.0.0. Failure to respect this can result
// in applications not being able to find libc++abi when they are loaded by dyld, if the
diff --git a/libcxxabi/test/vendor/llvm/apple-install-properties.sh.cpp b/libcxxabi/test/vendor/llvm/apple-install-properties.sh.cpp
new file mode 100644
index 000000000000000..0d6cfd84256ca0a
--- /dev/null
+++ b/libcxxabi/test/vendor/llvm/apple-install-properties.sh.cpp
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: stdlib=libc++ && target={{.+}}-apple-{{.+}}
+
+// This file checks various properties of the installation of libc++abi when built as
+// part of the LLVM releases, on Apple platforms.
+
+// Make sure we install the libc++abi headers in the right location.
+//
+// RUN: stat "%{include}/cxxabi.h"
+
+// Make sure we install libc++abi.dylib in the right location.
+//
+// RUN: stat "%{lib}/libc++abi.1.dylib"
+
+// Make sure we install a symlink from libc++abi.dylib to libc++abi.1.dylib.
+//
+// RUN: stat "%{lib}/libc++abi.dylib"
+// RUN: readlink "%{lib}/libc++abi.dylib" | grep "libc++abi.1.dylib"
+
+// Make sure we don't set a RPATH when we build the library. Since we are building a system
+// library, we are supposed to find our dependencies at the usual system-provided locations,
+// which doesn't require setting a RPATH in the library itself.
+//
+// RUN: otool -l "%{lib}/libc++abi.dylib" | grep --invert-match -e "LC_RPATH"
+
+// Make sure the compatibility_version of libc++abi is 1.0.0. Failure to respect this can result
+// in applications not being able to find libc++abi when they are loaded by dyld, if the
+// compatibility version was bumped.
+//
+// RUN: otool -L "%{lib}/libc++abi.dylib" | grep "libc++abi.1.dylib" | grep "compatibility version 1.0.0"
diff --git a/runtimes/CMakeLists.txt b/runtimes/CMakeLists.txt
index 010ec879e44a322..36c4e78c260758f 100644
--- a/runtimes/CMakeLists.txt
+++ b/runtimes/CMakeLists.txt
@@ -1,6 +1,8 @@
# This file handles building LLVM runtime sub-projects.
cmake_minimum_required(VERSION 3.20.0)
+set(CMAKE_BUILD_WITH_INSTALL_RPATH ON)
+
# Add path for custom and the LLVM build's modules to the CMake module path.
set(LLVM_COMMON_CMAKE_UTILS "${CMAKE_CURRENT_SOURCE_DIR}/../cmake")
include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
|
|
Edit: Sorry, I commented totally on the wrong PR here. |
|
Closing since I don't think forcing this setting is necessarily the right thing to do. Indeed, we now even run the tests against a fake-installed version of the library to ensure that we get the install-name and other similar things right. The build products itself shouldn't be used for anything authoritative IMO. |
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.
Make sure that we build libraries in the runtimes using the final RPATH they will use after installation. This ensures that when we link against these libraries in our tests, they use the same RPATH that they will use after being installed, which increases reproducibility.
As a side effect, it also solves the issue that libraries built from different build directories would have different binary signatures since they would have different offsets based on the length of the RPATH used during the build. This was undesirable since it made builds less reproducible.
Fixes #67375