From d6efc9811646edbfe13f06c2676fb469f1c155b1 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Wed, 24 Jun 2020 16:45:21 +0100 Subject: [PATCH] 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". To fix this get default triple from clang itself during the test instead of substituting it in via lit. --- clang/lib/Driver/Driver.cpp | 39 ++++++----- clang/test/Driver/program-path-priority.c | 109 ++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 clang/test/Driver/program-path-priority.c diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 24436ec..1f718b3 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 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. -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 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 0000000..ba893e7 --- /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: +/// -tool, tool, -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 + +/// -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 + +/// -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 + +/// -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, -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 + +/// -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 + +/// -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 + +/// -gcc has lowest priority so -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 -- 2.7.4