Fixed layout classes should still have resilient vtables

include/swift/AST/Decl.h

@@ -3589,6 +3589,17 @@ class ClassDecl final : public NominalTypeDecl {
     return SourceRange(ClassLoc, getBraces().End);
   }
 
+  /// Determine whether the member area of this class's metadata (which consists
+  /// of field offsets and vtable entries) is to be considered opaque by clients.
+  ///
+  /// Note that even @_fixed_layout classes have resilient metadata if they are
+  /// in a resilient module.
+  bool hasResilientMetadata() const;
+
+  /// Determine whether this class has resilient metadata when accessed from the
+  /// given module and resilience expansion.
+  bool hasResilientMetadata(ModuleDecl *M, ResilienceExpansion expansion) const;
+
   /// Determine whether this class has a superclass.
   bool hasSuperclass() const { return (bool)getSuperclassDecl(); }
 

lib/AST/Decl.cpp

@@ -1819,7 +1819,7 @@ bool AbstractStorageDecl::isResilient(ModuleDecl *M,
   case ResilienceExpansion::Minimal:
     return isResilient();
   case ResilienceExpansion::Maximal:
-    return isResilient() && M != getModuleContext();
+    return M != getModuleContext() && isResilient();
   }
   llvm_unreachable("bad resilience expansion");
 }
@@ -2915,7 +2915,7 @@ bool NominalTypeDecl::isResilient(ModuleDecl *M,
   case ResilienceExpansion::Minimal:
     return isResilient();
   case ResilienceExpansion::Maximal:
-    return isResilient() && M != getModuleContext();
+    return M != getModuleContext() && isResilient();
   }
   llvm_unreachable("bad resilience expansion");
 }
@@ -3355,6 +3355,36 @@ ClassDecl::ClassDecl(SourceLoc ClassLoc, Identifier Name, SourceLoc NameLoc,
   Bits.ClassDecl.HasMissingVTableEntries = 0;
 }
 
+bool ClassDecl::hasResilientMetadata() const {
+  // Imported classes don't have a vtable, etc, at all.
+  if (hasClangNode())
+    return false;
+
+  // If the module is not resilient, neither is the class metadata.
+  if (getParentModule()->getResilienceStrategy()
+          != ResilienceStrategy::Resilient)
+    return false;
+
+  // If the class is not public, we can't use it outside the module at all.
+  if (!getFormalAccessScope(/*useDC=*/nullptr,
+                            /*treatUsableFromInlineAsPublic=*/true).isPublic())
+    return false;
+
+  // Otherwise we access metadata members, such as vtable entries, resiliently.
+  return true;
+}
+
+bool ClassDecl::hasResilientMetadata(ModuleDecl *M,
+                                     ResilienceExpansion expansion) const {
+  switch (expansion) {
+  case ResilienceExpansion::Minimal:
+    return hasResilientMetadata();
+  case ResilienceExpansion::Maximal:
+    return M != getModuleContext() && hasResilientMetadata();
+  }
+  llvm_unreachable("bad resilience expansion");
+}
+
 DestructorDecl *ClassDecl::getDestructor() {
   auto results = lookupDirect(DeclBaseName::createDestructor());
   assert(!results.empty() && "Class without destructor?");

lib/IRGen/ClassMetadataVisitor.h

@@ -95,7 +95,7 @@ template <class Impl> class ClassMetadataVisitor
       if (superclassDecl->hasClangNode()) {
         // Nothing to do; Objective-C classes do not add new members to
         // Swift class metadata.
-      } else if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
+      } else if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal)) {
         // Runtime metadata instantiation will initialize our field offset
         // vector and vtable entries.
         //
@@ -118,14 +118,11 @@ template <class Impl> class ClassMetadataVisitor
     // This must always be the first item in the immediate members.
     asImpl().addGenericFields(theClass, theClass);
 
-    // If the class is resilient, we cannot make any assumptions about its
-    // member layout at all, so skip the rest of this method.
+    // If the class has resilient storage, we cannot make any assumptions about
+    // its storage layout, so skip the rest of this method.
     if (IGM.isResilient(theClass, ResilienceExpansion::Maximal))
       return;
 
-    // Add vtable entries.
-    asImpl().addVTableEntries(theClass);
-
     // A class only really *needs* a field-offset vector in the
     // metadata if:
     //   - it's in a generic context and
@@ -145,6 +142,14 @@ template <class Impl> class ClassMetadataVisitor
       addFieldEntries(field);
     }
     asImpl().noteEndOfFieldOffsets(theClass);
