[clang] d6efc98 - Reland "[clang][Driver] Correct tool search path priority"

David Spickett via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 25 01:35:11 PDT 2020


Author: David Spickett
Date: 2020-06-25T09:33:43+01:00
New Revision: d6efc9811646edbfe13f06c2676fb469f1c155b1

URL: https://github.com/llvm/llvm-project/commit/d6efc9811646edbfe13f06c2676fb469f1c155b1
DIFF: https://github.com/llvm/llvm-project/commit/d6efc9811646edbfe13f06c2676fb469f1c155b1.diff

LOG: Reland "[clang][Driver] Correct tool search path priority"

This reverts commit f570d5810485fa6fb2e1009f795a899d79bd429f.

The test was failing on MacOS if you set
LLVM_DEFAULT_TARGET_TRIPLE. For example if you set it to
"x86_64-apple-darwin" clang actually uses
"x86_64-apple-darwin<version>".

To fix this get default triple from clang itself during the
test instead of substituting it in via lit.

Added: 
    clang/test/Driver/program-path-priority.c

Modified: 
    clang/lib/Driver/Driver.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 24436ec4e8d7..1f718b30c551 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4766,13 +4766,11 @@ void Driver::generatePrefixedToolNames(
 }
 
 static bool ScanDirForExecutable(SmallString<128> &Dir,
-                                 ArrayRef<std::string> Names) {
-  for (const auto &Name : Names) {
-    llvm::sys::path::append(Dir, Name);
-    if (llvm::sys::fs::can_execute(Twine(Dir)))
-      return true;
-    llvm::sys::path::remove_filename(Dir);
-  }
+                                 const std::string &Name) {
+  llvm::sys::path::append(Dir, Name);
+  if (llvm::sys::fs::can_execute(Twine(Dir)))
+    return true;
+  llvm::sys::path::remove_filename(Dir);
   return false;
 }
 
@@ -4785,8 +4783,9 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {
   for (const auto &PrefixDir : PrefixDirs) {
     if (llvm::sys::fs::is_directory(PrefixDir)) {
       SmallString<128> P(PrefixDir);
-      if (ScanDirForExecutable(P, TargetSpecificExecutables))
-        return std::string(P.str());
+      for (const auto &TargetSpecificExecutable : TargetSpecificExecutables)
+        if (ScanDirForExecutable(P, TargetSpecificExecutable))
+          return std::string(P.str());
     } else {
       SmallString<128> P((PrefixDir + Name).str());
       if (llvm::sys::fs::can_execute(Twine(P)))
@@ -4795,17 +4794,25 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {
   }
 
   const ToolChain::path_list &List = TC.getProgramPaths();
-  for (const auto &Path : List) {
-    SmallString<128> P(Path);
-    if (ScanDirForExecutable(P, TargetSpecificExecutables))
-      return std::string(P.str());
-  }
+  for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) {
+    // For each possible name of the tool look for it in
+    // program paths first, then the path.
+    // Higher priority names will be first, meaning that
+    // a higher priority name in the path will be found
+    // instead of a lower priority name in the program path.
+    // E.g. <triple>-gcc on the path will be found instead
+    // of gcc in the program path
+    for (const auto &Path : List) {
+      SmallString<128> P(Path);
+      if (ScanDirForExecutable(P, TargetSpecificExecutable))
+        return std::string(P.str());
+    }
 
-  // If all else failed, search the path.
-  for (const auto &TargetSpecificExecutable : TargetSpecificExecutables)
+    // Fall back to the path
     if (llvm::ErrorOr<std::string> P =
             llvm::sys::findProgramByName(TargetSpecificExecutable))
       return *P;
+  }
 
   return std::string(Name);
 }

diff  --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c
new file mode 100644
index 000000000000..ba893e7e2e2c
--- /dev/null
+++ b/clang/test/Driver/program-path-priority.c
@@ -0,0 +1,109 @@
+/// Don't create symlinks on Windows
+// UNSUPPORTED: system-windows
+
+/// Check the priority used when searching for tools
+/// Names and locations are usually in this order:
+/// <triple>-tool, tool, <default triple>-tool
+/// program path, PATH
+/// (from highest to lowest priority)
+/// A higher priority name found in a lower priority
+/// location will win over a lower priority name in a
+/// higher priority location.
+/// Prefix dirs (added with -B) override the location,
+/// so only name priority is accounted for, unless we fail to find
+/// anything at all in the prefix.
+
+/// Symlink clang to a new dir which will be its
+/// "program path" for these tests
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: ln -s %clang %t/clang
+
+/// No gccs at all, nothing is found
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NO_NOTREAL_GCC %s
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
+// NO_NOTREAL_GCC-NOT: /gcc
+
+/// <triple>-gcc in program path is found
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+
+/// <triple>-gcc on the PATH is found
+// RUN: mkdir -p %t/env
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+
+/// <triple>-gcc in program path is preferred to one on the PATH
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+
+/// On program path, <triple>-gcc is preferred to plain gcc
+// RUN: touch %t/gcc && chmod +x %t/gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
+// NOTREAL_GCC_PREFERRED-NOT: /gcc
+
+/// <triple>-gcc on the PATH is preferred to gcc in program path
+// RUN: rm %t/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+
+/// <triple>-gcc on the PATH is preferred to gcc on the PATH
+// RUN: rm %t/gcc
+// RUN: touch %t/env/gcc && chmod +x %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+
+/// <default-triple>-gcc has lowest priority so <triple>-gcc
+/// on PATH beats default triple in program path
+/// Darwin triples have a version appended to them, even if set via
+/// LLVM_DEFAULT_TARGET_TRIPLE. So the only way to know for sure is to ask clang.
+// RUN: DEFAULT_TRIPLE=`%t/clang --version | grep "Target:" | cut -d ' ' -f2`
+// RUN: touch %t/$DEFAULT_TRIPLE-gcc && chmod +x %t/$DEFAULT_TRIPLE-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc
+
+/// plain gcc on PATH beats default triple in program path
+// RUN: rm %t/env/notreal-none-elf-gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+
+/// default triple only chosen when no others are present
+// RUN: rm %t/env/gcc
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc
+
+/// -B paths are searched separately so default triple will win
+/// if put in one of those even if other paths have higher priority names
+// RUN: mkdir -p %t/prefix
+// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \
+// RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_IN_PREFIX %s
+// DEFAULT_TRIPLE_IN_PREFIX: prefix/{{.*}}-gcc
+// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc
+
+/// Only if there is nothing in the prefix will we search other paths
+// RUN: rm %t/prefix/$DEFAULT_TRIPLE-gcc
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \
+// RUN:   FileCheck --check-prefix=EMPTY_PREFIX_DIR %s
+// EMPTY_PREFIX_DIR: notreal-none-elf-gcc


        


More information about the cfe-commits mailing list