Skip to content

Commit 67ec3b7

Browse files
slavapestovslavapestov
authored
Merge pull request #20850 from slavapestov/fixed-layout-classes
Fixed layout classes should still have resilient vtables
2 parents 1c703bf + f8d2881 commit 67ec3b7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+765
-289
lines changed

‎include/swift/AST/Decl.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,6 +3589,17 @@ class ClassDecl final : public NominalTypeDecl {
35893589
return SourceRange(ClassLoc, getBraces().End);
35903590
}
35913591

3592+
/// Determine whether the member area of this class's metadata (which consists
3593+
/// of field offsets and vtable entries) is to be considered opaque by clients.
3594+
///
3595+
/// Note that even @_fixed_layout classes have resilient metadata if they are
3596+
/// in a resilient module.
3597+
bool hasResilientMetadata() const;
3598+
3599+
/// Determine whether this class has resilient metadata when accessed from the
3600+
/// given module and resilience expansion.
3601+
bool hasResilientMetadata(ModuleDecl *M, ResilienceExpansion expansion) const;
3602+
35923603
/// Determine whether this class has a superclass.
35933604
bool hasSuperclass() const { return (bool)getSuperclassDecl(); }
35943605

‎lib/AST/Decl.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1819,7 +1819,7 @@ bool AbstractStorageDecl::isResilient(ModuleDecl *M,
18191819
case ResilienceExpansion::Minimal:
18201820
return isResilient();
18211821
case ResilienceExpansion::Maximal:
1822-
return isResilient() && M != getModuleContext();
1822+
return M != getModuleContext() && isResilient();
18231823
}
18241824
llvm_unreachable("bad resilience expansion");
18251825
}
@@ -2915,7 +2915,7 @@ bool NominalTypeDecl::isResilient(ModuleDecl *M,
29152915
case ResilienceExpansion::Minimal:
29162916
return isResilient();
29172917
case ResilienceExpansion::Maximal:
2918-
return isResilient() && M != getModuleContext();
2918+
return M != getModuleContext() && isResilient();
29192919
}
29202920
llvm_unreachable("bad resilience expansion");
29212921
}
@@ -3355,6 +3355,36 @@ ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
33553355
Bits.ClassDecl.HasMissingVTableEntries = 0;
33563356
}
33573357

3358+
bool ClassDecl::hasResilientMetadata() const {
3359+
// Imported classes don't have a vtable, etc, at all.
3360+
if (hasClangNode())
3361+
return false;
3362+
3363+
// If the module is not resilient, neither is the class metadata.
3364+
if (getParentModule()->getResilienceStrategy()
3365+
!= ResilienceStrategy::Resilient)
3366+
return false;
3367+
3368+
// If the class is not public, we can't use it outside the module at all.
3369+
if (!getFormalAccessScope(/*useDC=*/nullptr,
3370+
/*treatUsableFromInlineAsPublic=*/true).isPublic())
3371+
return false;
3372+
3373+
// Otherwise we access metadata members, such as vtable entries, resiliently.
3374+
return true;
3375+
}
3376+
3377+
bool ClassDecl::hasResilientMetadata(ModuleDecl *M,
3378+
ResilienceExpansion expansion) const {
3379+
switch (expansion) {
3380+
case ResilienceExpansion::Minimal:
3381+
return hasResilientMetadata();
3382+
case ResilienceExpansion::Maximal:
3383+
return M != getModuleContext() && hasResilientMetadata();
3384+
}
3385+
llvm_unreachable("bad resilience expansion");
3386+
}
3387+
33583388
DestructorDecl *ClassDecl::getDestructor() {
33593389
auto results = lookupDirect(DeclBaseName::createDestructor());
33603390
assert(!results.empty() && "Class without destructor?");

‎lib/IRGen/ClassMetadataVisitor.h

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ template <class Impl> class ClassMetadataVisitor
9595
if (superclassDecl->hasClangNode()) {
9696
// Nothing to do; Objective-C classes do not add new members to
9797
// Swift class metadata.
98-
} else if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
98+
} else if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal)) {
9999
// Runtime metadata instantiation will initialize our field offset
100100
// vector and vtable entries.
101101
//
@@ -118,14 +118,11 @@ template <class Impl> class ClassMetadataVisitor
118118
// This must always be the first item in the immediate members.
119119
asImpl().addGenericFields(theClass, theClass);
120120

121-
// If the class is resilient, we cannot make any assumptions about its
122-
// member layout at all, so skip the rest of this method.
121+
// If the class has resilient storage, we cannot make any assumptions about
122+
// its storage layout, so skip the rest of this method.
123123
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal))
124124
return;
125125

