arichardson updated this revision to Diff 336059.
arichardson added a comment.
Add the raw triple prefix for programs (but prefer the adjusted triple) and
always use the adjusted triple for libraries.
Should hopefully address the feedback from @joerg.
Also improve tests to check that we don't insert additional empty/"-unknown"
triple components.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100054/new/
https://reviews.llvm.org/D100054
Files:
clang/include/clang/Driver/Driver.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Driver/ToolChains/RISCVToolchain.cpp
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
clang/test/Driver/freebsd-m32.c
clang/test/Driver/linux-per-target-runtime-dir.c
Index: clang/test/Driver/linux-per-target-runtime-dir.c
===================================================================
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,18 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
// RUN: | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
// CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+/// Check that we handle flags such as -m32 when searching for the builtins:
+/// Previously clang would use the raw triple passed to -target to find builtins
+/// and sanitizer libraries, but this will result in build errors when compiling
+/// with flags such as -m32. Check that we use the adjusted triple instead:
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=i386-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: | FileCheck --check-prefix=CHECK-FILE-NAME-I386 %s
+// CHECK-FILE-NAME-I386: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu -m32 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: | FileCheck --check-prefix=CHECK-FILE-NAME-X8664-M32 %s
+// CHECK-FILE-NAME-X8664-M32: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
Index: clang/test/Driver/freebsd-m32.c
===================================================================
--- /dev/null
+++ clang/test/Driver/freebsd-m32.c
@@ -0,0 +1,76 @@
+/// Check that building with -m32 links with i386-freebsd12.2-ld/ instead of
+/// x86_64-freebsd12.2-ld and that we select the right sanitizer runtime.
+
+/// We should select x86_64-unknown-freebsd12.2-ld since it matches the triple argument
+/// Note: The paths specified by -B are not searched for triple-prefixed tools, so
+/// we also have to set $PATH.
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64-unknown-freebsd12.2 %s \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-64
+// PREFIXED-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// PREFIXED-64-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// Should not be passing an explicit linker emulation for the 64-bit case
+// PREFIXED-64-NOT: "-m"
+// RUN: env PATH=/this/path/does/not/exist %clang -no-canonical-prefixes \
+// RUN: -target x86_64-unknown-freebsd12.2 %s \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=MINUS-B-NO-TRIPLE-PREFIX
+// MINUS-B-NO-TRIPLE-PREFIX: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// MINUS-B-NO-TRIPLE-PREFIX-NEXT: "ld" "--eh-frame-hdr"
+// MINUS-B-NO-TRIPLE-PREFIX-NOT: "-m"
+
+/// The triple passed to clang -cc1 should be normalized, but the prefix when searching
+/// for ld should not be normalized. Since there is no x86_64--freebsd12.2-ld, passing
+/// -target x86_64--freebsd12.2 should not find a a valid linker:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64--freebsd12.2 %s \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-64
+// NO-NORMALIZE-LD-PREFIX-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// NO-NORMALIZE-LD-PREFIX-64-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-NOT: "-m"
+
+/// We should search for i386-unknown-freebsd12.2-ld when -m32 is passed (and also pass -m elf_i386_fbsd):
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64-unknown-freebsd12.2 %s -m32 \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-M32
+// PREFIXED-M32: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// PREFIXED-M32-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}i386-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// PREFIXED-M32-SAME: "-m" "elf_i386_fbsd"
+/// Only the -cc1 triple should be normalized when adjusted by -m32:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64-freebsd13 %s -m32 \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-M32
+// NO-NORMALIZE-LD-PREFIX-M32: "-cc1" "-triple" "i386-unknown-freebsd13"
+// NO-NORMALIZE-LD-PREFIX-M32-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-M32-SAME: "-m" "elf_i386_fbsd"
+
+/// Check that -m32 also affects the library name for the sanitizer runtime.
+/// This previous caused build issues since we were selecting "./lib/clang/13.0.0/lib/x86_64-unknown-freebsd12.2/libclang_rt.asan.a"
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64-unknown-freebsd12.2 %s -fsanitize=address \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=ASAN-64
+// ASAN-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// ASAN-64-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// ASAN-64-NOT: "-m"
+// ASAN-64-SAME: lib/freebsd/libclang_rt.asan-x86_64.a"
+// ASAN-64-NOT: "-m"
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN: -target x86_64-unknown-freebsd12.2 %s -fsanitize=address -m32 \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=ASAN-M32
+// ASAN-M32: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// ASAN-M32-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}i386-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// ASAN-M32-SAME: "-m" "elf_i386_fbsd"
+// ASAN-M32-SAME: lib/freebsd/libclang_rt.asan-i386.a"
+
+/// If there is no ld matching the -m32 triple, we should use the 64-bit linker as we
+/// can assume that it is also able to link 32-bit files. However, we still need to
+/// pass 32-bit flags and library paths:
+// Note: This also checks that the -m32 triple isn't normalized since there is a linker
+// that matches the normalized triple (i386-unknown-freebsd12.2-ld), but not one that
+// matches the adjusted triple (i386-freebsd12.2-ld).
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clangxx -no-canonical-prefixes \
+// RUN: -target x86_64-freebsd12.2 %s -fsanitize=address -m32 \
+// RUN: -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=ASAN-M32-64BIT-LINKER
+// ASAN-M32-64BIT-LINKER: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// ASAN-M32-64BIT-LINKER-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}x86_64-freebsd12.2-ld" "--eh-frame-hdr"
+// ASAN-M32-64BIT-LINKER-SAME: "-m" "elf_i386_fbsd"
+// ASAN-M32-64BIT-LINKER-SAME: lib/freebsd/libclang_rt.asan-i386.a"
Index: clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
===================================================================
--- /dev/null
+++ clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
@@ -0,0 +1 @@
+#!/bin/true
Index: clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
===================================================================
--- /dev/null
+++ clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
@@ -0,0 +1 @@
+#!/bin/true
Index: clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
===================================================================
--- /dev/null
+++ clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
@@ -0,0 +1 @@
+#!/bin/true
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===================================================================
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -127,7 +127,8 @@
llvm::sys::path::append(SysRootDir, LibDir, "..", TripleStr);
} else {
// Use the triple as provided to the driver. Unlike the parsed triple
- // this has not been normalized to always contain every field.
+ // this has not been normalized to always contain every field (but flags
+ // such as -m32 may have changed individual components).
llvm::sys::path::append(SysRootDir, getDriver().Dir, "..",
getDriver().getTargetTriple());
}
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -485,7 +485,8 @@
Optional<std::string> ToolChain::getRuntimePath() const {
SmallString<128> P;
- // First try the triple passed to driver as --target=<triple>.
+ // First try the triple passed to driver as --target=<triple> (but including
+ // adjustments made by flags such as -m32).
P.assign(D.ResourceDir);
llvm::sys::path::append(P, "lib", D.getTargetTriple());
if (getVFS().exists(P))
@@ -503,7 +504,8 @@
Optional<std::string> ToolChain::getCXXStdlibPath() const {
SmallString<128> P;
- // First try the triple passed to driver as --target=<triple>.
+ // First try the triple passed to driver as --target=<triple> (possibly
+ // adjusted for -m32, but not normalized).
P.assign(D.Dir);
llvm::sys::path::append(P, "..", "lib", D.getTargetTriple(), "c++");
if (getVFS().exists(P))
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -139,9 +139,10 @@
CCPrintHeadersFilename(), CCLogDiagnosticsFilename(),
CCCPrintBindings(false), CCPrintOptions(false), CCPrintHeaders(false),
CCLogDiagnostics(false), CCGenDiagnostics(false),
- CCPrintProcessStats(false), TargetTriple(TargetTriple),
- CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
- GenReproducer(false), SuppressMissingInputWarning(false) {
+ CCPrintProcessStats(false), RawTargetTriple(TargetTriple),
+ EffectiveTargetTriple(TargetTriple), CCCGenericGCCName(""), Saver(Alloc),
+ CheckInputsExist(true), GenReproducer(false),
+ SuppressMissingInputWarning(false) {
// Provide a sane fallback if no VFS is specified.
if (!this->VFS)
this->VFS = llvm::vfs::getRealFileSystem();
@@ -437,15 +438,16 @@
///
/// This routine provides the logic to compute a target triple from various
/// args passed to the driver and the default triple string.
-static llvm::Triple computeTargetTriple(const Driver &D,
- StringRef TargetTriple,
+static llvm::Triple computeTargetTriple(const Driver &D, StringRef TargetTriple,
const ArgList &Args,
- StringRef DarwinArchName = "") {
+ StringRef DarwinArchName = "",
+ bool Normalize = true) {
// FIXME: Already done in Compilation *Driver::BuildCompilation
if (const Arg *A = Args.getLastArg(options::OPT_target))
TargetTriple = A->getValue();
- llvm::Triple Target(llvm::Triple::normalize(TargetTriple));
+ llvm::Triple Target(Normalize ? llvm::Triple::normalize(TargetTriple)
+ : TargetTriple);
// GNU/Hurd's triples should have been -hurd-gnu*, but were historically made
// -gnu* only, and we can not change this, so we have to detect that case as
@@ -1109,15 +1111,15 @@
// and getToolChain is const.
if (IsCLMode()) {
// clang-cl targets MSVC-style Win32.
- llvm::Triple T(TargetTriple);
+ llvm::Triple T(RawTargetTriple);
T.setOS(llvm::Triple::Win32);
T.setVendor(llvm::Triple::PC);
T.setEnvironment(llvm::Triple::MSVC);
T.setObjectFormat(llvm::Triple::COFF);
- TargetTriple = T.str();
+ RawTargetTriple = T.str();
}
if (const Arg *A = Args.getLastArg(options::OPT_target))
- TargetTriple = A->getValue();
+ RawTargetTriple = A->getValue();
if (const Arg *A = Args.getLastArg(options::OPT_ccc_install_dir))
Dir = InstalledDir = A->getValue();
for (const Arg *A : Args.filtered(options::OPT_B)) {
@@ -1173,9 +1175,43 @@
// Perform the default argument translations.
DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs);
+ // If command line flags such as -m32, etc. changed parts of the triple that
+ // are not just changes to normalization, we also need to update the raw
+ // triple string that is used to find tools. This ensures e.g. that clang -m32
+ // searches for i386-*-ld instead of x86_64-*-ld when linking (and also uses
+ // the triple-prefixed library paths).
+ EffectiveTargetTriple =
+ computeTargetTriple(*this, RawTargetTriple, *UArgs, "");
+ // Note: It is important that we don't normalize this triple to avoid adding
+ // empty components (i.e. no additional -unknown compared to the raw one).
+ llvm::Triple NormalizedRawTriple(llvm::Triple::normalize(RawTargetTriple));
+ if (EffectiveTargetTriple != NormalizedRawTriple) {
+ // computeTargetTriple() may have added additional empty/-unknown
+ // components. De-normalize it to create the prefixed search paths:
+ // `clang -target x86_64-freebsd12 -m32` should search for programs and
+ // libraries using i386-freebsd12, not i386-unknown-freebsd12.
+ llvm::SmallVector<StringRef, 5> RawComponents;
+ StringRef(RawTargetTriple).split(RawComponents, '-');
+ llvm::SmallVector<StringRef, 5> EffectiveComponents;
+ StringRef(EffectiveTargetTriple.str()).split(EffectiveComponents, '-');
+ // Drop any empty/"unknown" components that are not also present in the raw
+ // target triple.
+ llvm::SmallString<64> AdjustedTriple = EffectiveComponents[0];
+ for (size_t EI = 1, RI = 1; EI < EffectiveComponents.size(); EI++) {
+ const StringRef EffectiveComp = EffectiveComponents[EI];
+ // Skip extra intermediate empty/unknown as well trailing empty parts.
+ if (EffectiveComp.empty() || EffectiveComp == "unknown") {
+ if (RI >= RawComponents.size() || EffectiveComp != RawComponents[RI])
+ continue;
+ }
+ AdjustedTriple += "-";
+ AdjustedTriple += EffectiveComp;
+ RI++;
+ }
+ TargetTripleStr = std::string(AdjustedTriple);
+ }
// Owned by the host.
- const ToolChain &TC = getToolChain(
- *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
+ const ToolChain &TC = getToolChain(*UArgs, EffectiveTargetTriple);
// The compilation takes ownership of Args.
Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
@@ -4561,9 +4597,9 @@
StringRef ArchName = BAA->getArchName();
if (!ArchName.empty())
- TC = &getToolChain(C.getArgs(),
- computeTargetTriple(*this, TargetTriple,
- C.getArgs(), ArchName));
+ TC = &getToolChain(
+ C.getArgs(),
+ computeTargetTriple(*this, RawTargetTriple, C.getArgs(), ArchName));
else
TC = &C.getDefaultToolChain();
@@ -4752,8 +4788,7 @@
}
const char *Driver::getDefaultImageName() const {
- llvm::Triple Target(llvm::Triple::normalize(TargetTriple));
- return Target.isOSWindows() ? "a.exe" : "a.out";
+ return EffectiveTargetTriple.isOSWindows() ? "a.exe" : "a.out";
}
/// Create output filename based on ArgValue, which could either be a
@@ -5067,8 +5102,15 @@
void Driver::generatePrefixedToolNames(
StringRef Tool, const ToolChain &TC,
SmallVectorImpl<std::string> &Names) const {
- // FIXME: Needs a better variable than TargetTriple
- Names.emplace_back((TargetTriple + "-" + Tool).str());
+ // Note: When searching e.g. for ld with `-target x86_64-freebsd -m32`, we
+ // prefer i386-freebsd-ld, but assume that x86_64-freebsd-ld can also be used
+ // for linking and should be preferred over a plain ld.
+ // However, it is important that the library search paths (e.g. for
+ // sanitizers) use the effective triple and not the raw target triple since we
+ // might otherwise end up trying to link a 32-bit program a 64-bit library.
+ Names.emplace_back((getTargetTriple() + "-" + Tool).str());
+ if (getTargetTriple() != RawTargetTriple)
+ Names.emplace_back((RawTargetTriple + "-" + Tool).str());
Names.emplace_back(Tool);
}
Index: clang/include/clang/Driver/Driver.h
===================================================================
--- clang/include/clang/Driver/Driver.h
+++ clang/include/clang/Driver/Driver.h
@@ -219,8 +219,13 @@
CC1ToolFunc CC1Main = nullptr;
private:
- /// Raw target triple.
- std::string TargetTriple;
+ /// Raw target triple as passed to -target (or the default value).
+ std::string RawTargetTriple;
+ /// Effective target triple (raw triple adjusted for -m32/etc. but not
+ /// normalized, since it is used as a path prefix).
+ std::string TargetTripleStr;
+ /// Normalized triple adjusted for command line flags (-m32/etc.)
+ llvm::Triple EffectiveTargetTriple;
/// Name to use when invoking gcc/g++.
std::string CCCGenericGCCName;
@@ -334,7 +339,11 @@
const std::string &getTitle() { return DriverTitle; }
void setTitle(std::string Value) { DriverTitle = std::move(Value); }
- std::string getTargetTriple() const { return TargetTriple; }
+ /// Get the non-normalized effective target triple. For example, clang
+ /// -target=x86_64-linux -m32 returns i386-linux, not i386-unknown-linux.
+ std::string getTargetTriple() const {
+ return TargetTripleStr.empty() ? RawTargetTriple : TargetTripleStr;
+ }
/// Get the path to the main clang executable.
const char *getClangProgramPath() const {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
