Author: Fangrui Song Date: 2024-02-28T15:12:31-08:00 New Revision: ff07c9b701dc6372f82d989d01768051e848b30d
URL: https://github.com/llvm/llvm-project/commit/ff07c9b701dc6372f82d989d01768051e848b30d DIFF: https://github.com/llvm/llvm-project/commit/ff07c9b701dc6372f82d989d01768051e848b30d.diff LOG: [Driver] Unify InstalledDir and Dir (#80527) `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 9ade6a9a747c49383ac747bd8ffaa6a0beaef71c). 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`. Added: Modified: clang/include/clang/Driver/Driver.h clang/test/Driver/darwin-header-search-libcxx.cpp clang/test/Driver/mingw-sysroot.cpp clang/test/Driver/no-canonical-prefixes.c clang/test/Driver/program-path-priority.c clang/test/Driver/rocm-detect.hip clang/tools/driver/driver.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index a5ca637853a6ae..73cf326101ffd5 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -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. @@ -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; } diff --git a/clang/test/Driver/darwin-header-search-libcxx.cpp b/clang/test/Driver/darwin-header-search-libcxx.cpp index 70cc06090a993d..5695f53683bab3 100644 --- a/clang/test/Driver/darwin-header-search-libcxx.cpp +++ b/clang/test/Driver/darwin-header-search-libcxx.cpp @@ -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 \ diff --git a/clang/test/Driver/mingw-sysroot.cpp b/clang/test/Driver/mingw-sysroot.cpp index 50152b2ca210d2..5d512e66697097 100644 --- a/clang/test/Driver/mingw-sysroot.cpp +++ b/clang/test/Driver/mingw-sysroot.cpp @@ -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" @@ -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" @@ -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" diff --git a/clang/test/Driver/no-canonical-prefixes.c b/clang/test/Driver/no-canonical-prefixes.c index fb54f85f959ae2..669e56639284a7 100644 --- a/clang/test/Driver/no-canonical-prefixes.c +++ b/clang/test/Driver/no-canonical-prefixes.c @@ -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: .{{$}} diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c index ee931dd7a9a35c..c940c4ced94420 100644 --- a/clang/test/Driver/program-path-priority.c +++ b/clang/test/Driver/program-path-priority.c @@ -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 @@ -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 @@ -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" diff --git a/clang/test/Driver/rocm-detect.hip b/clang/test/Driver/rocm-detect.hip index 0db994af556f3a..8b15c322e3fb36 100644 --- a/clang/test/Driver/rocm-detect.hip +++ b/clang/test/Driver/rocm-detect.hip @@ -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 @@ -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 diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 0dfb512adb0cc6..376025e3605be8 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -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 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits