IRGen: Treat class layout for classes from other modules a bit more opaquely.

We would miscompile in mixed-language-version projects when a Swift class was compiled for one language version, while using Objective-C-imported types that are only available to that version, and then imported into a Swift module with a different language version that wasn't able to see all of the properties because of incompatible imported types. This manifested in a number of ways:

- We assumed we could re-derive the constant field offsets of the class's ivars from the layout, which is wrong if properties are missing, causing accesses to final properties or subclass properties to go to the wrong offsets.
- We assumed we could re-derive the instance size and alignment of a class instance in total, causing code to allocate the wrong amount of memory.
- We neglected to account for the space that stored properties take up in the field offset vector of the class object, causing us to load vtable entries for following subclass methods from the wrong offsets.

Eventually, resilience should reduce our exposure to these kinds of problems. As an incremental step in the right direction, when we look at a class from another module in IRGen, treat it as always variably-sized, so we don't try to hardcode offsets, size, or alignment of its instances. When we import a class, and we're unable to import a stored property, leave behind a new kind of MissingMemberDecl that records the number of field offset vector slots it will take up, so that we lay out subclass objects and compute vtable offsets correctly. Fixes rdar://problem/35330067.

A side effect of this is that the RemoteAST library is no longer able to provide fixed field offsets for class ivars. This doesn't appear to impact the lldb test suite, and they will ultimately need to use more abstract access patterns to get ivar offsets from resilient classes (if they aren't already), so I just removed the RemoteAST test cases that tested for class field offsets for now.

Author: jckarter

include/swift/AST/Decl.h

@@ -617,8 +617,9 @@ class alignas(1 << DeclAlignInBits) Decl {
     unsigned : NumDeclBits;
 
     unsigned NumberOfVTableEntries : 2;
+    unsigned NumberOfFieldOffsetVectorEntries : 1;
   };
-  enum { NumMissingMemberDeclBits = NumDeclBits + 2 };
+  enum { NumMissingMemberDeclBits = NumDeclBits + 3 };
   static_assert(NumMissingMemberDeclBits <= 32, "fits in an unsigned");
 
 protected:
@@ -2987,6 +2988,26 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
                                ToStoredProperty(skipInaccessible));
   }
 
+private:
+  /// Predicate used to filter StoredPropertyRange.
+  struct ToStoredPropertyOrMissingMemberPlaceholder {
+    Optional<Decl *> operator()(Decl *decl) const;
+  };
+
+public:
+  /// A range for iterating the stored member variables of a structure.
+  using StoredPropertyOrMissingMemberPlaceholderRange
+    = OptionalTransformRange<DeclRange,
+                             ToStoredPropertyOrMissingMemberPlaceholder>;
+  
+  /// Return a collection of the stored member variables of this type, along
+  /// with placeholders for unimportable stored properties.
+  StoredPropertyOrMissingMemberPlaceholderRange
+  getStoredPropertiesAndMissingMemberPlaceholders() const {
+    return StoredPropertyOrMissingMemberPlaceholderRange(getMembers(),
+                             ToStoredPropertyOrMissingMemberPlaceholder());
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() >= DeclKind::First_NominalTypeDecl &&
@@ -6305,26 +6326,37 @@ class PostfixOperatorDecl : public OperatorDecl {
 class MissingMemberDecl : public Decl {
   DeclName Name;
 
-  MissingMemberDecl(DeclContext *DC, DeclName name, unsigned vtableEntries)
+  MissingMemberDecl(DeclContext *DC, DeclName name,
+                    unsigned vtableEntries,
+                    unsigned fieldOffsetVectorEntries)
       : Decl(DeclKind::MissingMember, DC), Name(name) {
     MissingMemberDeclBits.NumberOfVTableEntries = vtableEntries;
     assert(getNumberOfVTableEntries() == vtableEntries && "not enough bits");
+    MissingMemberDeclBits.NumberOfFieldOffsetVectorEntries =
+      fieldOffsetVectorEntries;
+    assert(getNumberOfFieldOffsetVectorEntries() == fieldOffsetVectorEntries
+           && "not enough bits");
     setImplicit();
   }
 public:
   static MissingMemberDecl *
   forMethod(ASTContext &ctx, DeclContext *DC, DeclName name,
             bool hasNormalVTableEntry) {
     assert(!name || name.isCompoundName());
-    return new (ctx) MissingMemberDecl(DC, name, hasNormalVTableEntry);
+    return new (ctx) MissingMemberDecl(DC, name, hasNormalVTableEntry, 0);
   }
 
   static MissingMemberDecl *
   forInitializer(ASTContext &ctx, DeclContext *DC, DeclName name,
                  bool hasNormalVTableEntry,
                  bool hasAllocatingVTableEntry) {
     unsigned entries = hasNormalVTableEntry + hasAllocatingVTableEntry;
-    return new (ctx) MissingMemberDecl(DC, name, entries);
+    return new (ctx) MissingMemberDecl(DC, name, entries, 0);
+  }
+  
+  static MissingMemberDecl *
+  forStoredProperty(ASTContext &ctx, DeclContext *DC, DeclName name) {
+    return new (ctx) MissingMemberDecl(DC, name, 0, 1);
   }
 
   DeclName getFullName() const {
@@ -6335,6 +6367,10 @@ class MissingMemberDecl : public Decl {
     return MissingMemberDeclBits.NumberOfVTableEntries;
   }
 
+  unsigned getNumberOfFieldOffsetVectorEntries() const {
+    return MissingMemberDeclBits.NumberOfFieldOffsetVectorEntries;
+  }
+
   SourceLoc getLoc() const {
     return SourceLoc();
   }
@@ -6369,6 +6405,21 @@ NominalTypeDecl::ToStoredProperty::operator()(Decl *decl) const {
   return None;
 }
 
+inline Optional<Decl *>
+NominalTypeDecl::ToStoredPropertyOrMissingMemberPlaceholder
+::operator()(Decl *decl) const {
+  if (auto var = dyn_cast<VarDecl>(decl)) {
+    if (!var->isStatic() && var->hasStorage())
+      return var;
+  }
+  if (auto missing = dyn_cast<MissingMemberDecl>(decl)) {
+    if (missing->getNumberOfFieldOffsetVectorEntries() > 0)
+      return missing;
+  }
+
+  return None;
+}
+
 inline void
 AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) {
   GetSetInfo.setInt(accessLevel);

lib/IRGen/ClassMetadataVisitor.h

@@ -127,15 +127,23 @@ template <class Impl> class ClassMetadataVisitor
     // But we currently always give classes field-offset vectors,
     // whether they need them or not.
     asImpl().noteStartOfFieldOffsets(theClass);
-    for (auto field : theClass->getStoredProperties()) {
+    for (auto field :
+           theClass->getStoredPropertiesAndMissingMemberPlaceholders()) {
       addFieldEntries(field);
     }
     asImpl().noteEndOfFieldOffsets(theClass);
   }
 
 private:
-  void addFieldEntries(VarDecl *field) {
-    asImpl().addFieldOffset(field);
+  void addFieldEntries(Decl *field) {
+    if (auto var = dyn_cast<VarDecl>(field)) {
+      asImpl().addFieldOffset(var);
+      return;
+    }
+    if (auto placeholder = dyn_cast<MissingMemberDecl>(field)) {
+      asImpl().addFieldOffsetPlaceholders(placeholder);
+      return;
+    }
   }
 };
 
@@ -171,6 +179,12 @@ class ClassMetadataScanner : public ClassMetadataVisitor<Impl> {
   }
   void addMethodOverride(SILDeclRef baseRef, SILDeclRef declRef) {}
   void addFieldOffset(VarDecl *var) { addPointer(); }
+  void addFieldOffsetPlaceholders(MissingMemberDecl *mmd) {
+    for (unsigned i = 0, e = mmd->getNumberOfFieldOffsetVectorEntries();
+         i < e; ++i) {
+      addPointer();
+    }
+  }
   void addGenericArgument(CanType argument, ClassDecl *forClass) {
     addPointer();
   }

lib/IRGen/GenClass.cpp

@@ -194,19 +194,19 @@ namespace {
     {
       // Start by adding a heap header.
       switch (refcounting) {
-      case swift::irgen::ReferenceCounting::Native:
+      case ReferenceCounting::Native:
         // For native classes, place a full object header.
         addHeapHeader();
         break;
-      case swift::irgen::ReferenceCounting::ObjC:
+      case ReferenceCounting::ObjC:
         // For ObjC-inheriting classes, we don't reliably know the size of the
         // base class, but NSObject only has an `isa` pointer at most.
         addNSObjectHeader();
         break;
-      case swift::irgen::ReferenceCounting::Block:
-      case swift::irgen::ReferenceCounting::Unknown:
-      case swift::irgen::ReferenceCounting::Bridge:
-      case swift::irgen::ReferenceCounting::Error:
+      case ReferenceCounting::Block:
+      case ReferenceCounting::Unknown:
+      case ReferenceCounting::Bridge:
+      case ReferenceCounting::Error:
         llvm_unreachable("not a class refcounting kind");
       }
 
@@ -317,6 +317,12 @@ namespace {
           NumInherited = Elements.size();
         }
       }
+      
+      // If this class was imported from another module, assume that we may
+      // not know its exact layout.
+      if (theClass->getModuleContext() != IGM.getSwiftModule()) {
+        ClassHasFixedSize = false;
+      }
 
       // Access strategies should be set by the abstract class layout,
       // not using the concrete type information we have.

lib/IRGen/GenMeta.cpp

@@ -3355,6 +3355,16 @@ namespace {
         }
       }
     }
+    
+    void addFieldOffsetPlaceholders(MissingMemberDecl *placeholder) {
+      for (unsigned i = 0,
+                    e = placeholder->getNumberOfFieldOffsetVectorEntries();
+           i < e; ++i) {
+        // Emit placeholder values for some number of stored properties we
+        // know exist but aren't able to reference directly.
+        B.addInt(IGM.SizeTy, 0);
+      }
+    }
 
     void addMethod(SILDeclRef fn) {
       // Find the vtable entry.
@@ -3386,8 +3396,9 @@ namespace {
       B.addBitCast(IGM.getDeletedMethodErrorFn(), IGM.FunctionPtrTy);
     }
 
-    void addPlaceholder(MissingMemberDecl *) {
-      llvm_unreachable("cannot generate metadata with placeholders in it");
+    void addPlaceholder(MissingMemberDecl *m) {
+      assert(m->getNumberOfVTableEntries() == 0
+             && "cannot generate metadata with placeholders in it");
     }
 
     void addMethodOverride(SILDeclRef baseRef, SILDeclRef declRef) {}
@@ -4044,7 +4055,7 @@ irgen::emitClassFragileInstanceSizeAndAlignMask(IRGenFunction &IGF,
   // FIXME: The below checks should capture this property already, but
   // resilient class metadata layout is not fully implemented yet.
   auto expansion = IGF.IGM.getResilienceExpansionForLayout(theClass);
-  if (IGF.IGM.isResilient(theClass, expansion)) {
+  if (theClass->getParentModule() != IGF.IGM.getSwiftModule()) {
     return emitClassResilientInstanceSizeAndAlignMask(IGF, theClass, metadata);
   }
 

lib/SILGen/SILGenType.cpp

@@ -274,8 +274,9 @@ class SILGenVTable : public SILVTableVisitor<SILGenVTable> {
     }
   }
 
-  void addPlaceholder(MissingMemberDecl *) {
-    llvm_unreachable("Should not be emitting class with missing members");
+  void addPlaceholder(MissingMemberDecl *m) {
+    assert(m->getNumberOfVTableEntries() == 0
+           && "Should not be emitting class with missing members");
   }
 };
 

lib/Serialization/Deserialization.cpp

@@ -2872,8 +2872,30 @@ ModuleFile::getDeclCheckedImpl(DeclID DID, Optional<DeclContext *> ForcedContext
     for (TypeID dependencyID : dependencyIDs) {
       auto dependency = getTypeChecked(dependencyID);
       if (!dependency) {
+        // Stored properties in classes still impact class object layout because
+        // their offset is computed and stored in the field offset vector.
+        DeclDeserializationError::Flags flags;
+        
+        if (!isStatic) {
+          switch ((StorageKind)storageKind) {
+          case StorageKind::Stored:
+          case StorageKind::StoredWithObservers:
+          case StorageKind::StoredWithTrivialAccessors:
+            flags |= DeclDeserializationError::Flag::NeedsFieldOffsetVectorEntry;
+            break;
+            
+          case StorageKind::Addressed:
+          case StorageKind::AddressedWithObservers:
+          case StorageKind::AddressedWithTrivialAccessors:
+          case StorageKind::Computed:
+          case StorageKind::ComputedWithMutableAddress:
+          case StorageKind::InheritedWithObservers:
+            break;
+          }
+        }
+        
         return llvm::make_error<TypeError>(
-            name, takeErrorInfo(dependency.takeError()));
+            name, takeErrorInfo(dependency.takeError()), flags);
       }
     }
 
@@ -4690,6 +4712,9 @@ Decl *handleErrorAndSupplyMissingClassMember(ASTContext &context,
     } else if (error.needsVTableEntry()) {
       suppliedMissingMember = MissingMemberDecl::forMethod(
           context, containingClass, error.getName(), error.needsVTableEntry());
+    } else if (error.needsFieldOffsetVectorEntry()) {
+      suppliedMissingMember = MissingMemberDecl::forStoredProperty(
+          context, containingClass, error.getName());
     }
     // FIXME: Handle other kinds of missing members: properties,
     // subscripts, and methods that don't need vtable entries.

lib/Serialization/DeserializationErrors.h

@@ -216,6 +216,7 @@ class DeclDeserializationError : public llvm::ErrorInfoBase {
     DesignatedInitializer = 1 << 0,
     NeedsVTableEntry = 1 << 1,
     NeedsAllocatingVTableEntry = 1 << 2,
+    NeedsFieldOffsetVectorEntry = 1 << 3,
   };
   using Flags = OptionSet<Flag>;
 
@@ -237,6 +238,9 @@ class DeclDeserializationError : public llvm::ErrorInfoBase {
   bool needsAllocatingVTableEntry() const {
     return flags.contains(Flag::NeedsAllocatingVTableEntry);
   }
+  bool needsFieldOffsetVectorEntry() const {
+    return flags.contains(Flag::NeedsFieldOffsetVectorEntry);
+  }
 
   bool isA(const void *const ClassID) const override {
     return ClassID == classID() || ErrorInfoBase::isA(ClassID);

test/IRGen/Inputs/mixed_mode/ObjCStuff.apinotes

@@ -0,0 +1,10 @@
+Name: ObjCStuff
+SwiftVersions:
+- Version: 4
+  Typedefs:
+   - Name: OJCCloudButt
+     SwiftName: OJCCloud.Butt
+- Version: 3
+  Typedefs:
+   - Name: OJCCloudButt
+     SwiftWrapper: none

test/IRGen/Inputs/mixed_mode/ObjCStuff.h

@@ -0,0 +1,7 @@
+@import Foundation;
+
+@interface OJCCloud: NSObject
+@end
+
+typedef int OJCCloudButt __attribute__((swift_wrapper(struct)));
+

test/IRGen/Inputs/mixed_mode/UsingObjCStuff.swift

@@ -0,0 +1,13 @@
+import Foundation
+import ObjCStuff
+
+open class ButtHolder {
+  public final var x: Int
+  public final var y: [OJCCloud.Butt: String]
+  @NSManaged public var magic: [OJCCloud.Butt: String]
+  public final var z: String
+
+  open func virtual() {}
+
+  public init() { fatalError() }
+}

test/IRGen/Inputs/mixed_mode/module.modulemap

@@ -0,0 +1,4 @@
+module ObjCStuff {
+  header "ObjCStuff.h"
+  export *
+}

test/IRGen/mixed_mode_class_with_unimportable_fields.sil

@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %target-swift-frontend -emit-module -o %t/UsingObjCStuff.swiftmodule -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 4 %S/Inputs/mixed_mode/UsingObjCStuff.swift
+// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 3 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
+// RUN: %target-swift-frontend -emit-ir -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %s | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-ptrsize
+
+// REQUIRES: objc_interop
+
+import Swift
+import UsingObjCStuff
+
+class SubButtHolder: ButtHolder {
+  var w: Double = 0
+}
+
+sil_vtable SubButtHolder {}
+
+// CHECK-LABEL: define {{.*}} @getHolder
+sil @getHolder: $@convention(thin) () -> @owned ButtHolder {
+entry:
+  // We should load the dimensions of the class instance from metadata, not try
+  // to hardcode constants.
+  // CHECK: [[METADATA:%.*]] = call %swift.type* @_T014UsingObjCStuff10ButtHolderCMa()
+  // CHECK-64: [[SIZE32:%.*]] = load i32
+  // CHECK-64: [[SIZE:%.*]] = zext i32 [[SIZE32]] to
+  // CHECK-32: [[SIZE:%.*]] = load i32
+  // CHECK: [[ALIGN16:%.*]] = load i16
+  // CHECK: [[ALIGN:%.*]] = zext i16 [[ALIGN16]] to [[WORD:i32|i64]]
+  // CHECK: call noalias %swift.refcounted* @swift_rt_swift_allocObject(%swift.type* [[METADATA]], [[WORD]] [[SIZE]], [[WORD]] [[ALIGN]])
+  %x = alloc_ref $ButtHolder
+  return %x : $ButtHolder
+}