Skip to content

Commit 71f116c

Browse files
committed
[Serialization] Recover in opaque type logic
If the underlying type of an opaque type references an implementation-only imported type, drop the underlying type information. Without this fix, once we enable deserialization safety, we see crashes or dropped decls in more existing tests: IRGen/mangle-opaque-return-type.swift IRGen/opaque_result_type_private_underlying.swift Serialization/Recovery/implementation-only-opaque-type.swift rdar://103238451
1 parent 8456a7e commit 71f116c

File tree

2 files changed

+84
-20
lines changed

2 files changed

+84
-20
lines changed

lib/Serialization/Deserialization.cpp

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ void InvalidRecordKindError::anchor() {}
158158
const char UnsafeDeserializationError::ID = '\0';
159159
void UnsafeDeserializationError::anchor() {}
160160

161+
static llvm::Error consumeErrorIfXRefNonLoadedModule(llvm::Error &&error);
162+
161163
/// Skips a single record in the bitstream.
162164
///
163165
/// Destroys the stream position if the next entry is not a record.
@@ -1360,7 +1362,12 @@ ModuleFile::getSubstitutionMapChecked(serialization::SubstitutionMapID id) {
13601362
SmallVector<Type, 4> replacementTypes;
13611363
replacementTypes.reserve(replacementTypeIDs.size());
13621364
for (auto typeID : replacementTypeIDs) {
1363-
replacementTypes.push_back(getType(typeID));
1365+
auto typeOrError = getTypeChecked(typeID);
1366+
if (!typeOrError) {
1367+
consumeError(typeOrError.takeError());
1368+
continue;
1369+
}
1370+
replacementTypes.push_back(typeOrError.get());
13641371
}
13651372

13661373
// Read the conformances.
@@ -3634,7 +3641,10 @@ class DeclDeserializer {
36343641
if (declOrOffset.isComplete())
36353642
return declOrOffset;
36363643

3637-
const auto resultType = MF.getType(resultInterfaceTypeID);
3644+
auto resultTypeOrError = MF.getTypeChecked(resultInterfaceTypeID);
3645+
if (!resultTypeOrError)
3646+
return resultTypeOrError.takeError();
3647+
const auto resultType = resultTypeOrError.get();
36383648
if (declOrOffset.isComplete())
36393649
return declOrOffset;
36403650

@@ -3707,9 +3717,13 @@ class DeclDeserializer {
37073717
std::move(needsNewVTableEntry));
37083718

37093719
if (opaqueReturnTypeID) {
3720+
auto declOrError = MF.getDeclChecked(opaqueReturnTypeID);
3721+
if (!declOrError)
3722+
return declOrError.takeError();
3723+
37103724
ctx.evaluator.cacheOutput(
37113725
OpaqueResultTypeRequest{fn},
3712-
cast<OpaqueTypeDecl>(MF.getDecl(opaqueReturnTypeID)));
3726+
cast<OpaqueTypeDecl>(declOrError.get()));
37133727
}
37143728

37153729
if (!isAccessor)
@@ -3850,26 +3864,31 @@ class DeclDeserializer {
38503864
opaqueDecl->setGenericSignature(GenericSignature());
38513865
if (underlyingTypeSubsID) {
38523866
auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
3853-
if (!subMapOrError)
3854-
return subMapOrError.takeError();
3855-
3856-
// Check whether there are any conditionally available substitutions.
3857-
// If there are, it means that "unique" we just read is a universally
3858-
// available substitution.
3859-
SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
3860-
limitedAvailability;
3867+
if (!subMapOrError) {
3868+
// If the underlying type references internal details, ignore it.
3869+
auto unconsumedError =
3870+
consumeErrorIfXRefNonLoadedModule(subMapOrError.takeError());
3871+
if (unconsumedError)
3872+
return std::move(unconsumedError);
3873+
} else {
3874+
// Check whether there are any conditionally available substitutions.
3875+
// If there are, it means that "unique" we just read is a universally
3876+
// available substitution.
3877+
SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
3878+
limitedAvailability;
38613879

3862-
deserializeConditionalSubstitutions(limitedAvailability);
3880+
deserializeConditionalSubstitutions(limitedAvailability);
38633881

3864-
if (limitedAvailability.empty()) {
3865-
opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
3866-
} else {
3867-
limitedAvailability.push_back(
3868-
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
3869-
ctx, {{VersionRange::empty(), /*unavailability=*/false}},
3870-
subMapOrError.get()));
3882+
if (limitedAvailability.empty()) {
3883+
opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
3884+
} else {
3885+
limitedAvailability.push_back(
3886+
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
3887+
ctx, {{VersionRange::empty(), /*unavailability=*/false}},
3888+
subMapOrError.get()));
38713889

3872-
opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
3890+
opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
3891+
}
38733892
}
38743893
}
38753894
return opaqueDecl;
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Test deserialization when the underlying type of an opaque type
2+
// depends on an implementation-only import.
3+
4+
// RUN: %empty-directory(%t)
5+
// RUN: split-file %s %t
6+
7+
// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
8+
// RUN: -emit-module-path=%t/BaseLib.swiftmodule %t/BaseLib.swift
9+
// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
10+
// RUN: -emit-module-path=%t/HiddenLib.swiftmodule %t/HiddenLib.swift -I %t
11+
// RUN: %target-swift-frontend -emit-module -enable-library-evolution \
12+
// RUN: -emit-module-path=%t/Lib.swiftmodule %t/Lib.swift -I %t
13+
14+
// RUN: %target-swift-frontend -I %t -emit-ir %t/Client.swift
15+
16+
//--- BaseLib.swift
17+
18+
public protocol Proto { }
19+
20+
//--- HiddenLib.swift
21+
22+
import BaseLib
23+
24+
public struct HiddenType : Proto {
25+
public init() {}
26+
}
27+
28+
//--- Lib.swift
29+
30+
import BaseLib
31+
@_implementationOnly import HiddenLib
32+
33+
public struct PublicStruct {
34+
public init() {}
35+
public func foo() -> some Proto {
36+
return HiddenType()
37+
}
38+
}
39+
40+
//--- Client.swift
41+
42+
import Lib
43+
44+
var s = PublicStruct()
45+
let r = s.foo()

0 commit comments

Comments
 (0)