+
+    // If the class has resilient metadata, we cannot make any assumptions
+    // about its metadata layout, so skip the rest of this method.
+    if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
+      return;
+
+    // Add vtable entries.
+    asImpl().addVTableEntries(theClass);
   }
 
 private:

lib/IRGen/GenCall.cpp

@@ -1510,11 +1510,14 @@ void CallEmission::emitToUnmappedExplosion(Explosion &out) {
 
   // For ABI reasons the result type of the call might not actually match the
   // expected result type.
+  //
+  // This can happen when calling C functions, or class method dispatch thunks
+  // for methods that have covariant ABI-compatible overrides.
   auto expectedNativeResultType = nativeSchema.getExpandedType(IGF.IGM);
   if (result->getType() != expectedNativeResultType) {
-    // This should only be needed when we call C functions.
-    assert(getCallee().getOrigFunctionType()->getLanguage() ==
-           SILFunctionLanguage::C);
+    auto origFnType = getCallee().getOrigFunctionType();
+    assert(origFnType->getLanguage() == SILFunctionLanguage::C ||
+           origFnType->getRepresentation() == SILFunctionTypeRepresentation::Method);
     result =
         IGF.coerceValue(result, expectedNativeResultType, IGF.IGM.DataLayout);
   }

lib/IRGen/GenClass.cpp

@@ -200,8 +200,8 @@ namespace {
     bool ClassHasGenericLayout = false;
 
     // Is this class or any of its superclasses resilient from the viewpoint
-    // of the current module? This means that their metadata can change size
-    // and field offsets, generic arguments and virtual methods must be
+    // of the current module? This means that their metadata can change size,
+    // hence field offsets, generic arguments and virtual methods must be
     // accessed relative to a metadata base global variable.
     bool ClassHasResilientAncestry = false;
 
@@ -261,14 +261,11 @@ namespace {
       return Elements;
     }
 
-    /// Does the class metadata have a completely known, static layout that
-    /// does not require initialization at runtime beyond registeration of
-    /// the class with the Objective-C runtime?
+    /// Do instances of the class have a completely known, static layout?
     bool isFixedSize() const {
       return !(ClassHasMissingMembers ||
                ClassHasResilientMembers ||
                ClassHasGenericLayout ||
-               ClassHasResilientAncestry ||
                ClassHasObjCAncestry);
     }
 
@@ -332,19 +329,21 @@ namespace {
         auto superclassDecl = superclassType.getClassOrBoundGenericClass();
         assert(superclassType && superclassDecl);
 
-        if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
-          // If the class is resilient, don't walk over its fields; we have to
-          // calculate the layout at runtime.
+        if (IGM.hasResilientMetadata(superclassDecl, ResilienceExpansion::Maximal))
           ClassHasResilientAncestry = true;
 
-          // Furthermore, if the superclass is generic, we have to assume
-          // that its layout depends on its generic parameters. But this only
-          // propagates down to subclasses whose superclass type depends on the
-          // subclass's generic context.
+        // If the superclass has resilient storage, don't walk its fields.
+        if (IGM.isResilient(superclassDecl, ResilienceExpansion::Maximal)) {
+          ClassHasResilientMembers = true;
+
+          // If the superclass is generic, we have to assume that its layout
+          // depends on its generic parameters. But this only propagates down to
+          // subclasses whose superclass type depends on the subclass's generic
+          // context.
           if (superclassType.hasArchetype())
             ClassHasGenericLayout = true;
         } else {
-          // Otherwise, we have total knowledge of the class and its
+          // Otherwise, we are allowed to have total knowledge of the superclass
           // fields, so walk them to compute the layout.
           addFieldsForClass(superclassDecl, superclassType, /*superclass=*/true);
         }
@@ -356,8 +355,11 @@ namespace {
       if (classHasIncompleteLayout(IGM, theClass))
         ClassHasMissingMembers = true;
 
-      if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
+      if (IGM.hasResilientMetadata(theClass, ResilienceExpansion::Maximal))
         ClassHasResilientAncestry = true;
+
+      if (IGM.isResilient(theClass, ResilienceExpansion::Maximal)) {
+        ClassHasResilientMembers = true;
         return;
       }
 

lib/IRGen/GenDecl.cpp

@@ -3859,6 +3859,20 @@ bool IRGenModule::isResilient(NominalTypeDecl *D, ResilienceExpansion expansion)
   return D->isResilient(getSwiftModule(), expansion);
 }
 
+/// Do we have to use resilient access patterns when working with this
+/// class?
+///
+/// For classes, this means that virtual method calls use dispatch thunks
+/// rather than accessing metadata members directly.
+bool IRGenModule::hasResilientMetadata(ClassDecl *D,
+                                       ResilienceExpansion expansion) {
+  if (expansion == ResilienceExpansion::Maximal &&
+      Types.getLoweringMode() == TypeConverter::Mode::CompletelyFragile) {
+    return false;
+  }
+  return D->hasResilientMetadata(getSwiftModule(), expansion);
+}
+
 // The most general resilience expansion where the given declaration is visible.
 ResilienceExpansion
 IRGenModule::getResilienceExpansionForAccess(NominalTypeDecl *decl) {

lib/IRGen/GenMeta.cpp

@@ -1381,7 +1381,7 @@ namespace {
                                   RequireMetadata_t requireMetadata)
       : super(IGM, Type, requireMetadata),
         VTable(IGM.getSILModule().lookUpVTable(getType())),
-        Resilient(IGM.isResilient(Type, ResilienceExpansion::Minimal)) {
+        Resilient(IGM.hasResilientMetadata(Type, ResilienceExpansion::Minimal)) {
 
       if (getType()->isForeign()) return;
 
@@ -1485,7 +1485,7 @@ namespace {
 
       // Only emit a method lookup function if the class is resilient
       // and has a non-empty vtable.
-      if (IGM.isResilient(getType(), ResilienceExpansion::Minimal))
+      if (IGM.hasResilientMetadata(getType(), ResilienceExpansion::Minimal))
         IGM.emitMethodLookupFunction(getType());
 
       auto offset = MetadataLayout->hasResilientSuperclass()
@@ -2250,7 +2250,7 @@ static void emitClassMetadataBaseOffset(IRGenModule &IGM,
   // Only classes defined in resilient modules, or those that have
   // a resilient superclass need this.
   if (!layout.hasResilientSuperclass() &&
-      !IGM.isResilient(classDecl, ResilienceExpansion::Minimal)) {
+      !IGM.hasResilientMetadata(classDecl, ResilienceExpansion::Minimal)) {
     return;
   }
 

lib/IRGen/IRGenModule.h

@@ -784,6 +784,7 @@ class IRGenModule {
   clang::CodeGen::CodeGenModule &getClangCGM() const;
 
   bool isResilient(NominalTypeDecl *decl, ResilienceExpansion expansion);
+  bool hasResilientMetadata(ClassDecl *decl, ResilienceExpansion expansion);
   ResilienceExpansion getResilienceExpansionForAccess(NominalTypeDecl *decl);
   ResilienceExpansion getResilienceExpansionForLayout(NominalTypeDecl *decl);
   ResilienceExpansion getResilienceExpansionForLayout(SILGlobalVariable *var);

lib/IRGen/IRGenSIL.cpp

@@ -5460,12 +5460,12 @@ void IRGenSILFunction::visitSuperMethodInst(swift::SuperMethodInst *i) {
   // its offset since methods can be re-ordered resiliently. Instead, we call
   // the class method lookup function, passing in a reference to the
   // method descriptor.
-  if (IGM.isResilient(classDecl, ResilienceExpansion::Maximal)) {
+  if (IGM.hasResilientMetadata(classDecl, ResilienceExpansion::Maximal)) {
     // Load the superclass of the static type of the 'self' value.
     llvm::Value *superMetadata;
     auto instanceTy = CanType(baseType.getASTType()->getMetatypeInstanceType());
-    if (!IGM.isResilient(instanceTy.getClassOrBoundGenericClass(),
-                         ResilienceExpansion::Maximal)) {
+    if (!IGM.hasResilientMetadata(instanceTy.getClassOrBoundGenericClass(),
+                                  ResilienceExpansion::Maximal)) {
       // It's still possible that the static type of 'self' is not resilient, in
       // which case we can assume its superclass.
       //
@@ -5544,8 +5544,8 @@ void IRGenSILFunction::visitClassMethodInst(swift::ClassMethodInst *i) {
   auto methodType = i->getType().castTo<SILFunctionType>();
 
   auto *classDecl = cast<ClassDecl>(method.getDecl()->getDeclContext());
-  if (IGM.isResilient(classDecl,
-                      ResilienceExpansion::Maximal)) {
+  if (IGM.hasResilientMetadata(classDecl,
+                               ResilienceExpansion::Maximal)) {
     auto *fnPtr = IGM.getAddrOfDispatchThunk(method, NotForDefinition);
     auto sig = IGM.getSignature(methodType);
     FunctionPointer fn(fnPtr, sig);

lib/IRGen/Linking.cpp

@@ -484,7 +484,7 @@ SILLinkage LinkEntity::getLinkage(ForDefinition_t forDefinition) const {
 
     auto linkage = getDeclLinkage(varDecl);
 
-    // Resilient classes don't expose field offset symbols.
+    // Classes with resilient storage don't expose field offset symbols.
     if (cast<ClassDecl>(varDecl->getDeclContext())->isResilient()) {
       assert(linkage != FormalLinkage::PublicNonUnique &&
             "Cannot have a resilient class with non-unique linkage");

lib/IRGen/MetadataLayout.cpp

@@ -280,7 +280,7 @@ ClassMetadataLayout::ClassMetadataLayout(IRGenModule &IGM, ClassDecl *decl)
         Layout.StartOfImmediateMembers = getNextOffset();
 
         if (Layout.HasResilientSuperclass ||
-            IGM.isResilient(forClass, ResilienceExpansion::Maximal)) {
+            IGM.hasResilientMetadata(forClass, ResilienceExpansion::Maximal)) {
           assert(!DynamicOffsetBase);
           DynamicOffsetBase = NextOffset;
         }

lib/SIL/SILDeclRef.cpp

@@ -928,6 +928,9 @@ SubclassScope SILDeclRef::getSubclassScope() const {
   assert(FD->getEffectiveAccess() <= classType->getEffectiveAccess() &&
          "class must be as visible as its members");
 
+  // FIXME: This is too narrow. Any class with resilient metadata should
+  // probably have this, at least for method overrides that don't add new
+  // vtable entries.
   if (classType->isResilient())
     return SubclassScope::Resilient;
 

lib/TBDGen/TBDGen.cpp

@@ -313,11 +313,11 @@ void TBDGenVisitor::visitClassDecl(ClassDecl *CD) {
   visitNominalTypeDecl(CD);
 
   auto hasResilientAncestor =
-      CD->isResilient(SwiftModule, ResilienceExpansion::Minimal);
+      CD->hasResilientMetadata(SwiftModule, ResilienceExpansion::Minimal);
   auto ancestor = CD->getSuperclassDecl();
   while (ancestor && !hasResilientAncestor) {
     hasResilientAncestor |=
-        ancestor->isResilient(SwiftModule, ResilienceExpansion::Maximal);
+        ancestor->hasResilientMetadata(SwiftModule, ResilienceExpansion::Maximal);
     ancestor = ancestor->getSuperclassDecl();
   }
 
@@ -338,7 +338,7 @@ void TBDGenVisitor::visitClassDecl(ClassDecl *CD) {
     void addMethod(SILDeclRef method) {
       assert(method.getDecl()->getDeclContext() == CD);
 
-      if (CD->isResilient()) {
+      if (CD->hasResilientMetadata()) {
         if (FirstTime) {
           FirstTime = false;
 

test/IRGen/class_metadata.swift

@@ -24,8 +24,8 @@ class A {}
 //   Field count.
 // CHECK-SAME: i32 0,
 //   Field offset vector offset.
-// CHECK-32-SAME: i32 14,
-// CHECK-64-SAME: i32 11,
+// CHECK-32-SAME: i32 13,
+// CHECK-64-SAME: i32 10,
 //   V-table offset.
 // CHECK-32-SAME: i32 13,
 // CHECK-64-SAME: i32 10,