Skip to content

Commit e727b65

Browse files
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 whe rdar://117607906
1 parent 41dc466 commit e727b65

File tree

6 files changed

+105
-11
lines changed

6 files changed

+105
-11
lines changed

include/swift/AST/Decl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3294,6 +3294,9 @@ class OpaqueTypeDecl final :
32943294
};
32953295
}
32963296

3297+
/// Should the underlying type be visible to clients outside of the module?
3298+
bool exportUnderlyingType() const;
3299+
32973300
/// The substitutions that map the generic parameters of the opaque type to
32983301
/// the unique underlying types, when that information is known.
32993302
llvm::Optional<SubstitutionMap> getUniqueUnderlyingTypeSubstitutions() const {

lib/AST/Decl.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9464,6 +9464,25 @@ GenericTypeParamDecl *OpaqueTypeDecl::getExplicitGenericParam(
94649464
return genericParamType->getDecl();
94659465
}
94669466

9467+
bool OpaqueTypeDecl::exportUnderlyingType() const {
9468+
auto mod = getDeclContext()->getParentModule();
9469+
if (mod->getResilienceStrategy() != ResilienceStrategy::Resilient)
9470+
return true;
9471+
9472+
ValueDecl *namingDecl = getNamingDecl();
9473+
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl))
9474+
return AFD->getResilienceExpansion() == ResilienceExpansion::Minimal;
9475+
9476+
if (auto *ASD = dyn_cast<AbstractStorageDecl>(namingDecl)) {
9477+
for (auto *accessor : ASD->getAllAccessors())
9478+
if (accessor->getResilienceExpansion() == ResilienceExpansion::Minimal)
9479+
return true;
9480+
return false;
9481+
}
9482+
9483+
llvm_unreachable("The naming decl is expected to be either an AFD or ASD");
9484+
}
9485+
94679486
llvm::Optional<unsigned>
94689487
OpaqueTypeDecl::getAnonymousOpaqueParamOrdinal(TypeRepr *repr) const {
94699488
assert(NamingDeclAndHasOpaqueReturnTypeRepr.getInt() &&

lib/Serialization/Deserialization.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4206,11 +4206,13 @@ class DeclDeserializer {
42064206
GenericSignatureID genericSigID;
42074207
SubstitutionMapID underlyingTypeSubsID;
42084208
uint8_t rawAccessLevel;
4209+
bool exportUnderlyingType;
42094210
decls_block::OpaqueTypeLayout::readRecord(scratch, contextID,
42104211
namingDeclID, interfaceSigID,
42114212
interfaceTypeID, genericSigID,
42124213
underlyingTypeSubsID,
4213-
rawAccessLevel);
4214+
rawAccessLevel,
4215+
exportUnderlyingType);
42144216

42154217
auto declContext = MF.getDeclContext(contextID);
42164218
auto interfaceSig = MF.getGenericSignature(interfaceSigID);
@@ -4244,16 +4246,25 @@ class DeclDeserializer {
42444246
else
42454247
opaqueDecl->setGenericSignature(GenericSignature());
42464248

4247-
auto *AFD = dyn_cast<AbstractFunctionDecl>(namingDecl);
4248-
if (MF.getResilienceStrategy() == ResilienceStrategy::Resilient &&
4249-
!MF.FileContext->getParentModule()->isMainModule() &&
4250-
AFD && AFD->getResilienceExpansion() != ResilienceExpansion::Minimal) {
4249+
if (!MF.FileContext->getParentModule()->isMainModule() &&
4250+
!exportUnderlyingType) {
42514251
// Do not try to read the underlying type information if the function
42524252
// is not inlinable in clients. This reflects the swiftinterface behavior
42534253
// in where clients are only aware of the underlying type when the body
42544254
// of the function is public.
4255+
LLVM_DEBUG(
4256+
llvm::dbgs() << "Ignoring underlying information for opaque type of '";
4257+
llvm::dbgs() << namingDecl->getName();
4258+
llvm::dbgs() << "'\n";
4259+
);
42554260

42564261
} else if (underlyingTypeSubsID) {
4262+
LLVM_DEBUG(
4263+
llvm::dbgs() << "Loading underlying information for opaque type of '";
4264+
llvm::dbgs() << namingDecl->getName();
4265+
llvm::dbgs() << "'\n";
4266+
);
4267+
42574268
auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
42584269
if (!subMapOrError) {
42594270
// 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 = 813; // VTable bit redefinition
61+
const uint16_t SWIFTMODULE_VERSION_MINOR = 814; // Opaque type export details
6262

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

lib/Serialization/Serialization.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4471,11 +4471,14 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
44714471
}
44724472
uint8_t rawAccessLevel =
44734473
getRawStableAccessLevel(opaqueDecl->getFormalAccess());
4474+
bool exportDetails = opaqueDecl->exportUnderlyingType();
4475+
44744476
unsigned abbrCode = S.DeclTypeAbbrCodes[OpaqueTypeLayout::Code];
44754477
OpaqueTypeLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
44764478
contextID.getOpaqueValue(), namingDeclID,
44774479
interfaceSigID, interfaceTypeID, genericSigID,
4478-
underlyingSubsID, rawAccessLevel);
4480+
underlyingSubsID, rawAccessLevel,
4481+
exportDetails);
44794482
writeGenericParams(opaqueDecl->getGenericParams());
44804483

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

test/Serialization/Safety/skip-reading-internal-details.swift

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
// RUN: -enable-library-evolution -swift-version 5 \
99
// RUN: -emit-module-path %t/HiddenLib.swiftmodule
1010
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
11-
// RUN: -enable-library-evolution \
11+
// RUN: -swift-version 5 -enable-library-evolution \
1212
// RUN: -emit-module-path %t/Lib.swiftmodule \
13-
// RUN: -emit-module-interface-path %t/Lib.swiftinterface
13+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface -verify
1414

1515
/// Build clients, with and without safety.
1616
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
@@ -107,12 +107,58 @@ public struct EV : V {
107107

108108
@available(SwiftStdlib 5.1, *)
109109
public extension V {
110+
private func referencedPrivateFunc(v: some V) -> some V { return v }
111+
112+
/// Hidden underlying types.
113+
// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivate()'
110114
@VB
111115
func opaqueReferencingPrivate() -> some V {
112116
referencedPrivateFunc(v: EV())
113117
}
114118

115-
private func referencedPrivateFunc(v: some V) -> some V { return v }
119+
// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVar'
120+
var opaqueReferencingPrivateVar: some V {
121+
referencedPrivateFunc(v: EV())
122+
}
123+
124+
// NEEDED: Ignoring underlying information for opaque type of 'opaqueReferencingPrivateVarPattern'
125+
var opaqueReferencingPrivateVarPattern: some V {
126+
get {
127+
referencedPrivateFunc(v: EV())
128+
}
129+
}
130+
131+
// NEEDED: Ignoring underlying information for opaque type of 'subscript(_:)'
132+
subscript(v: some V) -> some V {
133+
referencedPrivateFunc(v: v)
134+
}
135+
136+
/// Visible underlying types.
137+
// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueFunc()'
138+
@inlinable
139+
func inlinableOpaqueFunc() -> some V { EV() }
140+
141+
// NEEDED: Loading underlying information for opaque type of 'backdeployedOpaqueFunc()'
142+
@backDeployed(before: SwiftStdlib 5.1) // expected-warning 4 {{'@backDeployed' is unsupported on a instance method with a 'some' return type}}
143+
func backdeployedOpaqueFunc() -> some V { EV() }
144+
145+
// NEEDED: Loading underlying information for opaque type of 'aeicOpaqueFunc()'
146+
@_alwaysEmitIntoClient
147+
func aeicOpaqueFunc() -> some V { EV() }
148+
149+
// NEEDED: Loading underlying information for opaque type of 'transparentOpaqueFunc()'
150+
@_transparent
151+
func transparentOpaqueFunc() -> some V { EV() }
152+
153+
// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueVar'
154+
@inlinable
155+
var inlinableOpaqueVar: some V { EV() }
156+
157+
// NEEDED: Loading underlying information for opaque type of 'inlinableOpaqueVarPattern'
158+
var inlinableOpaqueVarPattern: some V {
159+
@inlinable
160+
get { EV() }
161+
}
116162
}
117163

118164
//--- Client.swift
@@ -127,4 +173,15 @@ x.notAMember() // expected-error {{value of type 'PublicStruct' has no member 'n
127173
if #available(SwiftStdlib 5.1, *) {
128174
let v = EV()
129175
let _ = v.opaqueReferencingPrivate()
176+
let _ = v.opaqueReferencingPrivateVar
177+
let _ = v.opaqueReferencingPrivateVarPattern
178+
let _ = v[v]
179+
180+
let _ = v.inlinableOpaqueFunc()
181+
let _ = v.backdeployedOpaqueFunc()
182+
let _ = v.aeicOpaqueFunc()
183+
let _ = v.transparentOpaqueFunc()
184+
185+
let _ = v.inlinableOpaqueVar
186+
let _ = v.inlinableOpaqueVarPattern
130187
}

0 commit comments

Comments
 (0)