Fix reflection subclassing

include/swift/Reflection/ReflectionContext.h

@@ -92,13 +92,13 @@ class ReflectionContext
         // Figure out where the stored properties of this class begin
         // by looking at the size of the superclass
         bool valid;
-        unsigned size, align;
-        std::tie(valid, size, align) =
-            this->readInstanceSizeAndAlignmentFromClassMetadata(MetadataAddress);
+        unsigned start;
+        std::tie(valid, start) =
+            this->readInstanceStartAndAlignmentFromClassMetadata(MetadataAddress);
 
         // Perform layout
         if (valid)
-          TI = TC.getClassInstanceTypeInfo(TR, size, align);
+          TI = TC.getClassInstanceTypeInfo(TR, start);
 
         break;
       }

include/swift/Reflection/TypeLowering.h

@@ -246,8 +246,7 @@ class TypeConverter {
   ///
   /// Not cached.
   const TypeInfo *getClassInstanceTypeInfo(const TypeRef *TR,
-                                           unsigned start,
-                                           unsigned align);
+                                           unsigned start);
 
 private:
   friend class swift::reflection::LowerType;

include/swift/Remote/MetadataReader.h

@@ -561,94 +561,28 @@ class MetadataReader {
   }
 
   /// Given a remote pointer to class metadata, attempt to discover its class
-  /// instance size and alignment.
-  std::tuple<bool, unsigned, unsigned>
-  readInstanceSizeAndAlignmentFromClassMetadata(StoredPointer MetadataAddress) {
+  /// instance size and whether fields should use the resilient layout strategy.
+  std::pair<bool, unsigned>
+  readInstanceStartAndAlignmentFromClassMetadata(StoredPointer MetadataAddress) {
     auto meta = readMetadata(MetadataAddress);
     if (!meta || meta->getKind() != MetadataKind::Class)
-      return std::make_tuple(false, 0, 0);
-
-    auto classMeta = cast<TargetClassMetadata<Runtime>>(meta);
-
-    // See swift_initClassMetadata_UniversalStrategy()
-    uint32_t size, align;
-
-    // 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);
-
-      // Get the address of the InstanceStart field.
-      auto address = roDataPtr + sizeof(uint32_t) * 1;
-
-      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);
+      return std::make_pair(false, 0);
 
-        // Round up to the alignment boundary.
-        size = (size + (align - 1)) & ~(align - 1);
+    // The following algorithm only works on the non-fragile Apple runtime.
 
-        return std::make_tuple(true, size, align);
-      }
+    // Grab the RO-data pointer.  This part is not ABI.
+    StoredPointer roDataPtr = readObjCRODataPtr(MetadataAddress);
+    if (!roDataPtr)
+      return std::make_pair(false, 0);
 
-      // Fall through.
-    }
+    // Get the address of the InstanceStart field.
+    auto address = roDataPtr + sizeof(uint32_t) * 1;
 
-    // No superclass, just an object header. 12 bytes on 32-bit, 16 on 64-bit
-    size = Reader->getPointerSize() + sizeof(uint64_t);
-    align = 1;
+    unsigned start;
+    if (!Reader->readInteger(RemoteAddress(address), &start))
+      return std::make_pair(false, 0);
 
-    return std::make_tuple(true, size, align);
+    return std::make_pair(true, start);
   }
 
   /// Given a remote pointer to metadata, attempt to turn it into a type.

lib/IRGen/GenEnum.cpp

