Thanks for the heads up Nico. Yes that was a mistake, it was reviewed as https://reviews.llvm.org/D79842 but somehow I told arc to land it as the duplicate diff instead.
I'll look into the failure. On Mon, 22 Jun 2020 at 13:48, Nico Weber <tha...@chromium.org> wrote: > > 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
