Skip to content

Commit f2c0aab

Browse files
committed
Serialization: Error when decoding invalid PlatformKind values.
Recently, unexpected binary module sharing between compilers that had inconsistent views of the `PlatformKind` enumeration caused me to spend several days debugging undefined behavior in the compiler. We should validate that enumeration values decoded during deserialization are valid, and fail fast when they are not to make debugging this kind of issue less time consuming. This change just validates the `PlatformKind` value decoded for an `AvailableAttr`, but more of deserialization could be updated to do similar validation. Resolves rdar://123770273
1 parent 8532092 commit f2c0aab

File tree

4 files changed

+71
-13
lines changed

4 files changed

+71
-13
lines changed

include/swift/AST/PlatformKind.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ StringRef platformString(PlatformKind platform);
4242
/// or None if such a platform kind does not exist.
4343
std::optional<PlatformKind> platformFromString(StringRef Name);
4444

45+
/// Safely converts the given unsigned value to a valid \c PlatformKind value or
46+
/// \c nullopt otherwise.
47+
std::optional<PlatformKind> platformFromUnsigned(unsigned value);
48+
4549
/// Returns a valid platform string that is closest to the candidate string
4650
/// based on edit distance. Returns \c None if the closest valid platform
4751
/// distance is not within a minimum threshold.

lib/AST/PlatformKind.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ std::optional<PlatformKind> swift::platformFromString(StringRef Name) {
5858
.Default(std::optional<PlatformKind>());
5959
}
6060

61+
std::optional<PlatformKind> swift::platformFromUnsigned(unsigned value) {
62+
PlatformKind platform = PlatformKind(value);
63+
switch (platform) {
64+
case PlatformKind::none:
65+
#define AVAILABILITY_PLATFORM(X, PrettyName) case PlatformKind::X:
66+
#include "swift/AST/PlatformKinds.def"
67+
return platform;
68+
}
69+
return std::nullopt;
70+
}
71+
6172
std::optional<StringRef>
6273
swift::closestCorrectedPlatformString(StringRef candidate) {
6374
auto lowerCasedCandidate = candidate.lower();

lib/Serialization/Deserialization.cpp

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ const char UnsafeDeserializationError::ID = '\0';
163163
void UnsafeDeserializationError::anchor() {}
164164
const char ModularizationError::ID = '\0';
165165
void ModularizationError::anchor() {}
166+
const char InvalidEnumValueError::ID = '\0';
167+
void InvalidEnumValueError::anchor() {}
166168

167169
/// Skips a single record in the bitstream.
168170
///
@@ -5295,8 +5297,9 @@ class DeclDeserializer {
52955297
return macro;
52965298
}
52975299

5298-
AvailableAttr *readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
5299-
StringRef blobData);
5300+
Expected<AvailableAttr *>
5301+
readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
5302+
StringRef blobData);
53005303
};
53015304
}
53025305

@@ -5349,7 +5352,7 @@ ModuleFile::getDeclChecked(
53495352
return declOrOffset;
53505353
}
53515354

5352-
AvailableAttr *
5355+
Expected<AvailableAttr *>
53535356
DeclDeserializer::readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
53545357
StringRef blobData) {
53555358
bool isImplicit;
@@ -5362,14 +5365,21 @@ DeclDeserializer::readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
53625365
DEF_VER_TUPLE_PIECES(Deprecated);
53635366
DEF_VER_TUPLE_PIECES(Obsoleted);
53645367
DeclID renameDeclID;
5365-
unsigned platform, messageSize, renameSize;
5368+
unsigned rawPlatform, messageSize, renameSize;
53665369

53675370
// Decode the record, pulling the version tuple information.
53685371
serialization::decls_block::AvailableDeclAttrLayout::readRecord(
53695372
scratch, isImplicit, isUnavailable, isDeprecated, isNoAsync,
5370-
isPackageDescriptionVersionSpecific, isSPI, LIST_VER_TUPLE_PIECES(Introduced),
5371-
LIST_VER_TUPLE_PIECES(Deprecated), LIST_VER_TUPLE_PIECES(Obsoleted),
5372-
platform, renameDeclID, messageSize, renameSize);
5373+
isPackageDescriptionVersionSpecific, isSPI,
5374+
LIST_VER_TUPLE_PIECES(Introduced), LIST_VER_TUPLE_PIECES(Deprecated),
5375+
LIST_VER_TUPLE_PIECES(Obsoleted), rawPlatform, renameDeclID, messageSize,
5376+
renameSize);
5377+
5378+
auto maybePlatform = platformFromUnsigned(rawPlatform);
5379+
if (!maybePlatform.has_value())
5380+
return llvm::make_error<InvalidEnumValueError>(rawPlatform, "PlatformKind");
5381+
5382+
PlatformKind platform = maybePlatform.value();
53735383

