Skip to content

Commit ed64fad

Browse files
committed
[SIL]/[SILOpt] Thunks and specializations shouldn't be connected to their parent class.
The "subclass scope" is meant to represent a connection to a vtable (and how public something needs to be), for things that end up in class vtables. Specializations and thunks are mostly internal implementation details and do not end up there, so subclass scope is not applicable to them. This stops the thunks and specializations being incorrectly public. (Note, there are some thunks that _are_ public facing: if a function has its signature optimized, the original entry point becomes a thunk, and this entry point is what ends up in vtables etc., so needs to remain around, which means keeping the same hacks for `private` members of an `open` class.) Fixes rdar://problem/40738913.
1 parent 413501b commit ed64fad

File tree

9 files changed

+125
-11
lines changed

9 files changed

+125
-11
lines changed

include/swift/SIL/SILFunction.h

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,33 @@ class SILFunction
229229
/// serialization.
230230
bool WasDeserializedCanonical = false;
231231

232+
static void
233+
validateSubclassScope(SubclassScope scope, IsThunk_t isThunk,
234+
const GenericSpecializationInformation *genericInfo) {
235+
#ifndef NDEBUG
236+
// The _original_ function for a method can turn into a thunk through
237+
// signature optimization, meaning it needs to retain its subclassScope, but
238+
// other thunks and specializations are implementation details and so
239+
// shouldn't be connected to their parent class.
240+
bool thunkCanHaveSubclassScope;
241+
switch (isThunk) {
242+
case IsNotThunk:
243+
case IsSignatureOptimizedThunk:
244+
thunkCanHaveSubclassScope = true;
245+
break;
246+
case IsThunk:
247+
case IsReabstractionThunk:
248+
thunkCanHaveSubclassScope = false;
249+
break;
250+
}
251+
auto allowsInterestingScopes = thunkCanHaveSubclassScope && !genericInfo;
252+
assert(
253+
allowsInterestingScopes ||
254+
scope == SubclassScope::NotApplicable &&
255+
"SubclassScope on specialization or non-signature-optimized thunk");
256+
#endif
257+
}
258+
232259
SILFunction(SILModule &module, SILLinkage linkage, StringRef mangledName,
233260
CanSILFunctionType loweredType, GenericEnvironment *genericEnv,
234261
Optional<SILLocation> loc, IsBare_t isBareSILFunction,
@@ -578,13 +605,20 @@ class SILFunction
578605

579606
/// Get this function's thunk attribute.
580607
IsThunk_t isThunk() const { return IsThunk_t(Thunk); }
581-
void setThunk(IsThunk_t isThunk) { Thunk = isThunk; }
608+
void setThunk(IsThunk_t isThunk) {
609+
validateSubclassScope(getClassSubclassScope(), isThunk, SpecializationInfo);
610+
Thunk = isThunk;
611+
}
582612

583613
/// Get the class visibility (relevant for class methods).
584614
SubclassScope getClassSubclassScope() const {
585615
return SubclassScope(ClassSubclassScope);
586616
}
587-
617+
void setClassSubclassScope(SubclassScope scope) {
618+
validateSubclassScope(scope, isThunk(), SpecializationInfo);
619+
ClassSubclassScope = static_cast<unsigned>(scope);
620+
}
621+
588622
/// Get this function's noinline attribute.
589623
Inline_t getInlineStrategy() const { return Inline_t(InlineStrategy); }
590624
void setInlineStrategy(Inline_t inStr) { InlineStrategy = inStr; }
@@ -660,6 +694,7 @@ class SILFunction
660694

661695
void setSpecializationInfo(const GenericSpecializationInformation *Info) {
662696
assert(!isSpecialization());
697+
validateSubclassScope(getClassSubclassScope(), isThunk(), Info);
663698
SpecializationInfo = Info;
664699
}
665700

lib/ParseSIL/ParseSIL.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,6 +912,8 @@ static bool parseDeclSILOptional(bool *isTransparent,
912912
*isCanonical = true;
913913
else if (isThunk && SP.P.Tok.getText() == "thunk")
914914
*isThunk = IsThunk;
915+
else if (isThunk && SP.P.Tok.getText() == "signature_optimized_thunk")
916+
*isThunk = IsSignatureOptimizedThunk;
915917
else if (isThunk && SP.P.Tok.getText() == "reabstraction_thunk")
916918
*isThunk = IsReabstractionThunk;
917919
else if (isGlobalInit && SP.P.Tok.getText() == "global_init")

lib/SIL/SILDeclRef.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,10 @@ SubclassScope SILDeclRef::getSubclassScope() const {
771771
if (context->isExtensionContext())
772772
return SubclassScope::NotApplicable;
773773

774+
// Various forms of thunks don't either.
775+
if (isThunk() || isForeign)
776+
return SubclassScope::NotApplicable;
777+
774778
auto *classType = context->getAsClassOrClassExtensionContext();
775779
if (!classType || classType->isFinal())
776780
return SubclassScope::NotApplicable;

lib/SIL/SILFunction.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ SILFunction::SILFunction(SILModule &Module, SILLinkage Linkage, StringRef Name,
102102
HasCReferences(false), IsWeakLinked(false),
103103
OptMode(OptimizationMode::NotSet), EffectsKindAttr(E),
104104
EntryCount(entryCount) {
105+
validateSubclassScope(classSubclassScope, isThunk, nullptr);
106+
105107
if (InsertBefore)
106108
Module.functions.insert(SILModule::iterator(InsertBefore), this);
107109
else

lib/SILOptimizer/FunctionSignatureTransforms/FunctionSignatureOpts.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,10 @@ void FunctionSignatureTransform::createFunctionSignatureOptimizedFunction() {
506506
NewF->setUnqualifiedOwnership();
507507
}
508508

509+
if (F->isSpecialization()) {
510+
NewF->setSpecializationInfo(F->getSpecializationInfo());
511+
}
512+
509513
// Then we transfer the body of F to NewF.
510514
NewF->spliceBody(F);
511515

lib/SILOptimizer/IPO/ClosureSpecializer.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,11 +635,15 @@ ClosureSpecCloner::initCloned(const CallSiteDescriptor &CallSiteDesc,
635635
// and not the original linkage.
636636
// Otherwise the new function could have an external linkage (in case the
637637
// original function was de-serialized) and would not be code-gen'd.
638+
// It's also important to disconnect this specialized function from any
639+
// classes (the classSubclassScope), because that may incorrectly
640+
// influence the linkage.
638641
getSpecializedLinkage(ClosureUser, ClosureUser->getLinkage()), ClonedName,
639642
ClonedTy, ClosureUser->getGenericEnvironment(),
640643
ClosureUser->getLocation(), IsBare, ClosureUser->isTransparent(),
641644
CallSiteDesc.isSerialized(), ClosureUser->getEntryCount(),
642-
ClosureUser->isThunk(), ClosureUser->getClassSubclassScope(),
645+
ClosureUser->isThunk(),
646+
/*classSubclassScope=*/SubclassScope::NotApplicable,
643647
ClosureUser->getInlineStrategy(), ClosureUser->getEffectsKind(),
644648
ClosureUser, ClosureUser->getDebugScope());
645649
if (!ClosureUser->hasQualifiedOwnership()) {

lib/SILOptimizer/Utils/Generics.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1840,6 +1840,7 @@ SILFunction *GenericFuncSpecializer::tryCreateSpecialization() {
18401840
auto *Caller = ReInfo.getApply() ? ReInfo.getApply().getFunction() : nullptr;
18411841
SubstitutionMap Subs = Caller ? ReInfo.getApply().getSubstitutionMap()
18421842
: ReInfo.getClonerParamSubstitutionMap();
1843+
SpecializedF->setClassSubclassScope(SubclassScope::NotApplicable);
18431844
SpecializedF->setSpecializationInfo(
18441845
GenericSpecializationInformation::create(Caller, GenericFunc, Subs));
18451846
return SpecializedF;

test/TBD/class_objc.swift.gyb

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %gyb %s > %t/main.swift
33

4-
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir=all %t/main.swift -disable-objc-attr-requires-foundation-module
4+
// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir=missing %t/main.swift -disable-objc-attr-requires-foundation-module
55

6-
// RUN: %target-swift-frontend -enable-resilience -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir=all %t/main.swift -disable-objc-attr-requires-foundation-module
6+
// RUN: %target-swift-frontend -enable-resilience -emit-ir -o- -parse-as-library -module-name test -import-objc-header %S/Inputs/objc_class_header.h -validate-tbd-against-ir=missing %t/main.swift -disable-objc-attr-requires-foundation-module
77

88
// With -enable-testing:
99

@@ -15,29 +15,56 @@
1515

1616
import Foundation
1717

18-
% names = ["Public", "Internal", "Private"]
18+
19+
%{
20+
names = ["Open", "Public", "Internal", "Private"]
21+
22+
METHODS = "{access} {override} func method{name}() {{}}"
23+
OBJC_METHOD = "@objc {access} {override} func method{name}ObjC() {{}}"
24+
def classBody(i, override=False):
25+
methods = []
26+
for name in names[i:]:
27+
is_private = name == "Private"
28+
args = {"access": name.lower(), "name": name,
29+
"override": "override" if override and not is_private else ""}
30+
methods.append(METHODS.format(**args))
31+
if not is_private:
32+
methods.append(OBJC_METHOD.format(**args))
33+
34+
return "\n".join(methods)
35+
}%
1936

2037
% for i, name in enumerate(names):
2138
% access = name.lower()
2239

2340
// a class by itself
24-
${access} class ${name}Empty: NSObject {}
41+
${access} class ${name}Empty: NSObject {
42+
${classBody(i)}
43+
}
2544

2645
// subclasses of that
2746
% for subname in names[i:]:
2847
% subaccess = subname.lower()
29-
${subaccess} class ${subname}Sub${name}Empty: ${name}Empty {}
48+
${subaccess} class ${subname}Sub${name}Empty: ${name}Empty {
49+
${classBody(i, override=True)}
50+
}
3051
%end
3152

32-
${access} class ${name}InheritObjCProtocol: NSObject, ObjCProtocol {}
53+
${access} class ${name}InheritObjCProtocol: NSObject, ObjCProtocol {
54+
${classBody(i)}
55+
}
3356

3457
// some bugs were revealed when there's @objc without inheriting from
3558
// NSObject.
36-
@objc ${access} class ${name}ObjCOnly {}
59+
@objc ${access} class ${name}ObjCOnly {
60+
${classBody(i)}
61+
}
3762

3863
%end
3964

4065
@usableFromInline
4166
@objc
42-
internal class InternalObjCOnlyUsableFromInline {}
67+
internal class InternalObjCOnlyUsableFromInline {
68+
${classBody(names.index("Internal"))}
69+
}
4370

test/TBD/specialization.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Validate the the specializations actually exist (if they don't then we're not
2+
// validating that they end up with the correct linkages):
3+
// RUN: %target-swift-frontend -emit-sil -o- -O -validate-tbd-against-ir=none %s | %FileCheck %s
4+
5+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all %s
6+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all -enable-resilience %s
7+
8+
// With -enable-testing:
9+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all -enable-testing %s
10+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all -enable-resilience -enable-testing %s
11+
12+
// rdar://problem/40738913
13+
14+
open class Foo {
15+
@inline(never)
16+
fileprivate func foo<T>(_: T.Type) {}
17+
}
18+
19+
open class Bar<T> {
20+
@inline(never)
21+
fileprivate func bar() {}
22+
}
23+
24+
25+
public func f() {
26+
Foo().foo(Int.self)
27+
Bar<Int>().bar()
28+
}
29+
30+
31+
// CHECK-LABEL: // specialized Foo.foo<A>(_:)
32+
// CHECK-NEXT: sil private [noinline] @$S14specialization3FooC3foo33_A6E3E43DB6679655BDF5A878ABC489A0LLyyxmlFSi_Tg5Tf4dd_n : $@convention(thin) () -> ()
33+
34+
// CHECK-LABEL: // specialized Bar.bar()
35+
// CHECK-NEXT: sil private [noinline] @$S14specialization3BarC3bar33_A6E3E43DB6679655BDF5A878ABC489A0LLyyFSi_Tg5Tf4d_n : $@convention(thin) () -> ()

0 commit comments

Comments
 (0)