Skip to content

Commit f2d1627

Browse files
xymusxymus
committed
[Serialization] Fix deserializing opaque types for computed properties and subscripts
A client shouldn't know about the underlying type of an opaque type unless it can see the body of the naming decl. Attempting to read it can lead to accessing a hidden dependency and a compiler crash. This was protected by a check specific to function decls but var decls and subscripts were not handled. To support them we have to move this logic to the writer side where we have access to the full AbstractStorageDecl and write in the swifmodule whether the underlying type should be visible outside of the module. rdar://117607906
1 parent d976ea6 commit f2d1627

File tree

7 files changed

+237
-8
lines changed

7 files changed

+237
-8
lines changed

‎include/swift/AST/Decl.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3324,6 +3324,9 @@ class OpaqueTypeDecl final :
33243324
};
33253325
}
33263326

3327+
/// Should the underlying type be visible to clients outside of the module?
3328+
bool exportUnderlyingType() const;
3329+
33273330
/// The substitutions that map the generic parameters of the opaque type to
33283331
/// the unique underlying types, when that information is known.
33293332
llvm::Optional<SubstitutionMap> getUniqueUnderlyingTypeSubstitutions() const {

‎lib/AST/Decl.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9547,6 +9547,25 @@ GenericTypeParamDecl *OpaqueTypeDecl::getExplicitGenericParam(
95479547
return genericParamType->getDecl();
95489548
}
95499549

9550+
bool OpaqueTypeDecl::exportUnderlyingType() const {
9551+
auto mod = getDeclContext()->getParentModule();
9552+
if (mod->getResilienceStrategy() != ResilienceStrategy::Resilient)
9553+
return true;
9554+
9555+
ValueDecl *namingDecl = getNamingDecl();
9556+
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl))
9557+
return AFD->getResilienceExpansion() == ResilienceExpansion::Minimal;
9558+
9559+
if (auto *ASD = dyn_cast<AbstractStorageDecl>(namingDecl)) {
9560+
for (auto *accessor : ASD->getAllAccessors())
9561+
if (accessor->getResilienceExpansion() == ResilienceExpansion::Minimal)
9562+
return true;
9563+
return false;
9564+
}
9565+
9566+
llvm_unreachable("The naming decl is expected to be either an AFD or ASD");
9567+
}
9568+
95509569
llvm::Optional<unsigned>
95519570
OpaqueTypeDecl::getAnonymousOpaqueParamOrdinal(TypeRepr *repr) const {
95529571
assert(NamingDeclAndHasOpaqueReturnTypeRepr.getInt() &&

‎lib/Serialization/Deserialization.cpp‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4221,11 +4221,13 @@ class DeclDeserializer {
42214221
GenericSignatureID genericSigID;
42224222
SubstitutionMapID underlyingTypeSubsID;
42234223
uint8_t rawAccessLevel;
4224+
bool exportUnderlyingType;
42244225
decls_block::OpaqueTypeLayout::readRecord(scratch, contextID,
42254226
namingDeclID, interfaceSigID,
42264227
interfaceTypeID, genericSigID,
42274228
underlyingTypeSubsID,
4228-
rawAccessLevel);
4229+
rawAccessLevel,
4230+
exportUnderlyingType);
42294231

42304232
auto declContext = MF.getDeclContext(contextID);
42314233
auto interfaceSigOrErr = MF.getGenericSignatureChecked(interfaceSigID);
@@ -4261,16 +4263,25 @@ class DeclDeserializer {
42614263
else
42624264
opaqueDecl->setGenericSignature(GenericSignature());
42634265

4264-
auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl);
4265-
if (MF.getResilienceStrategy() == ResilienceStrategy::Resilient &&
4266-
!MF.FileContext->getParentModule()->isMainModule() &&
4267-
AFD && AFD->getResilienceExpansion() != ResilienceExpansion::Minimal) {
4266+
if (!MF.FileContext->getParentModule()->isMainModule() &&
4267+
!exportUnderlyingType) {
42684268
// Do not try to read the underlying type information if the function
42694269
// is not inlinable in clients. This reflects the swiftinterface behavior
42704270
// in where clients are only aware of the underlying type when the body
42714271
// of the function is public.
4272+
LLVM_DEBUG(
4273+
llvm::dbgs() << "Ignoring underlying information for opaque type of '";
4274+
llvm::dbgs() << namingDecl->getName();
4275+
llvm::dbgs() << "'\n";
4276+
);
42724277

42734278
} else if (underlyingTypeSubsID) {
4279+
LLVM_DEBUG(
4280+
llvm::dbgs() << "Loading underlying information for opaque type of '";
4281+
llvm::dbgs() << namingDecl->getName();
4282+
llvm::dbgs() << "'\n";
4283+
);
4284+
42744285
auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
42754286
if (!subMapOrError) {
42764287
// If the underlying type references internal details, ignore it.

‎lib/Serialization/ModuleFormat.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5858
/// describe what change you made. The content of this comment isn't important;
5959
/// it just ensures a conflict if two people change the module format.
6060
/// Don't worry about adhering to the 80-column limit for this line.
61-
const uint16_t SWIFTMODULE_VERSION_MINOR = 816; // throw_addr
61+
const uint16_t SWIFTMODULE_VERSION_MINOR = 817; // Opaque type export details
6262

6363
/// A standard hash seed used for all string hashes in a serialized module.
6464
///
@@ -1641,7 +1641,8 @@ namespace decls_block {
16411641
TypeIDField, // interface type for opaque type
16421642
GenericSignatureIDField, // generic environment
16431643
SubstitutionMapIDField, // optional substitution map for underlying type
1644-
AccessLevelField // access level
1644+
AccessLevelField, // access level
1645+
BCFixed<1> // export underlying type details
16451646
// trailed by generic parameters
16461647
// trailed by conditional substitutions
16471648
>;

‎lib/Serialization/Serialization.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4490,11 +4490,14 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
44904490
}
44914491
uint8_t rawAccessLevel =
44924492
getRawStableAccessLevel(opaqueDecl->getFormalAccess());
4493+
bool exportDetails = opaqueDecl->exportUnderlyingType();
4494+
44934495
unsigned abbrCode = S.DeclTypeAbbrCodes[OpaqueTypeLayout::Code];
44944496
OpaqueTypeLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
44954497
contextID.getOpaqueValue(), namingDeclID,
44964498
interfaceSigID, interfaceTypeID, genericSigID,
4497-
underlyingSubsID, rawAccessLevel);
4499+
underlyingSubsID, rawAccessLevel,
4500+
exportDetails);
44984501
writeGenericParams(opaqueDecl->getGenericParams());
44994502

45004503
// Serialize all of the conditionally available substitutions expect the

‎test/Serialization/ignore-opaque-underlying-type-back-deploy.swift‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/// Variant of ignore-opaque-underlying-type because of the macOS host need.
2+
// RUN: %empty-directory(%t)
3+
// RUN: split-file %s %t
4+
5+
// REQUIRES: asserts
6+
// REQUIRES: OS=macosx
7+
8+
/// Resilient scenario, we ignore underlying type of non-inlinable functions.
9+
/// Build libraries.
10+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
11+
// RUN: -swift-version 5 -enable-library-evolution \
12+
// RUN: -emit-module-path %t/Lib.swiftmodule \
13+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface -verify
14+
15+
/// Build clients, with and without safety.
16+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
17+
// RUN: -verify -Xllvm -debug-only=Serialization \
18+
// RUN: -enable-deserialization-safety 2>&1 \
19+
// RUN: | %FileCheck %s
20+
21+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
22+
// RUN: -verify -Xllvm -debug-only=Serialization \
23+
// RUN: -disable-deserialization-safety 2>&1 \
24+
// RUN: | %FileCheck %s
25+
26+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
27+
// RUN: -verify -Xllvm -debug-only=Serialization \
28+
// RUN: -disable-access-control 2>&1 \
29+
// RUN: | %FileCheck %s
30+
31+
/// Build against the swiftinterface.
32+
// RUN: rm %t/Lib.swiftmodule
33+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
34+
// RUN: -verify -Xllvm -debug-only=Serialization \
35+
// RUN: -disable-deserialization-safety 2>&1 \
36+
// RUN: | %FileCheck %s
37+
38+
/// Non-resilient scenario, all underlying types are loaded.
39+
// RUN: %empty-directory(%t)
40+
// RUN: split-file %s %t
41+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
42+
// RUN: -swift-version 5 \
43+
// RUN: -emit-module-path %t/Lib.swiftmodule -verify
44+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
45+
// RUN: -verify -Xllvm -debug-only=Serialization \
46+
// RUN: -disable-deserialization-safety 2>&1 \
47+
// RUN: | %FileCheck --check-prefix=NON-RESILIENT %s
48+
// NON-RESILIENT-NOT: Ignoring underlying information
49+
50+
//--- Lib.swift
51+
public protocol V {}
52+
53+
public struct EV : V {
54+
public init () {}
55+
}
56+
57+
@available(SwiftStdlib 5.1, *)
58+
public extension V {
59+
// CHECK: Loading underlying information for opaque type of 'backdeployedOpaqueFunc()'
60+
@backDeployed(before: SwiftStdlib 5.1) // expected-warning 4 {{'@backDeployed' is unsupported on a instance method with a 'some' return type}}
61+
func backdeployedOpaqueFunc() -> some V { EV() }
62+
}
63+
64+
//--- Client.swift
65+
import Lib
66+
67+
if #available(SwiftStdlib 5.1, *) {
68+
let v = EV()
69+
let _ = v.backdeployedOpaqueFunc()
70+
}

‎test/Serialization/ignore-opaque-underlying-type.swift‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// REQUIRES: asserts
5+
6+
/// Resilient scenario, we ignore underlying type of non-inlinable functions.
7+
/// Build libraries.
8+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
9+
// RUN: -swift-version 5 -enable-library-evolution \
10+
// RUN: -emit-module-path %t/Lib.swiftmodule \
11+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface -verify
12+
13+
/// Build clients, with and without safety.
14+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
15+
// RUN: -verify -Xllvm -debug-only=Serialization \
16+
// RUN: -enable-deserialization-safety 2>&1 \
17+
// RUN: | %FileCheck %s
18+
19+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
20+
// RUN: -verify -Xllvm -debug-only=Serialization \
21+
// RUN: -disable-deserialization-safety 2>&1 \
22+
// RUN: | %FileCheck %s
23+
24+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
25+
// RUN: -verify -Xllvm -debug-only=Serialization \
26+
// RUN: -disable-access-control 2>&1 \
27+
// RUN: | %FileCheck %s
28+
29+
/// Build against the swiftinterface.
30+
// RUN: rm %t/Lib.swiftmodule
31+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
32+
// RUN: -verify -Xllvm -debug-only=Serialization \
33+
// RUN: -disable-deserialization-safety 2>&1 \
34+
// RUN: | %FileCheck %s
35+
36+
/// Non-resilient scenario, all underlying types are loaded.
37+
// RUN: %empty-directory(%t)
38+
// RUN: split-file %s %t
39+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift \
40+
// RUN: -swift-version 5 \
41+
// RUN: -emit-module-path %t/Lib.swiftmodule -verify
42+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
43+
// RUN: -verify -Xllvm -debug-only=Serialization \
44+
// RUN: -disable-deserialization-safety 2>&1 \
45+
// RUN: | %FileCheck --check-prefix=NON-RESILIENT %s
46+
// NON-RESILIENT-NOT: Ignoring underlying information
47+
48+
//--- Lib.swift
49+
public protocol V {}
50+
51+
public struct EV : V {
52+
public init () {}
53+
}
54+
55+
@available(SwiftStdlib 5.1, *)
56+
public extension V {
57+
private func referencedPrivateFunc(v: some V) -> some V { return v }
58+
59+
/// Hidden underlying types.
60+
// CHECK: Ignoring underlying information for opaque type of 'opaqueReferencingPrivate()'
61+
func opaqueReferencingPrivate() -> some V {
62+
referencedPrivateFunc(v: EV())
63+
}
64+
65+
// CHECK: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVar'
66+
var opaqueReferencingPrivateVar: some V {
67+
referencedPrivateFunc(v: EV())
68+
}
69+
70+
// CHECK: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVarPattern'
71+
var opaqueReferencingPrivateVarPattern: some V {
72+
get {
73+
referencedPrivateFunc(v: EV())
74+
}
75+
}
76+
77+
// CHECK: Ignoring underlying information for opaque type of 'subscript(_:)'
78+
subscript(v: some V) -> some V {
79+
referencedPrivateFunc(v: v)
80+
}
81+
82+
/// Visible underlying types.
83+
// CHECK: Loading underlying information for opaque type of 'inlinableOpaqueFunc()'
84+
@inlinable
85+
func inlinableOpaqueFunc() -> some V { EV() }
86+
87+
// CHECK: Loading underlying information for opaque type of 'aeicOpaqueFunc()'
88+
@_alwaysEmitIntoClient
89+
func aeicOpaqueFunc() -> some V { EV() }
90+
91+
// CHECK: Loading underlying information for opaque type of 'transparentOpaqueFunc()'
92+
@_transparent
93+
func transparentOpaqueFunc() -> some V { EV() }
94+
95+
// CHECK: Loading underlying information for opaque type of 'inlinableOpaqueVar'
96+
@inlinable
97+
var inlinableOpaqueVar: some V { EV() }
98+
99+
// CHECK: Loading underlying information for opaque type of 'inlinableOpaqueVarPattern'
100+
var inlinableOpaqueVarPattern: some V {
101+
@inlinable
102+
get { EV() }
103+
}
104+
}
105+
106+
//--- Client.swift
107+
import Lib
108+
109+
if #available(SwiftStdlib 5.1, *) {
110+
let v = EV()
111+
let _ = v.opaqueReferencingPrivate()
112+
let _ = v.opaqueReferencingPrivateVar
113+
let _ = v.opaqueReferencingPrivateVarPattern
114+
let _ = v[v]
115+
116+
let _ = v.inlinableOpaqueFunc()
117+
let _ = v.aeicOpaqueFunc()
118+
let _ = v.transparentOpaqueFunc()
119+
120+
let _ = v.inlinableOpaqueVar
121+
let _ = v.inlinableOpaqueVarPattern
122+
}

0 commit comments

Comments
 (0)