Merge pull request #5705 from slavapestov/reflection-fixes

Author: swift-ci

include/swift/Reflection/ReflectionContext.h

@@ -93,16 +93,13 @@ class ReflectionContext
         // by looking at the size of the superclass
         bool valid;
         unsigned size, align;
-        auto super =
-            this->readSuperClassFromClassMetadata(MetadataAddress);
-        if (super) {
-          std::tie(valid, size, align) =
-              this->readInstanceSizeAndAlignmentFromClassMetadata(super);
-
-          // Perform layout
-          if (valid)
-            TI = TC.getClassInstanceTypeInfo(TR, size, align);
-        }
+        std::tie(valid, size, align) =
+            this->readInstanceSizeAndAlignmentFromClassMetadata(MetadataAddress);
+
+        // Perform layout
+        if (valid)
+          TI = TC.getClassInstanceTypeInfo(TR, size, align);
+
         break;
       }
       default:

include/swift/Reflection/TypeLowering.h

@@ -117,7 +117,9 @@ class TypeInfo {
            unsigned Size, unsigned Alignment,
            unsigned Stride, unsigned NumExtraInhabitants)
     : Kind(Kind), Size(Size), Alignment(Alignment), Stride(Stride),
-      NumExtraInhabitants(NumExtraInhabitants) {}
+      NumExtraInhabitants(NumExtraInhabitants) {
+    assert(Alignment > 0);
+  }
 
   TypeInfoKind getKind() const { return Kind; }
 

include/swift/Remote/MetadataReader.h

