<div dir="ltr">Hi,<div><br></div><div><a href="https://reviews.llvm.org/D79988">https://reviews.llvm.org/D79988</a> is apparently a "Restricted Differential Revision" and I don't have permissions to do that. This being an open source project, this is not something we do. I'm guessing it's not intentional?<br></div><div><br></div><div>This also breaks check-clang on macOS: <a href="http://45.33.8.238/mac/15950/step_7.txt">http://45.33.8.238/mac/15950/step_7.txt</a> Please take a look and revert if it takes a while to investigate.</div><div><br></div><div>Nico</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 22, 2020 at 4:41 AM David Spickett via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: David Spickett<br>
Date: 2020-06-22T09:41:13+01:00<br>
New Revision: 028571d60843cb87e2637ef69ee09090d4526c62<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62.diff</a><br>
<br>
LOG: [clang][Driver] Correct tool search path priority<br>
<br>
Summary:<br>
As seen in:<br>
<a href="https://bugs.llvm.org/show_bug.cgi?id=45693" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_bug.cgi?id=45693</a><br>
<br>
When clang looks for a tool it has a set of<br>
possible names for it, in priority order.<br>
Previously it would look for these names in<br>
the program path. Then look for all the names<br>
in the PATH.<br>
<br>
This means that aarch64-none-elf-gcc on the PATH<br>
would lose to gcc in the program path.<br>
(which was /usr/bin in the bug's case)<br>
<br>
This changes that logic to search each name in both<br>
possible locations, then move to the next name.<br>
Which is more what you would expect to happen when<br>
using a non default triple.<br>
<br>
(-B prefixes maybe should follow this logic too,<br>
but are not changed in this patch)<br>
<br>
Subscribers: kristof.beyls, cfe-commits<br>
<br>
Tags: #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D79988" rel="noreferrer" target="_blank">https://reviews.llvm.org/D79988</a><br>
<br>
Added: <br>
clang/test/Driver/program-path-priority.c<br>
<br>
Modified: <br>
clang/lib/Driver/Driver.cpp<br>
clang/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a><br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp<br>
index a48761af400f..27477553963c 100644<br>
--- a/clang/lib/Driver/Driver.cpp<br>
+++ b/clang/lib/Driver/Driver.cpp<br>
@@ -4717,13 +4717,11 @@ void Driver::generatePrefixedToolNames(<br>
}<br>
<br>
static bool ScanDirForExecutable(SmallString<128> &Dir,<br>
- ArrayRef<std::string> Names) {<br>
- for (const auto &Name : Names) {<br>
- llvm::sys::path::append(Dir, Name);<br>
- if (llvm::sys::fs::can_execute(Twine(Dir)))<br>
- return true;<br>
- llvm::sys::path::remove_filename(Dir);<br>
- }<br>
+ const std::string &Name) {<br>
+ llvm::sys::path::append(Dir, Name);<br>
+ if (llvm::sys::fs::can_execute(Twine(Dir)))<br>
+ return true;<br>
+ llvm::sys::path::remove_filename(Dir);<br>
return false;<br>
}<br>
<br>
@@ -4736,8 +4734,9 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {<br>
for (const auto &PrefixDir : PrefixDirs) {<br>
if (llvm::sys::fs::is_directory(PrefixDir)) {<br>
SmallString<128> P(PrefixDir);<br>
- if (ScanDirForExecutable(P, TargetSpecificExecutables))<br>
- return std::string(P.str());<br>
+ for (const auto &TargetSpecificExecutable : TargetSpecificExecutables)<br>
+ if (ScanDirForExecutable(P, TargetSpecificExecutable))<br>
+ return std::string(P.str());<br>
} else {<br>
SmallString<128> P((PrefixDir + Name).str());<br>
if (llvm::sys::fs::can_execute(Twine(P)))<br>
@@ -4746,17 +4745,25 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const {<br>
}<br>
<br>
const ToolChain::path_list &List = TC.getProgramPaths();<br>
- for (const auto &Path : List) {<br>
- SmallString<128> P(Path);<br>
- if (ScanDirForExecutable(P, TargetSpecificExecutables))<br>
- return std::string(P.str());<br>
- }<br>
+ for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) {<br>
+ // For each possible name of the tool look for it in<br>
+ // program paths first, then the path.<br>
+ // Higher priority names will be first, meaning that<br>
+ // a higher priority name in the path will be found<br>
+ // instead of a lower priority name in the program path.<br>
+ // E.g. <triple>-gcc on the path will be found instead<br>
+ // of gcc in the program path<br>
+ for (const auto &Path : List) {<br>
+ SmallString<128> P(Path);<br>
+ if (ScanDirForExecutable(P, TargetSpecificExecutable))<br>
+ return std::string(P.str());<br>
+ }<br>
<br>
- // If all else failed, search the path.<br>
- for (const auto &TargetSpecificExecutable : TargetSpecificExecutables)<br>
+ // Fall back to the path<br>
if (llvm::ErrorOr<std::string> P =<br>
llvm::sys::findProgramByName(TargetSpecificExecutable))<br>
return *P;<br>
+ }<br>
<br>
return std::string(Name);<br>
}<br>
<br>
diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c<br>
new file mode 100644<br>
index 000000000000..48f23917812e<br>
--- /dev/null<br>
+++ b/clang/test/Driver/program-path-priority.c<br>
@@ -0,0 +1,106 @@<br>
+/// Don't create symlinks on Windows<br>
+// UNSUPPORTED: system-windows<br>
+<br>
+/// Check the priority used when searching for tools<br>
+/// Names and locations are usually in this order:<br>
+/// <triple>-tool, tool, <default triple>-tool<br>
+/// program path, PATH<br>
+/// (from highest to lowest priority)<br>
+/// A higher priority name found in a lower priority<br>
+/// location will win over a lower priority name in a<br>
+/// higher priority location.<br>
+/// Prefix dirs (added with -B) override the location,<br>
+/// so only name priority is accounted for, unless we fail to find<br>
+/// anything at all in the prefix.<br>
+<br>
+/// Copy clang to a new dir which will be its<br>
+/// "program path" for these tests<br>
+// RUN: rm -rf %t && mkdir -p %t<br>
+// RUN: ln -s %clang %t/clang<br>
+<br>
+/// No gccs at all, nothing is found<br>
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s<br>
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc<br>
+// NO_NOTREAL_GCC-NOT: /gcc<br>
+<br>
+/// <triple>-gcc in program path is found<br>
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc<br>
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s<br>
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc<br>
+<br>
+/// <triple>-gcc on the PATH is found<br>
+// RUN: mkdir -p %t/env<br>
+// RUN: rm %t/notreal-none-elf-gcc<br>
+// RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s<br>
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc<br>
+<br>
+/// <triple>-gcc in program path is preferred to one on the PATH<br>
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s<br>
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc<br>
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc<br>
+<br>
+/// On program path, <triple>-gcc is preferred to plain gcc<br>
+// RUN: touch %t/gcc && chmod +x %t/gcc<br>
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s<br>
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc<br>
+// NOTREAL_GCC_PREFERRED-NOT: /gcc<br>
+<br>
+/// <triple>-gcc on the PATH is preferred to gcc in program path<br>
+// RUN: rm %t/notreal-none-elf-gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s<br>
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc<br>
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc<br>
+<br>
+/// <triple>-gcc on the PATH is preferred to gcc on the PATH<br>
+// RUN: rm %t/gcc<br>
+// RUN: touch %t/env/gcc && chmod +x %t/env/gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s<br>
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc<br>
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc<br>
+<br>
+/// <default-triple>-gcc has lowest priority so <triple>-gcc<br>
+/// on PATH beats default triple in program path<br>
+// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s<br>
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc<br>
+<br>
+/// plain gcc on PATH beats default triple in program path<br>
+// RUN: rm %t/env/notreal-none-elf-gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s<br>
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc<br>
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc<br>
+<br>
+/// default triple only chosen when no others are present<br>
+// RUN: rm %t/env/gcc<br>
+// RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s<br>
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc<br>
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc<br>
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc<br>
+<br>
+/// -B paths are searched separately so default triple will win<br>
+/// if put in one of those even if other paths have higher priority names<br>
+// RUN: mkdir -p %t/prefix<br>
+// RUN: mv %t/%target_triple-gcc %t/prefix<br>
+// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc<br>
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=DEFAULT_TRIPLE_IN_PREFIX %s<br>
+// DEFAULT_TRIPLE_IN_PREFIX: prefix/{{.*}}-gcc<br>
+// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc<br>
+<br>
+/// Only if there is nothing in the prefix will we search other paths<br>
+// RUN: rm %t/prefix/%target_triple-gcc<br>
+// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \<br>
+// RUN: FileCheck --check-prefix=EMPTY_PREFIX_DIR %s<br>
+// EMPTY_PREFIX_DIR: notreal-none-elf-gcc<br>
<br>
diff --git a/clang/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a> b/clang/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a><br>
index 413f81175420..b1f4abe4ec3a 100644<br>
--- a/clang/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a><br>
+++ b/clang/test/<a href="http://lit.cfg.py" rel="noreferrer" target="_blank">lit.cfg.py</a><br>
@@ -46,6 +46,8 @@<br>
config.substitutions.append(<br>
('%src_include_dir', config.clang_src_dir + '/include'))<br>
<br>
+config.substitutions.append(<br>
+ ('%target_triple', config.target_triple))<br>
<br>
# Propagate path to symbolizer for ASan/MSan.<br>
llvm_config.with_system_environment(<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>