Implement MultiPayloadEnum support for projectEnumValue (#30635)

This code rearchitects and simplifies the projectEnumValue support by
introducing a new `TypeInfo` subclass for each kind of enum, including trivial,
no-payload, single-payload, and three different classes for multi-payload enums:

* "UnsupportedEnum" that we don't understand.  This returns "don't know" answers for all requests in cases where the runtime lacks enough information to accurately handle a particular enum.

* MP Enums that only use a separate tag value.  This includes generic enums and other dynamic layouts, as well as enums whose payloads have no spare bits.

* MP Enums that use spare bits, possibly in addition to a separate tag.  This logic can only be used, of course, if we can in fact compute a spare bit mask that agrees with the compiler.

The final challenge is to choose one of the above three handlings for every MPE.  Currently, we do not have an accurate source of information for the spare bit mask, so we never choose the third option above.  We use the second option for dynamic MPE layouts (including generics) and the first for everything else.

TODO: Once we can arrange for the compiler to expose spare bit mask data, we'll be able to use that to drive more MPE cases.

Author: tbkka

include/swift/Reflection/ReflectionContext.h

@@ -711,176 +711,32 @@ class ReflectionContext
     }
   }
 
+  /// Projects the value of an enum.
+  ///
+  /// Takes the address and typeref for an enum and determines the
+  /// index of the currently-selected case within the enum.
+  /// You can use this index with `swift_reflection_childOfTypeRef`
+  /// to get detailed information about the specific case.
+  ///
+  /// Returns true if the enum case could be successfully determined.  In
+  /// particular, note that this code may return false for valid in-memory data
+  /// if the compiler used a strategy we do not yet understand.
   bool projectEnumValue(RemoteAddress EnumAddress,
                         const TypeRef *EnumTR,
                         int *CaseIndex) {
-    if (EnumTR == nullptr)
+    // Get the TypeInfo and sanity-check it
+    if (EnumTR == nullptr) {
       return false;
-    auto EnumTI = getTypeInfo(EnumTR);
-    if (EnumTI == nullptr)
-      return false;
-
-    auto EnumRecordTI = dyn_cast<const RecordTypeInfo>(EnumTI);
-    if (EnumRecordTI == nullptr)
-      return false;
-    auto EnumSize = EnumRecordTI->getSize();
-
-    auto Fields = EnumRecordTI->getFields();
-    auto FieldCount = Fields.size();
-    if (FieldCount == 0) {
-      return false;  // No fields?
-    }
-    if (FieldCount == 1) {
-      *CaseIndex = 0; // Only possible field
-      return true;
-    }
-
-    switch (EnumRecordTI->getRecordKind()) {
-
-    case RecordKind::NoPayloadEnum: {
-      if (EnumSize == 0) {
-        *CaseIndex = 0;
-        return true;
-      }
-      return getReader().readInteger(EnumAddress, EnumSize, CaseIndex);
-    }
-
-    case RecordKind::SinglePayloadEnum: {
-      FieldInfo PayloadCase = Fields[0];
-      if (!PayloadCase.TR)
-        return false;
-      unsigned long NonPayloadCaseCount = FieldCount - 1;
-      unsigned long PayloadExtraInhabitants = PayloadCase.TI.getNumExtraInhabitants();
-      unsigned Discriminator = 0;
-      auto PayloadSize = PayloadCase.TI.getSize();
-      if (NonPayloadCaseCount >= PayloadExtraInhabitants) {
-        // There are more cases than inhabitants, we need a separate discriminator.
-        auto TagInfo = getEnumTagCounts(PayloadSize, NonPayloadCaseCount, 1);
-        auto TagSize = TagInfo.numTagBytes;
-        auto TagAddress = RemoteAddress(EnumAddress.getAddressData() + PayloadSize);
-        if (!getReader().readInteger(TagAddress, TagSize, &Discriminator)) {
-          printf(">>>> readXI failed to read discriminator\n\n");
-          return false;
-        }
-      }
-
-      if (PayloadSize == 0) {
-        // Payload carries no information, so discriminator fully determines the case
-        *CaseIndex = Discriminator;
-        return true;
-      } else if (Discriminator == 0) {
-        // The payload area carries all the information...
-        if (PayloadExtraInhabitants == 0) {
-          *CaseIndex = 0;
-          return true;
-        }
-        int XITag = 0;
-        if (!PayloadCase.TI.readExtraInhabitantIndex(getReader(), EnumAddress, &XITag)) {
-          return false;
-        }
-        if (XITag < 0) { // Valid (not extra) inhabitant
-          *CaseIndex = 0; // Payload case is always #0
-          return true;
-        } else if ((unsigned)XITag <= NonPayloadCaseCount) {
-          *CaseIndex = XITag + 1;
-          return true;
-        }
-        return false;
-      } else {
-        // No payload: Payload area is reused for more cases
-        uint32_t PayloadTag = 0;
-        auto PayloadTagSize = std::min(PayloadSize, decltype(PayloadSize)(sizeof(PayloadTag)));
-        if (!getReader().readInteger(EnumAddress, PayloadTagSize, &PayloadTag)) {
-          return false;
-        }
-        auto XICases = 1U + PayloadExtraInhabitants; // Cases coded with XIs when discriminator = 0
-        auto PayloadCases = 1U << (PayloadTagSize * 8U);
-        *CaseIndex = XICases + (Discriminator - 1) * PayloadCases + PayloadTag;
-        return true;
-      }
     }
-
-    case RecordKind::MultiPayloadEnum: {
-      // Collect basic statistics about the enum
-      unsigned long PayloadCaseCount = 0;
-      unsigned long NonPayloadCaseCount = 0;
-      unsigned long PayloadSize = 0;
-      for (auto Field : Fields) {
-        if (Field.TR != 0) {
-          PayloadCaseCount += 1;
-          if (Field.TI.getSize() > PayloadSize) {
-            PayloadSize = Field.TI.getSize();
-          }
-        } else {
-          NonPayloadCaseCount += 1;
-        }
-      }
-      if (EnumSize > PayloadSize) {
-        // If the compiler laid this out with a separate tag, use that.
-        unsigned tag = 0;
-        auto TagSize = EnumSize - PayloadSize;
-        auto TagAddress = remote::RemoteAddress(EnumAddress.getAddressData() + PayloadSize);
-        if (!getReader().readInteger(TagAddress, TagSize, &tag)
-           || tag >= Fields.size()) {
-          return false;
-        }
-        if (tag < PayloadCaseCount) {
-          *CaseIndex = tag;
-          return true;
-        }
-        auto PayloadTagSize = std::min(PayloadSize, 4UL);
-        // Treat the tag as a page selector; payload carries the offset within the page
-        auto Page = tag - PayloadCaseCount;
-        // Zero for 32-bit because we'll never have more than one page
-        auto PageSize = PayloadTagSize >= 4 ? 0 : 1 << (PayloadSize * 8U);
-        auto PageStart = Page * PageSize;
-        unsigned PayloadTag;
-        if (!getReader().readInteger(EnumAddress, PayloadTagSize, &PayloadTag)) {
-          return false;
-        }
-        *CaseIndex = PageStart + PayloadTag + PayloadCaseCount;
-        return true;
-      } else {
-        // XXX TODO: If the payloads have common spare bits (e.g., all pointers)
-        // then use those to decode the case.
-        return false;
-      }
-      break;
-    }
-
-    default:
-      // Unknown record kind.
-      break;
-    }
-    return false;
-  }
-
-  bool getEnumCaseTypeRef(const TypeRef *EnumTR,
-                          unsigned CaseIndex,
-                          std::string &Name,
-                          const TypeRef **OutPayloadTR) {
-    *OutPayloadTR = nullptr;
-
-    if (EnumTR == nullptr)
-      return false;
-
-    auto EnumTI = getTypeInfo(EnumTR);
-    if (EnumTI == nullptr)
+    auto TI = getTypeInfo(EnumTR);
+    if (TI == nullptr) {
       return false;
-
-    auto EnumRecordTI = dyn_cast<const RecordTypeInfo>(EnumTI);
-    if (EnumRecordTI == nullptr)
-      return false;
-
-    auto NumCases = EnumRecordTI->getNumFields();
-    if (CaseIndex >= NumCases) {
+    }
+    auto EnumTI = dyn_cast<const EnumTypeInfo>(TI);
+    if (EnumTI == nullptr){
       return false;
-    } else {
-      const auto Case = EnumRecordTI->getFields()[CaseIndex];
-      Name = Case.Name;
-      *OutPayloadTR = Case.TR;
-      return true;
     }
+    return EnumTI->projectEnumValue(getReader(), EnumAddress, CaseIndex);
   }
 
   /// Return a description of the layout of a value with the given type.

include/swift/Reflection/TypeLowering.h

@@ -42,6 +42,23 @@ class EnumTypeInfoBuilder;
 class RecordTypeInfoBuilder;
 class ExistentialTypeInfoBuilder;
 
+enum class EnumKind : unsigned {
+  // An enum with no payload cases. The record will have no fields, but
+  // will have the correct size.
+  NoPayloadEnum,
+
+  // An enum with a single payload case and zero or more no-payload
+  // cases.  The no-payload cases may be encoded with an extra tag
+  // byte or as invalid payload values ("extra inhabitants").
+  SinglePayloadEnum,
+
+  // An enum with multiple payload cases and zero or more non-payload
+  // cases.  The selector that indicates what case is currently active
+  // may be encoded in unused "spare bits" common to all payloads and/or
+  // may use a separate tag byte.
+  MultiPayloadEnum,
+};
+
 enum class RecordKind : unsigned {
   Invalid,
 
@@ -51,18 +68,6 @@ enum class RecordKind : unsigned {
   // A Swift struct type.
   Struct,
 
-  // An enum with no payload cases. The record will have no fields, but
-  // will have the correct size.
-  NoPayloadEnum,
-
-  // An enum with a single payload case. The record consists of a single
-  // field, being the enum payload.
-  SinglePayloadEnum,
-
-  // An enum with multiple payload cases. The record consists of a multiple
-  // fields, one for each enum payload.
-  MultiPayloadEnum,
-
   // A Swift-native function is always a function pointer followed by a
   // retainable, nullable context pointer.
   ThickFunction,
@@ -107,6 +112,7 @@ enum class TypeInfoKind : unsigned {
   Record,
   Reference,
   Invalid,
+  Enum,
 };
 
 class TypeInfo {
@@ -209,6 +215,65 @@ class RecordTypeInfo : public TypeInfo {
   }
 };
 
+/// Enums
+class EnumTypeInfo : public TypeInfo {
+  EnumKind SubKind;
+  std::vector<FieldInfo> Cases;
+
+protected:
+  EnumTypeInfo(unsigned Size, unsigned Alignment,
+               unsigned Stride, unsigned NumExtraInhabitants,
+               bool BitwiseTakable,
+               EnumKind SubKind, const std::vector<FieldInfo> &Cases)
+    : TypeInfo(TypeInfoKind::Enum, Size, Alignment, Stride,
+               NumExtraInhabitants, BitwiseTakable),
+      SubKind(SubKind), Cases(Cases) {}
+
+public:
+  EnumKind getEnumKind() const { return SubKind; }
+  const std::vector<FieldInfo> &getCases() const { return Cases; }
+  unsigned getNumCases() const { return Cases.size(); }
+  unsigned getNumPayloadCases() const {
+    auto Cases = getCases();
+    return std::count_if(Cases.begin(), Cases.end(),
+                         [](const FieldInfo &Case){return Case.TR != 0;});
+  }
+  // Size of the payload area.
+  unsigned getPayloadSize() const {
+    return EnumTypeInfo::getPayloadSizeForCases(Cases);
+  }
+
+  static unsigned getPayloadSizeForCases(const std::vector<FieldInfo> &Cases) {
+    unsigned size = 0;
+    for (auto Case : Cases) {
+      if (Case.TR != 0 && Case.TI.getSize() > size) {
+        size = Case.TI.getSize();
+      }
+    }
+    return size;
+  }
+
+  // Returns true if this enum is `Optional`
+  // (This was factored out of a piece of code that was just
+  // checking the EnumKind.  This is vastly better than that,
+  // but could probably be improved further.)
+  bool isOptional() const {
+    return
+      SubKind == EnumKind::SinglePayloadEnum
+      && Cases.size() == 2
+      && Cases[0].Name == "some"
+      && Cases[1].Name == "none";
+  }
+
+  virtual bool projectEnumValue(remote::MemoryReader &reader,
+                                remote::RemoteAddress address,
+                                int *CaseIndex) const = 0;
+
+  static bool classof(const TypeInfo *TI) {
+    return TI->getKind() == TypeInfoKind::Enum;
+  }
+};
+
 /// References to classes, closure contexts and anything else with an
 /// 'isa' pointer
 class ReferenceTypeInfo : public TypeInfo {

include/swift/Remote/MemoryReader.h

@@ -62,21 +62,29 @@ class MemoryReader {
   }
 
   /// Attempts to read an integer of the specified size from the given
-  /// address in the remote process.
+  /// address in the remote process.  Following `storeEnumElement`
+  /// in EnumImpl.h, this reads arbitrary-size integers by ignoring
+  /// high-order bits that are outside the range of `IntegerType`.
   ///
-  /// Returns false if the operation failed, or the request size is
-  /// larger than the provided destination.
+  /// Returns false if the operation failed.
   template <typename IntegerType>
   bool readInteger(RemoteAddress address, size_t bytes, IntegerType *dest) {
     *dest = 0;
-    if ((bytes > sizeof(IntegerType))
-        || !readBytes(address, (uint8_t *)dest, bytes)) {
-      return false;
-    }
+    size_t readSize = std::min(bytes, sizeof(IntegerType));
     // FIXME: Assumes host and target have the same endianness.
     // TODO: Query DLQ for endianness of target, compare to endianness of host.
 #if defined(__BIG_ENDIAN__)
-    *dest >>= (sizeof(IntegerType) - bytes);
+    // Read low-order bits of source ...
+    if (!readBytes(address + (bytes - readSize), (uint8_t *)dest, readSize)) {
+      return false;
+    }
+    // ... align result to low-order bits of *dest
+    *dest >>= 8 * (sizeof(IntegerType) - readSize);
+#else
+    // Read from low-order bytes of integer
+    if (!readBytes(address, (uint8_t *)dest, readSize)) {
+      return false;
+    }
 #endif
     return true;
   }

include/swift/Remote/RemoteAddress.h

@@ -47,6 +47,18 @@ class RemoteAddress {
   uint64_t getAddressData() const {
     return Data;
   }
+
+  template<typename IntegerType>
+  RemoteAddress& operator+=(const IntegerType& rhs) {
+    Data += rhs;
+    return *this;
+  }
+
+  template<typename IntegerType>
+  friend RemoteAddress operator+(RemoteAddress lhs,
+                                 const IntegerType& rhs) {
+    return lhs += rhs;
+  }
 };
 
 /// A symbolic relocated absolute pointer value.

include/swift/SwiftRemoteMirror/SwiftRemoteMirror.h

@@ -241,6 +241,8 @@ int swift_reflection_projectExistential(SwiftReflectionContextRef ContextRef,
 ///
 /// Takes the address and typeref for an enum and determines the
 /// index of the currently-selected case within the enum.
+/// You can use this index with `swift_reflection_childOfTypeRef`
+/// to get detailed information about the specific case.
 ///
 /// Returns true if the enum case could be successfully determined.
 /// In particular, note that this code may fail for valid in-memory data
@@ -251,21 +253,6 @@ int swift_reflection_projectEnumValue(SwiftReflectionContextRef ContextRef,
                                       swift_typeref_t EnumTypeRef,
                                       int *CaseIndex);
 
-/// Finds information about a particular enum case.
-///
-/// Given an enum typeref and index of a case, returns:
-/// * Typeref of the associated payload or zero if there is no payload
-/// * Name of the case if known.
-///
-/// The Name points to a freshly-allocated C string on the heap.  You
-/// are responsible for freeing the string when you are finished.
-SWIFT_REMOTE_MIRROR_LINKAGE
-int swift_reflection_getEnumCaseTypeRef(SwiftReflectionContextRef ContextRef,
-                                        swift_typeref_t EnumTypeRef,
-                                        int CaseIndex,
-                                        char **CaseName,
-                                        swift_typeref_t *PayloadTypeRef);
-
 /// Dump a brief description of the typeref as a tree to stderr.
 SWIFT_REMOTE_MIRROR_LINKAGE
 void swift_reflection_dumpTypeRef(swift_typeref_t OpaqueTypeRef);