@@ -5684,6 +5684,12 @@ const TypeInfo *TypeConverter::convertEnumType(TypeBase *key, CanType type,
 void IRGenModule::emitEnumDecl(EnumDecl *theEnum) {
   emitEnumMetadata(*this, theEnum);
   emitNestedTypeDecls(theEnum->getMembers());
+
+  if (shouldEmitOpaqueTypeMetadataRecord(theEnum)) {
+    emitOpaqueTypeMetadataRecord(theEnum);
+    return;
+  }
+
   emitFieldMetadataRecord(theEnum);
 }
 

lib/IRGen/GenReflection.cpp

@@ -557,6 +557,17 @@ void IRGenModule::emitOpaqueTypeMetadataRecord(const NominalTypeDecl *nominalDec
   builder.emit();
 }
 
+bool IRGenModule::shouldEmitOpaqueTypeMetadataRecord(
+    const NominalTypeDecl *nominalDecl) {
+  if (nominalDecl->getAttrs().hasAttribute<AlignmentAttr>()) {
+    auto &ti = getTypeInfoForUnlowered(nominalDecl->getDeclaredTypeInContext());
+    if (isa<FixedTypeInfo>(ti))
+      return true;
+  }
+
+  return false;
+}
+
 /// Builds a constant LLVM struct describing the layout of a fixed-size
 /// SIL @box. These look like closure contexts, but without any necessary
 /// bindings or metadata sources, and only a single captured value.

lib/IRGen/GenStruct.cpp

@@ -839,6 +839,12 @@ unsigned irgen::getPhysicalStructFieldIndex(IRGenModule &IGM, SILType baseType,
 void IRGenModule::emitStructDecl(StructDecl *st) {
   emitStructMetadata(*this, st);
   emitNestedTypeDecls(st->getMembers());
+
+  if (shouldEmitOpaqueTypeMetadataRecord(st)) {
+    emitOpaqueTypeMetadataRecord(st);
+    return;
+  }
+
   emitFieldMetadataRecord(st);
 }
 

lib/IRGen/IRGenModule.h

@@ -771,6 +771,10 @@ class IRGenModule {
   /// from this module.
   void emitOpaqueTypeMetadataRecord(const NominalTypeDecl *nominalDecl);
 
+  /// Some nominal type declarations require us to emit a fixed-size type
+  /// descriptor, because they have special layout considerations.
+  bool shouldEmitOpaqueTypeMetadataRecord(const NominalTypeDecl *nominalDecl);
+
   /// Emit reflection metadata records for builtin and imported types referenced
   /// from this module.
   void emitBuiltinReflectionMetadata();

stdlib/public/Reflection/TypeLowering.cpp

@@ -1251,8 +1251,7 @@ const TypeInfo *TypeConverter::getTypeInfo(const TypeRef *TR) {
 }
 
 const TypeInfo *TypeConverter::getClassInstanceTypeInfo(const TypeRef *TR,
-                                                        unsigned start,
-                                                        unsigned align) {
+                                                        unsigned start) {
   const FieldDescriptor *FD = getBuilder().getFieldTypeInfo(TR);
   if (FD == nullptr) {
     DEBUG(std::cerr << "No field descriptor: "; TR->dump());

validation-test/Reflection/Inputs/ObjCClasses/ObjCClasses.h

@@ -0,0 +1,21 @@
+#ifndef SWIFT_TEST_OBJC_CLASSES_H
+#define SWIFT_TEST_OBJC_CLASSES_H
+
+#import <Foundation/Foundation.h>
+
+@interface FirstClass : NSObject
+@property void *x;
+@end
+
+@interface SecondClass : NSObject
+@property void *x;
+@property void *y;
+@end
+
+@interface ThirdClass : NSObject
+@property void *x;
+@property void *y;
+@property void *z;
+@end
+
+#endif

validation-test/Reflection/Inputs/ObjCClasses/ObjCClasses.m

@@ -0,0 +1,16 @@
+#import "ObjCClasses.h"
+
+@implementation FirstClass : NSObject
+@synthesize x;
+@end
+
+@implementation SecondClass : NSObject
+@synthesize x;
+@synthesize y;
+@end
+
+@implementation ThirdClass : NSObject
+@synthesize x;
+@synthesize y;
+@synthesize z;
+@end

validation-test/Reflection/Inputs/ObjCClasses/module.map

@@ -0,0 +1,3 @@
+module ObjCClasses {
+  header "ObjCClasses.h"
+}

validation-test/Reflection/inherits_NSObject.swift

@@ -6,6 +6,7 @@
 // REQUIRES: executable_test
 
 import Foundation
+import simd
 
 import SwiftReflectionTest
 
@@ -14,11 +15,6 @@ class BaseNSClass : NSObject {
   var x: Bool = false
 }
 
-class DerivedNSClass : BaseNSClass {
-  var y: Bool = false
-  var z: Int = 0
-}
-
 let baseClass = BaseNSClass()
 reflect(object: baseClass)
 
@@ -42,16 +38,21 @@ reflect(object: baseClass)
 // CHECK-32: (class inherits_NSObject.BaseNSClass)
 
 // CHECK-32: Type info:
-// 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: (class_instance size=17 alignment=4 stride=20 num_extra_inhabitants=0
+// CHECK-32-NEXT:   (field name=w offset=12
 // 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:   (field name=x offset=16
 // 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)))))
 
+class DerivedNSClass : BaseNSClass {
+  var y: Bool = false
+  var z: Int = 0
+}
+
 let derivedClass = DerivedNSClass()
 reflect(object: derivedClass)
 
@@ -85,4 +86,106 @@ reflect(object: derivedClass)
 // CHECK-32-NEXT:       (field name=_value offset=0
 // CHECK-32-NEXT:         (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=0)))))
 
+// Note: dynamic layout starts at offset 8, not 16
+class GenericBaseNSClass<T> : NSObject {
+  var w: T = 0 as! T
+}
+
+let genericBaseClass = GenericBaseNSClass<Int>()
+reflect(object: genericBaseClass)
+
+// CHECK-64: Reflecting an object.
+// CHECK-64: Type reference:
+// CHECK-64: (bound_generic_class inherits_NSObject.GenericBaseNSClass
+// CHECK-64:   (struct Swift.Int))
+
+// CHECK-64: Type info:
+// CHECK-64-NEXT: (class_instance size=16 alignment=8 stride=16 num_extra_inhabitants=0
+// CHECK-64-NEXT:   (field name=w offset=8
+// 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: (bound_generic_class inherits_NSObject.GenericBaseNSClass
+// CHECK-32:   (struct Swift.Int))
+
+// CHECK-32: Type info:
+// CHECK-32-NEXT: (class_instance size=8 alignment=4 stride=8 num_extra_inhabitants=0
+// CHECK-32-NEXT:   (field name=w offset=4
+// 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)))))
+
+class AlignedNSClass : NSObject {
+  var w: Int = 0
+  var x: int4 = [1,2,3,4]
+}
+
+let alignedClass = AlignedNSClass()
+reflect(object: alignedClass)
+
+// CHECK-64: Reflecting an object.
+// CHECK-64: Type reference:
+// CHECK-64: (class inherits_NSObject.AlignedNSClass)
+
+// CHECK-64: Type info:
+// CHECK-64-NEXT: (class_instance size=48 alignment=16 stride=48 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=32
+// CHECK-64-NEXT:     (builtin size=16 alignment=16 stride=16 num_extra_inhabitants=0)))
+
+// CHECK-32: Reflecting an object.
+// CHECK-32: Type reference:
+// CHECK-32: (class inherits_NSObject.AlignedNSClass)
+
+// CHECK-32: Type info:
+// CHECK-32-NEXT: (class_instance size=32 alignment=16 stride=32 num_extra_inhabitants=0
+// CHECK-32-NEXT:   (field name=w offset=12
+// 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=16
+// CHECK-32-NEXT:     (builtin size=16 alignment=16 stride=16 num_extra_inhabitants=0)))
+
+class GenericAlignedNSClass<T> : NSObject {
+  var w: T = 0 as! T
+  var x: int4 = [1,2,3,4]
+}
+
+let genericAlignedClass = GenericAlignedNSClass<Int>()
+reflect(object: genericAlignedClass)
+
+// CHECK-64: Reflecting an object.
+// CHECK-64: Type reference:
+// CHECK-64: (bound_generic_class inherits_NSObject.GenericAlignedNSClass
+// CHECK-64:   (struct Swift.Int))
+
+// CHECK-64: Type info:
+// CHECK-64-NEXT: (class_instance size=48 alignment=16 stride=48 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=32
+// CHECK-64-NEXT:     (builtin size=16 alignment=16 stride=16 num_extra_inhabitants=0)))
+
+// CHECK-32: Reflecting an object.
+// CHECK-32: Type reference:
+// CHECK-32: (bound_generic_class inherits_NSObject.GenericAlignedNSClass
+// CHECK-32:   (struct Swift.Int))
+
+// CHECK-32: Type info:
+// CHECK-32-NEXT: (class_instance size=48 alignment=16 stride=48 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=32
+// CHECK-32-NEXT:     (builtin size=16 alignment=16 stride=16 num_extra_inhabitants=0)))
+
 doneReflecting()