[Reflection] Add lightweight error handling to ReflectionContext

include/swift/RemoteInspection/ReflectionContext.h

@@ -22,6 +22,7 @@
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Object/COFF.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/Memory.h"
 #include "llvm/ADT/STLExtras.h"
 
@@ -1276,6 +1277,15 @@ class ReflectionContext
     }
   }
 
+  llvm::Expected<const TypeInfo &>
+  getTypeInfo(const TypeRef &TR, remote::TypeInfoProvider *ExternalTypeInfo) {
+    auto &TC = getBuilder().getTypeConverter();
+    const TypeInfo *TI = TC.getTypeInfo(&TR, ExternalTypeInfo);
+    if (!TI)
+      return llvm::createStringError(TC.takeLastError());
+    return *TI;
+  }
+
   /// Given a typeref, attempt to calculate the unaligned start of this
   /// instance's fields. For example, for a type without a superclass, the start
   /// of the instance fields would after the word for the isa pointer and the

include/swift/RemoteInspection/TypeLowering.h

@@ -373,8 +373,8 @@ class ArrayTypeInfo : public TypeInfo {
 class TypeConverter {
   TypeRefBuilder &Builder;
   std::vector<std::unique_ptr<const TypeInfo>> Pool;
-  llvm::DenseMap<std::pair<const TypeRef *, remote::TypeInfoProvider::IdType>,
-                 const TypeInfo *> Cache;
+  using KeyT = std::pair<const TypeRef *, remote::TypeInfoProvider::IdType>;
+  llvm::DenseMap<KeyT, const TypeInfo *> Cache;
   llvm::DenseSet<const TypeRef *> RecursionCheck;
   llvm::DenseMap<std::pair<unsigned, unsigned>,
                  const ReferenceTypeInfo *> ReferenceCache;
@@ -391,9 +391,25 @@ class TypeConverter {
   const TypeInfo *DefaultActorStorageTI = nullptr;
   const TypeInfo *EmptyTI = nullptr;
 
+  /// Used for lightweight error handling. We don't have access to
+  /// llvm::Expected<> here, so TypeConverter just stores a pointer to the last
+  /// encountered error instead that is stored in the cache.
+  using TCError = std::pair<const char *, const TypeRef *>;
+  TCError LastError = {nullptr, nullptr};
+  std::unique_ptr<llvm::DenseMap<KeyT, TCError>> ErrorCache;
+
 public:
   explicit TypeConverter(TypeRefBuilder &Builder) : Builder(Builder) {}
 
+  /// Called by LLDB.
+  void enableErrorCache() {
+    ErrorCache = std::make_unique<llvm::DenseMap<KeyT, TCError>>();
+  }
+  void setError(const char *msg, const TypeRef *TR) { LastError = {msg, TR}; }
+
+  /// Retreive the error and reset it.
+  std::string takeLastError();
+
   TypeRefBuilder &getBuilder() { return Builder; }
 
   /// Tests if the type is concrete enough that its size is known.
@@ -474,8 +490,10 @@ class RecordTypeInfoBuilder {
     : TC(TC), Size(0), Alignment(1), NumExtraInhabitants(0),
       BitwiseTakable(true), Kind(Kind), Empty(true), Invalid(false) {}
 
-  bool isInvalid() const {
-    return Invalid;
+  bool isInvalid() const { return Invalid; }
+  void markInvalid(const char *msg, const TypeRef *TR = nullptr) {
+    Invalid = true;
+    TC.setError(msg, TR);
   }
 
   unsigned addField(unsigned fieldSize, unsigned fieldAlignment,

stdlib/public/RemoteInspection/TypeLowering.cpp

@@ -1160,6 +1160,12 @@ class ExistentialTypeInfoBuilder {
   unsigned WitnessTableCount;
   bool Invalid;
 
+  void markInvalid(const char *msg, const TypeRef *TR = nullptr) {
+    Invalid = true;
+    DEBUG_LOG(fprintf(stderr, "%s\n", msg); if (TR) TR->dump());
+    TC.setError(msg, TR);
+  }
+
   bool isSingleError() const {
     // If we changed representation, it means we added a
     // superclass constraint or an AnyObject member.
@@ -1191,8 +1197,7 @@ class ExistentialTypeInfoBuilder {
       auto *NTD = dyn_cast<NominalTypeRef>(P);
       auto *OP = dyn_cast<ObjCProtocolTypeRef>(P);
       if (!NTD && !OP) {
-        DEBUG_LOG(fprintf(stderr, "Bad protocol: "); P->dump())
-        Invalid = true;
+        markInvalid("bad protocol", P);
         continue;
       }
 
@@ -1204,8 +1209,7 @@ class ExistentialTypeInfoBuilder {
 
       auto FD = TC.getBuilder().getFieldDescriptor(P);
       if (FD == nullptr) {
-        DEBUG_LOG(fprintf(stderr, "No field descriptor: "); P->dump())
-        Invalid = true;
+        markInvalid("no field descriptor", P);
         continue;
       }
 
@@ -1224,16 +1228,12 @@ class ExistentialTypeInfoBuilder {
             // layering.
             auto *SuperclassTI = TC.getTypeInfo(Superclass, nullptr);
             if (SuperclassTI == nullptr) {
-              DEBUG_LOG(fprintf(stderr, "No TypeInfo for superclass: ");
-                        Superclass->dump());
-              Invalid = true;
+              markInvalid("no type info for superclass", Superclass);
               continue;
             }
 
             if (!isa<ReferenceTypeInfo>(SuperclassTI)) {
-              DEBUG_LOG(fprintf(stderr, "Superclass not a reference type: ");
-                        SuperclassTI->dump());
-              Invalid = true;
+              markInvalid("superclass not a reference type", Superclass);
               continue;
             }
 
@@ -1252,7 +1252,7 @@ class ExistentialTypeInfoBuilder {
         case FieldDescriptorKind::Enum:
         case FieldDescriptorKind::MultiPayloadEnum:
         case FieldDescriptorKind::Class:
-          Invalid = true;
+          markInvalid("unexpected field descriptor kind");
           continue;
       }
     }
@@ -1283,8 +1283,7 @@ class ExistentialTypeInfoBuilder {
       if (!isa<NominalTypeRef>(T) &&
           !isa<BoundGenericTypeRef>(T) &&
           !isa<ObjCClassTypeRef>(T)) {
-        DEBUG_LOG(fprintf(stderr, "Bad existential member: "); T->dump())
-        Invalid = true;
+        markInvalid("bad existential member", T);
         return;
       }
 
@@ -1296,8 +1295,7 @@ class ExistentialTypeInfoBuilder {
 
       const auto &FD = TC.getBuilder().getFieldDescriptor(T);
       if (FD == nullptr) {
-        DEBUG_LOG(fprintf(stderr, "No field descriptor: "); T->dump())
-        Invalid = true;
+        markInvalid("no field descriptor", T);
         return;
       }
 
@@ -1313,8 +1311,7 @@ class ExistentialTypeInfoBuilder {
         break;
 
       default:
-        DEBUG_LOG(fprintf(stderr, "Bad existential member: "); T->dump())
-        Invalid = true;
+        markInvalid("bad existential member", T);
         return;
       }
     }
@@ -1324,10 +1321,6 @@ class ExistentialTypeInfoBuilder {
     Representation = ExistentialTypeRepresentation::Class;
   }
 
-  void markInvalid() {
-    Invalid = true;
-  }
-
   const TypeInfo *build(remote::TypeInfoProvider *ExternalTypeInfo) {
     examineProtocols();
 
@@ -1407,7 +1400,7 @@ class ExistentialTypeInfoBuilder {
 
     if (ObjC) {
       if (WitnessTableCount > 0) {
-        DEBUG_LOG(fprintf(stderr, "@objc existential with witness tables\n"));
+        markInvalid("@objc existential with witness tables");
         return nullptr;
       }
 
@@ -1480,8 +1473,7 @@ void RecordTypeInfoBuilder::addField(
     remote::TypeInfoProvider *ExternalTypeInfo) {
   const TypeInfo *TI = TC.getTypeInfo(TR, ExternalTypeInfo);
   if (TI == nullptr) {
-    DEBUG_LOG(fprintf(stderr, "No TypeInfo for field type: "); TR->dump());
-    Invalid = true;
+    markInvalid("no TypeInfo for field type", TR);
     return;
   }
 
@@ -1563,6 +1555,18 @@ const ReferenceTypeInfo *TypeConverter::getReferenceTypeInfo(
   return TI;
 }
 
+std::string TypeConverter::takeLastError() {
+  if (!LastError.first)
+    return {};
+  std::stringstream s;
+  s << LastError.first << ": ";
+  if (LastError.second)
+    LastError.second->dump(s);
+
+  LastError = {nullptr, nullptr};
+  return s.str();
+}
+
 /// Thin functions consist of a function pointer. We do not use
 /// Builtin.RawPointer here, since the extra inhabitants differ.
 const TypeInfo *
@@ -1996,6 +2000,12 @@ class EnumTypeInfoBuilder {
   std::vector<FieldInfo> Cases;
   bool Invalid;
 
+  void markInvalid(const char *msg, const TypeRef *TR = nullptr) {
+    Invalid = true;
+    DEBUG_LOG(fprintf(stderr, "%s\n", msg); if (TR) TR->dump());
+    TC.setError(msg, TR);
+  }
+
   const TypeRef *getCaseTypeRef(FieldTypeInfo Case) {
     // An indirect case is like a payload case with an argument type
     // of Builtin.NativeObject.
@@ -2015,8 +2025,7 @@ class EnumTypeInfoBuilder {
   void addCase(const std::string &Name, const TypeRef *TR,
                const TypeInfo *TI) {
     if (TI == nullptr) {
-      DEBUG_LOG(fprintf(stderr, "No TypeInfo for case type: "); TR->dump());
-      Invalid = true;
+      markInvalid("no type info for case type", TR);
       static TypeInfo emptyTI;
       Cases.push_back({Name, /*offset=*/0, /*value=*/-1, TR, emptyTI});
     } else {
@@ -2045,7 +2054,7 @@ class EnumTypeInfoBuilder {
 
     std::vector<FieldTypeInfo> Fields;
     if (!TC.getBuilder().getFieldTypeRefs(TR, FD, ExternalTypeInfo, Fields)) {
-      Invalid = true;
+      markInvalid("cannot not get field types", TR);
       return nullptr;
     }
 
@@ -2060,7 +2069,7 @@ class EnumTypeInfoBuilder {
         auto *CaseTI = TC.getTypeInfo(CaseTR, ExternalTypeInfo);
         if (CaseTI == nullptr) {
           // We don't have typeinfo; something is very broken.
-          Invalid = true;
+          markInvalid("no type info for single enum case", CaseTR);
           return nullptr;
 	} else if (Case.Indirect) {
 	  // An indirect case is non-empty (it stores a pointer)
@@ -2655,8 +2664,11 @@ TypeConverter::getTypeInfo(const TypeRef *TR,
       ExternalTypeInfo ? ExternalTypeInfo->getId() : 0;
   // See if we already computed the result
   auto found = Cache.find({TR, ExternalTypeInfoId});
-  if (found != Cache.end())
+  if (found != Cache.end()) {
+    if (!found->second && ErrorCache)
+      LastError = ErrorCache->lookup({TR, ExternalTypeInfoId});
     return found->second;
+  }
 
   // Detect invalid recursive value types (IRGen should not emit
   // them in the first place, but there might be bugs)
@@ -2668,6 +2680,11 @@ TypeConverter::getTypeInfo(const TypeRef *TR,
   // Compute the result and cache it
   auto *TI = LowerType(*this, ExternalTypeInfo).visit(TR);
   Cache.insert({{TR, ExternalTypeInfoId}, TI});
+  if (!TI && ErrorCache) {
+    if (!LastError.first)
+      LastError = {"cannot decode or find", TR};
+    ErrorCache->insert({{TR, ExternalTypeInfoId}, LastError});
+  }
 
   RecursionCheck.erase(TR);