Hi, https://reviews.llvm.org/D79988 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?
This also breaks check-clang on macOS: http://45.33.8.238/mac/15950/step_7.txt Please take a look and revert if it takes a while to investigate. Nico On Mon, Jun 22, 2020 at 4:41 AM David Spickett via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > Author: David Spickett > Date: 2020-06-22T09:41:13+01:00 > New Revision: 028571d60843cb87e2637ef69ee09090d4526c62 > > URL: > https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62 > DIFF: > https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62.diff > > LOG: [clang][Driver] Correct tool search path priority > > Summary: > 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) > > Subscribers: kristof.beyls, cfe-commits > > Tags: #clang > > Differential Revision: https://reviews.llvm.org/D79988 > > Added: > clang/test/Driver/program-path-priority.c > > Modified: > clang/lib/Driver/Driver.cpp > clang/test/lit.cfg.py > > Removed: > > > > > ################################################################################ > diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp > index a48761af400f..27477553963c 100644 > --- a/clang/lib/Driver/Driver.cpp > +++ b/clang/lib/Driver/Driver.cpp > @@ -4717,13 +4717,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; > } > > @@ -4736,8 +4734,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))) > @@ -4746,17 +4745,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..48f23917812e > --- /dev/null > +++ b/clang/test/Driver/program-path-priority.c > @@ -0,0 +1,106 @@ > +/// 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. > + > +/// Copy 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 > +// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_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/%target_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/%target_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 > > diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py > index 413f81175420..b1f4abe4ec3a 100644 > --- a/clang/test/lit.cfg.py > +++ b/clang/test/lit.cfg.py > @@ -46,6 +46,8 @@ > config.substitutions.append( > ('%src_include_dir', config.clang_src_dir + '/include')) > > +config.substitutions.append( > + ('%target_triple', config.target_triple)) > > # Propagate path to symbolizer for ASan/MSan. > llvm_config.with_system_environment( > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
