IRGen: Correctly compute instanceStart for a Swift class that starts with an empty field and is followed by a resilient field

We used to crash for classes that have an empty and a resilient field
during intialization if the object was in the shared cache.

  class CrashInInit {
    var empty = EmptyStruct()
    var resilient = ResilientThing()
  }

What happened was that for such a class we we would compute a
instanceStart of 0. The shared cache builder would then slide the value
of the constant ivar offset for the empty field from 0 to 16. However,
the field offset for empty fields is assumed to be zero and the runtime
does not compute a different value for the empty field and so the field
offset for the empty field remains 0. The runtime then trys to reconcile
the field offset (0) and the ivar offset (16) trying to write to the ivar
offset. However, the ivar offset is marked as constant and so we
crashed.
This can be avoided by correctly computing the instanceStart for such a
class to be 16 such that the shared cache builder does not update the
value of the empty field.

rdar://rdar://58458169

Author: aschwaighofer

lib/IRGen/ClassLayout.cpp

@@ -29,12 +29,14 @@ ClassLayout::ClassLayout(const StructLayoutBuilder &builder,
                          llvm::Type *classTy,
                          ArrayRef<VarDecl *> allStoredProps,
                          ArrayRef<FieldAccess> allFieldAccesses,
-                         ArrayRef<ElementLayout> allElements)
+                         ArrayRef<ElementLayout> allElements,
+                         Size headerSize)
   : MinimumAlign(builder.getAlignment()),
     MinimumSize(builder.getSize()),
     IsFixedLayout(builder.isFixedLayout()),
     Options(options),
     Ty(classTy),
+    HeaderSize(headerSize),
     AllStoredProperties(allStoredProps),
     AllFieldAccesses(allFieldAccesses),
     AllElements(allElements) { }
@@ -46,12 +48,33 @@ Size ClassLayout::getInstanceStart() const {
     elements = elements.drop_front();
 
     // Ignore empty elements.
+    bool haveSeenEmpty = false;
     if (element.isEmpty()) {
       continue;
     } else if (element.hasByteOffset()) {
       // FIXME: assumes layout is always sequential!
       return element.getByteOffset();
     } else {
+      // We used to crash for classes that have an empty and a resilient field
+      // during intialization.
+      //   class CrashInInit {
+      //     var empty = EmptyStruct()
+      //     var resilient = ResilientThing()
+      //   }
+      // What happened was that for such a class we we would compute a
+      // instanceStart of 0. The shared cache builder would then slide the value
+      // of the constant ivar offset for the empty field from 0 to 16. However
+      // the field offset for empty fields is assume to be zero and the runtime
+      // does not compute a different value for the empty field and so the field
+      // offset for the empty field stays 0. The runtime then trys to reconcile
+      // the field offset and the ivar offset trying to write to the ivar
+      // offset. However, the ivar offset is marked as constant and so we
+      // crashed.
+      // This can be avoided by correctly computing the instanceStart for such a
+      // class to be 16 such that the shared cache builder does not update the
+      // value of the empty field.
+      if (!Options.contains(ClassMetadataFlags::ClassHasObjCAncestry))
+        return HeaderSize;
       return Size(0);
     }
   }

lib/IRGen/ClassLayout.h

@@ -114,6 +114,9 @@ class ClassLayout {
   /// The LLVM type for instances of this class.
   llvm::Type *Ty;
 
+  /// The header size of this class.
+  Size HeaderSize;
+
   /// Lazily-initialized array of all fragile stored properties directly defined
   /// in the class itself.
   ArrayRef<VarDecl *> AllStoredProperties;
@@ -131,7 +134,8 @@ class ClassLayout {
               llvm::Type *classTy,
               ArrayRef<VarDecl *> allStoredProps,
               ArrayRef<FieldAccess> allFieldAccesses,
-              ArrayRef<ElementLayout> allElements);
+              ArrayRef<ElementLayout> allElements,
+              Size headerSize);
 
   Size getInstanceStart() const;
 

lib/IRGen/GenClass.cpp

@@ -165,6 +165,8 @@ namespace {
 
     ClassMetadataOptions Options;
 
+    Size HeaderSize;
+
   public:
     ClassLayoutBuilder(IRGenModule &IGM, SILType classType,
                        ReferenceCounting refcounting,
@@ -178,11 +180,13 @@ namespace {
       case ReferenceCounting::Native:
         // For native classes, place a full object header.
         addHeapHeader();
+        HeaderSize = CurSize;
         break;
       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();
+        HeaderSize = CurSize;
         break;
       case ReferenceCounting::Block:
       case ReferenceCounting::Unknown:
@@ -222,7 +226,7 @@ namespace {
       auto allElements = IGM.Context.AllocateCopy(Elements);
 
       return ClassLayout(*this, Options, classTy,
-                         allStoredProps, allFieldAccesses, allElements);
+                         allStoredProps, allFieldAccesses, allElements, HeaderSize);
     }
 
   private:

test/IRGen/metadata.swift

@@ -1,6 +1,21 @@
 // RUN: %empty-directory(%t)
 // RUN: %target-swift-frontend -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule -module-name=resilient_struct %S/../Inputs/resilient_struct.swift
-// RUN: %target-swift-frontend -module-name A -I %t  %S/Inputs/metadata2.swift -primary-file %s -emit-ir | %FileCheck %s
+// RUN: %target-swift-frontend -module-name A -I %t  %S/Inputs/metadata2.swift -primary-file %s -emit-ir | %FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-%target-os
+
+import resilient_struct
+
+enum Singleton {
+  case only
+}
+
+// CHECK: @"$s1A1GC14zeroSizedFieldAA9SingletonOvpWvd" = hidden constant i{{(64|32)}} 0
+// Check that the instance start is after the header (at 8 or 16).
+// CHECK: _DATA__TtC1A1G = private constant {{.*}} { i32 128, i32 {{(16|8)}}
+
+class G {
+  var zeroSizedField = Singleton.only
+  var r = ResilientInt(i:1)
+}
 
 // CHECK-LABEL: define {{.*}}swiftcc %swift.metadata_response @"$s1A12MyControllerCMr"(%swift.type*, i8*, i8**)
 // CHECK-NOT: ret