swift-reflection-test: Increase reading robustness

- Check return values from `read`
- Make the reflstr section optional, as it can be stripped by
  -strip-reflection-names.

Author: bitjammer

include/swift/Reflection/ReflectionContext.h

@@ -564,6 +564,8 @@ class ReflectionContext {
     for (auto Info : ReflectionInfos) {
       for (auto &FieldDescriptor : Info.fieldmd) {
         auto CandidateMangledName = FieldDescriptor.MangledTypeName.get();
+        if (!CandidateMangledName)
+          continue;
         if (MangledName.compare(CandidateMangledName) != 0)
           continue;
         for (auto &Field : FieldDescriptor) {

tools/swift-reflection-test/swift-reflection-test.cpp

@@ -204,47 +204,77 @@ static int doDumpReflectionSections(std::string BinaryFilename,
 }
 
 namespace {
+template <typename Runtime>
 struct Section {
-  addr_t StartAddress;
-  addr_t Size;
-
-  addr_t getEndAddress() const {
+  using StoredPointer = typename Runtime::StoredPointer;
+  StoredPointer StartAddress;
+  StoredPointer Size;
+  StoredPointer getEndAddress() const {
     return StartAddress + Size;
   }
 };
 
+template <typename Runtime>
 struct RemoteReflectionInfo {
+  using StoredPointer = typename Runtime::StoredPointer;
+  using StoredSize = typename Runtime::StoredSize;
+
   const std::string ImageName;
-  const Section fieldmd;
-  const Section assocty;
-  const Section reflstr;
-  const Section typeref;
-  const addr_t StartAddress;
-  const size_t TotalSize;
-
-  RemoteReflectionInfo(std::string ImageName, Section fieldmd, Section assocty,
-                       Section reflstr, Section typeref)
-    : fieldmd(fieldmd), assocty(assocty), reflstr(reflstr), typeref(typeref),
+  const Section<Runtime> fieldmd;
+  const Section<Runtime> assocty;
+  const llvm::Optional<Section<Runtime>> reflstr;
+  const Section<Runtime> typeref;
+  const StoredPointer StartAddress;
+  const StoredSize TotalSize;
+
+  RemoteReflectionInfo(std::string ImageName,
+                       Section<Runtime> fieldmd,
+                       Section<Runtime> assocty,
+                       llvm::Optional<Section<Runtime>> reflstr,
+                       Section<Runtime> typeref)
+    : ImageName(ImageName),
+      fieldmd(fieldmd),
+      assocty(assocty),
+      reflstr(reflstr),
+      typeref(typeref),
       StartAddress(std::min({
-        fieldmd.StartAddress, typeref.StartAddress,
-        reflstr.StartAddress, assocty.StartAddress})),
-      TotalSize(std::max({fieldmd.getEndAddress(), assocty.getEndAddress(),
-                         reflstr.getEndAddress(), typeref.getEndAddress()}) - StartAddress) {}
+        fieldmd.StartAddress,
+        typeref.StartAddress,
+        reflstr.hasValue()
+          ? reflstr.getValue().StartAddress
+          : fieldmd.StartAddress,
+        assocty.StartAddress})),
+      TotalSize(std::max({
+        fieldmd.getEndAddress(),
+        assocty.getEndAddress(),
+        reflstr.hasValue()
+          ? reflstr.getValue().getEndAddress()
+          : fieldmd.getEndAddress(),
+        typeref.getEndAddress()
+      }) - StartAddress) {}
 };
 }
 
-template <typename StoredPointer>
+template <typename Runtime>
 class PipeMemoryReader : public MemoryReader {
+  using StoredPointer = typename Runtime::StoredPointer;
+
   static constexpr size_t ReadEnd = 0;
   static constexpr size_t WriteEnd = 1;
-  static constexpr size_t ParentEnd = 1;
-  static constexpr size_t ChildEnd = 0;
 
   int to_child[2];
   int from_child[2];
 
 public:
 
+  int getParentReadFD() const {
+    return from_child[ReadEnd];
+  }
+
+  int getChildWriteFD() const {
+    return from_child[WriteEnd];
+  }
+
   int getParentWriteFD() const {
     return to_child[WriteEnd];
   }
@@ -253,13 +283,6 @@ class PipeMemoryReader : public MemoryReader {
     return to_child[ReadEnd];
   }
 
-  int getParentReadFD() const {
-    return from_child[ReadEnd];
-  }
-
-  int getChildWriteFD() const {
-    return from_child[WriteEnd];
-  }
 
   PipeMemoryReader() {
     if (pipe(to_child))
@@ -278,19 +301,33 @@ class PipeMemoryReader : public MemoryReader {
     return 8;
   }
 
+  template <typename T>
+  void collectBytesFromPipe(T *Value, size_t Size) {
+    auto Dest = reinterpret_cast<uint8_t *>(&Value);
+    while (Size) {
+      auto bytesRead = read(getParentReadFD(), Value, Size);
+      if (bytesRead <= 0)
+        errorAndExit("collectBytesFromPipe");
+      Size -= bytesRead;
+      Dest += bytesRead;
+    }
+  }
+
   bool readBytes(addr_t Address, uint8_t *Dest, uint64_t Size) override {
+
+    StoredPointer TargetAddress = (StoredPointer)Address;
     write(getParentWriteFD(), REQUEST_READ_BYTES, 2);
-    write(getParentWriteFD(), &Address, sizeof(Address));
+    write(getParentWriteFD(), &TargetAddress, sizeof(TargetAddress));
     write(getParentWriteFD(), &Size, sizeof(Size));
-    auto bytesRead = read(getParentReadFD(), Dest, Size);
-    return bytesRead == (int64_t)Size;
+    collectBytesFromPipe(Dest, Size);
+    return true;
   }
 
-  uint64_t getParentWriteFD(addr_t Address) {
+  uint64_t getParentWriteFD(StoredPointer Address) {
     write(getParentWriteFD(), REQUEST_STRING_LENGTH, 2);
     StoredPointer Length;
     write(getParentWriteFD(), &Address, sizeof(Address));
-    read(getParentReadFD(), &Length, sizeof(Length));
+    collectBytesFromPipe(&Length, sizeof(Length));
     return static_cast<uint64_t>(Length);
   }
 
@@ -299,14 +336,14 @@ class PipeMemoryReader : public MemoryReader {
     write(getParentWriteFD(), REQUEST_SYMBOL_ADDRESS, 2);
     write(getParentWriteFD(), SymbolName.c_str(), SymbolName.size());
     write(getParentWriteFD(), "\n", 1);
-    read(getParentReadFD(), &Address, sizeof(Address));
-    return static_cast<addr_t>(Address);
+    collectBytesFromPipe(&Address, sizeof(Address));
+    return static_cast<StoredPointer>(Address);
   }
 
-  addr_t receiveInstanceAddress() {
+  StoredPointer receiveInstanceAddress() {
     write(getParentWriteFD(), REQUEST_INSTANCE_ADDRESS, 2);
-    addr_t InstanceAddress = 0;
-    read(getParentReadFD(), &InstanceAddress, sizeof(InstanceAddress));
+    StoredPointer InstanceAddress = 0;
+    collectBytesFromPipe(&InstanceAddress, sizeof(InstanceAddress));
     return InstanceAddress;
   }
 
@@ -316,50 +353,52 @@ class PipeMemoryReader : public MemoryReader {
 
   uint8_t receivePointerSize() {
     write(getParentWriteFD(), REQUEST_POINTER_SIZE, 2);
-    uint8_t PointerSize;
-    read(getParentReadFD(), &PointerSize, sizeof(PointerSize));
+    uint8_t PointerSize = 0;
+    collectBytesFromPipe(&PointerSize, sizeof(PointerSize));
     return PointerSize;
   }
 
   std::vector<ReflectionInfo> receiveReflectionInfo() {
     write(getParentWriteFD(), REQUEST_REFLECTION_INFO, 2);
     uint64_t NumReflectionInfos = 0;
-    read(getParentReadFD(), &NumReflectionInfos, sizeof(NumReflectionInfos));
+    collectBytesFromPipe(&NumReflectionInfos, sizeof(NumReflectionInfos));
 
-    std::vector<RemoteReflectionInfo> RemoteInfos;
+    std::vector<RemoteReflectionInfo<Runtime>> RemoteInfos;
     for (uint64_t i = 0; i < NumReflectionInfos; ++i) {
       uint64_t ImageNameLength;
-      read(getParentReadFD(), &ImageNameLength, sizeof(ImageNameLength));
+      collectBytesFromPipe(&ImageNameLength, sizeof(ImageNameLength));
       char c;
       std::string ImageName;
       for (uint64_t i = 0; i < ImageNameLength; ++i) {
-        read(getParentReadFD(), &c, 1);
+        collectBytesFromPipe(&c, 1);
         ImageName.push_back(c);
       }
 
-      addr_t fieldmd_start;
-      addr_t fieldmd_size;
-      addr_t typeref_start;
-      addr_t typeref_size;
-      addr_t reflstr_start;
-      addr_t reflstr_size;
-      addr_t assocty_start;
-      addr_t assocty_size;
-
-      read(getParentReadFD(), &fieldmd_start, sizeof(fieldmd_start));
-      read(getParentReadFD(), &fieldmd_size, sizeof(fieldmd_size));
-      read(getParentReadFD(), &typeref_start, sizeof(typeref_start));
-      read(getParentReadFD(), &typeref_size, sizeof(typeref_size));
-      read(getParentReadFD(), &reflstr_start, sizeof(reflstr_start));
-      read(getParentReadFD(), &reflstr_size, sizeof(reflstr_size));
-      read(getParentReadFD(), &assocty_start, sizeof(assocty_start));
-      read(getParentReadFD(), &assocty_size, sizeof(assocty_size));
+      StoredPointer fieldmd_start;
+      StoredPointer fieldmd_size;
+      StoredPointer typeref_start;
+      StoredPointer typeref_size;
+      StoredPointer reflstr_start;
+      StoredPointer reflstr_size;
+      StoredPointer assocty_start;
+      StoredPointer assocty_size;
+
+      collectBytesFromPipe(&fieldmd_start, sizeof(fieldmd_start));
+      collectBytesFromPipe(&fieldmd_size, sizeof(fieldmd_size));
+      collectBytesFromPipe(&typeref_start, sizeof(typeref_start));
+      collectBytesFromPipe(&typeref_size, sizeof(typeref_size));
+      collectBytesFromPipe(&reflstr_start, sizeof(reflstr_start));
+      collectBytesFromPipe(&reflstr_size, sizeof(reflstr_size));
+      collectBytesFromPipe(&assocty_start, sizeof(assocty_start));
+      collectBytesFromPipe(&assocty_size, sizeof(assocty_size));
 
       RemoteInfos.push_back({
         ImageName,
         {fieldmd_start, fieldmd_size},
         {typeref_start, typeref_size},
-        {reflstr_start, reflstr_size},
+        reflstr_size > 0
+          ? llvm::Optional<Section<Runtime>>({reflstr_start, reflstr_size})
+          : llvm::None,
         {assocty_start, assocty_size},
       });
     }
@@ -369,29 +408,39 @@ class PipeMemoryReader : public MemoryReader {
 
       auto buffer = (uint8_t *)malloc(RemoteInfo.TotalSize);
 
-      readBytes(RemoteInfo.StartAddress, buffer, RemoteInfo.TotalSize);
-
-      auto fieldmd_base = buffer + RemoteInfo.fieldmd.StartAddress - RemoteInfo.StartAddress;
-      auto typeref_base = buffer + RemoteInfo.typeref.StartAddress - RemoteInfo.StartAddress;
-      auto reflstr_base = buffer + RemoteInfo.reflstr.StartAddress - RemoteInfo.StartAddress;
-      auto assocty_base = buffer + RemoteInfo.assocty.StartAddress - RemoteInfo.StartAddress;
+      if (!readBytes(RemoteInfo.StartAddress, buffer, RemoteInfo.TotalSize))
+        errorAndExit("Couldn't read reflection information");
+
+      auto fieldmd_base
+        = buffer + RemoteInfo.fieldmd.StartAddress - RemoteInfo.StartAddress;
+      auto typeref_base
+        = buffer + RemoteInfo.typeref.StartAddress - RemoteInfo.StartAddress;
+      auto reflstr_base
+        = RemoteInfo.reflstr.hasValue()
+          ? buffer + RemoteInfo.reflstr.getValue().StartAddress
+             - RemoteInfo.StartAddress
+          : 0;
+      auto assocty_base
+        = buffer + RemoteInfo.assocty.StartAddress - RemoteInfo.StartAddress;
       ReflectionInfo Info {
         RemoteInfo.ImageName,
         {fieldmd_base, fieldmd_base + RemoteInfo.fieldmd.Size},
         {typeref_base, typeref_base + RemoteInfo.typeref.Size},
-        {reflstr_base, reflstr_base + RemoteInfo.reflstr.Size},
+        {reflstr_base, reflstr_base + (RemoteInfo.reflstr.hasValue()
+          ? RemoteInfo.reflstr.getValue().Size
+          : 0)},
         {assocty_base, assocty_base + RemoteInfo.assocty.Size},
       };
       Infos.push_back(Info);
     }
     return Infos;
   }
 
-  uint64_t getStringLength(addr_t Address) {
+  uint64_t getStringLength(StoredPointer Address) {
     write(getParentWriteFD(), REQUEST_STRING_LENGTH, 2);
     StoredPointer Length;
     write(getParentWriteFD(), &Address, sizeof(Address));
-    read(getParentReadFD(), &Length, sizeof(Length));
+    collectBytesFromPipe(&Length, sizeof(Length));
     return static_cast<uint64_t>(Length);
   }
 
@@ -411,7 +460,7 @@ template <typename Runtime>
 static int doDumpHeapInstance(std::string BinaryFilename) {
   using StoredPointer = typename Runtime::StoredPointer;
 
-  PipeMemoryReader<StoredPointer> Pipe;
+  PipeMemoryReader<Runtime> Pipe;
 
   pid_t pid = fork();
   switch (pid) {
@@ -436,12 +485,12 @@ static int doDumpHeapInstance(std::string BinaryFilename) {
       if (PointerSize != Runtime::PointerSize)
         errorAndExit("Child process had unexpected architecture");
 
-      addr_t instance = Pipe.receiveInstanceAddress();
+      StoredPointer instance = Pipe.receiveInstanceAddress();
       assert(instance);
       std::cerr << "Parent: instance pointer in child address space: 0x";
       std::cerr << std::hex << instance << std::endl;
 
-      addr_t isa;
+      StoredPointer isa;
       if (!Pipe.readInteger(instance, &isa))
         errorAndExit("Couldn't get heap object's metadata address");