Skip to content

Commit ddc8e21

Browse files
eecksteineeckstein
committed
CrossModuleOptimization: fix crash when importing a module as implementationOnly
If a function uses a type imported as implementationOnly (or similar), it cannot be serialized. I added a new API in ModuleDecl (canBeUsedForCrossModuleOptimization), which performs this check. rdar://72864719
1 parent 903de11 commit ddc8e21

File tree

6 files changed

+101
-6
lines changed

6 files changed

+101
-6
lines changed

‎include/swift/AST/Module.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,10 @@ class ModuleDecl : public DeclContext, public TypeDecl {
626626
/// This assumes that \p module was imported.
627627
bool isImportedImplementationOnly(const ModuleDecl *module) const;
628628

629+
/// Returns true if a function, which is using \p nominal, can be serialized
630+
/// by cross-module-optimization.
631+
bool canBeUsedForCrossModuleOptimization(NominalTypeDecl *nominal) const;
632+
629633
/// Finds all top-level decls of this module.
630634
///
631635
/// This does a simple local lookup, not recursively looking through imports.

‎lib/AST/Module.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,6 +2012,30 @@ bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
20122012
return true;
20132013
}
20142014

2015+
bool ModuleDecl::
2016+
canBeUsedForCrossModuleOptimization(NominalTypeDecl *nominal) const {
2017+
ModuleDecl *moduleOfNominal = nominal->getParentModule();
2018+
2019+
// If the nominal is defined in the same module, it's fine.
2020+
if (moduleOfNominal == this)
2021+
return true;
2022+
2023+
// See if nominal is imported in a "regular" way, i.e. not with
2024+
// @_implementationOnly or @_spi.
2025+
ModuleDecl::ImportFilter filter = {
2026+
ModuleDecl::ImportFilterKind::Exported,
2027+
ModuleDecl::ImportFilterKind::Default};
2028+
SmallVector<ImportedModule, 4> results;
2029+
getImportedModules(results, filter);
2030+
2031+
auto &imports = getASTContext().getImportCache();
2032+
for (auto &desc : results) {
2033+
if (imports.isImportedBy(moduleOfNominal, desc.importedModule))
2034+
return true;
2035+
}
2036+
return false;
2037+
}
2038+
20152039
void SourceFile::lookupImportedSPIGroups(
20162040
const ModuleDecl *importedModule,
20172041
llvm::SmallSetVector<Identifier, 4> &spiGroups) const {

‎lib/SILOptimizer/IPO/CrossModuleSerializationSetup.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ class CrossModuleSerializationSetup {
4545
llvm::SmallVector<SILFunction *, 16> workList;
4646
llvm::SmallPtrSet<SILFunction *, 16> functionsHandled;
4747

48+
llvm::DenseMap<SILType, bool> typesChecked;
4849
llvm::SmallPtrSet<TypeBase *, 16> typesHandled;
4950

5051
SILModule &M;
@@ -62,6 +63,8 @@ class CrossModuleSerializationSetup {
6263

6364
bool canSerialize(SILInstruction *inst, bool lookIntoThunks);
6465

66+
bool canSerialize(SILType type);
67+
6568
void setUpForSerialization(SILFunction *F);
6669

6770
void prepareInstructionForSerialization(SILInstruction *inst);
@@ -320,6 +323,12 @@ bool CrossModuleSerializationSetup::canSerialize(SILFunction *F,
320323

321324
bool CrossModuleSerializationSetup::canSerialize(SILInstruction *inst,
322325
bool lookIntoThunks) {
326+
327+
for (SILValue result : inst->getResults()) {
328+
if (!canSerialize(result->getType()))
329+
return false;
330+
}
331+
323332
if (auto *FRI = dyn_cast<FunctionRefBaseInst>(inst)) {
324333
SILFunction *callee = FRI->getReferencedFunctionOrNull();
325334
return canUseFromInline(callee, lookIntoThunks);
@@ -344,6 +353,31 @@ bool CrossModuleSerializationSetup::canSerialize(SILInstruction *inst,
344353
return true;
345354
}
346355

356+
bool CrossModuleSerializationSetup::canSerialize(SILType type) {
357+
auto iter = typesChecked.find(type);
358+
if (iter != typesChecked.end())
359+
return iter->getSecond();
360+
361+
ModuleDecl *mod = M.getSwiftModule();
362+
bool success = !type.getASTType().findIf(
363+
[mod](Type rawSubType) {
364+
CanType subType = rawSubType->getCanonicalType();
365+
if (NominalTypeDecl *subNT = subType->getNominalOrBoundGenericNominal()) {
366+
367+
// Exclude types which are defined in an @_implementationOnly imported
368+
// module. Such modules are not transitively available.
369+
if (!mod->canBeUsedForCrossModuleOptimization(subNT)) {
370+
llvm::dbgs() << " === " << mod->getName() << ", " <<
371+
subNT->getParentModule()->getName() << ", " << subNT->getName() << '\n';
372+
return true;
373+
}
374+
}
375+
return false;
376+
});
377+
typesChecked[type] = success;
378+
return success;
379+
}
380+
347381
/// Returns true if the function \p func can be used from a serialized function.
348382
///
349383
/// If \p lookIntoThunks is true, serializable shared thunks are also accepted.

‎test/SILOptimizer/Inputs/cross-module.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import Submodule
2+
@_implementationOnly import PrivateSubmodule
23

34
private enum PE<T> {
45
case A
@@ -267,4 +268,11 @@ public func callUnrelated<T>(_ t: T) -> T {
267268
return t
268269
}
269270

271+
public func callImplementationOnly<T>(_ t: T) -> T {
272+
let p = PrivateStr(i: 27)
273+
print(p.test())
274+
return t
275+
}
276+
277+
270278
public let globalLet = 529387

‎test/SILOptimizer/Inputs/cross-private-submodule.swift‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
public struct PrivateStr {
3+
var i: Int
4+
5+
public init(i: Int) {
6+
self.i = i
7+
}
8+
9+
@inline(never)
10+
public func test() -> Int {
11+
return i
12+
}
13+
}
14+

‎test/SILOptimizer/cross-module-optimization.swift‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,17 @@
22

33
// RUN: %empty-directory(%t)
44
// RUN: %target-build-swift -O -wmo -parse-as-library -cross-module-optimization -emit-module -emit-module-path=%t/Submodule.swiftmodule -module-name=Submodule %S/Inputs/cross-submodule.swift -c -o %t/submodule.o
5+
// RUN: %target-build-swift -O -wmo -parse-as-library -cross-module-optimization -emit-module -emit-module-path=%t/PrivateSubmodule.swiftmodule -module-name=PrivateSubmodule %S/Inputs/cross-private-submodule.swift -c -o %t/privatesubmodule.o
56
// RUN: %target-build-swift -O -wmo -parse-as-library -cross-module-optimization -emit-module -emit-module-path=%t/Test.swiftmodule -module-name=Test -I%t %S/Inputs/cross-module.swift -c -o %t/test.o
67
// RUN: %target-build-swift -O -wmo -module-name=Main -I%t %s -c -o %t/main.o
7-
// RUN: %target-swiftc_driver %t/main.o %t/test.o %t/submodule.o -o %t/a.out
8+
// RUN: %target-swiftc_driver %t/main.o %t/test.o %t/submodule.o %t/privatesubmodule.o -o %t/a.out
89
// RUN: %target-codesign %t/a.out
910
// RUN: %target-run %t/a.out | %FileCheck %s -check-prefix=CHECK-OUTPUT
1011

1112
// Check if it also works if the main module is compiled with -Onone:
1213

1314
// RUN: %target-build-swift -Onone -wmo -module-name=Main -I%t %s -c -o %t/main-onone.o
14-
// RUN: %target-swiftc_driver %t/main-onone.o %t/test.o %t/submodule.o -o %t/a.out
15+
// RUN: %target-swiftc_driver %t/main-onone.o %t/test.o %t/submodule.o %t/privatesubmodule.o -o %t/a.out
1516
// RUN: %target-codesign %t/a.out
1617
// RUN: %target-run %t/a.out | %FileCheck %s -check-prefix=CHECK-OUTPUT
1718

@@ -29,19 +30,19 @@ import Test
2930
func testNestedTypes() {
3031
let c = Container()
3132

32-
// CHECK-OUTPUT: [Test.Container.Base]
33+
// CHECK-OUTPUT [Test.Container.Base]
3334
// CHECK-OUTPUT: 27
3435
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test9ContainerV9testclassyxxlFSi_Tg5
3536
print(c.testclass(27))
36-
// CHECK-OUTPUT: [Test.Container.Base]
37+
// CHECK-OUTPUT [Test.Container.Base]
3738
// CHECK-OUTPUT: 27
3839
// CHECK-SIL-DAG: sil shared_external {{.*}} @$s4Test9ContainerV13testclass_genyxxlF
3940
print(c.testclass_gen(27))
40-
// CHECK-OUTPUT: [Test.PE<Swift.Int>.B(27)]
41+
// CHECK-OUTPUT [Test.PE<Swift.Int>.B(27)]
4142
// CHECK-OUTPUT: 27
4243
// CHECK-SIL-DAG: sil shared [noinline] @$s4Test9ContainerV8testenumyxxlFSi_Tg5
4344
print(c.testenum(27))
44-
// CHECK-OUTPUT: [Test.PE<Swift.Int>.B(27)]
45+
// CHECK-OUTPUT [Test.PE<Swift.Int>.B(27)]
4546
// CHECK-OUTPUT: 27
4647
// CHECK-SIL-DAG: sil shared_external {{.*}} @$s4Test9ContainerV12testenum_genyxxlF
4748
print(c.testenum_gen(27))
@@ -141,6 +142,15 @@ func testGlobal() {
141142
// CHECK-SIL2: } // end sil function '$s4Main10testGlobalyyF'
142143
}
143144

145+
// CHECK-SIL2-LABEL: sil hidden [noinline] @$s4Main22testImplementationOnlyyyF
146+
@inline(never)
147+
func testImplementationOnly() {
148+
// CHECK-OUTPUT: 27
149+
// CHECK-SIL2: function_ref @$s4Test22callImplementationOnlyyxxlF
150+
print(callImplementationOnly(27))
151+
// CHECK-SIL2: } // end sil function '$s4Main22testImplementationOnlyyyF'
152+
}
153+
144154
testNestedTypes()
145155
testClass()
146156
testError()
@@ -150,4 +160,5 @@ testClosures()
150160
testKeypath()
151161
testMisc()
152162
testGlobal()
163+
testImplementationOnly()
153164

0 commit comments

Comments
 (0)