[Driver] Fix rpath for compiler-rt

[yaxunl]

Summary

The compiler-rt library path can be either {resource_dir}/lib/{triple}
or {resource_dir}/lib/{OS}/{arch} depending on whether
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default is ON or OFF.
Currently, the rpath added by -rtlib-add-rpath only adds
the latter. This patch checks both and adds the one that exists.

[MaskRay]

This change is correct for Linux. llvm/CMakeLists.txt says:

if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390")
  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)

Some rpath using OSes (notably macOS) use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default=OFF. Is the rpath setting ever usable on macOS?
If not, the change is correct.

[yaxunl]

In D146686#4215577, @MaskRay wrote:

This change is correct for Linux. llvm/CMakeLists.txt says:

if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390")
  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)

Some rpath using OSes (notably macOS) use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default=OFF. Is the rpath setting ever usable on macOS?
If not, the change is correct.

How about I try to add both? there is check whether the dir exist then adding it, so only the existing one will be added. then it should work in either case.

[MaskRay]

In D146686#4216612, @yaxunl wrote:

In D146686#4215577, @MaskRay wrote:

This change is correct for Linux. llvm/CMakeLists.txt says:

if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390")
  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)

Some rpath using OSes (notably macOS) use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default=OFF. Is the rpath setting ever usable on macOS?
If not, the change is correct.

How about I try to add both? there is check whether the dir exist then adding it, so only the existing one will be added. then it should work in either case.

Yes. Detecting both locations will be nice.

[yaxunl]

adds check for both {resource_dir}/lib/{triple} and {resource_dir}/lib/{OS}/{arch}

[MaskRay]

The new test can be placed in arch-specific-libdir-rpath.c. One or two additional RUN lines seem sufficient, no need to duplicate too many.

Also, use --target= for new tests and avoid ^//$.

clang/lib/Driver/ToolChains/OHOS.cpp

140 ↗

(On Diff #508312)

delete braces

[yaxunl]

revised by Fangrui's comments

[MaskRay]

clang/lib/Driver/ToolChain.cpp

640

unneeded blank line

[yaxunl]

clang/lib/Driver/ToolChain.cpp

640

will remove when committing

clang/lib/Driver/ToolChain.cpp

ToolChain::path_list ToolChain::getStdlibPaths() const {
   llvm::sys::path::append(P, "..", "lib", getTripleString());
   Paths.push_back(std::string(P.str()));
 
   return Paths;
 }
 
 std::string ToolChain::getArchSpecificLibPath() const {
   SmallString<128> Path(getDriver().ResourceDir);
-  llvm::sys::path::append(Path, "lib", getOSLibName(),
-                          llvm::Triple::getArchTypeName(getArch()));
+  llvm::sys::path::append(Path, "lib", getTriple().str());
   return std::string(Path.str());
 }
 
 bool ToolChain::needsProfileRT(const ArgList &Args) {
   if (Args.hasArg(options::OPT_noprofilelib))
     return false;
 
   return Args.hasArg(options::OPT_fprofile_generate) ||
          Args.hasArg(options::OPT_fprofile_generate_EQ) ||
          Args.hasArg(options::OPT_fcs_profile_generate) ||
          Args.hasArg(options::OPT_fcs_profile_generate_EQ) ||
          Args.hasArg(options::OPT_fprofile_instr_generate) ||
          Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
          Args.hasArg(options::OPT_fcreate_profile) ||
          Args.hasArg(options::OPT_forder_file_instrumentation);
 }
 
 bool ToolChain::needsGCovInstrumentation(const llvm::opt::ArgList &Args) {
   return Args.hasArg(options::OPT_coverage) ||
          Args.hasFlag(options::OPT_fprofile_arcs, options::OPT_fno_profile_arcs,
                       false);
 }

clang/test/Driver/arch-specific-libdir-rpath.c

 // 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:     -frtlib-add-rpath \
-// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64,RPATH-X86_64 %s
+// RUN:   | FileCheck --check-prefixes=RESDIR,LIBPATH-X86_64-ANDROID,RPATH-X86_64-ANDROID %s
 //
 // Add LIBPATH, RPATH for OpenMP
 // RUN: %clang %s -### 2>&1 -target x86_64-linux -fopenmp \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
 // 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)

 // Do not add LIBPATH or RPATH if arch-specific subdir doesn't exist
 // RUN: %clang %s -### 2>&1 -target x86_64-linux \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     -frtlib-add-rpath \
 // RUN:   | FileCheck --check-prefixes=RESDIR,NO-LIBPATH,NO-RPATH %s
 
 // RESDIR: "-resource-dir" "[[RESDIR:[^"]*]]"
 //
-// LIBPATH-X86_64: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)x86_64}}
-// RPATH-X86_64:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)x86_64}}"
+// LIBPATH-X86_64: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux}}
+// RPATH-X86_64:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux}}"
+//
+// LIBPATH-X86_64-ANDROID: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux-android}}
+// RPATH-X86_64-ANDROID:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux-android}}"
 //
-// NO-LIBPATH-X86_64-NOT: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)x86_64}}
-// NO-RPATH-X86_64-NOT:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)x86_64}}"
+// NO-LIBPATH-X86_64-NOT: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux}}
+// NO-RPATH-X86_64-NOT:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)x86_64-unknown-linux}}"
 //
-// LIBPATH-AARCH64: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)aarch64}}
-// RPATH-AARCH64:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)linux(/|\\\\)aarch64}}"
+// LIBPATH-AARCH64: -L[[RESDIR]]{{(/|\\\\)lib(/|\\\\)aarch64-unknown-linux}}
+// RPATH-AARCH64:   "-rpath" "[[RESDIR]]{{(/|\\\\)lib(/|\\\\)aarch64-unknown-linux}}"
 //
 // NO-LIBPATH-NOT: "-L{{[^"]*Inputs(/|\\\\)resource_dir}}"
 // NO-RPATH-NOT:   "-rpath" {{.*(/|\\\\)Inputs(/|\\\\)resource_dir}}

clang/test/Driver/arch-specific-libdir.c

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

 // RUN:   | FileCheck --check-prefixes=FILEPATH,ARCHDIR-aarch64 %s
 //
 // RUN: %clang %s -### 2>&1 -target aarch64-unknown-linux \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:   | FileCheck --check-prefixes=FILEPATH,NO-ARCHDIR %s
 //
 //
 // FILEPATH: "-x" "c" "[[FILE_PATH:.*]]{{(/|\\\\).*}}.c"
-// ARCHDIR-i386:    -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)linux(/|\\\\)i386}}
-// ARCHDIR-x86_64:  -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)linux(/|\\\\)x86_64}}
-// ARCHDIR-arm:     -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)linux(/|\\\\)arm}}
-// ARCHDIR-aarch64: -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)linux(/|\\\\)aarch64}}
+// ARCHDIR-i386:    -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)i386-unknown-linux}}
+// ARCHDIR-i686:    -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)i686-unknown-linux}}
+// ARCHDIR-x86_64:  -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)x86_64-unknown-linux}}
+// ARCHDIR-arm:     -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)arm-unknown-linux}}
+// ARCHDIR-aarch64: -L[[FILE_PATH]]{{(/|\\\\)Inputs(/|\\\\)resource_dir_with_arch_subdir(/|\\\\)lib(/|\\\\)aarch64-unknown-linux}}
 //
 // Have a stricter check for no-archdir - that the driver doesn't add any
 // subdirectory from the provided resource directory.
 // NO-ARCHDIR-NOT: -L[[FILE_PATH]]/Inputs/resource_dir