Skip to content

Commit c8d7e94

Browse files
elshelsh
committed
[PackageCMO] Make binary module work for package-external client.
A binary module with PackageCMO includes instructions that are typically disallowed in resilient mode. If the client module belongs to the same package, these instructions can be deserialized and inlined during optimization. However, this must be prevented for clients outside the package, as such instructions are invalid beyond the package domain and could trigger an assertion failure. Resolves rdar://135345358
1 parent 9c44b79 commit c8d7e94

File tree

3 files changed

+162
-1
lines changed

3 files changed

+162
-1
lines changed

‎include/swift/Serialization/SerializedSILLoader.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ class SerializedSILLoader {
4747
explicit SerializedSILLoader(
4848
ASTContext &ctx, SILModule *SILMod,
4949
DeserializationNotificationHandlerSet *callbacks);
50-
5150
public:
5251
/// Create a new loader.
5352
///

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,25 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
584584

585585
if (funcTyID == 0)
586586
return MF->diagnoseFatal("SILFunction typeID is 0");
587+
588+
// A function with [serialized_for_package] and its body should only be
589+
// deserialized in clients within the same package as its defining module;
590+
// for external clients, it should appear only as a declaration. If the
591+
// function has shared linkage, it must have been internal or private in
592+
// its defining module that's optimized, thus should remain inaccessible
593+
// outside the package boundary. This ensures that instructions, which may
594+
// only be valid in resilient mode when package optimization is enabled,
595+
// aren't inlined at the call site, preventing potential assert fails during
596+
// sil-verify.
597+
if (serializedKind == SerializedKind_t::IsSerializedForPackage) {
598+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule())) {
599+
if (rawLinkage == (unsigned int)(SILLinkage::Shared))
600+
return nullptr;
601+
if (!declarationOnly)
602+
declarationOnly = true;
603+
}
604+
}
605+
587606
auto astType = MF->getTypeChecked(funcTyID);
588607
if (!astType) {
589608
if (!existingFn || errorIfEmptyBody) {
@@ -3828,6 +3847,13 @@ SILVTable *SILDeserializer::readVTable(DeclID VId) {
38283847
return nullptr;
38293848
}
38303849

3850+
// Vtable with [serialized_for_package], i.e. optimized with Package CMO,
3851+
// should only be deserialized into a client if its defning module and the
3852+
// client are in the package.
3853+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule()) &&
3854+
SerializedKind_t(Serialized) == SerializedKind_t::IsSerializedForPackage)
3855+
return nullptr;
3856+
38313857
ClassDecl *theClass = cast<ClassDecl>(MF->getDecl(ClassID));
38323858

38333859
PrettyStackTraceDecl trace("deserializing SIL vtable for", theClass);
@@ -4201,6 +4227,13 @@ llvm::Expected<SILWitnessTable *>
42014227
MF->fatal("invalid linkage code");
42024228
}
42034229

4230+
// Witness with [serialized_for_package], i.e. optimized with Package CMO,
4231+
// should only be deserialized into a client if its defning module and the
4232+
// client are in the package.
4233+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule()) &&
4234+
SerializedKind_t(Serialized) == SerializedKind_t::IsSerializedForPackage)
4235+
return nullptr;
4236+
42044237
// Deserialize Conformance.
42054238
auto maybeConformance = MF->getConformanceChecked(conformance);
42064239
if (!maybeConformance)

