Skip to content

Commit 263d154

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 2adb84d commit 263d154

File tree

2 files changed

+85
-20
lines changed

2 files changed

+85
-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.
@@ -3601,7 +3608,10 @@ class DeclDeserializer {
36013608
if (declOrOffset.isComplete())
36023609
return declOrOffset;
36033610

3604-
const auto resultType = MF.getType(resultInterfaceTypeID);
3611+
auto resultTypeOrError = MF.getTypeChecked(resultInterfaceTypeID);
3612+
if (!resultTypeOrError)
3613+
return resultTypeOrError.takeError();
3614+
const auto resultType = resultTypeOrError.get();
36053615
if (declOrOffset.isComplete())
36063616
return declOrOffset;
36073617

@@ -3674,9 +3684,13 @@ class DeclDeserializer {
36743684
std::move(needsNewVTableEntry));
36753685

36763686
if (opaqueReturnTypeID) {
3687+
auto declOrError = MF.getDeclChecked(opaqueReturnTypeID);
3688+
if (!declOrError)
3689+
return declOrError.takeError();
3690+
36773691
ctx.evaluator.cacheOutput(
36783692
OpaqueResultTypeRequest{fn},
3679-
cast<OpaqueTypeDecl>(MF.getDecl(opaqueReturnTypeID)));
3693+
cast<OpaqueTypeDecl>(declOrError.get()));
36803694
}
36813695

36823696
if (!isAccessor)
@@ -3817,26 +3831,31 @@ class DeclDeserializer {
38173831
opaqueDecl->setGenericSignature(GenericSignature());
38183832
if (underlyingTypeSubsID) {
38193833
auto subMapOrError = MF.getSubstitutionMapChecked(underlyingTypeSubsID);
3820-
if (!subMapOrError)
3821-
return subMapOrError.takeError();
3822-
3823-
// Check whether there are any conditionally available substitutions.
3824-
// If there are, it means that "unique" we just read is a universally
3825-
// available substitution.
3826-
SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
3827-
limitedAvailability;
3834+
if (!subMapOrError) {
3835+
// If the underlying type references internal details, ignore it.
3836+
auto unconsumedError =
3837+
consumeErrorIfXRefNonLoadedModule(subMapOrError.takeError());
3838+
if (unconsumedError)
3839+
return unconsumedError;
3840+
} else {
3841+
// Check whether there are any conditionally available substitutions.
3842+
// If there are, it means that "unique" we just read is a universally
3843+
// available substitution.
3844+
SmallVector<OpaqueTypeDecl::ConditionallyAvailableSubstitutions *>
3845+
limitedAvailability;
38283846

3829-
deserializeConditionalSubstitutions(limitedAvailability);
3847+
deserializeConditionalSubstitutions(limitedAvailability);
38303848

3831-
if (limitedAvailability.empty()) {
3832-
opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
3833-
} else {
3834-
limitedAvailability.push_back(
3835-
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
3836-
ctx, {{VersionRange::empty(), /*unavailability=*/false}},
3837-
subMapOrError.get()));
3849+
if (limitedAvailability.empty()) {
3850+
opaqueDecl->setUniqueUnderlyingTypeSubstitutions(subMapOrError.get());
3851+
} else {
3852+
limitedAvailability.push_back(
3853+
OpaqueTypeDecl::ConditionallyAvailableSubstitutions::get(
3854+
ctx, {{VersionRange::empty(), /*unavailability=*/false}},
3855+
subMapOrError.get()));
38383856

3839-
opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
3857+
opaqueDecl->setConditionallyAvailableSubstitutions(limitedAvailability);
3858+
}
38403859
}
38413860
}
38423861
return opaqueDecl;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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+
// RUN: -disable-deserialization-recovery
16+
17+
//--- BaseLib.swift
18+
19+
public protocol Proto { }
20+
21+
//--- HiddenLib.swift
22+
23+
import BaseLib
24+
25+
public struct HiddenType : Proto {
26+
public init() {}
27+
}
28+
29+
//--- Lib.swift
30+
31+
import BaseLib
32+
@_implementationOnly import HiddenLib
33+
34+
public struct PublicStruct {
35+
public init() {}
36+
public func foo() -> some Proto {
37+
return HiddenType()
38+
}
39+
}
40+
41+
//--- Client.swift
42+
43+
import Lib
44+
45+
var s = PublicStruct()
46+
let r = s.foo()

0 commit comments

Comments
 (0)