126-
// Add vtable entries.
127-
asImpl().addVTableEntries(theClass);
128-
129126
// A class only really *needs* a field-offset vector in the
130127
// metadata if:
131128
// - it's in a generic context and
@@ -145,6 +142,14 @@ template <class Impl> class ClassMetadataVisitor
145142
addFieldEntries(field);
146143
}
147144
asImpl().noteEndOfFieldOffsets(theClass);
145+
146+
// If the class has resilient metadata, we cannot make any assumptions
147+
// about its metadata layout, so skip the rest of this method.
148+
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
149+
return;
150+
151+
// Add vtable entries.
152+
asImpl().addVTableEntries(theClass);
148153
}
149154

150155
private:

‎lib/IRGen/GenCall.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,11 +1510,14 @@ void CallEmission::emitToUnmappedExplosion(Explosion &out) {
15101510

15111511
// For ABI reasons the result type of the call might not actually match the
15121512
// expected result type.
1513+
//
1514+
// This can happen when calling C functions, or class method dispatch thunks
1515+
// for methods that have covariant ABI-compatible overrides.
15131516
auto expectedNativeResultType = nativeSchema.getExpandedType(IGF.IGM);
15141517
if (result->getType() != expectedNativeResultType) {
1515-
// This should only be needed when we call C functions.
1516-
assert(getCallee().getOrigFunctionType()->getLanguage() ==
1517-
SILFunctionLanguage::C);
1518+
auto origFnType = getCallee().getOrigFunctionType();
1519+
assert(origFnType->getLanguage() == SILFunctionLanguage::C ||
1520+
origFnType->getRepresentation() == SILFunctionTypeRepresentation::Method);
15181521
result =
15191522
IGF.coerceValue(result, expectedNativeResultType, IGF.IGM.DataLayout);
15201523
}

‎lib/IRGen/GenClass.cpp

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ namespace {
200200
bool ClassHasGenericLayout = false;
201201

202202
// Is this class or any of its superclasses resilient from the viewpoint
203-
// of the current module? This means that their metadata can change size
204-
// and field offsets, generic arguments and virtual methods must be
203+
// of the current module? This means that their metadata can change size,
204+
// hence field offsets, generic arguments and virtual methods must be
205205
// accessed relative to a metadata base global variable.
206206
bool ClassHasResilientAncestry = false;
207207

@@ -261,14 +261,11 @@ namespace {
261261
return Elements;
262262
}
263263

264-
/// Does the class metadata have a completely known, static layout that
265-
/// does not require initialization at runtime beyond registeration of
266-
/// the class with the Objective-C runtime?
264+
/// Do instances of the class have a completely known, static layout?
267265
bool isFixedSize() const {
268266
return !(ClassHasMissingMembers ||
269267
ClassHasResilientMembers ||
270268
ClassHasGenericLayout ||
271-
ClassHasResilientAncestry ||
272269
ClassHasObjCAncestry);
273270
}
274271

@@ -332,19 +329,21 @@ namespace {
332329
auto superclassDecl = superclassType.getClassOrBoundGenericClass();
333330
assert(superclassType && superclassDecl);
334331

335-
if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
336-
// If the class is resilient, don't walk over its fields; we have to
337-
// calculate the layout at runtime.
332+
if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal))
338333
ClassHasResilientAncestry = true;
339334

340-
// Furthermore, if the superclass is generic, we have to assume
341-
// that its layout depends on its generic parameters. But this only
342-
// propagates down to subclasses whose superclass type depends on the
343-
// subclass's generic context.
335+
// If the superclass has resilient storage, don't walk its fields.
336+
if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
337+
ClassHasResilientMembers = true;
338+
339+
// If the superclass is generic, we have to assume that its layout
340+
// depends on its generic parameters. But this only propagates down to
341+
// subclasses whose superclass type depends on the subclass's generic
342+
// context.
344343
if (superclassType.hasArchetype())
345344
ClassHasGenericLayout = true;
346345
} else {
347-
// Otherwise, we have total knowledge of the class and its
346+
// Otherwise, we are allowed to have total knowledge of the superclass
348347
// fields, so walk them to compute the layout.
349348
addFieldsForClass(superclassDecl, superclassType, /*superclass=*/true);
350349
}
@@ -356,8 +355,11 @@ namespace {
356355
if (classHasIncompleteLayout(IGM, theClass))
357356
ClassHasMissingMembers = true;
358357

359-
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
358+
if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
360359
ClassHasResilientAncestry = true;
360+
361+
if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
362+
ClassHasResilientMembers = true;
361363
return;
362364
}
363365