‎test/SILOptimizer/package-cmo-deserialize-for-external-client.swift

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
5+
// RUN: -module-name Lib -package-name pkg \
6+
// RUN: -enable-library-evolution -swift-version 5 \
7+
// RUN: -O -wmo -experimental-allow-non-resilient-access -experimental-package-cmo \
8+
// RUN: -emit-module -emit-module-path %t/Lib.swiftmodule
9+
// RUN: %target-sil-opt %t/Lib.swiftmodule -o %t/Lib.sil
10+
11+
// RUN: %target-swift-frontend -emit-sil -O %t/Client.swift -I %t -package-name pkg \
12+
// RUN: -Xllvm -sil-disable-pass=DeadFunctionAndGlobalElimination -o %t/InPkgClient.sil
13+
14+
// RUN: %target-swift-frontend -emit-sil -O %t/Client.swift -I %t \
15+
// RUN: -Xllvm -sil-disable-pass=DeadFunctionAndGlobalElimination -o %t/ExtClient.sil
16+
17+
/// TEST how functions (and bodies) and tables optimized with [serialized_for_package] in Lib are
18+
/// deserialized into Client depending on package boundary.
19+
///
20+
/// Test 1: They should be deserialized into Client as Lib and Client are in the same package.
21+
// RUN: %FileCheck -check-prefix=CHECK-INPKG %s < %t/InPkgClient.sil
22+
23+
// Pub.init(_:) is removed after getting lined below.
24+
// CHECK-INPKG-NOT: @$s3Lib3PubCyACSicfc
25+
26+
// Pub.__allocating_init(_:)
27+
// CHECK-INPKG: sil public_external @$s3Lib3PubCyACSicfC : $@convention(method) (Int, @thick Pub.Type) -> @owned Pub {
28+
// Pub.init(_:) is inlined in this block, as its body was deserialized.
29+
// CHECK-INPKG: ref_element_addr {{.*}} : $Pub, #Pub.pubVar
30+
31+
// CHECK-INPKG: sil_vtable Pub {
32+
// CHECK-INPKG: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
33+
// CHECK-INPKG: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
34+
// CHECK-INPKG: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
35+
// CHECK-INPKG: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
36+
// CHECK-INPKG: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
37+
// CHECK-INPKG: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
38+
// CHECK-INPKG: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
39+
// CHECK-INPKG: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
40+
41+
// CHECK-INPKG: sil_witness_table public_external Pub: PubProto module Lib {
42+
// CHECK-INPKG: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
43+
// CHECK-INPKG: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
44+
// CHECK-INPKG: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
45+
46+
47+
/// Test 2: They should NOT be deserialized into Client as Lib and Client are NOT in the same package;
48+
/// only declaration should be deserialized, not the body.
49+
// RUN: %FileCheck -check-prefix=CHECK-EXT %s < %t/ExtClient.sil
50+
51+
// CHECK-EXT-NOT: sil_vtable Pub {
52+
// CHECK-EXT-NOT: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
53+
// CHECK-EXT-NOT: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
54+
// CHECK-EXT-NOT: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
55+
// CHECK-EXT-NOT: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
56+
// CHECK-EXT-NOT: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
57+
// CHECK-EXT-NOT: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
58+
// CHECK-EXT-NOT: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
59+
// CHECK-EXT-NOT: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
60+
61+
// CHECK-EXT-NOT: sil_witness_table public_external Pub: PubProto module Lib {
62+
// CHECK-EXT-NOT: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
63+
// CHECK-EXT-NOT: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
64+
// CHECK-EXT-NOT: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
65+
66+
// Pub.init(_:) is just a decl; does not contain the body.
67+
// CHECK-EXT: sil @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub
68+
69+
70+
/// Verify functions and tables are optimized with Package CMO in Lib.
71+
// RUN: %FileCheck -check-prefix=CHECK-LIB %s < %t/Lib.sil
72+
73+
// CHECK-LIB: sil [serialized] [exact_self_class] [canonical] @$s3Lib3PubCyACSicfC : $@convention(method) (Int, @thick Pub.Type) -> @owned Pub {
74+
// CHECK-LIB: function_ref @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub
75+
// Pub.init(_:)
76+
77+
// CHECK-LIB: sil [serialized_for_package] [canonical] @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub {
78+
// CHECK-LIB: ref_element_addr {{.*}} : $Pub, #Pub.pubVar
79+
// Pub.__allocating_init(_:)
80+
81+
// Pub.pkgVar.getter
82+
// CHECK-LIB: sil package [serialized_for_package] [canonical] @$s3Lib3PubC6pkgVarSivg : $@convention(method) (@guaranteed Pub) -> Int {
83+
84+
// CHECK-LIB: sil_vtable [serialized_for_package] Pub {
85+
// CHECK-LIB: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
86+
// CHECK-LIB: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
87+
// CHECK-LIB: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
88+
// CHECK-LIB: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
89+
// CHECK-LIB: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
90+
// CHECK-LIB: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
91+
// CHECK-LIB: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
92+
// CHECK-LIB: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
93+
94+
// CHECK-LIB: sil_witness_table [serialized_for_package] Pub: PubProto module Lib {
95+
// CHECK-LIB: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
96+
// CHECK-LIB: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
97+
// CHECK-LIB: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
98+
99+
100+
//--- Lib.swift
101+
102+
public protocol PubProto {
103+
var pubVar: Int { get set }
104+
}
105+
106+
public class Pub: PubProto {
107+
public var pubVar: Int = 1
108+
package var pkgVar: Int = 2
109+
public init(_ arg: Int) {
110+
pubVar = arg
111+
pkgVar = arg
112+
}
113+
}
114+
115+
//--- Client.swift
116+
import Lib
117+
118+
public func test(_ arg: Int) -> Int {
119+
let x = Pub(arg)
120+
x.pubVar = 3
121+
return x.run()
122+
}
123+
124+
public extension PubProto {
125+
func run() -> Int {
126+
return pubVar
127+
}
128+
}
129+

0 commit comments

Comments
 (0)