Correctly project enums with zero-sized payloads

I earlier overhauled the enum layout logic to correctly consider
enums with generic cases and cases that have zero size.
This updates the enum projection logic to use that information
as well.

In particular, this fixes a bug where an MPE with zero-sized cases
would be incorrectly projected by RemoteMirror (with consequences
for the `leaks` tool and lldb).

Resolves rdar://111705059

Author: tbkka

stdlib/public/RemoteInspection/TypeLowering.cpp

@@ -653,13 +653,16 @@ class SinglePayloadEnumTypeInfo: public EnumTypeInfo {
 // }
 // ```
 class TaggedMultiPayloadEnumTypeInfo: public EnumTypeInfo {
+  unsigned NumEffectivePayloadCases;
 public:
   TaggedMultiPayloadEnumTypeInfo(unsigned Size, unsigned Alignment,
                            unsigned Stride, unsigned NumExtraInhabitants,
                            bool BitwiseTakable,
-                           const std::vector<FieldInfo> &Cases)
+                           const std::vector<FieldInfo> &Cases,
+                           unsigned NumEffectivePayloadCases)
     : EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants,
-                   BitwiseTakable, EnumKind::MultiPayloadEnum, Cases) {
+                   BitwiseTakable, EnumKind::MultiPayloadEnum, Cases),
+      NumEffectivePayloadCases(NumEffectivePayloadCases) {
     // Definition of "multi-payload enum"
     assert(getCases().size() > 1); // At least 2 cases
     assert(Cases[0].TR != 0); // At least 2 payloads
@@ -698,7 +701,7 @@ class TaggedMultiPayloadEnumTypeInfo: public EnumTypeInfo {
                         remote::RemoteAddress address,
                         int *CaseIndex) const override {
     unsigned long PayloadSize = getPayloadSize();
-    unsigned PayloadCount = getNumPayloadCases();
+    unsigned PayloadCount = NumEffectivePayloadCases;
     unsigned NumCases = getNumCases();
     unsigned TagSize = getSize() - PayloadSize;
     unsigned tag = 0;
@@ -1030,15 +1033,24 @@ class BitMask {
 // bits in the payload.
 class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
   BitMask spareBitsMask;
+  // "Effective" payload cases includes those with
+  // generic payload and non-generic cases that are
+  // statically known to have non-zero size.
+  // It does not include cases with payloads that are
+  // non-generic and zero-sized (these are treated as
+  // non-payload cases for many purposes).
+  unsigned NumEffectivePayloadCases;
 public:
   MultiPayloadEnumTypeInfo(unsigned Size, unsigned Alignment,
                            unsigned Stride, unsigned NumExtraInhabitants,
                            bool BitwiseTakable,
                            const std::vector<FieldInfo> &Cases,
-                           BitMask spareBitsMask)
+                           BitMask spareBitsMask,
+                           unsigned NumEffectivePayloadCases)
     : EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants,
                    BitwiseTakable, EnumKind::MultiPayloadEnum, Cases),
-      spareBitsMask(spareBitsMask) {
+      spareBitsMask(spareBitsMask),
+      NumEffectivePayloadCases(NumEffectivePayloadCases) {
     assert(Cases[0].TR != 0);
     assert(Cases[1].TR != 0);
     assert(getNumNonEmptyPayloadCases() > 1);
@@ -1125,7 +1137,6 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
                         remote::RemoteAddress address,
                         int *CaseIndex) const override {
     unsigned long payloadSize = getPayloadSize();
-    unsigned NumPayloadCases = getNumPayloadCases();
 
     // Extra Tag (if any) holds upper bits of case value
     auto extraTagSize = getSize() - payloadSize;
@@ -1156,7 +1167,7 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
     }
 
     // If the above identifies a payload case, we're done
-    if (static_cast<unsigned>(tagValue) < NumPayloadCases) {
+    if (static_cast<unsigned>(tagValue) < NumEffectivePayloadCases) {
       *CaseIndex = tagValue;
       return true;
     }
@@ -1173,9 +1184,9 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
 
     int ComputedCase = 0;
     if (occupiedBitCount >= 32) {
-      ComputedCase = payloadValue + NumPayloadCases;
+      ComputedCase = payloadValue + NumEffectivePayloadCases;
     } else {
-      ComputedCase = (((tagValue - NumPayloadCases) << occupiedBitCount) |  payloadValue) + NumPayloadCases;
+      ComputedCase = (((tagValue - NumEffectivePayloadCases) << occupiedBitCount) |  payloadValue) + NumEffectivePayloadCases;
     }
 
     if (static_cast<unsigned>(ComputedCase) < getNumCases()) {
@@ -1193,8 +1204,8 @@ class MultiPayloadEnumTypeInfo: public EnumTypeInfo {
   // * The remainder of the payload bits (for non-payload cases)
   // This computes the bits used for the payload tag.
   BitMask getMultiPayloadTagBitsMask() const {
-    auto payloadTagValues = getNumPayloadCases() - 1;
-    if (getNumCases() > getNumPayloadCases()) {
+    auto payloadTagValues = NumEffectivePayloadCases - 1;
+    if (getNumCases() > NumEffectivePayloadCases) {
       payloadTagValues += 1;
     }
     int payloadTagBits = 0;
@@ -2257,7 +2268,7 @@ class EnumTypeInfoBuilder {
         Stride = 1;
       return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
         Size, Alignment, Stride, NumExtraInhabitants,
-        BitwiseTakable, Cases);
+        BitwiseTakable, Cases, EffectivePayloadCases);
     }
 
     // This is a multi-payload enum that:
@@ -2291,7 +2302,7 @@ class EnumTypeInfoBuilder {
         // If there are no spare bits, use the "simple" tag-only implementation.
         return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
           Size, Alignment, Stride, NumExtraInhabitants,
-          BitwiseTakable, Cases);
+          BitwiseTakable, Cases, EffectivePayloadCases);
       }
 
 #if 0  // TODO: This should be !defined(NDEBUG)
@@ -2314,7 +2325,8 @@ class EnumTypeInfoBuilder {
       // Use compiler-provided spare bit information
       return TC.makeTypeInfo<MultiPayloadEnumTypeInfo>(
         Size, Alignment, Stride, NumExtraInhabitants,
-        BitwiseTakable, Cases, spareBitsMask);
+        BitwiseTakable, Cases, spareBitsMask,
+        EffectivePayloadCases);
     }
 
     // Either there was no compiler data or it didn't make sense
@@ -2339,13 +2351,14 @@ class EnumTypeInfoBuilder {
       // above only returns an empty mask when the mask is really empty,
       return TC.makeTypeInfo<TaggedMultiPayloadEnumTypeInfo>(
         Size, Alignment, Stride, NumExtraInhabitants,
-        BitwiseTakable, Cases);
+        BitwiseTakable, Cases, EffectivePayloadCases);
     } else {
       // General case can mix spare bits and extra discriminator
       // It obviously relies on having an accurate spare bit mask.
       return TC.makeTypeInfo<MultiPayloadEnumTypeInfo>(
         Size, Alignment, Stride, NumExtraInhabitants,
-        BitwiseTakable, Cases, spareBitsMask);
+        BitwiseTakable, Cases, spareBitsMask,
+        EffectivePayloadCases);
     }
   }
 };

