[Driver] Add flag to request arch-specific-subdir in -rpath

[pirama]

Summary

This patch adds -f[no-]rtlib-add-rpath, which if enabled, embeds the
arch-specific subdirectory in resource directory using -rpath (instead
of doing so only during native compilation).

This patch also re-enables test arch-specific-libdir.c which was
silently unsupported because of the REQUIRES tag 'linux'.

[rnk]

Can you add a flag pair to control the insertion of rpath into the final binary? As things are currently, clang is basically leaking paths from the machine doing the linking into the final binary, which often will not run on the same machine. I was thinking -f[no-]compiler-rt-rpath or something, but openmp is not part of compiler-rt. Name recommendations welcome. We might also want to reconsider the default setting.

[pirama]

In D30700#694511, @rnk wrote:

I was thinking -f[no-]compiler-rt-rpath or something, but openmp is not part of compiler-rt. Name recommendations welcome. \

Maybe -f[no-]rtlib-add-rpath?

We might also want to reconsider the default setting.

I also feel defaulting to not adding rpath makes sense.

[rnk]

Both suggestions sound good to me. Thanks!

[Hahnfeld]

No build system will ever set -frtlib-add-rpath to enable this "feature". I'm for keeping this opt-out until we have configuration files to set this by default. Making it opt-in would weaken its main reason of existence: Not to break simple binaries for the user, and we can just drop it.

[pirama]

• Rebase
• Added command line flag and updated tests.

[rnk]

In D30700#695358, @Hahnfeld wrote:

No build system will ever set -frtlib-add-rpath to enable this "feature". I'm for keeping this opt-out until we have configuration files to set this by default. Making it opt-in would weaken its main reason of existence: Not to break simple binaries for the user, and we can just drop it.

I don't agree. If we want to be good citizens on Linux, what's supposed to happen is that we install our shared libraries into /usr/lib/${distro_target}, which is what GCC does with its shared compiler runtime libraries. GCC does not add system-specific absolute rpaths to the binaries it produces.

[pirama]

*Actually* add the command line flags.

[rnk]

lgtm, thanks!

[pirama]

Update commit message

cfe/trunk/include/clang/Driver/Options.td

 def resource_dir : Separate<["-"], "resource-dir">,
   Flags<[DriverOption, CC1Option, CoreOption, HelpHidden]>,
   HelpText<"The directory which holds the compiler resource files">;
 def resource_dir_EQ : Joined<["-"], "resource-dir=">, Flags<[DriverOption, CoreOption]>,
   Alias<resource_dir>;
 def rpath : Separate<["-"], "rpath">, Flags<[LinkerInput]>, Group<Link_Group>;
 def rtlib_EQ : Joined<["-", "--"], "rtlib=">,
   HelpText<"Compiler runtime library to use">;
+def frtlib_add_rpath: Flag<["-"], "frtlib-add-rpath">, Flags<[NoArgumentUnused]>,
+  HelpText<"Add -rpath with architecture-specific resource directory to the linker flags">;
+def fno_rtlib_add_rpath: Flag<["-"], "fno-rtlib-add-rpath">, Flags<[NoArgumentUnused]>,
+  HelpText<"Do not add -rpath with architecture-specific resource directory to the linker flags">;
 def r : Flag<["-"], "r">, Flags<[LinkerInput,NoArgumentUnused]>,
         Group<Link_Group>;
 def save_temps_EQ : Joined<["-", "--"], "save-temps=">, Flags<[DriverOption]>,
   HelpText<"Save intermediate compilation results.">;
 def save_temps : Flag<["-", "--"], "save-temps">, Flags<[DriverOption]>,
   Alias<save_temps_EQ>, AliasArgs<["cwd"]>,
   HelpText<"Save intermediate compilation results">;
 def save_stats_EQ : Joined<["-", "--"], "save-stats=">, Flags<[DriverOption]>,

cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ)) {
     else
       CmdArgs.push_back(
           Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
   }
 }
 
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
                                  ArgStringList &CmdArgs) {
-  // In the cross-compilation case, arch-specific library path is likely
-  // unavailable at runtime.
-  if (TC.isCrossCompiling()) return;
-
   std::string CandidateRPath = TC.getArchSpecificLibPath();
   if (TC.getVFS().exists(CandidateRPath)) {
     CmdArgs.push_back("-rpath");
     CmdArgs.push_back(Args.MakeArgString(CandidateRPath.c_str()));
   }
 }
 
 void tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,

cfe/trunk/test/Driver/arch-specific-libdir-rpath.c

 // Test that the driver adds an arch-specific subdirectory in
-// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath' for native
-// compilations.
+// {RESOURCE_DIR}/lib/linux to the linker search path and to '-rpath'
 //
