Skip to content

Commit 26bf0b4

Browse files
authored
[clang][Driver] Add a custom error option in multilib.yaml. (#105684)
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.
1 parent 30d56be commit 26bf0b4

File tree

13 files changed

+237
-86
lines changed

13 files changed

+237
-86
lines changed

clang/docs/Multilib.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ For a more comprehensive example see
200200
# to be a match.
201201
Flags: [--target=thumbv7m-none-eabi, -mfpu=fpv4-sp-d16]
202202
203+
# If there is no multilib available for a particular set of flags, and the
204+
# other multilibs are not adequate fallbacks, then you can define a variant
205+
# record with a FatalError key in place of the Dir key.
206+
- FatalError: this multilib collection has no hard-float ABI support
207+
Flags: [--target=thumbv7m-none-eabi, -mfloat-abi=hard]
208+
203209
204210
# The second section of the file is a list of regular expressions that are
205211
# used to map from flags generated from command line options to custom flags.

clang/include/clang/Basic/DiagnosticDriverKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,8 @@ def warn_drv_missing_multilib : Warning<
810810
InGroup<DiagGroup<"missing-multilib">>;
811811
def note_drv_available_multilibs : Note<
812812
"available multilibs are:%0">;
813+
def err_drv_multilib_custom_error : Error<
814+
"multilib configuration error: %0">;
813815

814816
def err_drv_experimental_crel : Error<
815817
"-Wa,--allow-experimental-crel must be specified to use -Wa,--crel. "

clang/include/clang/Driver/Multilib.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,16 @@
1818
#include "llvm/Support/SourceMgr.h"
1919
#include <cassert>
2020
#include <functional>
21+
#include <optional>
2122
#include <string>
2223
#include <utility>
2324
#include <vector>
2425

2526
namespace clang {
2627
namespace driver {
2728

29+
class Driver;
30+
2831
/// This corresponds to a single GCC Multilib, or a segment of one controlled
2932
/// by a command line flag.
3033
/// See also MultilibBuilder for building a multilib by mutating it
@@ -48,13 +51,19 @@ class Multilib {
4851
// directory is not mutually exclusive with anything else.
4952
std::string ExclusiveGroup;
5053

54+
// Some Multilib objects don't actually represent library directories you can
55+
// select. Instead, they represent failures of multilib selection, of the
56+
// form 'Sorry, we don't have any library compatible with these constraints'.
57+
std::optional<std::string> FatalError;
58+
5159
public:
5260
/// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the
5361
/// sysroot string so they must either be empty or begin with a '/' character.
5462
/// This is enforced with an assert in the constructor.
5563
Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
5664
StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(),
57-
StringRef ExclusiveGroup = {});
65+
StringRef ExclusiveGroup = {},
66+
std::optional<StringRef> FatalError = std::nullopt);
5867

5968
/// Get the detected GCC installation path suffix for the multi-arch
6069
/// target variant. Always starts with a '/', unless empty
@@ -84,6 +93,10 @@ class Multilib {
8493
{ return GCCSuffix.empty() && OSSuffix.empty() && IncludeSuffix.empty(); }
8594

8695
bool operator==(const Multilib &Other) const;
96+
97+
bool isFatalError() const { return FatalError.has_value(); }
98+
99+
const std::string &getFatalError() const { return FatalError.value(); }
87100
};
88101

89102
raw_ostream &operator<<(raw_ostream &OS, const Multilib &M);
@@ -129,7 +142,7 @@ class MultilibSet {
129142
const_iterator end() const { return Multilibs.end(); }
130143

131144
/// Select compatible variants, \returns false if none are compatible
132-
bool select(const Multilib::flags_list &Flags,
145+
bool select(const Driver &D, const Multilib::flags_list &Flags,
133146
llvm::SmallVectorImpl<Multilib> &) const;
134147

135148
unsigned size() const { return Multilibs.size(); }

clang/lib/Driver/Driver.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2296,7 +2296,8 @@ bool Driver::HandleImmediateArgs(Compilation &C) {
22962296

22972297
if (C.getArgs().hasArg(options::OPT_print_multi_lib)) {
22982298
for (const Multilib &Multilib : TC.getMultilibs())
2299-
llvm::outs() << Multilib << "\n";
2299+
if (!Multilib.isFatalError())
2300+
llvm::outs() << Multilib << "\n";
23002301
return false;
23012302
}
23022303

clang/lib/Driver/Multilib.cpp

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "clang/Driver/Multilib.h"
1010
#include "clang/Basic/LLVM.h"
1111
#include "clang/Basic/Version.h"
12+
#include "clang/Driver/Driver.h"
1213
#include "llvm/ADT/DenseSet.h"
1314
#include "llvm/ADT/SmallString.h"
1415
#include "llvm/ADT/StringRef.h"
@@ -31,9 +32,10 @@ using namespace llvm::sys;
3132

3233
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
3334
StringRef IncludeSuffix, const flags_list &Flags,
34-
StringRef ExclusiveGroup)
35+
StringRef ExclusiveGroup,
36+
std::optional<StringRef> FatalError)
3537
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
36-
Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
38+
Flags(Flags), ExclusiveGroup(ExclusiveGroup), FatalError(FatalError) {
3739
assert(GCCSuffix.empty() ||
3840
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
3941
assert(OSSuffix.empty() ||
@@ -94,7 +96,7 @@ MultilibSet &MultilibSet::FilterOut(FilterCallback F) {
9496

9597
void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); }
9698

97-
bool MultilibSet::select(const Multilib::flags_list &Flags,
99+
bool MultilibSet::select(const Driver &D, const Multilib::flags_list &Flags,
98100
llvm::SmallVectorImpl<Multilib> &Selected) const {
99101
llvm::StringSet<> FlagSet(expandFlags(Flags));
100102
Selected.clear();
@@ -121,6 +123,14 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
121123
continue;
122124
}
123125

126+
// If this multilib is actually a placeholder containing a fatal
127+
// error message written by the multilib.yaml author, display that
128+
// error message, and return failure.
129+
if (M.isFatalError()) {
130+
D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getFatalError();
131+
return false;
132+
}
133+
124134
// Select this multilib.
125135
Selected.push_back(M);
126136
}
@@ -162,7 +172,8 @@ namespace {
162172
static const VersionTuple MultilibVersionCurrent(1, 0);
163173

164174
struct MultilibSerialization {
165-
std::string Dir;
175+
std::string Dir; // if this record successfully selects a library dir
176+
std::string FatalError; // if this record reports a fatal error message
166177
std::vector<std::string> Flags;
167178
std::string Group;
168179
};
@@ -205,11 +216,16 @@ struct MultilibSetSerialization {
205216

206217
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
207218
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
208-
io.mapRequired("Dir", V.Dir);
219+
io.mapOptional("Dir", V.Dir);
220+
io.mapOptional("FatalError", V.FatalError);
209221
io.mapRequired("Flags", V.Flags);
210222
io.mapOptional("Group", V.Group);
211223
}
212224
static std::string validate(IO &io, MultilibSerialization &V) {
225+
if (V.Dir.empty() && V.FatalError.empty())
226+
return "one of the 'Dir' and 'FatalError' keys must be specified";
227+
if (!V.Dir.empty() && !V.FatalError.empty())
228+
return "the 'Dir' and 'FatalError' keys may not both be specified";
213229
if (StringRef(V.Dir).starts_with("/"))
214230
return "paths must be relative but \"" + V.Dir + "\" starts with \"/\"";
215231
return std::string{};
@@ -295,14 +311,18 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
295311
multilib_list Multilibs;
296312
Multilibs.reserve(MS.Multilibs.size());
297313
for (const auto &M : MS.Multilibs) {
298-
std::string Dir;
299-
if (M.Dir != ".")
300-
Dir = "/" + M.Dir;
301-
// We transfer M.Group straight into the ExclusiveGroup parameter for the
302-
// Multilib constructor. If we later support more than one type of group,
303-
// we'll have to look up the group name in MS.Groups, check its type, and
304-
// decide what to do here.
305-
Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group);
314+
if (!M.FatalError.empty()) {
315+
Multilibs.emplace_back("", "", "", M.Flags, M.Group, M.FatalError);
316+
} else {
317+
std::string Dir;
318+
if (M.Dir != ".")
319+
Dir = "/" + M.Dir;
320+
// We transfer M.Group straight into the ExclusiveGroup parameter for the
321+
// Multilib constructor. If we later support more than one type of group,
322+
// we'll have to look up the group name in MS.Groups, check its type, and
323+
// decide what to do here.
324+
Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group);
325+
}
306326
}
307327

308328
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));

clang/lib/Driver/ToolChains/BareMetal.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ static bool findRISCVMultilibs(const Driver &D,
5858

5959
Result.Multilibs =
6060
MultilibSetBuilder().Either(Imac, Imafdc).makeMultilibSet();
61-
return Result.Multilibs.select(Flags, Result.SelectedMultilibs);
61+
return Result.Multilibs.select(D, Flags, Result.SelectedMultilibs);
6262
}
6363
if (TargetTriple.isRISCV32()) {
6464
MultilibBuilder Imac =
@@ -92,7 +92,7 @@ static bool findRISCVMultilibs(const Driver &D,
9292

9393
Result.Multilibs =
9494
MultilibSetBuilder().Either(I, Im, Iac, Imac, Imafc).makeMultilibSet();
95-
return Result.Multilibs.select(Flags, Result.SelectedMultilibs);
95+
return Result.Multilibs.select(D, Flags, Result.SelectedMultilibs);
9696
}
9797
return false;
9898
}
@@ -182,12 +182,13 @@ static void findMultilibsFromYAML(const ToolChain &TC, const Driver &D,
182182
if (ErrorOrMultilibSet.getError())
183183
return;
184184
Result.Multilibs = ErrorOrMultilibSet.get();
185-
if (Result.Multilibs.select(Flags, Result.SelectedMultilibs))
185+
if (Result.Multilibs.select(D, Flags, Result.SelectedMultilibs))
186186
return;
187187
D.Diag(clang::diag::warn_drv_missing_multilib) << llvm::join(Flags, " ");
188188
std::stringstream ss;
189189
for (const Multilib &Multilib : Result.Multilibs)
190-
ss << "\n" << llvm::join(Multilib.flags(), " ");
190+
if (!Multilib.isFatalError())
191+
ss << "\n" << llvm::join(Multilib.flags(), " ");
191192
D.Diag(clang::diag::note_drv_available_multilibs) << ss.str();
192193
}
193194

clang/lib/Driver/ToolChains/Fuchsia.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
324324

325325
Multilibs.setFilePathsCallback(FilePaths);
326326

327-
if (Multilibs.select(Flags, SelectedMultilibs)) {
327+
if (Multilibs.select(D, Flags, SelectedMultilibs)) {
328328
// Ensure that -print-multi-directory only outputs one multilib directory.
329329
Multilib LastSelected = SelectedMultilibs.back();
330330
SelectedMultilibs = {LastSelected};

0 commit comments

Comments
 (0)