53745384
ValueDecl *renameDecl = nullptr;
53755385
if (renameDeclID) {
@@ -5391,7 +5401,7 @@ DeclDeserializer::readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
53915401
platformAgnostic = PlatformAgnosticAvailabilityKind::Deprecated;
53925402
else if (isNoAsync)
53935403
platformAgnostic = PlatformAgnosticAvailabilityKind::NoAsync;
5394-
else if (((PlatformKind)platform) == PlatformKind::none &&
5404+
else if (platform == PlatformKind::none &&
53955405
(!Introduced.empty() || !Deprecated.empty() || !Obsoleted.empty()))
53965406
platformAgnostic =
53975407
isPackageDescriptionVersionSpecific
@@ -5402,9 +5412,9 @@ DeclDeserializer::readAvailable_DECL_ATTR(SmallVectorImpl<uint64_t> &scratch,
54025412
platformAgnostic = PlatformAgnosticAvailabilityKind::None;
54035413

54045414
auto attr = new (ctx) AvailableAttr(
5405-
SourceLoc(), SourceRange(), (PlatformKind)platform, message, rename,
5406-
renameDecl, Introduced, SourceRange(), Deprecated, SourceRange(),
5407-
Obsoleted, SourceRange(), platformAgnostic, isImplicit, isSPI);
5415+
SourceLoc(), SourceRange(), platform, message, rename, renameDecl,
5416+
Introduced, SourceRange(), Deprecated, SourceRange(), Obsoleted,
5417+
SourceRange(), platformAgnostic, isImplicit, isSPI);
54085418
return attr;
54095419
}
54105420

@@ -5643,7 +5653,11 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
56435653
}
56445654

56455655
case decls_block::Available_DECL_ATTR: {
5646-
Attr = readAvailable_DECL_ATTR(scratch, blobData);
5656+
auto attrOrError = readAvailable_DECL_ATTR(scratch, blobData);
5657+
if (!attrOrError)
5658+
return MF.diagnoseFatal(attrOrError.takeError());
5659+
5660+
Attr = attrOrError.get();
56475661
break;
56485662
}
56495663

@@ -5753,7 +5767,10 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
57535767
}
57545768

57555769
auto attr = readAvailable_DECL_ATTR(scratch, blobData);
5756-
availabilityAttrs.push_back(attr);
5770+
if (!attr)
5771+
return MF.diagnoseFatal(attr.takeError());
5772+
5773+
availabilityAttrs.push_back(attr.get());
57575774
restoreOffset2.cancel();
57585775
--numAvailabilityAttrs;
57595776
}

lib/Serialization/DeserializationErrors.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,32 @@ class UnsafeDeserializationError : public llvm::ErrorInfo<UnsafeDeserializationE
646646
}
647647
};
648648

649+
class InvalidEnumValueError
650+
: public llvm::ErrorInfo<InvalidEnumValueError, DeclDeserializationError> {
651+
friend ErrorInfo;
652+
static const char ID;
653+
void anchor() override;
654+
655+
unsigned enumValue;
656+
const char *enumDescription;
657+
658+
public:
659+
explicit InvalidEnumValueError(unsigned enumValue,
660+
const char *enumDescription) {
661+
this->enumValue = enumValue;
662+
this->enumDescription = enumDescription;
663+
}
664+
665+
void log(raw_ostream &OS) const override {
666+
OS << "invalid value " << enumValue << " for enumeration "
667+
<< enumDescription;
668+
}
669+
670+
std::error_code convertToErrorCode() const override {
671+
return llvm::inconvertibleErrorCode();
672+
}
673+
};
674+
649675
class PrettyStackTraceModuleFile : public llvm::PrettyStackTraceEntry {
650676
const char *Action;
651677
const ModuleFile &MF;

0 commit comments

Comments
 (0)