‎lib/IRGen/GenDecl.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,6 +3859,20 @@ bool IRGenModule::isResilient(NominalTypeDecl *D, ResilienceExpansion expansion)
38593859
return D->isResilient(getSwiftModule(), expansion);
38603860
}
38613861

3862+
/// Do we have to use resilient access patterns when working with this
3863+
/// class?
3864+
///
3865+
/// For classes, this means that virtual method calls use dispatch thunks
3866+
/// rather than accessing metadata members directly.
3867+
bool IRGenModule::hasResilientMetadata(ClassDecl *D,
3868+
ResilienceExpansion expansion) {
3869+
if (expansion == ResilienceExpansion::Maximal &&
3870+
Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
3871+
return false;
3872+
}
3873+
return D->hasResilientMetadata(getSwiftModule(), expansion);
3874+
}
3875+
38623876
// The most general resilience expansion where the given declaration is visible.
38633877
ResilienceExpansion
38643878
IRGenModule::getResilienceExpansionForAccess(NominalTypeDecl *decl) {

‎lib/IRGen/GenMeta.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,7 +1381,7 @@ namespace {
13811381
RequireMetadata_t requireMetadata)
13821382
: super(IGM, Type, requireMetadata),
13831383
VTable(IGM.getSILModule().lookUpVTable(getType())),
1384-
Resilient(IGM.isResilient(Type, ResilienceExpansion::Minimal)) {
1384+
Resilient(IGM.hasResilientMetadata(Type, ResilienceExpansion::Minimal)) {
13851385

13861386
if (getType()->isForeign()) return;
13871387

@@ -1485,7 +1485,7 @@ namespace {
14851485

14861486
// Only emit a method lookup function if the class is resilient
14871487
// and has a non-empty vtable.
1488-
if (IGM.isResilient(getType(), ResilienceExpansion::Minimal))
1488+
if (IGM.hasResilientMetadata(getType(), ResilienceExpansion::Minimal))
14891489
IGM.emitMethodLookupFunction(getType());
14901490

14911491
auto offset = MetadataLayout->hasResilientSuperclass()
@@ -2250,7 +2250,7 @@ static void emitClassMetadataBaseOffset(IRGenModule &IGM,
22502250
// Only classes defined in resilient modules, or those that have
22512251
// a resilient superclass need this.
22522252
if (!layout.hasResilientSuperclass() &&
2253-
!IGM.isResilient(classDecl, ResilienceExpansion::Minimal)) {
2253+
!IGM.hasResilientMetadata(classDecl, ResilienceExpansion::Minimal)) {
22542254
return;
22552255
}
22562256

‎lib/IRGen/IRGenModule.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,7 @@ class IRGenModule {
784784
clang::CodeGen::CodeGenModule &getClangCGM() const;
785785

786786
bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion);
787+
bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion);
787788
ResilienceExpansion getResilienceExpansionForAccess(NominalTypeDecl *decl);
788789
ResilienceExpansion getResilienceExpansionForLayout(NominalTypeDecl *decl);
789790
ResilienceExpansion getResilienceExpansionForLayout(SILGlobalVariable *var);

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5460,12 +5460,12 @@ void IRGenSILFunction::visitSuperMethodInst(swift::SuperMethodInst *i) {
54605460
// its offset since methods can be re-ordered resiliently. Instead, we call
54615461
// the class method lookup function, passing in a reference to the
54625462
// method descriptor.
5463-
if (IGM.isResilient(classDecl, ResilienceExpansion::Maximal)) {
5463+
if (IGM.hasResilientMetadata(classDecl, ResilienceExpansion::Maximal)) {
54645464
// Load the superclass of the static type of the 'self' value.
54655465
llvm::Value *superMetadata;
54665466
auto instanceTy = CanType(baseType.getASTType()->getMetatypeInstanceType());
5467-
if (!IGM.isResilient(instanceTy.getClassOrBoundGenericClass(),
5468-
ResilienceExpansion::Maximal)) {
5467+
if (!IGM.hasResilientMetadata(instanceTy.getClassOrBoundGenericClass(),
5468+
ResilienceExpansion::Maximal)) {
54695469
// It's still possible that the static type of 'self' is not resilient, in
54705470
// which case we can assume its superclass.
54715471
//
@@ -5544,8 +5544,8 @@ void IRGenSILFunction::visitClassMethodInst(swift::ClassMethodInst *i) {
55445544
auto methodType = i->getType().castTo<SILFunctionType>();
55455545

55465546
auto *classDecl = cast<ClassDecl>(method.getDecl()->getDeclContext());
5547-
if (IGM.isResilient(classDecl,
5548-
ResilienceExpansion::Maximal)) {
5547+
if (IGM.hasResilientMetadata(classDecl,
5548+
ResilienceExpansion::Maximal)) {
55495549
auto *fnPtr = IGM.getAddrOfDispatchThunk(method, NotForDefinition);
55505550
auto sig = IGM.getSignature(methodType);
55515551
FunctionPointer fn(fnPtr, sig);

‎lib/IRGen/Linking.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ SILLinkage LinkEntity::getLinkage(ForDefinition_t forDefinition) const {
484484

485485
auto linkage = getDeclLinkage(varDecl);
486486

487-
// Resilient classes don't expose field offset symbols.
487+
// Classes with resilient storage don't expose field offset symbols.
488488
if (cast<ClassDecl>(varDecl->getDeclContext())->isResilient()) {
489489
assert(linkage != FormalLinkage::PublicNonUnique &&
490490
"Cannot have a resilient class with non-unique linkage");

‎lib/IRGen/MetadataLayout.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ ClassMetadataLayout::ClassMetadataLayout(IRGenModule &IGM, ClassDecl *decl)
280280
Layout.StartOfImmediateMembers = getNextOffset();
281281

282282
if (Layout.HasResilientSuperclass ||
283-
IGM.isResilient(forClass, ResilienceExpansion::Maximal)) {
283+
IGM.hasResilientMetadata(forClass, ResilienceExpansion::Maximal)) {
284284
assert(!DynamicOffsetBase);
285285
DynamicOffsetBase = NextOffset;
286286
}

‎lib/SIL/SILDeclRef.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,9 @@ SubclassScope SILDeclRef::getSubclassScope() const {
928928
assert(FD->getEffectiveAccess() <= classType->getEffectiveAccess() &&
929929
"class must be as visible as its members");
930930

931+
// FIXME: This is too narrow. Any class with resilient metadata should
932+
// probably have this, at least for method overrides that don't add new
933+
// vtable entries.
931934
if (classType->isResilient())
932935
return SubclassScope::Resilient;
933936

‎lib/TBDGen/TBDGen.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,11 @@ void TBDGenVisitor::visitClassDecl(ClassDecl *CD) {
313313
visitNominalTypeDecl(CD);
314314

315315
auto hasResilientAncestor =
316-
CD->isResilient(SwiftModule, ResilienceExpansion::Minimal);
316+
CD->hasResilientMetadata(SwiftModule, ResilienceExpansion::Minimal);
317317
auto ancestor = CD->getSuperclassDecl();
318318
while (ancestor && !hasResilientAncestor) {
319319
hasResilientAncestor |=
320-
ancestor->isResilient(SwiftModule, ResilienceExpansion::Maximal);
320+
ancestor->hasResilientMetadata(SwiftModule, ResilienceExpansion::Maximal);
321321
ancestor = ancestor->getSuperclassDecl();
322322
}
323323

@@ -338,7 +338,7 @@ void TBDGenVisitor::visitClassDecl(ClassDecl *CD) {
338338
void addMethod(SILDeclRef method) {
339339
assert(method.getDecl()->getDeclContext() == CD);
340340

341-
if (CD->isResilient()) {
341+
if (CD->hasResilientMetadata()) {
342342
if (FirstTime) {
343343
FirstTime = false;
344344

‎test/IRGen/class_metadata.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ class A {}
2424
// Field count.
2525
// CHECK-SAME: i32 0,
2626
// Field offset vector offset.
27-
// CHECK-32-SAME: i32 14,
28-
// CHECK-64-SAME: i32 11,
27+
// CHECK-32-SAME: i32 13,
28+
// CHECK-64-SAME: i32 10,
2929
// V-table offset.
3030
// CHECK-32-SAME: i32 13,
3131
// CHECK-64-SAME: i32 10,

0 commit comments

Comments
 (0)