DavidSpickett created this revision.
DavidSpickett added reviewers: rogfer01, ddunbar, chandlerc.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.
DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.


================
Comment at: clang/test/Driver/program-path-priority.c:72
+// <default-triple>-gcc has lowest priority
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
----------------
Pretty sure I'm stretching the limits here, probably not suitable for Windows. 
I hoped to be able to capture the default triple in a CHECK line, then use it 
in a following RUN line. (though thinking about how FileCheck works, that isn't 
possible)


As seen in:
https://bugs.llvm.org/show_bug.cgi?id=45693

When clang looks for a tool it has a set of
possible names for it, in priority order.
Previously it would look for these names in
the program path. Then look for all the names
in the PATH.

This means that aarch64-none-elf-gcc on the PATH
would lose to gcc in the program path.
(which was /usr/bin in the bug's case)

This changes that logic to search each name in both
possible locations, then move to the next name.
Which is more what you would expect to happen when
using a non default triple.

(-B prefixes maybe should follow this logic too,
but are not changed in this patch)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79842

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/program-path-priority.c

Index: clang/test/Driver/program-path-priority.c
===================================================================
--- /dev/null
+++ clang/test/Driver/program-path-priority.c
@@ -0,0 +1,113 @@
+// Check the priority used when searching for tools
+// Names and locations have a set priority. (highest to lowest)
+// <triple>-tool, tool, <defaul triple>-tool
+// program path, PATH
+// Such that a more specific name found in the PATH
+// will win over a less specific name found in the program path
+// Prefix dirs (added with -B) overrride the location,
+// so only name priority is accounted for, unless we fail to find
+// anything at all in the prefix.
+
+// Copy clang to a new dir which will be its
+// "program path" for these tests
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp %clang %t
+
+// 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
+// RUN: 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
+// RUN: 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
+// RUN: 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
+// RUN: 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
+// RUN: 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
+// RUN: default_triple=$(%t/clang --version | grep -oP "(?<=Target:\s).*")
+// RUN: touch %t/${default_triple}-gcc
+// RUN: chmod +x %t/${default_triple}-gcc
+
+// <triple>-gcc on PATH beats default triple in program path
+// RUN: touch %t/${default_triple}-gcc
+// RUN: 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
+// RUN: 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
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4702,13 +4702,11 @@
 }
 
 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;
 }
 
@@ -4721,8 +4719,10 @@
   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)))
@@ -4731,17 +4731,24 @@
   }
 
   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.
+    // This means that if the names are: triple-gcc, gcc
+    // And triple-gcc is only on the path, we will find it
+    // first. Over gcc which may be in a program path like
+    // /usr/bin.
+    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);
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to