-// -rpath only gets added during native compilation.  To keep the test simple,
-// just test for x86_64-linux native compilation.
-// REQUIRES: x86_64-linux
+// Test the default behavior when neither -frtlib-add-rpath nor
+// -fno-rtlib-add-rpath is specified, which is to skip -rpath
+// RUN: %clang %s -### 2>&1 -target x86_64-linux \
+// RUN:     -fsanitize=address -shared-libasan \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:     -frtlib-add-rpath \
+// 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 -### 2>&1 -target x86_64-linux \
+// RUN:     -fsanitize=address -shared-libasan \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH-X86_64 %s
+//
+// Test that -rpath is added only under the right circumstance even if
+// -frtlib-add-rpath is specified.
 //
 // Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
-// RUN: %clang %s -### 2>&1 -fsanitize=undefined \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
+//
+// Add LIBPATH but no RPATH for -fsanitizer=address w/o -shared-libasan
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fsanitize=undefined \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH, RPATH for -fsanitize=address -shared-libasan
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN:     -fsanitize=address -shared-libasan \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+//
+// Add LIBPATH, RPATH for -fsanitize=address -shared-libasan on aarch64
+// RUN: %clang %s -### 2>&1 -target aarch64-linux \
+// RUN:     -fsanitize=address -shared-libasan \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-AArch64,RPATH-AArch64 %s
 //
 // Add LIBPATH, RPATH with -fsanitize=address for Android
 // RUN: %clang %s -### 2>&1 -target x86_64-linux-android -fsanitize=address \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH, RPATH for OpenMP
-// RUN: %clang %s -### 2>&1 -fopenmp \
+// RUN: %clang %s -### 2>&1 -target x86_64-linux -fopenmp \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
 //
 // Add LIBPATH but no RPATH for ubsan (or any other sanitizer)
 // RUN: %clang %s -### 2>&1 -fsanitize=undefined \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Add LIBPATH but no RPATH if no sanitizer or runtime is specified
 // RUN: %clang %s -### 2>&1 \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,LIBPATH,NO-RPATH %s
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,NO-RPATH %s
 //
 // Do not add LIBPATH or RPATH if arch-specific subdir doesn't exist
 // RUN: %clang %s -### 2>&1 \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
-// RUN:   | FileCheck --check-prefixes=FILEPATH,NO-LIBPATH,NO-RPATH %s
-//
+// RUN:     -frtlib-add-rpath \
+// RUN:   | FileCheck --check-prefixes=RESDIR,NO-LIBPATH,NO-RPATH %s
 //
-// FILEPATH: "-x" "c" "[[FILE_PATH:.*]]/{{.*}}.c"
-// LIBPATH: -L[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64
-// RPATH: "-rpath" "[[FILE_PATH]]/Inputs/resource_dir_with_arch_subdir/lib/linux/x86_64"
-// NO-LIBPATH-NOT: -L{{.*}}Inputs/resource_dir
-// NO-RPATH-NOT: "-rpath" {{.*}}/Inputs/resource_dir
+// RESDIR: "-resource-dir" "[[RESDIR:[^ ]*]]"
+// LIBPATH-X86_64:  -L[[RESDIR]]/lib/linux/x86_64
+// RPATH-X86_64:    "-rpath" "[[RESDIR]]/lib/linux/x86_64"
+// LIBPATH-AArch64: -L[[RESDIR]]/lib/linux/aarch64
+// RPATH-AArch64:   "-rpath" "[[RESDIR]]/lib/linux/aarch64"
+// NO-LIBPATH-NOT:  -L{{.*}}Inputs/resource_dir
+// NO-RPATH-NOT:    "-rpath" {{.*}}/Inputs/resource_dir

cfe/trunk/test/Driver/arch-specific-libdir.c

 // Test that the driver adds an arch-specific subdirectory in
 // {RESOURCE_DIR}/lib/linux to the search path.
 //
-// REQUIRES: linux
-//
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-i386 %s
 //
 // RUN: %clang %s -### 2>&1 -target i386-unknown-linux \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
 //

cfe/trunk/test/lit.cfg

if platform.system() not in ['Darwin', 'Fuchsia']:
     config.available_features.add('libgcc')
 
 # Native compilation: Check if triples match.
 # FIXME: Consider cases that target can be executed
 # even if host_triple were different from target_triple.
 if config.host_triple == config.target_triple:
     config.available_features.add("native")
 
-# Test Driver/arch-specific-libdir-rpath.c is restricted to x86_64-linux
-if re.match(r'^x86_64.*-linux', config.target_triple):
-    config.available_features.add("x86_64-linux")
-
 # Case-insensitive file system
 def is_filesystem_case_insensitive():
     handle, path = tempfile.mkstemp(prefix='case-test', dir=config.test_exec_root)
     isInsensitive = os.path.exists(
         os.path.join(
             os.path.dirname(path),
             os.path.basename(path).upper()
             ))