Skip to content

Commit

Permalink
[Driver] Unify InstalledDir and Dir (#80527)
Browse files Browse the repository at this point in the history
`Driver::ClangExecutable` is derived from:

* (-canonical-prefixes default): `realpath` on the executable path
* (-no-canonical-prefixes) argv[0] (consult PATH if argv[0] is a word)

`Dir` and `ResourceDir` are derived from `ClangExecutable`. Both
variables are used to derive certain include and library paths.

`InstalledDir` is a related concept used to derive certain other paths.
`InstalledDir` is weird in the -canonical-prefixes mode: Clang
calls `make_absolute` but does not follow symlinks
(FIXME from 9ade6a9).
This causes some search and library paths to be mix-and-matched.

The "Do a PATH lookup, if there are no directory components." logic
makes things worse.
`InstalledDir` is different when you invoke it via `PATH`:
```
% which clang
/usr/bin/clang
% clang -v |& grep InstalledDir
InstalledDir: /usr/bin
% /usr/lib/llvm-16/bin/clang -v |& grep InstalledDir
InstalledDir: /usr/lib/llvm-16/bin
```

I believe `InstalledDir` was a partial solution to
`-no-canonical-prefixes` and should be eventually removed.

This patch removes `SetInstallDir` and relies on Driver::Driver to set
`InstalledDir` to `Dir`. The behavior for regular file `clang` or
`-no-canonical-prefixes` is unchanged.

If a user creates a symlink to the regular file `clang` and uses the
default `-canonical-prefixes`, they now consistently get search and
library paths relative to the regular file `clang`, not mix-and-match
paths.

If a user creates a symlink to the regular file `clang` and replaces
some directorys from the actual installation, they should change the
symlink to a wrapper that calls the underlying clang with
`-no-canonical-prefixes`.
  • Loading branch information
MaskRay committed Feb 28, 2024
1 parent c345198 commit ff07c9b
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 39 deletions.
3 changes: 1 addition & 2 deletions clang/include/clang/Driver/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class Driver {
/// Target and driver mode components extracted from clang executable name.
ParsedClangName ClangNameParts;

/// The path to the installed clang directory, if any.
/// TODO: Remove this in favor of Dir.
std::string InstalledDir;

/// The path to the compiler resource directory.
Expand Down Expand Up @@ -433,7 +433,6 @@ class Driver {
return InstalledDir.c_str();
return Dir.c_str();
}
void setInstalledDir(StringRef Value) { InstalledDir = std::string(Value); }

bool isSaveTempsEnabled() const { return SaveTemps != SaveTempsNone; }
bool isSaveTempsObj() const { return SaveTemps == SaveTempsObj; }
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/darwin-header-search-libcxx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@
// RUN: ln -sf %t/install/bin/clang %t/symlinked1/bin/clang
// RUN: mkdir -p %t/symlinked1/include/c++/v1

// RUN: %t/symlinked1/bin/clang -### %s -fsyntax-only 2>&1 \
// RUN: %t/symlinked1/bin/clang -### %s -no-canonical-prefixes -fsyntax-only 2>&1 \
// RUN: --target=x86_64-apple-darwin \
// RUN: -stdlib=libc++ \
// RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_cxx_v1 \
Expand Down
12 changes: 7 additions & 5 deletions clang/test/Driver/mingw-sysroot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@
// CHECK_TESTROOT_GCC_EXPLICIT: "-internal-isystem" "{{[^"]+}}/testroot-gcc{{/|\\\\}}include"


// If there's a matching sysroot next to the clang binary itself, prefer that
// If -no-canonical-prefixes and there's a matching sysroot next to the clang binary itself, prefer that
// over a gcc in the path:

// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG %s
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC2 %s
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s -no-canonical-prefixes 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG %s
// CHECK_TESTROOT_GCC2: "{{[^"]+}}/testroot-gcc{{/|\\\\}}x86_64-w64-mingw32{{/|\\\\}}include"
// CHECK_TESTROOT_CLANG: "{{[^"]+}}/testroot-clang{{/|\\\\}}x86_64-w64-mingw32{{/|\\\\}}include"


Expand Down Expand Up @@ -82,7 +84,7 @@
// that indicates that we did choose the right base, even if this particular directory
// actually doesn't exist here.

// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang-native/bin/clang -target x86_64-w64-mingw32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_NATIVE %s
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang-native/bin/clang -no-canonical-prefixes --target=x86_64-w64-mingw32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_NATIVE %s
// CHECK_TESTROOT_CLANG_NATIVE: "{{[^"]+}}/testroot-clang-native{{/|\\\\}}x86_64-w64-mingw32{{/|\\\\}}include"


Expand All @@ -93,12 +95,12 @@
// that defaults to x86_64 mingw, but it's easier to test this in cross setups
// with symlinks, like the other tests here.)

// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang -no-canonical-prefixes --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|\\\\}}i686-w64-mingw32{{/|\\\\}}include"


// If the user calls clang with a custom literal triple, make sure this maps
// to sysroots with the matching spelling.

// RUN: %T/testroot-custom-triple/bin/clang --target=x86_64-w64-mingw32foo -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CUSTOM_TRIPLE %s
// RUN: %T/testroot-custom-triple/bin/clang -no-canonical-prefixes --target=x86_64-w64-mingw32foo -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CUSTOM_TRIPLE %s
// CHECK_TESTROOT_CUSTOM_TRIPLE: "{{[^"]+}}/testroot-custom-triple{{/|\\\\}}x86_64-w64-mingw32foo{{/|\\\\}}include"
2 changes: 1 addition & 1 deletion clang/test/Driver/no-canonical-prefixes.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
// RUN: | FileCheck --check-prefix=NON-CANONICAL %s
//
// FIXME: This should really be '.real'.
// CANONICAL: InstalledDir: {{.*}}.fake
// CANONICAL: InstalledDir: {{.*}}bin
// CANONICAL: {{[/|\\]*}}clang{{.*}}" -cc1
//
// NON-CANONICAL: InstalledDir: .{{$}}
Expand Down
13 changes: 8 additions & 5 deletions clang/test/Driver/program-path-priority.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
// 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"
// PROG_PATH_NOTREAL_GCC: notreal-none-unknown-elf

/// <triple>-gcc on the PATH is found
// RUN: mkdir -p %t/env
Expand All @@ -57,7 +57,7 @@
// 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: notreal-none-unknown-elf"
// NOTREAL_GCC_PREFERRED-NOT: /gcc"

/// <triple>-gcc on the PATH is preferred to gcc in program path
Expand Down Expand Up @@ -125,6 +125,9 @@
/// Only if there is nothing in the prefix will we search other paths
/// -f in case $DEFAULT_TRIPLE == %target_triple
// RUN: rm -f %t/prefix/$DEFAULT_TRIPLE-gcc %t/prefix/%target_triple-gcc %t/prefix/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"
// RUN: env "PATH=" %t/clang -### -canonical-prefixes --target=notreal-none-elf %s -B %t/prefix 2>&1 | \
// RUN: FileCheck --check-prefix=EMPTY_PREFIX_DIR1 %s
// EMPTY_PREFIX_DIR1: gcc"
// RUN: env "PATH=" %t/clang -### -no-canonical-prefixes --target=notreal-none-elf %s -B %t/prefix 2>&1 | \
// RUN: FileCheck --check-prefix=EMPTY_PREFIX_DIR2 %s
// EMPTY_PREFIX_DIR2: notreal-none-elf-gcc"
4 changes: 2 additions & 2 deletions clang/test/Driver/rocm-detect.hip
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
// RUN: rm -rf %t/rocm-spack
// RUN: cp -r %S/Inputs/rocm-spack %t
// RUN: ln -fs %clang %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -no-canonical-prefixes -v \
// RUN: -resource-dir=%t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/lib/clang \
// RUN: -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 --print-rocm-search-dirs %s 2>&1 \
// RUN: | FileCheck -check-prefixes=SPACK %s
Expand All @@ -111,7 +111,7 @@
// ROCm release. --hip-path and --rocm-device-lib-path can be used to specify them.

// RUN: cp -r %t/rocm-spack/hip-* %t/rocm-spack/hip-4.0.0-abcd
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -v \
// RUN: %t/rocm-spack/llvm-amdgpu-4.0.0-ieagcs7inf7runpyfvepqkurasoglq4z/bin/clang -### -no-canonical-prefixes -v \
// RUN: -target x86_64-linux-gnu --cuda-gpu-arch=gfx900 \
// RUN: --hip-path=%t/rocm-spack/hip-4.0.0-abcd \
// RUN: %s 2>&1 | FileCheck -check-prefixes=SPACK-SET %s
Expand Down
23 changes: 0 additions & 23 deletions clang/tools/driver/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -323,28 +323,6 @@ static void FixupDiagPrefixExeName(TextDiagnosticPrinter *DiagClient,
DiagClient->setPrefix(std::string(ExeBasename));
}

static void SetInstallDir(SmallVectorImpl<const char *> &argv,
Driver &TheDriver, bool CanonicalPrefixes) {
// Attempt to find the original path used to invoke the driver, to determine
// the installed path. We do this manually, because we want to support that
// path being a symlink.
SmallString<128> InstalledPath(argv[0]);

// Do a PATH lookup, if there are no directory components.
if (llvm::sys::path::filename(InstalledPath) == InstalledPath)
if (llvm::ErrorOr<std::string> Tmp = llvm::sys::findProgramByName(
llvm::sys::path::filename(InstalledPath.str())))
InstalledPath = *Tmp;

// FIXME: We don't actually canonicalize this, we just make it absolute.
if (CanonicalPrefixes)
llvm::sys::fs::make_absolute(InstalledPath);

StringRef InstalledPathParent(llvm::sys::path::parent_path(InstalledPath));
if (llvm::sys::fs::exists(InstalledPathParent))
TheDriver.setInstalledDir(InstalledPathParent);
}

static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV,
const llvm::ToolContext &ToolContext) {
// If we call the cc1 tool from the clangDriver library (through
Expand Down Expand Up @@ -484,7 +462,6 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) {
ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);

Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
SetInstallDir(Args, TheDriver, CanonicalPrefixes);
auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(ProgName);
TheDriver.setTargetAndMode(TargetAndMode);
// If -canonical-prefixes is set, GetExecutablePath will have resolved Path
Expand Down

0 comments on commit ff07c9b

Please sign in to comment.