[clang] [Clang] Add 'CLANG_ALLOW_IMPLICIT_RPATH' to enable toolchain use of -rpath (PR #82004)

Joseph Huber via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 16 07:50:41 PST 2024


https://github.com/jhuber6 created https://github.com/llvm/llvm-project/pull/82004

Summary:
The original discussion in https://reviews.llvm.org/D118493 was that
`clang` should not be adding `-rpath` implicitly for toolchains. The
main motivation behind that change was the 'Fedora' toolchain
disallowing usage of `-rpath` in their tools. This patch introduces a
new clang configuration called `CLANG_ALLOW_IMPLICT_RPATH` that defaults
to on.

The motivation behind this patch is the fact that the LLVM offloading
toolchain is currently in a nearly unusable state for casual users. This
is a problem for other runtimes like `libc++` or `libunwind`, however
`libomptarget` and other offloading runtimes like HIP are much difficult
to support. Currently, `libomptarget` is only supported to be used with
the same build that created the executable. Furthermore, `libomptarget`
is currently shipped by four different vendors, which are not mutually
compatible. Because this is totally broken as far as I'm aware all the
vendor compilers reverted the original patch turning this off.

The presented solution was to advise users to use Clang configruation
files. Since then, this was added to the documentation at the website
https://openmp.llvm.org/SupportAndFAQ.html#q-what-are-the-llvm-components-used-in-offloading-and-how-are-they-found
but this has not had any noticable effect on the number of people I've
had to coach through their broken environment since the patch was
reverted and is roughly equivalent to the currenlty accepted solution of
using LD_LIBRARY_PATH.

This would be solved with static linking, but offloading runtimes also
manage thred pools and device resources that cannot be duplicated. The
Fedora distributors were the only group that were against doing this by
default, so the suggestion is that Fedora uses
`-DCLANG_ALLOW_IMPLICIT_RPATH=OFF` when building.


>From 7ddd4ad90cda446b94d72b707015767ea759d121 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Fri, 16 Feb 2024 09:18:12 -0600
Subject: [PATCH] [Clang] Add 'CLANG_ALLOW_IMPLICIT_RPATH' to enable toolchain
 use of -rpath

Summary:
The original discussion in https://reviews.llvm.org/D118493 was that
`clang` should not be adding `-rpath` implicitly for toolchains. The
main motivation behind that change was the 'Fedora' toolchain
disallowing usage of `-rpath` in their tools. This patch introduces a
new clang configuration called `CLANG_ALLOW_IMPLICT_RPATH` that defaults
to on.

The motivation behind this patch is the fact that the LLVM offloading
toolchain is currently in a nearly unusable state for casual users. This
is a problem for other runtimes like `libc++` or `libunwind`, however
`libomptarget` and other offloading runtimes like HIP are much difficult
to support. Currently, `libomptarget` is only supported to be used with
the same build that created the executable. Furthermore, `libomptarget`
is currently shipped by four different vendors, which are not mutually
compatible. Because this is totally broken as far as I'm aware all the
vendor compilers reverted the original patch turning this off.

The presented solution was to advise users to use Clang configruation
files. Since then, this was added to the documentation at the website
https://openmp.llvm.org/SupportAndFAQ.html#q-what-are-the-llvm-components-used-in-offloading-and-how-are-they-found
but this has not had any noticable effect on the number of people I've
had to coach through their broken environment since the patch was
reverted and is roughly equivalent to the currenlty accepted solution of
using LD_LIBRARY_PATH.

This would be solved with static linking, but offloading runtimes also
manage thred pools and device resources that cannot be duplicated. The
Fedora distributors were the only group that were against doing this by
default, so the suggestion is that Fedora uses
`-DCLANG_ALLOW_IMPLICIT_RPATH=OFF` when building.
---
 clang/CMakeLists.txt                           | 2 ++
 clang/include/clang/Config/config.h.cmake      | 3 +++
 clang/lib/Driver/ToolChains/CommonArgs.cpp     | 3 ++-
 clang/test/Driver/arch-specific-libdir-rpath.c | 7 -------
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 47fc2e4886cfc2..02e0a8f80d9c59 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -213,6 +213,8 @@ set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 
 option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON)
 
+option(CLANG_ALLOW_IMPLICIT_RPATH "Allow clang to add -rpath automatically" ON)
+
 set(CLANG_DEFAULT_LINKER "" CACHE STRING
   "Default linker to use (linker name or absolute path, empty for platform default)")
 
diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake
index 4015ac8040861c..6ca8cc75953094 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -63,6 +63,9 @@
 /* Define if dladdr() is available on this platform. */
 #cmakedefine CLANG_HAVE_DLADDR ${CLANG_HAVE_DLADDR}
 
+/* Define if we have sys/resource.h (rlimits) */
+#cmakedefine01 CLANG_ALLOW_IMPLICIT_RPATH
+
 /* Linker version detected at compile time. */
 #cmakedefine HOST_LINK_VERSION "${HOST_LINK_VERSION}"
 
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 0fd7b8424eb4ba..cc8e4ffa571a82 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1107,7 +1107,8 @@ void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
                                  ArgStringList &CmdArgs) {
   if (!Args.hasFlag(options::OPT_frtlib_add_rpath,
-                    options::OPT_fno_rtlib_add_rpath, false))
+                    options::OPT_fno_rtlib_add_rpath,
+                    CLANG_ALLOW_IMPLICIT_RPATH))
     return;
 
   for (const auto &CandidateRPath : TC.getArchSpecificLibPaths()) {
diff --git a/clang/test/Driver/arch-specific-libdir-rpath.c b/clang/test/Driver/arch-specific-libdir-rpath.c
index 1e6bbbc5929ac2..de114c67862dea 100644
--- a/clang/test/Driver/arch-specific-libdir-rpath.c
+++ b/clang/test/Driver/arch-specific-libdir-rpath.c
@@ -1,13 +1,6 @@
 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
-// Test the default behavior when neither -frtlib-add-rpath nor
-// -fno-rtlib-add-rpath is specified, which is to skip -rpath
-// RUN: %clang %s -### --target=x86_64-linux \
-// RUN:     -fsanitize=address -shared-libasan \
-// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir 2>&1 \
-// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
-//
 // Test that -rpath is not added under -fno-rtlib-add-rpath even if other
 // conditions are met.
 // RUN: %clang %s -### --target=x86_64-linux \



More information about the cfe-commits mailing list