Skip to content

Commit 4e80111

Browse files
xymusxymus
committed
[Serialization] Main deserialization safety logic on the writer side
When writing decls to a swiftmodule files, the serialization logic evaluates whether the decl will be safe to deserialize. This is inferred from the access level of the decl, whether it's local, if the module is built for testing, etc. If the decl in unsafe to deserialize, a record will be written down before the decl itself in the swiftmodule file. On the reader side, attempting to deserialize a decl marked as unsafe raises a deserialization error early. This error is handled by the existing deserialization recovery logic. In theory, we want to consider as safe only decls that are actually needed by the client. Marking as many internal details as possible as unsafe will prevent more errors. Getting the right scope may require more work in the future.
1 parent ce91d77 commit 4e80111

File tree

3 files changed

+399
-0
lines changed

3 files changed

+399
-0
lines changed

‎lib/Serialization/Serialization.cpp

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@
7070

7171
#include <vector>
7272

73+
#define DEBUG_TYPE "Serialization"
74+
7375
using namespace swift;
7476
using namespace swift::serialization;
7577
using namespace llvm::support;
@@ -3088,6 +3090,171 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
30883090
}
30893091
}
30903092

3093+
/// Determine if \p decl is safe to deserialize when it's public
3094+
/// or otherwise needed by the client in normal builds, this should usually
3095+
/// correspond to logic in type-checking ensuring these safe decls don't
3096+
/// refer to implementation details. We have to be careful not to mark
3097+
/// anything needed by a client as unsafe as the client will reject reading
3098+
/// it, but at the same time keep the safety checks precise to avoid
3099+
/// XRef errors and such.
3100+
///
3101+
/// \p decl should be either an \c ExtensionDecl or a \c ValueDecl.
3102+
static bool declIsDeserializationSafe(const Decl *decl) {
3103+
if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
3104+
// Consider extensions as safe as their extended type.
3105+
auto nominalType = ext->getExtendedNominal();
3106+
if (!nominalType ||
3107+
!declIsDeserializationSafe(nominalType))
3108+
return false;
3109+
3110+
// We can mark the extension unsafe only if it has no public members.
3111+
auto members = ext->getMembers();
3112+
int membersCount = 0;
3113+
auto hasSafeMembers = std::any_of(members.begin(), members.end(),
3114+
[&membersCount](const Decl *D) -> bool {
3115+
membersCount ++;
3116+
if (auto VD = dyn_cast<ValueDecl>(D))
3117+
return declIsDeserializationSafe(VD);
3118+
return true;
3119+
});
3120+
if (hasSafeMembers)
3121+
return true;
3122+
3123+
// We can mark the extension unsafe only if it has no public
3124+
// conformances.
3125+
auto protocols = ext->getLocalProtocols(
3126+
ConformanceLookupKind::OnlyExplicit);
3127+
bool hasSafeConformances = std::any_of(protocols.begin(),
3128+
protocols.end(),
3129+
declIsDeserializationSafe);
3130+
if (hasSafeConformances)
3131+
return true;
3132+
3133+
// Truly empty extensions are safe, it may happen in swiftinterfaces.
3134+
if (membersCount == 0 && protocols.size() == 0)
3135+
return true;
3136+
3137+
return false;
3138+
}
3139+
3140+
auto value = cast<ValueDecl>(decl);
3141+
3142+
// A decl is safe if formally accessible publicly.
3143+
auto accessScope = value->getFormalAccessScope(/*useDC=*/nullptr,
3144+
/*treatUsableFromInlineAsPublic=*/true);
3145+
if (accessScope.isPublic())
3146+
return true;
3147+
3148+
// Testable allows access to internal details.
3149+
if (value->getDeclContext()->getParentModule()->isTestingEnabled() &&
3150+
accessScope.isInternal())
3151+
return true;
3152+
3153+
if (auto accessor = dyn_cast<AccessorDecl>(value))
3154+
// Accessors are as safe as their storage.
3155+
if (declIsDeserializationSafe(accessor->getStorage()))
3156+
return true;
3157+
3158+
// Frozen fields are always safe.
3159+
if (auto var = dyn_cast<VarDecl>(value)) {
3160+
if (var->isLayoutExposedToClients())
3161+
return true;
3162+
3163+
// Consider all lazy var storage as "safe".
3164+
// FIXME: We should keep track of what lazy var is associated to the
3165+
// storage for them to preserve the same safeness.
3166+
if (var->isLazyStorageProperty())
3167+
return true;
3168+
3169+
// Property wrappers storage is as safe as the wrapped property.
3170+
if (VarDecl *wrapped = var->getOriginalWrappedProperty())
3171+
if (declIsDeserializationSafe(wrapped))
3172+
return true;
3173+
}
3174+
3175+
return false;
3176+
}
3177+
3178+
/// Write a \c DeserializationSafetyLayout record only when \p decl is unsafe
3179+
/// to deserialize.
3180+
///
3181+
/// \sa declIsDeserializationSafe
3182+
void writeDeserializationSafety(const Decl *decl) {
3183+
using namespace decls_block;
3184+
3185+
auto DC = decl->getDeclContext();
3186+
if (!DC->getParentModule()->isResilient())
3187+
return;
3188+
3189+
// Everything should be safe in a swiftinterface. So, don't emit any safety
3190+
// record when building a swiftinterface in release builds. Debug builds
3191+
// instead print inconsistencies.
3192+
auto parentSF = DC->getParentSourceFile();
3193+
bool fromModuleInterface = parentSF &&
3194+
parentSF->Kind == SourceFileKind::Interface;
3195+
#if NDEBUG
3196+
if (fromModuleInterface)
3197+
return;
3198+
#endif
3199+
3200+
// Private imports allow safe access to everything.
3201+
if (DC->getParentModule()->arePrivateImportsEnabled())
3202+
return;
3203+
3204+
// Ignore things with no access level.
3205+
// Note: There's likely room to report some of these as unsafe to prevent
3206+
// failures.
3207+
if (isa<GenericTypeParamDecl>(decl) ||
3208+
isa<OpaqueTypeDecl>(decl) ||
3209+
isa<ParamDecl>(decl) ||
3210+
isa<EnumCaseDecl>(decl) ||
3211+
isa<EnumElementDecl>(decl))
3212+
return;
3213+
3214+
if (!isa<ValueDecl>(decl) && !isa<ExtensionDecl>(decl))
3215+
return;
3216+
3217+
// Don't look at decls inside functions and
3218+
// check the ValueDecls themselves.
3219+
auto declIsSafe = DC->isLocalContext() ||
3220+
declIsDeserializationSafe(decl);
3221+
#ifdef NDEBUG
3222+
// In release builds, bail right away if the decl is safe.
3223+
// In debug builds, wait to bail after the debug prints and asserts.
3224+
if (declIsSafe)
3225+
return;
3226+
#endif
3227+
3228+
// Write a human readable name to an identifier.
3229+
SmallString<64> out;
3230+
llvm::raw_svector_ostream outStream(out);
3231+
if (auto val = dyn_cast<ValueDecl>(decl)) {
3232+
outStream << val->getName();
3233+
} else if (auto ext = dyn_cast<ExtensionDecl>(decl)) {
3234+
outStream << "extension ";
3235+
if (auto nominalType = ext->getExtendedNominal())
3236+
outStream << nominalType->getName();
3237+
}
3238+
auto name = S.getASTContext().getIdentifier(out);
3239+
3240+
LLVM_DEBUG(
3241+
llvm::dbgs() << "Serialization safety, "
3242+
<< (declIsSafe? "safe" : "unsafe")
3243+
<< ": '" << name << "'\n";
3244+
assert((declIsSafe || !fromModuleInterface) &&
3245+
"All swiftinterface decls should be deserialization safe");
3246+
);
3247+
3248+
#ifndef NDEBUG
3249+
if (declIsSafe)
3250+
return;
3251+
#endif
3252+
3253+
auto abbrCode = S.DeclTypeAbbrCodes[DeserializationSafetyLayout::Code];
3254+
DeserializationSafetyLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
3255+
S.addDeclBaseNameRef(name));
3256+
}
3257+
30913258
void writeForeignErrorConvention(const ForeignErrorConvention &fec) {
30923259
using namespace decls_block;
30933260

@@ -3405,6 +3572,8 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
34053572
if (D->isInvalid())
34063573
writeDeclErrorFlag();
34073574

3575+
writeDeserializationSafety(D);
3576+
34083577
// Emit attributes (if any).
34093578
for (auto Attr : D->getAttrs())
34103579
writeDeclAttribute(D, Attr);

‎test/Serialization/Safety/unsafe-decls.swift

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// RUN: %empty-directory(%t)
2+
3+
// REQUIRES: asserts
4+
5+
// RUN: %target-swift-frontend -emit-module %s \
6+
// RUN: -enable-library-evolution -swift-version 5 \
7+
// RUN: -enable-deserialization-safety \
8+
// RUN: -Xllvm -debug-only=Serialization 2>&1 \
9+
// RUN: | %FileCheck --check-prefixes=SAFETY-PRIVATE,SAFETY-INTERNAL %s
10+
11+
// RUN: %target-swift-frontend -emit-module %s \
12+
// RUN: -enable-library-evolution -swift-version 5 \
13+
// RUN: -enable-deserialization-safety \
14+
// RUN: -Xllvm -debug-only=Serialization \
15+
// RUN: -enable-testing 2>&1 \
16+
// RUN: | %FileCheck --check-prefixes=SAFETY-PRIVATE,NO-SAFETY-INTERNAL-NOT %s
17+
18+
/// Don't mark decls as unsafe when private import is enabled.
19+
// RUN: %target-swift-frontend -emit-module %s \
20+
// RUN: -enable-library-evolution -swift-version 5 \
21+
// RUN: -enable-deserialization-safety \
22+
// RUN: -Xllvm -debug-only=Serialization \
23+
// RUN: -enable-private-imports 2>&1 \
24+
// RUN: | %FileCheck --check-prefixes=DISABLED %s
25+
26+
/// Don't mark decls as unsafe without library evolution.
27+
// RUN: %target-swift-frontend -emit-module %s \
28+
// RUN: -enable-deserialization-safety -swift-version 5 \
29+
// RUN: -Xllvm -debug-only=Serialization 2>&1 \
30+
// RUN: | %FileCheck --check-prefixes=DISABLED %s
31+
32+
// DISABLED-NOT: Serialization safety
33+
34+
public protocol PublicProto {}
35+
// SAFETY-PRIVATE: Serialization safety, safe: 'PublicProto'
36+
internal protocol InternalProto {}
37+
// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalProto'
38+
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalProto'
39+
private protocol PrivateProto {}
40+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'PrivateProto'
41+
fileprivate protocol FileprivateProto {}
42+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'FileprivateProto'
43+
44+
internal struct InternalStruct : PublicProto {
45+
// SAFETY-INTERNAL: Serialization safety, unsafe: 'InternalStruct'
46+
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'InternalStruct'
47+
}
48+
49+
public struct PublicStruct {
50+
// SAFETY-PRIVATE: Serialization safety, safe: 'PublicStruct'
51+
52+
public typealias publicTypealias = Int
53+
// SAFETY-PRIVATE: Serialization safety, safe: 'publicTypealias'
54+
typealias internalTypealias = Int
55+
// SAFETY-INTERNAL: Serialization safety, unsafe: 'internalTypealias'
56+
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'internalTypealias'
57+
// SAFETY-PRIVATE: Serialization safety, safe: 'localTypealias'
58+
59+
public init(publicInit a: Int) {}
60+
// SAFETY-PRIVATE: Serialization safety, safe: 'init(publicInit:)'
61+
internal init(internalInit a: Int) {}
62+
// SAFETY-INTERNAL: Serialization safety, unsafe: 'init(internalInit:)'
63+
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'init(internalInit:)'
64+
private init(privateInit a: Int) {}
65+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'init(privateInit:)'
66+
fileprivate init(fileprivateInit a: Int) {}
67+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'init(fileprivateInit:)'
68+
69+
@inlinable public func inlinableFunc() {
70+
typealias localTypealias = Int
71+
}
72+
// SAFETY-PRIVATE: Serialization safety, safe: 'inlinableFunc()'
73+
public func publicFunc() {}
74+
// SAFETY-PRIVATE: Serialization safety, safe: 'publicFunc()'
75+
public func opaqueTypeFunc() -> some PublicProto {
76+
return InternalStruct()
77+
}
78+
// SAFETY-PRIVATE: Serialization safety, safe: 'opaqueTypeFunc()'
79+
internal func internalFunc() {}
80+
// SAFETY-INTERNAL: Serialization safety, unsafe: 'internalFunc()'
81+
// NO-SAFETY-INTERNAL: Serialization safety, safe: 'internalFunc()'
82+
private func privateFunc() {}
83+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'privateFunc()'
84+
fileprivate func fileprivateFunc() {}
85+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'fileprivateFunc()'
86+
87+
public var publicProperty = "Some text"
88+
// SAFETY-PRIVATE: Serialization safety, safe: 'publicProperty'
89+
public private(set) var halfPublicProperty = "Some text"
90+
// SAFETY-PRIVATE: Serialization safety, safe: 'halfPublicProperty'
91+
private var privateProperty = "Some text"
92+
// SAFETY-PRIVATE: Serialization safety, unsafe: 'privateProperty'
93+
}

0 commit comments

Comments
 (0)