From 028571d60843cb87e2637ef69ee09090d4526c62 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 11 May 2020 17:13:00 +0100 Subject: [PATCH] [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 --- clang/lib/Driver/Driver.cpp | 39 ++++---- clang/test/Driver/program-path-priority.c | 106 ++++++++++++++++++++++ clang/test/lit.cfg.py | 2 + 3 files changed, 131 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 a48761af400fda..27477553963ca9 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 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. -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 00000000000000..48f23917812eb8 --- /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: +/// -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. + +/// 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 + +/// -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 +// 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 413f811754204e..b1f4abe4ec3a93 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(