Skip to content

Commit 6c6cd41

Browse files
committed
[Serialization] Public overrides of internal decls are ignorable by clients
This change reflects the behavior of `DeclAttribute.printImpl` that prints the `override` keyword in a swiftinterface only when the overriden decl is also public. This issue was detected when working on deserialization safety by public overrides of private functions in the following tests: test/Interpreter/vtables_multifile.swift test/Interpreter/vtables_multifile_testable.swift test/SILGen/accessibility_vtables_testable.swift test/SILGen/accessibility_vtables_usableFromInline.swift test/SILGen/vtables_multifile.swift
1 parent 559ae7f commit 6c6cd41

File tree

2 files changed

+83
-4
lines changed

2 files changed

+83
-4
lines changed

lib/Serialization/Serialization.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3345,15 +3345,27 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
33453345
///
33463346
/// This should be kept conservative. Compiler crashes are still better than
33473347
/// miscompiles.
3348-
static bool overriddenDeclAffectsABI(const ValueDecl *overridden) {
3348+
static bool overriddenDeclAffectsABI(const ValueDecl *override,
3349+
const ValueDecl *overridden) {
33493350
if (!overridden)
33503351
return false;
3351-
// There's one case where we know a declaration doesn't affect the ABI of
3352+
// There's a few cases where we know a declaration doesn't affect the ABI of
33523353
// its overrides after they've been compiled: if the declaration is '@objc'
33533354
// and 'dynamic'. In that case, all accesses to the method or property will
33543355
// go through the Objective-C method tables anyway.
33553356
if (overridden->hasClangNode() || overridden->shouldUseObjCDispatch())
33563357
return false;
3358+
3359+
// In a public-override-internal case, the override doesn't have ABI
3360+
// implications.
3361+
auto isPublic = [](const ValueDecl *VD) {
3362+
return VD->getFormalAccessScope(VD->getDeclContext(),
3363+
/*treatUsableFromInlineAsPublic*/true)
3364+
.isPublic();
3365+
};
3366+
if (isPublic(override) && !isPublic(overridden))
3367+
return false;
3368+
33573369
return true;
33583370
}
33593371

@@ -4044,7 +4056,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
40444056
fn->isImplicitlyUnwrappedOptional(),
40454057
S.addDeclRef(fn->getOperatorDecl()),
40464058
S.addDeclRef(fn->getOverriddenDecl()),
4047-
overriddenDeclAffectsABI(fn->getOverriddenDecl()),
4059+
overriddenDeclAffectsABI(fn, fn->getOverriddenDecl()),
40484060
fn->getName().getArgumentNames().size() +
40494061
fn->getName().isCompoundName(),
40504062
rawAccessLevel,
@@ -4136,7 +4148,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
41364148
uint8_t(getStableAccessorKind(fn->getAccessorKind()));
41374149

41384150
bool overriddenAffectsABI =
4139-
overriddenDeclAffectsABI(fn->getOverriddenDecl());
4151+
overriddenDeclAffectsABI(fn, fn->getOverriddenDecl());
41404152

41414153
Type ty = fn->getInterfaceType();
41424154
SmallVector<IdentifierID, 4> dependencies;
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/// Deserialization can ignore public overrides to internal methods.
2+
3+
// RUN: %empty-directory(%t)
4+
// RUN: split-file %s %t
5+
6+
/// Build the library.
7+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
8+
// RUN: -enable-library-evolution -swift-version 5 \
9+
// RUN: -emit-module-path %t/Lib.swiftmodule \
10+
// RUN: -emit-module-interface-path %t/Lib.swiftinterface
11+
12+
/// Build against the swiftmodule.
13+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
14+
// RUN: -enable-deserialization-safety
15+
16+
/// Build against the swiftinterface.
17+
// RUN: rm %t/Lib.swiftmodule
18+
// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t \
19+
// RUN: -enable-deserialization-safety
20+
21+
//--- Lib.swift
22+
23+
open class Base {
24+
public func publicMethod() -> Int {
25+
return 1
26+
}
27+
28+
fileprivate func fileprivateMethod() -> Int {
29+
return 1
30+
}
31+
32+
internal func internalMethod() -> Int {
33+
return 1
34+
}
35+
}
36+
37+
open class Derived : Base {
38+
open override func publicMethod() -> Int {
39+
return super.publicMethod() + 1
40+
}
41+
42+
open override func fileprivateMethod() -> Int {
43+
return super.fileprivateMethod() + 1
44+
}
45+
46+
open override func internalMethod() -> Int {
47+
return super.internalMethod() + 1
48+
}
49+
}
50+
51+
//--- Client.swift
52+
53+
import Lib
54+
55+
public class OtherFinalDerived : Derived {
56+
public override func publicMethod() -> Int {
57+
return super.publicMethod() + 1
58+
}
59+
60+
public override func fileprivateMethod() -> Int {
61+
return super.fileprivateMethod() + 1
62+
}
63+
64+
public override func internalMethod() -> Int {
65+
return super.internalMethod() + 1
66+
}
67+
}

0 commit comments

Comments
 (0)