Skip to content

Commit a29fbe6

Browse files
elshelsh
committed
Non-package module should load swiftinterface instead of
binary module with PackageCMO enabled. If the binary module optimized with Package CMO is loaded for a non-package client module, the client module does not understand the serialized content of the optimized binary module because it contains attributes and instructions specific to the optimization, i.e. [serialized_for_package] definition that contains operations on non-loadable types that are normally not allowed. This can cause an assert or a crash in SIL verifier. This PR checks whether the optimized binary module and the client module are in the same package, and require loading swiftinterface module if not. Resolves rdar://134584629
1 parent 98595fc commit a29fbe6

File tree

11 files changed

+119
-67
lines changed

11 files changed

+119
-67
lines changed

‎include/swift/Serialization/Validation.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,8 @@ struct SearchPath {
272272
/// \param[out] dependencies If present, will be populated with list of
273273
/// input files the module depends on, if present in INPUT_BLOCK.
274274
ValidationInfo validateSerializedAST(
275-
StringRef data, bool requiresOSSAModules,
276-
StringRef requiredSDK,
277-
ExtendedValidationInfo *extendedInfo = nullptr,
275+
StringRef data, bool requiresOSSAModules, StringRef requiredSDK,
276+
StringRef PackageName, ExtendedValidationInfo *extendedInfo = nullptr,
278277
SmallVectorImpl<SerializationOptions::FileDependency> *dependencies =
279278
nullptr,
280279
SmallVectorImpl<SearchPath> *searchPaths = nullptr);

‎lib/ASTSectionImporter/ASTSectionImporter.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ swift::parseASTSection(MemoryBufferSerializedModuleLoader &Loader,
5959
while (!buf.empty()) {
6060
auto info = serialization::validateSerializedAST(
6161
buf, Loader.isRequiredOSSAModules(),
62-
/*requiredSDK*/StringRef());
62+
/*requiredSDK*/ StringRef(), /*packageName*/ StringRef());
6363

6464
assert(info.name.size() < (2 << 10) && "name failed sanity check");
6565

‎lib/Frontend/CompilerInvocation.cpp‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3635,12 +3635,9 @@ bool CompilerInvocation::parseArgs(
36353635
serialization::Status
36363636
CompilerInvocation::loadFromSerializedAST(StringRef data) {
36373637
serialization::ExtendedValidationInfo extendedInfo;
3638-
serialization::ValidationInfo info =
3639-
serialization::validateSerializedAST(
3640-
data,
3641-
getSILOptions().EnableOSSAModules,
3642-
LangOpts.SDKName,
3643-
&extendedInfo);
3638+
serialization::ValidationInfo info = serialization::validateSerializedAST(
3639+
data, getSILOptions().EnableOSSAModules, LangOpts.SDKName,
3640+
LangOpts.PackageName, &extendedInfo);
36443641

36453642
if (info.status != serialization::Status::Valid)
36463643
return info.status;
@@ -3675,10 +3672,8 @@ CompilerInvocation::setUpInputForSILTool(
36753672
InputFile(inputFilename, bePrimary, fileBufOrErr.get().get(), file_types::TY_SIL));
36763673

36773674
auto result = serialization::validateSerializedAST(
3678-
fileBufOrErr.get()->getBuffer(),
3679-
getSILOptions().EnableOSSAModules,
3680-
LangOpts.SDKName,
3681-
&extendedInfo);
3675+
fileBufOrErr.get()->getBuffer(), getSILOptions().EnableOSSAModules,
3676+
LangOpts.SDKName, LangOpts.PackageName, &extendedInfo);
36823677
bool hasSerializedAST = result.status == serialization::Status::Valid;
36833678

36843679
if (hasSerializedAST) {

‎lib/Frontend/ModuleInterfaceLoader.cpp‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,10 @@ namespace path = llvm::sys::path;
214214

215215
static bool serializedASTLooksValid(const llvm::MemoryBuffer &buf,
216216
bool requiresOSSAModules,
217-
StringRef requiredSDK) {
218-
auto VI = serialization::validateSerializedAST(buf.getBuffer(),
219-
requiresOSSAModules,
220-
requiredSDK);
217+
StringRef requiredSDK,
218+
StringRef packageName) {
219+
auto VI = serialization::validateSerializedAST(
220+
buf.getBuffer(), requiresOSSAModules, requiredSDK, packageName);
221221
return VI.status == serialization::Status::Valid;
222222
}
223223

@@ -495,11 +495,13 @@ class UpToDateModuleCheker {
495495
// Clear the existing dependencies, because we're going to re-fill them
496496
// in validateSerializedAST.
497497
allDeps.clear();
498-
498+
serialization::ExtendedValidationInfo extendedInfo;
499499
LLVM_DEBUG(llvm::dbgs() << "Validating deps of " << path << "\n");
500+
500501
auto validationInfo = serialization::validateSerializedAST(
501-
buf.getBuffer(), requiresOSSAModules,
502-
ctx.LangOpts.SDKName, /*ExtendedValidationInfo=*/nullptr, &allDeps);
502+
buf.getBuffer(), requiresOSSAModules, ctx.LangOpts.SDKName,
503+
ctx.LangOpts.PackageName,
504+
/*ExtendedValidationInfo=*/&extendedInfo, &allDeps);
503505

504506
if (validationInfo.status != serialization::Status::Valid) {
505507
rebuildInfo.setSerializationStatus(path, validationInfo.status);
@@ -659,9 +661,9 @@ class ModuleInterfaceLoaderImpl {
659661
if (!modBuf)
660662
return false;
661663

662-
auto looksValid = serializedASTLooksValid(*modBuf.get(),
663-
requiresOSSAModules,
664-
ctx.LangOpts.SDKName);
664+
auto looksValid =
665+
serializedASTLooksValid(*modBuf.get(), requiresOSSAModules,
666+
ctx.LangOpts.SDKName, ctx.LangOpts.PackageName);
665667
if (!looksValid)
666668
return false;
667669

@@ -2473,9 +2475,8 @@ bool ExplicitSwiftModuleLoader::canImportModule(
24732475
}
24742476

24752477
auto metaData = serialization::validateSerializedAST(
2476-
(*moduleBuf)->getBuffer(),
2477-
Ctx.SILOpts.EnableOSSAModules,
2478-
Ctx.LangOpts.SDKName);
2478+
(*moduleBuf)->getBuffer(), Ctx.SILOpts.EnableOSSAModules,
2479+
Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName);
24792480
versionInfo->setVersion(metaData.userModuleVersion,
24802481
ModuleVersionSourceKind::SwiftBinaryModule);
24812482
return true;
@@ -2817,7 +2818,7 @@ bool ExplicitCASModuleLoader::canImportModule(
28172818
}
28182819
auto metaData = serialization::validateSerializedAST(
28192820
(*moduleBuf)->getBuffer(), Ctx.SILOpts.EnableOSSAModules,
2820-
Ctx.LangOpts.SDKName);
2821+
Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName);
28212822
versionInfo->setVersion(metaData.userModuleVersion,
28222823
ModuleVersionSourceKind::SwiftBinaryModule);
28232824
return true;

‎lib/Serialization/ModuleFile.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,8 @@ ModuleFile::getModuleName(ASTContext &Ctx, StringRef modulePath,
407407
serialization::ValidationInfo loadInfo = ModuleFileSharedCore::load(
408408
"", "", std::move(newBuf), nullptr, nullptr,
409409
/*isFramework=*/isFramework, Ctx.SILOpts.EnableOSSAModules,
410-
Ctx.LangOpts.SDKName, Ctx.SearchPathOpts.DeserializedPathRecoverer,
411-
loadedModuleFile);
410+
Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName,
411+
Ctx.SearchPathOpts.DeserializedPathRecoverer, loadedModuleFile);
412412
Name = loadedModuleFile->Name.str();
413413
return std::move(moduleBuf.get());
414414
}

‎lib/Serialization/ModuleFileSharedCore.cpp‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,9 @@ static bool readOptionsBlock(llvm::BitstreamCursor &cursor,
216216

217217
static ValidationInfo validateControlBlock(
218218
llvm::BitstreamCursor &cursor, SmallVectorImpl<uint64_t> &scratch,
219-
std::pair<uint16_t, uint16_t> expectedVersion,
220-
bool requiresOSSAModules,
221-
bool requiresRevisionMatch,
222-
StringRef requiredSDK,
223-
ExtendedValidationInfo *extendedInfo,
224-
PathObfuscator &pathRecoverer) {
219+
std::pair<uint16_t, uint16_t> expectedVersion, bool requiresOSSAModules,
220+
bool requiresRevisionMatch, StringRef requiredSDK, StringRef packageName,
221+
ExtendedValidationInfo *extendedInfo, PathObfuscator &pathRecoverer) {
225222
// The control block is malformed until we've at least read a major version
226223
// number.
227224
ValidationInfo result;
@@ -253,7 +250,16 @@ static ValidationInfo validateControlBlock(
253250
result.status = Status::Malformed;
254251
return result;
255252
}
256-
if (!readOptionsBlock(cursor, scratch, *extendedInfo, pathRecoverer)) {
253+
254+
if (readOptionsBlock(cursor, scratch, *extendedInfo, pathRecoverer)) {
255+
// The client module must be in the same package as the
256+
// (optimized) binary module being loaded.
257+
if (extendedInfo && extendedInfo->serializePackageEnabled() &&
258+
extendedInfo->getModulePackageName() != packageName) {
259+
result.status = Status::Malformed;
260+
return result;
261+
}
262+
} else {
257263
result.status = Status::Malformed;
258264
return result;
259265
}
@@ -579,9 +585,8 @@ bool serialization::isSerializedAST(StringRef data) {
579585
}
580586

581587
ValidationInfo serialization::validateSerializedAST(
582-
StringRef data, bool requiresOSSAModules,
583-
StringRef requiredSDK,
584-
ExtendedValidationInfo *extendedInfo,
588+
StringRef data, bool requiresOSSAModules, StringRef requiredSDK,
589+
StringRef packageName, ExtendedValidationInfo *extendedInfo,
585590
SmallVectorImpl<SerializationOptions::FileDependency> *dependencies,
586591
SmallVectorImpl<SearchPath> *searchPaths) {
587592
ValidationInfo result;
@@ -625,8 +630,7 @@ ValidationInfo serialization::validateSerializedAST(
625630
cursor, scratch,
626631
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
627632
requiresOSSAModules,
628-
/*requiresRevisionMatch=*/true,
629-
requiredSDK,
633+
/*requiresRevisionMatch=*/true, requiredSDK, packageName,
630634
extendedInfo, localObfuscator);
631635
if (result.status != Status::Valid)
632636
return result;
@@ -1161,8 +1165,10 @@ bool ModuleFileSharedCore::readModuleDocIfPresent(PathObfuscator &pathRecoverer)
11611165
info = validateControlBlock(
11621166
docCursor, scratch, {SWIFTDOC_VERSION_MAJOR, SWIFTDOC_VERSION_MINOR},
11631167
RequiresOSSAModules,
1164-
/*requiresRevisionMatch*/false,
1165-
/*requiredSDK*/StringRef(), /*extendedInfo*/nullptr, pathRecoverer);
1168+
/*requiresRevisionMatch*/ false,
1169+
/*requiredSDK*/ StringRef(),
1170+
/*packageName*/ StringRef(),
1171+
/*extendedInfo*/ nullptr, pathRecoverer);
11661172
if (info.status != Status::Valid)
11671173
return false;
11681174
// Check that the swiftdoc is actually for this module.
@@ -1306,8 +1312,10 @@ bool ModuleFileSharedCore::readModuleSourceInfoIfPresent(PathObfuscator &pathRec
13061312
infoCursor, scratch,
13071313
{SWIFTSOURCEINFO_VERSION_MAJOR, SWIFTSOURCEINFO_VERSION_MINOR},
13081314
RequiresOSSAModules,
1309-
/*requiresRevisionMatch*/false,
1310-
/*requiredSDK*/StringRef(), /*extendedInfo*/nullptr, pathRecoverer);
1315+
/*requiresRevisionMatch*/ false,
1316+
/*requiredSDK*/ StringRef(),
1317+
/*packageName*/ StringRef(),
1318+
/*extendedInfo*/ nullptr, pathRecoverer);
13111319
if (info.status != Status::Valid)
13121320
return false;
13131321
// Check that the swiftsourceinfo is actually for this module.
@@ -1381,10 +1389,9 @@ ModuleFileSharedCore::ModuleFileSharedCore(
13811389
std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
13821390
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
13831391
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
1384-
bool isFramework,
1385-
bool requiresOSSAModules,
1386-
StringRef requiredSDK,
1387-
serialization::ValidationInfo &info, PathObfuscator &pathRecoverer)
1392+
bool isFramework, bool requiresOSSAModules, StringRef requiredSDK,
1393+
StringRef packageName, serialization::ValidationInfo &info,
1394+
PathObfuscator &pathRecoverer)
13881395
: ModuleInputBuffer(std::move(moduleInputBuffer)),
13891396
ModuleDocInputBuffer(std::move(moduleDocInputBuffer)),
13901397
ModuleSourceInfoInputBuffer(std::move(moduleSourceInfoInputBuffer)),
@@ -1436,8 +1443,8 @@ ModuleFileSharedCore::ModuleFileSharedCore(
14361443
cursor, scratch,
14371444
{SWIFTMODULE_VERSION_MAJOR, SWIFTMODULE_VERSION_MINOR},
14381445
RequiresOSSAModules,
1439-
/*requiresRevisionMatch=*/true, requiredSDK,
1440-
&extInfo, pathRecoverer);
1446+
/*requiresRevisionMatch=*/true, requiredSDK, packageName, &extInfo,
1447+
pathRecoverer);
14411448
if (info.status != Status::Valid) {
14421449
error(info.status);
14431450
return;

‎lib/Serialization/ModuleFileSharedCore.h‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -417,10 +417,9 @@ class ModuleFileSharedCore {
417417
std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
418418
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
419419
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
420-
bool isFramework,
421-
bool requiresOSSAModules,
422-
StringRef requiredSDK,
423-
serialization::ValidationInfo &info, PathObfuscator &pathRecoverer);
420+
bool isFramework, bool requiresOSSAModules, StringRef requiredSDK,
421+
StringRef packageName, serialization::ValidationInfo &info,
422+
PathObfuscator &pathRecoverer);
424423

425424
/// Change the status of the current module.
426425
Status error(Status issue) {
@@ -555,15 +554,14 @@ class ModuleFileSharedCore {
555554
std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
556555
std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
557556
std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
558-
bool isFramework, bool requiresOSSAModules,
559-
StringRef requiredSDK, PathObfuscator &pathRecoverer,
557+
bool isFramework, bool requiresOSSAModules, StringRef requiredSDK,
558+
StringRef packageName, PathObfuscator &pathRecoverer,
560559
std::shared_ptr<const ModuleFileSharedCore> &theModule) {
561560
serialization::ValidationInfo info;
562561
auto *core = new ModuleFileSharedCore(
563562
std::move(moduleInputBuffer), std::move(moduleDocInputBuffer),
564563
std::move(moduleSourceInfoInputBuffer), isFramework,
565-
requiresOSSAModules, requiredSDK, info,
566-
pathRecoverer);
564+
requiresOSSAModules, requiredSDK, packageName, info, pathRecoverer);
567565
if (!moduleInterfacePath.empty()) {
568566
ArrayRef<char> path;
569567
core->allocateBuffer(path, moduleInterfacePath);

‎lib/Serialization/SerializedModuleLoader.cpp‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ SerializedModuleLoaderBase::scanModuleFile(Twine modulePath, bool isFramework,
452452
std::shared_ptr<const ModuleFileSharedCore> loadedModuleFile;
453453
serialization::ValidationInfo loadInfo = ModuleFileSharedCore::load(
454454
"", "", std::move(moduleBuf.get()), nullptr, nullptr, isFramework,
455-
isRequiredOSSAModules(), Ctx.LangOpts.SDKName,
455+
isRequiredOSSAModules(), Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName,
456456
Ctx.SearchPathOpts.DeserializedPathRecoverer, loadedModuleFile);
457457

458458
if (Ctx.SearchPathOpts.ScannerModuleValidation) {
@@ -922,8 +922,7 @@ LoadedFile *SerializedModuleLoaderBase::loadAST(
922922
moduleInterfacePath, moduleInterfaceSourcePath,
923923
std::move(moduleInputBuffer), std::move(moduleDocInputBuffer),
924924
std::move(moduleSourceInfoInputBuffer), isFramework,
925-
isRequiredOSSAModules(),
926-
Ctx.LangOpts.SDKName,
925+
isRequiredOSSAModules(), Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName,
927926
Ctx.SearchPathOpts.DeserializedPathRecoverer, loadedModuleFileCore);
928927
SerializedASTFile *fileUnit = nullptr;
929928

@@ -1443,7 +1442,7 @@ bool SerializedModuleLoaderBase::canImportModule(
14431442
if (moduleInputBuffer) {
14441443
auto metaData = serialization::validateSerializedAST(
14451444
moduleInputBuffer->getBuffer(), Ctx.SILOpts.EnableOSSAModules,
1446-
Ctx.LangOpts.SDKName);
1445+
Ctx.LangOpts.SDKName, Ctx.LangOpts.PackageName);
14471446

14481447
// If we only found binary module, make sure that is valid.
14491448
if (metaData.status != serialization::Status::Valid &&

‎test/ModuleInterface/load_interface_for_external_client.swift‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %empty-directory(%t/ModuleCache)
3+
// RUN: split-file %s %t
4+
5+
// RUN: %target-swift-frontend -emit-module %t/Bar.swift -I %t \
6+
// RUN: -module-name Bar -package-name barpkg \
7+
// RUN: -enable-library-evolution -swift-version 5 \
8+
// RUN: -module-cache-path %t/ModuleCache \
9+
// RUN: -emit-module -emit-module-path %t/Bar.swiftmodule \
10+
// RUN: -O -wmo -allow-non-resilient-access -package-cmo \
11+
// RUN: -disable-print-package-name-for-non-package-interface \
12+
// RUN: -emit-module-interface-path %t/Bar.swiftinterface \
13+
// RUN: -emit-private-module-interface-path %t/Bar.private.swiftinterface \
14+
// RUN: -emit-package-module-interface-path %t/Bar.package.swiftinterface
15+
16+
// RUN: %target-swift-frontend -typecheck %t/InPkgClient.swift -I %t \
17+
// RUN: -package-name barpkg \
18+
// RUN: -Rmodule-loading 2> %t/load-pcmo-module.txt
19+
// RUN: %FileCheck -check-prefix=LOAD-BINARY %s < %t/load-pcmo-module.txt
20+
21+
// RUN: %target-swift-frontend -typecheck %t/ExtClient.swift -I %t \
22+
// RUN: -Rmodule-loading 2> %t/load-interface.txt
23+
// RUN: %FileCheck -check-prefix=LOAD-INTERFACE %s < %t/load-interface.txt
24+
25+
// LOAD-BINARY: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar.swiftmodule'
26+
27+
// LOAD-INTERFACE: remark: loaded module 'Bar'; source: '{{.*}}Bar.private.swiftinterface', loaded: '{{.*}}Bar-{{.*}}.swiftmodule'
28+
29+
//--- Bar.swift
30+
public class PubKlass {
31+
public var pubVar = "1"
32+
public init() {}
33+
}
34+
35+
package class PkgKlass {
36+
package var pkgVar = "2"
37+
package init() {}
38+
}
39+
40+
//--- InPkgClient.swift
41+
import Bar
42+
43+
func foo() {
44+
print(PubKlass().pubVar, PkgKlass().pkgVar)
45+
}
46+
47+
//--- ExtClient.swift
48+
import Bar
49+
50+
func foo() {
51+
print(PubKlass().pubVar)
52+
}

‎tools/lldb-moduleimport-test/lldb-moduleimport-test.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ static bool validateModule(
5151
llvm::SmallVectorImpl<swift::serialization::SearchPath> &searchPaths) {
5252
info = swift::serialization::validateSerializedAST(
5353
data, requiresOSSAModules,
54-
/*requiredSDK*/ StringRef(), &extendedInfo, /* dependencies*/ nullptr,
55-
&searchPaths);
54+
/*requiredSDK*/ StringRef(),
55+
/*packageName*/ StringRef(), &extendedInfo,
56+
/* dependencies*/ nullptr, &searchPaths);
5657
if (info.status != swift::serialization::Status::Valid) {
5758
llvm::outs() << "error: validateSerializedAST() failed\n";
5859
return false;

0 commit comments

Comments
 (0)