Merge pull request #29219 from aschwaighofer/irgen_fix_zero_size_field_instanceStart_2

IRGen: Correctly compute instanceStart for a Swift class that starts …

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,24 @@
 // 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-macosx: _DATA__TtC1A1G = private constant {{.*}} { i32 128, i32 {{(16|8)}}
+// CHECK-ios: _DATA__TtC1A1G = private constant {{.*}} { i32 128, i32 {{(16|8)}}
+// CHECK-watchos: _DATA__TtC1A1G = private constant {{.*}} { i32 128, i32 {{(16|8)}}
+// CHECK-tvos: _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