llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Simon Tatham (statham-arm) <details> <summary>Changes</summary> Sometimes a collection of multilibs has a gap in it, where a set of driver command-line options can't work with any of the available libraries. For example, the Arm MVE extension requires special startup code (you need to initialize FPSCR.LTPSIZE), and also benefits greatly from -mfloat-abi=hard. So a multilib provider might build a library for systems without MVE, and another for MVE with -mfloat-abi=hard, anticipating that that's what most MVE users would want. But then if a user compiles for MVE _without_ -mfloat-abi=hard, thhey can't use either of those libraries – one has an ABI mismatch, and the other will fail to set up LTPSIZE. In that situation, it's useful to include a multilib.yaml entry for the unworkable intermediate situation, and have it map to a fatal error message rather than a set of actual libraries. Then the user gets a build failure with a sensible explanation, instead of selecting an unworkable library and silently generating bad output. The new regression test demonstrates this case. This patch introduces extra syntax into multilib.yaml, so that a record in the `Variants` list can omit the `Dir` key, and in its place, provide a `FatalError` key. Then, if that variant is selected, the error message is emitted as a clang diagnostic, and multilib selection fails. In order to emit the error message in `MultilibSet::select`, I had to pass a `Driver &` to that function, which involved plumbing one through to every call site, and in the unit tests, constructing one specially. --- Patch is 36.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105684.diff 13 Files Affected: - (modified) clang/docs/Multilib.rst (+6) - (modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+2) - (modified) clang/include/clang/Driver/Multilib.h (+15-2) - (modified) clang/lib/Driver/Driver.cpp (+2-1) - (modified) clang/lib/Driver/Multilib.cpp (+34-13) - (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+5-4) - (modified) clang/lib/Driver/ToolChains/Fuchsia.cpp (+1-1) - (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+31-26) - (modified) clang/lib/Driver/ToolChains/OHOS.cpp (+4-3) - (added) clang/test/Driver/baremetal-multilib-custom-error.yaml (+63) - (modified) clang/unittests/Driver/MultilibBuilderTest.cpp (+6-3) - (modified) clang/unittests/Driver/MultilibTest.cpp (+51-36) - (modified) clang/unittests/Driver/SimpleDiagnosticConsumer.h (+16) ``````````diff diff --git a/clang/docs/Multilib.rst b/clang/docs/Multilib.rst index 063fe9a336f2fe..6d77fda3623b20 100644 --- a/clang/docs/Multilib.rst +++ b/clang/docs/Multilib.rst @@ -200,6 +200,12 @@ For a more comprehensive example see # to be a match. Flags: [--target=thumbv7m-none-eabi, -mfpu=fpv4-sp-d16] + # If there is no multilib available for a particular set of flags, and the + # other multilibs are not adequate fallbacks, then you can define a variant + # record with a FatalError key in place of the Dir key. + - FatalError: this multilib collection has no hard-float ABI support + Flags: [--target=thumbv7m-none-eabi, -mfloat-abi=hard] + # The second section of the file is a list of regular expressions that are # used to map from flags generated from command line options to custom flags. diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index ba90742fbdaabc..97573fcf20c1fb 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -810,6 +810,8 @@ def warn_drv_missing_multilib : Warning< InGroup<DiagGroup<"missing-multilib">>; def note_drv_available_multilibs : Note< "available multilibs are:%0">; +def err_drv_multilib_custom_error : Error< + "multilib configuration error: %0">; def err_drv_experimental_crel : Error< "-Wa,--allow-experimental-crel must be specified to use -Wa,--crel. " diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h index 9a2cc9bb1ba134..2b6a64187f7783 100644 --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -18,6 +18,7 @@ #include "llvm/Support/SourceMgr.h" #include <cassert> #include <functional> +#include <optional> #include <string> #include <utility> #include <vector> @@ -25,6 +26,8 @@ namespace clang { namespace driver { +class Driver; + /// This corresponds to a single GCC Multilib, or a segment of one controlled /// by a command line flag. /// See also MultilibBuilder for building a multilib by mutating it @@ -48,13 +51,19 @@ class Multilib { // directory is not mutually exclusive with anything else. std::string ExclusiveGroup; + // Some Multilib objects don't actually represent library directories you can + // select. Instead, they represent failures of multilib selection, of the + // form 'Sorry, we don't have any library compatible with these constraints'. + std::optional<std::string> FatalError; + public: /// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the /// sysroot string so they must either be empty or begin with a '/' character. /// This is enforced with an assert in the constructor. Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {}, StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(), - StringRef ExclusiveGroup = {}); + StringRef ExclusiveGroup = {}, + std::optional<StringRef> FatalError = std::nullopt); /// Get the detected GCC installation path suffix for the multi-arch /// target variant. Always starts with a '/', unless empty @@ -84,6 +93,10 @@ class Multilib { { return GCCSuffix.empty() && OSSuffix.empty() && IncludeSuffix.empty(); } bool operator==(const Multilib &Other) const; + + bool isFatalError() const { return FatalError.has_value(); } + + const std::string &getFatalError() const { return FatalError.value(); } }; raw_ostream &operator<<(raw_ostream &OS, const Multilib &M); @@ -130,7 +143,7 @@ class MultilibSet { /// Select compatible variants, \returns false if none are compatible bool select(const Multilib::flags_list &Flags, - llvm::SmallVectorImpl<Multilib> &) const; + llvm::SmallVectorImpl<Multilib> &, const Driver &D) const; unsigned size() const { return Multilibs.size(); } diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e12416e51f8d24..0ad911972a11ed 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2296,7 +2296,8 @@ bool Driver::HandleImmediateArgs(Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_multi_lib)) { for (const Multilib &Multilib : TC.getMultilibs()) - llvm::outs() << Multilib << "\n"; + if (!Multilib.isFatalError()) + llvm::outs() << Multilib << "\n"; return false; } diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp index 9c091bbfdabab1..4114b7faa15270 100644 --- a/clang/lib/Driver/Multilib.cpp +++ b/clang/lib/Driver/Multilib.cpp @@ -9,6 +9,7 @@ #include "clang/Driver/Multilib.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/Version.h" +#include "clang/Driver/Driver.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringRef.h" @@ -31,9 +32,10 @@ using namespace llvm::sys; Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix, StringRef IncludeSuffix, const flags_list &Flags, - StringRef ExclusiveGroup) + StringRef ExclusiveGroup, + std::optional<StringRef> FatalError) : GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix), - Flags(Flags), ExclusiveGroup(ExclusiveGroup) { + Flags(Flags), ExclusiveGroup(ExclusiveGroup), FatalError(FatalError) { assert(GCCSuffix.empty() || (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1)); assert(OSSuffix.empty() || @@ -95,7 +97,8 @@ MultilibSet &MultilibSet::FilterOut(FilterCallback F) { void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); } bool MultilibSet::select(const Multilib::flags_list &Flags, - llvm::SmallVectorImpl<Multilib> &Selected) const { + llvm::SmallVectorImpl<Multilib> &Selected, + const Driver &D) const { llvm::StringSet<> FlagSet(expandFlags(Flags)); Selected.clear(); @@ -121,6 +124,14 @@ bool MultilibSet::select(const Multilib::flags_list &Flags, continue; } + // If this multilib is actually a placeholder containing a fatal + // error message written by the multilib.yaml author, display that + // error message, and return failure. + if (M.isFatalError()) { + D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getFatalError(); + return false; + } + // Select this multilib. Selected.push_back(M); } @@ -162,7 +173,8 @@ namespace { static const VersionTuple MultilibVersionCurrent(1, 0); struct MultilibSerialization { - std::string Dir; + std::string Dir; // if this record successfully selects a library dir + std::string FatalError; // if this record reports a fatal error message std::vector<std::string> Flags; std::string Group; }; @@ -205,11 +217,16 @@ struct MultilibSetSerialization { template <> struct llvm::yaml::MappingTraits<MultilibSerialization> { static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) { - io.mapRequired("Dir", V.Dir); + io.mapOptional("Dir", V.Dir); + io.mapOptional("FatalError", V.FatalError); io.mapRequired("Flags", V.Flags); io.mapOptional("Group", V.Group); } static std::string validate(IO &io, MultilibSerialization &V) { + if (V.Dir.empty() && V.FatalError.empty()) + return "one of the 'Dir' and 'FatalError' keys must be specified"; + if (!V.Dir.empty() && !V.FatalError.empty()) + return "the 'Dir' and 'FatalError' keys may not both be specified"; if (StringRef(V.Dir).starts_with("/")) return "paths must be relative but \"" + V.Dir + "\" starts with \"/\""; return std::string{}; @@ -295,14 +312,18 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input, multilib_list Multilibs; Multilibs.reserve(MS.Multilibs.size()); for (const auto &M : MS.Multilibs) { - std::string Dir; - if (M.Dir != ".") - Dir = "/" + M.Dir; - // We transfer M.Group straight into the ExclusiveGroup parameter for the - // Multilib constructor. If we later support more than one type of group, - // we'll have to look up the group name in MS.Groups, check its type, and - // decide what to do here. - Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group); + if (!M.FatalError.empty()) { + Multilibs.emplace_back("", "", "", M.Flags, M.Group, M.FatalError); + } else { + std::string Dir; + if (M.Dir != ".") + Dir = "/" + M.Dir; + // We transfer M.Group straight into the ExclusiveGroup parameter for the + // Multilib constructor. If we later support more than one type of group, + // we'll have to look up the group name in MS.Groups, check its type, and + // decide what to do here. + Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group); + } } return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers)); diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp index 0f5f32f3f8f495..e26cdae6bf0391 100644 --- a/clang/lib/Driver/ToolChains/BareMetal.cpp +++ b/clang/lib/Driver/ToolChains/BareMetal.cpp @@ -58,7 +58,7 @@ static bool findRISCVMultilibs(const Driver &D, Result.Multilibs = MultilibSetBuilder().Either(Imac, Imafdc).makeMultilibSet(); - return Result.Multilibs.select(Flags, Result.SelectedMultilibs); + return Result.Multilibs.select(Flags, Result.SelectedMultilibs, D); } if (TargetTriple.isRISCV32()) { MultilibBuilder Imac = @@ -92,7 +92,7 @@ static bool findRISCVMultilibs(const Driver &D, Result.Multilibs = MultilibSetBuilder().Either(I, Im, Iac, Imac, Imafc).makeMultilibSet(); - return Result.Multilibs.select(Flags, Result.SelectedMultilibs); + return Result.Multilibs.select(Flags, Result.SelectedMultilibs, D); } return false; } @@ -182,12 +182,13 @@ static void findMultilibsFromYAML(const ToolChain &TC, const Driver &D, if (ErrorOrMultilibSet.getError()) return; Result.Multilibs = ErrorOrMultilibSet.get(); - if (Result.Multilibs.select(Flags, Result.SelectedMultilibs)) + if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D)) return; D.Diag(clang::diag::warn_drv_missing_multilib) << llvm::join(Flags, " "); std::stringstream ss; for (const Multilib &Multilib : Result.Multilibs) - ss << "\n" << llvm::join(Multilib.flags(), " "); + if (!Multilib.isFatalError()) + ss << "\n" << llvm::join(Multilib.flags(), " "); D.Diag(clang::diag::note_drv_available_multilibs) << ss.str(); } diff --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp b/clang/lib/Driver/ToolChains/Fuchsia.cpp index 6daa73c7a54c88..d4145685c19b5f 100644 --- a/clang/lib/Driver/ToolChains/Fuchsia.cpp +++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp @@ -324,7 +324,7 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple, Multilibs.setFilePathsCallback(FilePaths); - if (Multilibs.select(Flags, SelectedMultilibs)) { + if (Multilibs.select(Flags, SelectedMultilibs, D)) { // Ensure that -print-multi-directory only outputs one multilib directory. Multilib LastSelected = SelectedMultilibs.back(); SelectedMultilibs = {LastSelected}; diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp index 52c2ee90b1b286..f65ba639fa5239 100644 --- a/clang/lib/Driver/ToolChains/Gnu.cpp +++ b/clang/lib/Driver/ToolChains/Gnu.cpp @@ -1041,7 +1041,8 @@ static bool isMSP430(llvm::Triple::ArchType Arch) { return Arch == llvm::Triple::msp430; } -static bool findMipsCsMultilibs(const Multilib::flags_list &Flags, +static bool findMipsCsMultilibs(const Driver &D, + const Multilib::flags_list &Flags, FilterNonExistent &NonExistent, DetectedMultilibs &Result) { // Check for Code Sourcery toolchain multilibs @@ -1135,7 +1136,7 @@ static bool findMipsCsMultilibs(const Multilib::flags_list &Flags, if (CSMipsMultilibs.size() < DebianMipsMultilibs.size()) std::iter_swap(Candidates, Candidates + 1); for (const MultilibSet *Candidate : Candidates) { - if (Candidate->select(Flags, Result.SelectedMultilibs)) { + if (Candidate->select(Flags, Result.SelectedMultilibs, D)) { if (Candidate == &DebianMipsMultilibs) Result.BiarchSibling = Multilib(); Result.Multilibs = *Candidate; @@ -1145,7 +1146,8 @@ static bool findMipsCsMultilibs(const Multilib::flags_list &Flags, return false; } -static bool findMipsAndroidMultilibs(llvm::vfs::FileSystem &VFS, StringRef Path, +static bool findMipsAndroidMultilibs(const Driver &D, + llvm::vfs::FileSystem &VFS, StringRef Path, const Multilib::flags_list &Flags, FilterNonExistent &NonExistent, DetectedMultilibs &Result) { @@ -1184,14 +1186,15 @@ static bool findMipsAndroidMultilibs(llvm::vfs::FileSystem &VFS, StringRef Path, MS = &AndroidMipselMultilibs; else if (VFS.exists(Path + "/32")) MS = &AndroidMips64elMultilibs; - if (MS->select(Flags, Result.SelectedMultilibs)) { + if (MS->select(Flags, Result.SelectedMultilibs, D)) { Result.Multilibs = *MS; return true; } return false; } -static bool findMipsMuslMultilibs(const Multilib::flags_list &Flags, +static bool findMipsMuslMultilibs(const Driver &D, + const Multilib::flags_list &Flags, FilterNonExistent &NonExistent, DetectedMultilibs &Result) { // Musl toolchain multilibs @@ -1218,14 +1221,15 @@ static bool findMipsMuslMultilibs(const Multilib::flags_list &Flags, {"/../sysroot" + M.osSuffix() + "/usr/include"}); }); } - if (MuslMipsMultilibs.select(Flags, Result.SelectedMultilibs)) { + if (MuslMipsMultilibs.select(Flags, Result.SelectedMultilibs, D)) { Result.Multilibs = MuslMipsMultilibs; return true; } return false; } -static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags, +static bool findMipsMtiMultilibs(const Driver &D, + const Multilib::flags_list &Flags, FilterNonExistent &NonExistent, DetectedMultilibs &Result) { // CodeScape MTI toolchain v1.2 and early. @@ -1403,7 +1407,7 @@ static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags, }); } for (auto *Candidate : {&MtiMipsMultilibsV1, &MtiMipsMultilibsV2}) { - if (Candidate->select(Flags, Result.SelectedMultilibs)) { + if (Candidate->select(Flags, Result.SelectedMultilibs, D)) { Result.Multilibs = *Candidate; return true; } @@ -1411,7 +1415,8 @@ static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags, return false; } -static bool findMipsImgMultilibs(const Multilib::flags_list &Flags, +static bool findMipsImgMultilibs(const Driver &D, + const Multilib::flags_list &Flags, FilterNonExistent &NonExistent, DetectedMultilibs &Result) { // CodeScape IMG toolchain v1.2 and early. @@ -1509,7 +1514,7 @@ static bool findMipsImgMultilibs(const Multilib::flags_list &Flags, }); } for (auto *Candidate : {&ImgMultilibsV1, &ImgMultilibsV2}) { - if (Candidate->select(Flags, Result.SelectedMultilibs)) { + if (Candidate->select(Flags, Result.SelectedMultilibs, D)) { Result.Multilibs = *Candidate; return true; } @@ -1556,25 +1561,25 @@ bool clang::driver::findMIPSMultilibs(const Driver &D, addMultilibFlag(!isMipsEL(TargetArch), "-EB", Flags); if (TargetTriple.isAndroid()) - return findMipsAndroidMultilibs(D.getVFS(), Path, Flags, NonExistent, + return findMipsAndroidMultilibs(D, D.getVFS(), Path, Flags, NonExistent, Result); if (TargetTriple.getVendor() == llvm::Triple::MipsTechnologies && TargetTriple.getOS() == llvm::Triple::Linux && TargetTriple.getEnvironment() == llvm::Triple::UnknownEnvironment) - return findMipsMuslMultilibs(Flags, NonExistent, Result); + return findMipsMuslMultilibs(D, Flags, NonExistent, Result); if (TargetTriple.getVendor() == llvm::Triple::MipsTechnologies && TargetTriple.getOS() == llvm::Triple::Linux && TargetTriple.isGNUEnvironment()) - return findMipsMtiMultilibs(Flags, NonExistent, Result); + return findMipsMtiMultilibs(D, Flags, NonExistent, Result); if (TargetTriple.getVendor() == llvm::Triple::ImaginationTechnologies && TargetTriple.getOS() == llvm::Triple::Linux && TargetTriple.isGNUEnvironment()) - return findMipsImgMultilibs(Flags, NonExistent, Result); + return findMipsImgMultilibs(D, Flags, NonExistent, Result); - if (findMipsCsMultilibs(Flags, NonExistent, Result)) + if (findMipsCsMultilibs(D, Flags, NonExistent, Result)) return true; // Fallback to the regular toolchain-tree structure. @@ -1582,7 +1587,7 @@ bool clang::driver::findMIPSMultilibs(const Driver &D, Result.Multilibs.push_back(Default); Result.Multilibs.FilterOut(NonExistent); - if (Result.Multilibs.select(Flags, Result.SelectedMultilibs)) { + if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D)) { Result.BiarchSibling = Multilib(); return true; } @@ -1629,7 +1634,7 @@ static void findAndroidArmMultilibs(const Driver &D, addMultilibFlag(IsArmV7Mode, "-march=armv7-a", Flags); addMultilibFlag(IsThumbMode, "-mthumb", Flags); - if (AndroidArmMultilibs.select(Flags, Result.SelectedMultilibs)) + if (AndroidArmMultilibs.select(Flags, Result.SelectedMultilibs, D)) Result.Multilibs = AndroidArmMultilibs; } @@ -1655,7 +1660,7 @@ static bool findMSP430Multilibs(const Driver &D, addMultilibFlag(Args.hasFlag(options::OPT_fexceptions, options::OPT_fno_exceptions, false), "-exceptions", Flags); - if (Result.Multilibs.select(Flags, Result.SelectedMultilibs)) + if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D)) return true; return false; @@ -1722,7 +1727,7 @@ static void findCSKYMultilibs(const Driver &D, const llvm::Triple &TargetTriple, .makeMultilibSet() .FilterOut(NonExistent); - if (CSKYMultilibs.select(Flags, Result.SelectedMultilibs)) + if (CSKYMultilibs.select(Flags, Result.SelectedMultilibs, D)) Result.Multilibs = CSKYMultilibs; } @@ -1737,11 +1742,11 @@ static void findCSKYMultilibs(const Driver &D, const llvm::Triple &TargetTriple, /// march=rv32ima are not compatible, because software and hardware /// atomic operation can't work together correctly. static bool -selectRISCVMultilib(const MultilibSet &RISCVMultilibSet, StringRef Arch, - const Multilib::flags_list &Flags, +selectRISCVMultilib(const Driver &D, const MultilibSet &RISCVMultilibSet, + StringRef Arch, const Multilib::flags_list &Flags, llvm::SmallVectorImpl<Multilib> &SelectedMultilibs) { // Try to find the perfect matching multi-lib first. - if (RISCVMultilibSet.select(Flags, SelectedMultilibs)) + if (RISCVMultilibSet.select(Flags, SelectedMultilibs, D)) return true; Multilib::flags_list NewFlags; @@ -1833,7 +1838,7 @@ selectRISCVMultilib(const MultilibSet &RISCVMultilibSet, StringRef Arch, MultilibSet NewRISCVMultilibs = MultilibSetBuilder().Either(NewMultilibs).makeMultilibSet(); - if (NewRISCVMultilibs.select(NewFlags, SelectedMultilibs)) + if (NewRISCVMultilibs.select(NewFlags, SelectedMultilibs, D)) for (const Multilib &NewSelectedM : SelectedMultilibs) for (const auto &M : RISCVMultilibSet) // Look up the corresponding multi-lib entry in original multi-lib set. @@ -1894,7 +1899,7 @@ static void findRISCVBareMetalMultilibs(const Driver &D, } } - if (selectRISCVMultilib(RISCVMultilibs, MArch, Flags, + if (selectRISCVMultilib(D, RISCVMultilibs, MArch, Flags, Result.SelectedMultilibs)) Result.Multilibs = RISCVMultilibs; } @@ -1937,7 +1942,7 @@ static void findRISCVMultilibs(const Driver &D, add... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/105684 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
