Skip to content

Commit 1718362

Browse files
committed
[Serialization] Harden the compiler tag check on swiftmodules
When two different serialization formats share a version number but are different enough, it can defeat the check to restrict loading swiftmodules built by the same compiler. Add a backup check in case the REVISION block is unseen, for swiftmodules only not swiftdoc or sourceinfo. rdar://93188070
1 parent 1757061 commit 1718362

File tree

1 file changed

+22
-5
lines changed

1 file changed

+22
-5
lines changed

lib/Serialization/ModuleFileSharedCore.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,14 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor,
172172
static ValidationInfo validateControlBlock(
173173
llvm::BitstreamCursor &cursor, SmallVectorImpl<uint64_t> &scratch,
174174
std::pair<uint16_t, uint16_t> expectedVersion, bool requiresOSSAModules,
175+
bool requiresRevisionMatch,
175176
ExtendedValidationInfo *extendedInfo,
176177
PathObfuscator &pathRecoverer) {
177178
// The control block is malformed until we've at least read a major version
178179
// number.
179180
ValidationInfo result;
180181
bool versionSeen = false;
182+
bool revisionSeen = false;
181183

182184
while (!cursor.AtEndOfStream()) {
183185
Expected<llvm::BitstreamEntry> maybeEntry = cursor.advance();
@@ -308,6 +310,8 @@ static ValidationInfo validateControlBlock(
308310
break;
309311
}
310312
case control_block::REVISION: {
313+
revisionSeen = true;
314+
311315
// Tagged compilers should only load modules if they were
312316
// produced by the exact same compiler tag.
313317

@@ -331,8 +335,12 @@ static ValidationInfo validateControlBlock(
331335
if (isCompilerTagged) {
332336
StringRef compilerRevision = forcedDebugRevision ?
333337
forcedDebugRevision : version::getSwiftRevision();
334-
if (moduleRevision != compilerRevision)
338+
if (moduleRevision != compilerRevision) {
335339
result.status = Status::RevisionIncompatible;
340+
341+
// We can't trust this module format at this point.
342+
return result;
343+
}
336344
}
337345
break;
338346
}
@@ -349,6 +357,13 @@ static ValidationInfo validateControlBlock(
349357
}
350358
}
351359

360+
// Last resort check in cases where the format is broken enough that
361+
// we didn't read the REVISION block, report such a case as incompatible.
362+
if (requiresRevisionMatch &&
363+
!revisionSeen &&
364+
result.status == Status::Valid)
365+
result.status = Status::RevisionIncompatible;
366+
352367
return result;
353368
}
354369

@@ -471,7 +486,8 @@ ValidationInfo serialization::validateSerializedAST(
471486
result = validateControlBlock(
472487
cursor, scratch,
473488
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
474-
requiresOSSAModules, extendedInfo, localObfuscator);
489+
requiresOSSAModules, /*requiresRevisionMatch=*/true,
490+
extendedInfo, localObfuscator);
475491
if (result.status == Status::Malformed)
476492
return result;
477493
} else if (dependencies &&
@@ -979,7 +995,7 @@ bool ModuleFileSharedCore::readModuleDocIfPresent(PathObfuscator &pathRecoverer)
979995

980996
info = validateControlBlock(
981997
docCursor, scratch, {SWIFTDOC_VERSION_MAJOR, SWIFTDOC_VERSION_MINOR},
982-
RequiresOSSAModules,
998+
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
983999
/*extendedInfo*/ nullptr, pathRecoverer);
9841000
if (info.status != Status::Valid)
9851001
return false;
@@ -1123,7 +1139,7 @@ bool ModuleFileSharedCore::readModuleSourceInfoIfPresent(PathObfuscator &pathRec
11231139
info = validateControlBlock(
11241140
infoCursor, scratch,
11251141
{SWIFTSOURCEINFO_VERSION_MAJOR, SWIFTSOURCEINFO_VERSION_MINOR},
1126-
RequiresOSSAModules,
1142+
RequiresOSSAModules, /*requiresRevisionMatch=*/false,
11271143
/*extendedInfo*/ nullptr,
11281144
pathRecoverer);
11291145
if (info.status != Status::Valid)
@@ -1251,7 +1267,8 @@ ModuleFileSharedCore::ModuleFileSharedCore(
12511267
info = validateControlBlock(
12521268
cursor, scratch,
12531269
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
1254-
RequiresOSSAModules, &extInfo, pathRecoverer);
1270+
RequiresOSSAModules, /*requiresRevisionMatch=*/true,
1271+
&extInfo, pathRecoverer);
12551272
if (info.status != Status::Valid) {
12561273
error(info.status);
12571274
return;

0 commit comments

Comments
 (0)