validation-test/Reflection/reflect_Enum_values2.swift

@@ -0,0 +1,72 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values2
+// RUN: %target-codesign %t/reflect_Enum_values2
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values2 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+enum EnumB {
+    case a
+    case b(Int8)
+    case c(Int8)
+    case d(Void)
+    case e(Void)
+    case RIGHT(Int8)
+    case f(Int8)
+    case g(Void)
+    case LEFT(Int8, Int64)
+}
+
+class PlaceSummaryUnit {
+  let t: EnumB
+  init(t: EnumB) { self.t = t }
+}
+
+reflect(object: PlaceSummaryUnit(t: .RIGHT(5)))
+
+// CHECKALL: Reflecting an object.
+// CHECKALL-NEXT: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (class reflect_Enum_values2.PlaceSummaryUnit)
+
+// CHECKALL: Type info
+
+// TODO: Check the type layout for 64- and 32-bit targets
+
+reflect(enumValue: PlaceSummaryUnit(t: .RIGHT(5)).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
+// CHECKALL-NEXT: Value: .RIGHT(_)
+
+reflect(enumValue: PlaceSummaryUnit(t: .a).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
+// CHECKALL-NEXT: Value: .a
+
+reflect(enumValue: PlaceSummaryUnit(t: .LEFT(1,2)).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
+// CHECKALL-NEXT: Value: .LEFT(_)
+
+reflect(enumValue: PlaceSummaryUnit(t: .d(())).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (enum reflect_Enum_values2.EnumB)
+// CHECKALL-NEXT: Value: .d(_)
+
+doneReflecting()
+
+// CHECKALL: Done.
+

validation-test/Reflection/reflect_Enum_values3.swift

@@ -0,0 +1,85 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_values3
+// RUN: %target-codesign %t/reflect_Enum_values3
+
+// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_values3 | tee /dev/stderr | %FileCheck %s --check-prefix=CHECK%target-ptrsize --check-prefix=CHECKALL --dump-input=fail %add_num_extra_inhabitants
+
+// REQUIRES: objc_interop
+// REQUIRES: executable_test
+// UNSUPPORTED: use_os_stdlib
+
+import SwiftReflectionTest
+
+enum EnumB<T> {
+    case a
+    case b(Int8)
+    case c(T)
+    case d(Void)
+    case e(Void)
+    case RIGHT(Int8)
+    case f(Int8)
+    case g(Void)
+    case LEFT(Int8, Int64)
+}
+
+class PlaceSummaryUnit<T> {
+  let t: EnumB<T>
+  init(t: EnumB<T>) { self.t = t }
+}
+
+reflect(object: PlaceSummaryUnit<Int8>(t: .RIGHT(5)))
+
+// CHECKALL: Reflecting an object.
+// CHECKALL-NEXT: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_class reflect_Enum_values3.PlaceSummaryUnit
+// CHECKALL-NEXT:   (struct Swift.Int8))
+
+// CHECKALL: Type info
+
+// TODO: Check the type layout for 64- and 32-bit targets
+
+reflect(enumValue: PlaceSummaryUnit<Int8>(t: .RIGHT(5)).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
+// CHECKALL-NEXT:   (struct Swift.Int8))
+// CHECKALL-NEXT: Value: .RIGHT(_)
+
+reflect(enumValue: PlaceSummaryUnit<Int8>(t: .a).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
+// CHECKALL-NEXT:   (struct Swift.Int8))
+// CHECKALL-NEXT: Value: .a
+
+reflect(enumValue: PlaceSummaryUnit<Int8>(t: .c(7)).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
+// CHECKALL-NEXT:   (struct Swift.Int8))
+// CHECKALL-NEXT: Value: .c(_)
+
+reflect(enumValue: PlaceSummaryUnit<Int8>(t: .LEFT(1,2)).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
+// CHECKALL-NEXT:   (struct Swift.Int8))
+// CHECKALL-NEXT: Value: .LEFT(_)
+
+reflect(enumValue: PlaceSummaryUnit<Int8>(t: .d(())).t)
+
+// CHECKALL: Reflecting an enum value.
+// CHECKALL-NEXT: Type reference:
+// CHECKALL-NEXT: (bound_generic_enum reflect_Enum_values3.EnumB
+// CHECKALL-NEXT:   (struct Swift.Int8))
+// CHECKALL-NEXT: Value: .d(_)
+
+doneReflecting()
+
+// CHECKALL: Done.
+