@@ -564,33 +564,90 @@ class MetadataReader {
   /// instance size and alignment.
   std::tuple<bool, unsigned, unsigned>
   readInstanceSizeAndAlignmentFromClassMetadata(StoredPointer MetadataAddress) {
-    auto superMeta = readMetadata(MetadataAddress);
-    if (!superMeta || superMeta->getKind() != MetadataKind::Class)
+    auto meta = readMetadata(MetadataAddress);
+    if (!meta || meta->getKind() != MetadataKind::Class)
       return std::make_tuple(false, 0, 0);
 
-    auto super = cast<TargetClassMetadata<Runtime>>(superMeta);
+    auto classMeta = cast<TargetClassMetadata<Runtime>>(meta);
 
     // See swift_initClassMetadata_UniversalStrategy()
     uint32_t size, align;
-    if (super->isTypeMetadata()) {
-      size = super->getInstanceSize();
-      align = super->getInstanceAlignMask() + 1;
-    } else {
+
+    // If this class is defined in Objective-C, return the value of the
+    // InstanceStart field from the ROData.
+    if (!classMeta->isTypeMetadata()) {
       // The following algorithm only works on the non-fragile Apple runtime.
 
       // Grab the RO-data pointer.  This part is not ABI.
       StoredPointer roDataPtr = readObjCRODataPtr(MetadataAddress);
       if (!roDataPtr)
         return std::make_tuple(false, 0, 0);
 
-      auto address = roDataPtr + sizeof(uint32_t) * 2;
+      // Get the address of the InstanceStart field.
+      auto address = roDataPtr + sizeof(uint32_t) * 1;
 
-      align = 16; // malloc alignment guarantee
+      align = 16;
 
       if (!Reader->readInteger(RemoteAddress(address), &size))
         return std::make_tuple(false, 0, 0);
+
+      assert((size & (align - 1)) == 0);
+      return std::make_tuple(true, size, align);
+    }
+
+    // Otherwise, it is a Swift class. Get the superclass.
+    auto superAddr = readSuperClassFromClassMetadata(MetadataAddress);
+    if (superAddr) {
+      auto superMeta = readMetadata(superAddr);
+      if (!superMeta || superMeta->getKind() != MetadataKind::Class)
+        return std::make_tuple(false, 0, 0);
+
+      auto superclassMeta = cast<TargetClassMetadata<Runtime>>(superMeta);
+
+      // If the superclass is an Objective-C class, we start layout
+      // from the InstanceSize of the superclass, aligned up to
+      // 16 bytes.
+      if (superclassMeta->isTypeMetadata()) {
+        // Superclass is a Swift class. Get the size of an instance,
+        // and start layout from that.
+        size = superclassMeta->getInstanceSize();
+        align = 1;
+
+        return std::make_tuple(true, size, align);
+      }
+
+      std::string superName;
+      if (!readObjCClassName(superAddr, superName))
+        return std::make_tuple(false, 0, 0);
+
+      if (superName != "SwiftObject") {
+        // Grab the RO-data pointer.  This part is not ABI.
+        StoredPointer roDataPtr = readObjCRODataPtr(superAddr);
+        if (!roDataPtr)
+          return std::make_tuple(false, 0, 0);
+
+        // Get the address of the InstanceSize field.
+        auto address = roDataPtr + sizeof(uint32_t) * 2;
+
+        // malloc alignment boundary.
+        align = 16;
+
+        if (!Reader->readInteger(RemoteAddress(address), &size))
+          return std::make_tuple(false, 0, 0);
+
+        // Round up to the alignment boundary.
+        size = (size + (align - 1)) & ~(align - 1);
+
+        return std::make_tuple(true, size, align);
+      }
+
+      // Fall through.
     }
 
+    // No superclass, just an object header. 12 bytes on 32-bit, 16 on 64-bit
+    size = Reader->getPointerSize() + sizeof(uint64_t);
+    align = 1;
+
     return std::make_tuple(true, size, align);
   }
 

stdlib/public/Reflection/TypeLowering.cpp

@@ -378,6 +378,8 @@ class ExistentialTypeInfoBuilder {
 unsigned RecordTypeInfoBuilder::addField(unsigned fieldSize,
                                          unsigned fieldAlignment,
                                          unsigned numExtraInhabitants) {
+  assert(fieldAlignment > 0);
+
   // Align the current size appropriately
   Size = ((Size + fieldAlignment - 1) & ~(fieldAlignment - 1));
 
@@ -873,7 +875,7 @@ class EnumTypeInfoBuilder {
 
 public:
   EnumTypeInfoBuilder(TypeConverter &TC)
-    : TC(TC), Size(0), Alignment(0), NumExtraInhabitants(0),
+    : TC(TC), Size(0), Alignment(1), NumExtraInhabitants(0),
       Kind(RecordKind::Invalid), Invalid(false) {}
 
   const TypeInfo *build(const TypeRef *TR, const FieldDescriptor *FD) {
@@ -1264,9 +1266,9 @@ const TypeInfo *TypeConverter::getClassInstanceTypeInfo(const TypeRef *TR,
     // TypeRef to make field types concrete.
     RecordTypeInfoBuilder builder(*this, RecordKind::ClassInstance);
 
-    // Start layout from the given instance start offset.
-    // Extra inhabitants do not matter for a class instance payload.
-    builder.addField(start, align, /*numExtraInhabitants=*/0);
+    // Start layout from the given instance start offset. This should
+    // be the superclass instance size.
+    builder.addField(start, 1, /*numExtraInhabitants=*/0);
 
     for (auto Field : getBuilder().getFieldTypeRefs(TR, FD))
       builder.addField(Field.Name, Field.TR);

test/Reflection/typeref_lowering.swift

@@ -913,11 +913,11 @@ V12TypeLowering10EnumStruct
 // CHECK-64: (struct TypeLowering.EnumStruct)
 // CHECK-64-NEXT: (struct size=81 alignment=8 stride=88 num_extra_inhabitants=0
 // CHECK-64-NEXT:   (field name=empty offset=0
-// CHECK-64-NEXT:     (no_payload_enum size=0 alignment=0 stride=1 num_extra_inhabitants=0))
+// CHECK-64-NEXT:     (no_payload_enum size=0 alignment=1 stride=1 num_extra_inhabitants=0))
 // CHECK-64-NEXT:   (field name=noPayload offset=0
-// CHECK-64-NEXT:     (no_payload_enum size=1 alignment=0 stride=1 num_extra_inhabitants=0))
-// CHECK-64-NEXT:   (field name=sillyNoPayload offset=0
-// CHECK-64-NEXT:     (no_payload_enum size=1 alignment=0 stride=1 num_extra_inhabitants=0))
+// CHECK-64-NEXT:     (no_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0))
+// CHECK-64-NEXT:   (field name=sillyNoPayload offset=1
+// CHECK-64-NEXT:     (no_payload_enum size=1 alignment=1 stride=1 num_extra_inhabitants=0))
 // CHECK-64-NEXT:   (field name=singleton offset=8
 // CHECK-64-NEXT:     (reference kind=strong refcounting=native))
 // CHECK-64-NEXT:   (field name=singlePayload offset=16

validation-test/Reflection/functions.swift

@@ -164,14 +164,14 @@ class CapturingClass {
   // CHECK-64: (class functions.CapturingClass)
 
   // CHECK-64: Type info:
-  // CHECK-64: (class_instance size=16 alignment=16 stride=16
+  // CHECK-64: (class_instance size=16 alignment=1 stride=16
 
   // CHECK-32: Reflecting an object.
   // CHECK-32: Type reference:
   // CHECK-32: (class functions.CapturingClass)
 
   // CHECK-32: Type info:
-  // CHECK-32: (class_instance size=12 alignment=16 stride=16
+  // CHECK-32: (class_instance size=12 alignment=1 stride=12
   @_semantics("optimize.sil.never")
   func arity0Capture1() -> () -> () {
     let closure = {

validation-test/Reflection/inherits_NSObject.swift

@@ -9,62 +9,80 @@ import Foundation
 
 import SwiftReflectionTest
 
-class InheritsNSObject : NSObject {}
-class ContainsInheritsNSObject : NSObject {
-    var a: InheritsNSObject
-    var _b: InheritsNSObject
-
-    var b: InheritsNSObject {
-        get { return _b }
-        set (newVal) { _b = newVal }
-    }
-
-    override init() {
-        a = InheritsNSObject()
-        _b = InheritsNSObject()
-    }
+class BaseNSClass : NSObject {
+  var w: Int = 0
+  var x: Bool = false
 }
 
-let inheritsNSObject = InheritsNSObject()
-reflect(object: inheritsNSObject)
+class DerivedNSClass : BaseNSClass {
+  var y: Bool = false
+  var z: Int = 0
+}
+
+let baseClass = BaseNSClass()
+reflect(object: baseClass)
 
 // CHECK-64: Reflecting an object.
 // CHECK-64: Type reference:
-// CHECK-64: (class inherits_NSObject.InheritsNSObject)
+// CHECK-64: (class inherits_NSObject.BaseNSClass)
 
 // CHECK-64: Type info:
-// CHECK-64: (class_instance size=8 alignment=16 stride=16 num_extra_inhabitants=0)
+// CHECK-64-NEXT: (class_instance size=25 alignment=8 stride=32 num_extra_inhabitants=0
+// CHECK-64-NEXT:   (field name=w offset=16
+// CHECK-64-NEXT:     (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0
+// CHECK-64-NEXT:       (field name=_value offset=0
+// CHECK-64-NEXT:         (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=0))))
+// CHECK-64-NEXT:   (field name=x offset=24
+// CHECK-64-NEXT:     (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254
+// CHECK-64-NEXT:       (field name=_value offset=0
+// CHECK-64-NEXT:         (builtin size=1 alignment=1 stride=1 num_extra_inhabitants=254)))))
 
 // CHECK-32: Reflecting an object.
 // CHECK-32: Type reference:
-// CHECK-32: (class inherits_NSObject.InheritsNSObject)
+// CHECK-32: (class inherits_NSObject.BaseNSClass)
 
 // CHECK-32: Type info:
-// CHECK-32: (class_instance size=4 alignment=16 stride=16 num_extra_inhabitants=0)
-
-let sc = ContainsInheritsNSObject()
-reflect(object: sc)
+// CHECK-32-NEXT: (class_instance size=21 alignment=4 stride=24 num_extra_inhabitants=0
+// CHECK-32-NEXT:   (field name=w offset=16
+// CHECK-32-NEXT:     (struct size=4 alignment=4 stride=4 num_extra_inhabitants=0
+// CHECK-32-NEXT:       (field name=_value offset=0
+// CHECK-32-NEXT:         (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=0))))
+// CHECK-32-NEXT:   (field name=x offset=20
+// CHECK-32-NEXT:     (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254
+// CHECK-32-NEXT:       (field name=_value offset=0
+// CHECK-32-NEXT:         (builtin size=1 alignment=1 stride=1 num_extra_inhabitants=254)))))
+
+let derivedClass = DerivedNSClass()
+reflect(object: derivedClass)
 
 // CHECK-64: Reflecting an object.
 // CHECK-64: Type reference:
-// CHECK-64: (class inherits_NSObject.ContainsInheritsNSObject)
+// CHECK-64: (class inherits_NSObject.DerivedNSClass)
 
-// CHECK-64:        Type info:
-// CHECK-64:        (class_instance size=24 alignment=16 stride=32 num_extra_inhabitants=0
-// CHECK-64-NEXT:   (field name=a offset=8
-// CHECK-64-NEXT:     (reference kind=strong refcounting=unknown))
-// CHECK-64-NEXT:   (field name=_b offset=16
-// CHECK-64-NEXT:     (reference kind=strong refcounting=unknown)))
+// CHECK-64: Type info:
+// CHECK-64-NEXT: (class_instance size=40 alignment=8 stride=40 num_extra_inhabitants=0
+// CHECK-64-NEXT:   (field name=y offset=25
+// CHECK-64-NEXT:     (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254
+// CHECK-64-NEXT:       (field name=_value offset=0
+// CHECK-64-NEXT:         (builtin size=1 alignment=1 stride=1 num_extra_inhabitants=254))))
+// CHECK-64-NEXT:   (field name=z offset=32
+// CHECK-64-NEXT:     (struct size=8 alignment=8 stride=8 num_extra_inhabitants=0
+// CHECK-64-NEXT:       (field name=_value offset=0
+// CHECK-64-NEXT:         (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=0)))))
 
 // CHECK-32: Reflecting an object.
 // CHECK-32: Type reference:
-// CHECK-32: (class inherits_NSObject.ContainsInheritsNSObject)
-
-// CHECK-32:        Type info:
-// CHECK-32:        (class_instance size=12 alignment=16 stride=16 num_extra_inhabitants=0
-// CHECK-32-NEXT:   (field name=a offset=4
-// CHECK-32-NEXT:     (reference kind=strong refcounting=unknown))
-// CHECK-32-NEXT:   (field name=_b offset=8
-// CHECK-32-NEXT:     (reference kind=strong refcounting=unknown)))
+// CHECK-32: (class inherits_NSObject.DerivedNSClass)
+
+// CHECK-32: Type info:
+// CHECK-32-NEXT: (class_instance size=24 alignment=4 stride=24 num_extra_inhabitants=0
+// CHECK-32-NEXT:   (field name=y offset=17
+// CHECK-32-NEXT:     (struct size=1 alignment=1 stride=1 num_extra_inhabitants=254
+// CHECK-32-NEXT:       (field name=_value offset=0
+// CHECK-32-NEXT:         (builtin size=1 alignment=1 stride=1 num_extra_inhabitants=254))))
+// CHECK-32-NEXT:   (field name=z offset=20
+// CHECK-32-NEXT:     (struct size=4 alignment=4 stride=4 num_extra_inhabitants=0
+// CHECK-32-NEXT:       (field name=_value offset=0
+// CHECK-32-NEXT:         (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=0)))))
 
 doneReflecting()