Minidump: Add support for the MemoryList stream

[labath]

Summary

the stream format is exactly the same as for ThreadList and ModuleList
streams, only the entry types are slightly different, so the changes in
this patch are just straight-forward applications of established
patterns.

[jhenderson]

include/llvm/Object/Minidump.h

85–86

These two lines talk about threads. Is that a copy/paste error?

include/llvm/ObjectYAML/MinidumpYAML.h

92

Would it make more sense to call this ParsedMermoryList, to match the StreamType?

test/tools/obj2yaml/basic-minidump.yaml

54–57

I'd probably find this neater if the Indentation of values for each entry were more consistent, but I'm not too fussed.

Also, in the ThreadList above, the Content is not quoted, but here it is. Please standardise it on one or the other.

unittests/Object/MinidumpTest.cpp

483

thread? Copy/paste error?

[labath]

Address review comments.

include/llvm/Object/Minidump.h

85–86

Yep, sorry. This is the downside of "straight-forward applications of established
patterns" :/

include/llvm/ObjectYAML/MinidumpYAML.h

92

Not really, because this represents only one entry in the MemoryList stream, and not list as a whole. However, we could call it ParsedMemoryDescriptor, as that's the type of the list entries.

test/tools/obj2yaml/basic-minidump.yaml

54–57

Done. The different quoting of is actually a relict of how obj2yaml prints BinaryRef values (they omit quotes if the data happens to be contain hex (A-F) characters). Do you think it would be worth making this output more consistent too?

[jhenderson]

LGTM, though I haven't bothered to inspect the detail of the minidump format. I'll leave it up to you whether you think it's worth getting a separate pair of eyes.

test/tools/obj2yaml/basic-minidump.yaml

54–57

Probably, but that sounds like an unrelated changes.

[labath]

Thanks. This is very straight-forward, so I don't think another set of eyes is needed (though I expect Greg is keeping at least one eye on this).

include/llvm/Object/Minidump.h

public:
   /// returned if the file does not contain this stream, or if the stream is
   /// not large enough to contain the number of threads declared in the stream
   /// header. The consistency of the Thread entries themselves is not checked in
   /// any way.
   Expected<ArrayRef<minidump::Thread>> getThreadList() const {
     return getListStream<minidump::Thread>(minidump::StreamType::ThreadList);
   }
 
+  /// Returns the list of memory ranges embedded in the MemoryList stream. An
+  /// error is returned if the file does not contain this stream, or if the
+  /// stream is not large enough to contain the number of memory descriptors
+  /// declared in the stream header. The consistency of the MemoryDescriptor
+  /// entries themselves is not checked in any way.
+  Expected<ArrayRef<minidump::MemoryDescriptor>> getMemoryList() const {
+    return getListStream<minidump::MemoryDescriptor>(
+        minidump::StreamType::MemoryList);
+  }
+
 private:
   static Error createError(StringRef Str) {
     return make_error<GenericBinaryError>(Str, object_error::parse_failed);
   }
 
   static Error createEOFError() {
     return make_error<GenericBinaryError>("Unexpected EOF",
                                           object_error::unexpected_eof);

include/llvm/ObjectYAML/MinidumpYAML.h

 /// corresponds to the Stream Type field in the minidump file. The "Kind" field
 /// specifies how are we going to treat it. For highly specialized streams (e.g.
 /// SystemInfo), there is a 1:1 mapping between Types and Kinds, but in general
 /// one stream Kind can be used to represent multiple stream Types (e.g. any
 /// unrecognised stream Type will be handled via RawContentStream). The mapping
 /// from Types to Kinds is fixed and given by the static getKind function.
 struct Stream {
   enum class StreamKind {
+    MemoryList,
     ModuleList,
     RawContent,
     SystemInfo,
     TextContent,
     ThreadList,
   };
 
   Stream(StreamKind Kind, minidump::StreamType Type) : Kind(Kind), Type(Type) {}

 struct ParsedThread {
   static constexpr Stream::StreamKind Kind = Stream::StreamKind::ThreadList;
   static constexpr minidump::StreamType Type = minidump::StreamType::ThreadList;
 
   minidump::Thread Entry;
   yaml::BinaryRef Stack;
   yaml::BinaryRef Context;
 };
+
+/// A structure containing all data describing a single memory region.
+struct ParsedMemoryDescriptor {
+  static constexpr Stream::StreamKind Kind = Stream::StreamKind::MemoryList;
+  static constexpr minidump::StreamType Type = minidump::StreamType::MemoryList;
+
+  minidump::MemoryDescriptor Entry;
+  yaml::BinaryRef Content;
+};
 } // namespace detail
 
 using ModuleListStream = detail::ListStream<detail::ParsedModule>;
 using ThreadListStream = detail::ListStream<detail::ParsedThread>;
+using MemoryListStream = detail::ListStream<detail::ParsedMemoryDescriptor>;
 
 /// A minidump stream represented as a sequence of hex bytes. This is used as a
 /// fallback when no other stream kind is suitable.
 struct RawContentStream : public Stream {
   yaml::BinaryRef Content;
   yaml::Hex32 Size;
 
   RawContentStream(minidump::StreamType Type, ArrayRef<uint8_t> Content = {})

 LLVM_YAML_DECLARE_ENUM_TRAITS(llvm::minidump::StreamType)
 
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::ArmInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::OtherInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::VSFixedFileInfo)
 
 LLVM_YAML_DECLARE_MAPPING_TRAITS(
+    llvm::MinidumpYAML::MemoryListStream::entry_type)
+LLVM_YAML_DECLARE_MAPPING_TRAITS(
     llvm::MinidumpYAML::ModuleListStream::entry_type)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(
     llvm::MinidumpYAML::ThreadListStream::entry_type)
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr<llvm::MinidumpYAML::Stream>)
+LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::MemoryListStream::entry_type)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ModuleListStream::entry_type)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::MinidumpYAML::ThreadListStream::entry_type)
 
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::MinidumpYAML::Object)
 
 #endif // LLVM_OBJECTYAML_MINIDUMPYAML_H

lib/Object/Minidump.cpp

if (ListOffset + sizeof(T) * ListSize < OptionalStream->size())
     ListOffset = 8;
 
   return getDataSliceAs<T>(*OptionalStream, ListOffset, ListSize);
 }
 template Expected<ArrayRef<Module>>
     MinidumpFile::getListStream(StreamType) const;
 template Expected<ArrayRef<Thread>>
     MinidumpFile::getListStream(StreamType) const;
+template Expected<ArrayRef<MemoryDescriptor>>
+    MinidumpFile::getListStream(StreamType) const;
 
 Expected<ArrayRef<uint8_t>>
 MinidumpFile::getDataSlice(ArrayRef<uint8_t> Data, size_t Offset, size_t Size) {
   // Check for overflow.
   if (Offset + Size < Offset || Offset + Size < Size ||
       Offset + Size > Data.size())
     return createEOFError();
   return Data.slice(Offset, Size);

lib/ObjectYAML/MinidumpYAML.cpp

static inline void mapOptionalHex(yaml::IO &IO, const char *Key,
                                   typename EndianType::value_type Default) {
   mapOptionalAs<typename HexType<EndianType>::type>(IO, Key, Val, Default);
 }
 
 Stream::~Stream() = default;
 
 Stream::StreamKind Stream::getKind(StreamType Type) {
   switch (Type) {
+  case StreamType::MemoryList:
+    return StreamKind::MemoryList;
   case StreamType::ModuleList:
     return StreamKind::ModuleList;
   case StreamType::SystemInfo:
     return StreamKind::SystemInfo;
   case StreamType::LinuxCPUInfo:
   case StreamType::LinuxProcStatus:
   case StreamType::LinuxLSBRelease:
   case StreamType::LinuxCMDLine:
   case StreamType::LinuxMaps:
   case StreamType::LinuxProcStat:
   case StreamType::LinuxProcUptime:
     return StreamKind::TextContent;
   case StreamType::ThreadList:
     return StreamKind::ThreadList;
   default:
     return StreamKind::RawContent;
   }
 }
 
 std::unique_ptr<Stream> Stream::create(StreamType Type) {
   StreamKind Kind = getKind(Type);
   switch (Kind) {
+  case StreamKind::MemoryList:
+    return llvm::make_unique<MemoryListStream>();
   case StreamKind::ModuleList:
     return llvm::make_unique<ModuleListStream>();
   case StreamKind::RawContent:
     return llvm::make_unique<RawContentStream>(Type);
   case StreamKind::SystemInfo:
     return llvm::make_unique<SystemInfoStream>();
   case StreamKind::TextContent:
     return llvm::make_unique<TextContentStream>(Type);

 }
 
 static StringRef streamValidate(RawContentStream &Stream) {
   if (Stream.Size.value < Stream.Content.binary_size())
     return "Stream size must be greater or equal to the content size";
   return "";
 }
 
+void yaml::MappingTraits<MemoryListStream::entry_type>::mapping(
+    IO &IO, MemoryListStream::entry_type &Range) {
+  MappingContextTraits<MemoryDescriptor, yaml::BinaryRef>::mapping(
+      IO, Range.Entry, Range.Content);
+}
+
+static void streamMapping(yaml::IO &IO, MemoryListStream &Stream) {
+  IO.mapRequired("Memory Ranges", Stream.Entries);
+}
+
 static void streamMapping(yaml::IO &IO, ModuleListStream &Stream) {
   IO.mapRequired("Modules", Stream.Entries);
 }
 
 static void streamMapping(yaml::IO &IO, SystemInfoStream &Stream) {
   SystemInfo &Info = Stream.Info;
   IO.mapRequired("Processor Arch", Info.ProcessorArch);
   mapOptional(IO, "Processor Level", Info.ProcessorLevel, 0);
void yaml::MappingTraits<std::unique_ptr<Stream>>::mapping(
   StreamType Type;
   if (IO.outputting())
     Type = S->Type;
   IO.mapRequired("Type", Type);
 
   if (!IO.outputting())
     S = MinidumpYAML::Stream::create(Type);
   switch (S->Kind) {
+  case MinidumpYAML::Stream::StreamKind::MemoryList:
+    streamMapping(IO, llvm::cast<MemoryListStream>(*S));
+    break;
   case MinidumpYAML::Stream::StreamKind::ModuleList:
     streamMapping(IO, llvm::cast<ModuleListStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::RawContent:
     streamMapping(IO, llvm::cast<RawContentStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::SystemInfo:
     streamMapping(IO, llvm::cast<SystemInfoStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::TextContent:
     streamMapping(IO, llvm::cast<TextContentStream>(*S));
     break;
   case MinidumpYAML::Stream::StreamKind::ThreadList:
     streamMapping(IO, llvm::cast<ThreadListStream>(*S));
     break;
   }
 }
 
 StringRef yaml::MappingTraits<std::unique_ptr<Stream>>::validate(
     yaml::IO &IO, std::unique_ptr<MinidumpYAML::Stream> &S) {
   switch (S->Kind) {
   case MinidumpYAML::Stream::StreamKind::RawContent:
     return streamValidate(cast<RawContentStream>(*S));
+  case MinidumpYAML::Stream::StreamKind::MemoryList:
   case MinidumpYAML::Stream::StreamKind::ModuleList:
   case MinidumpYAML::Stream::StreamKind::SystemInfo:
   case MinidumpYAML::Stream::StreamKind::TextContent:
   case MinidumpYAML::Stream::StreamKind::ThreadList:
     return "";
   }
   llvm_unreachable("Fully covered switch above!");
 }
 
 void yaml::MappingTraits<Object>::mapping(IO &IO, Object &O) {
   IO.mapTag("!minidump", true);
   mapOptionalHex(IO, "Signature", O.Header.Signature, Header::MagicSignature);
   mapOptionalHex(IO, "Version", O.Header.Version, Header::MagicVersion);
   mapOptionalHex(IO, "Flags", O.Header.Flags, 0);
   IO.mapRequired("Streams", O.Streams);
 }
 
 static LocationDescriptor layout(BlobAllocator &File, yaml::BinaryRef Data) {
   return {support::ulittle32_t(Data.binary_size()),
           support::ulittle32_t(File.allocateBytes(Data))};
 }
 
+static void layout(BlobAllocator &File, MemoryListStream::entry_type &Range) {
+  Range.Entry.Memory = layout(File, Range.Content);
+}
+
 static void layout(BlobAllocator &File, ModuleListStream::entry_type &M) {
   M.Entry.ModuleNameRVA = File.allocateString(M.Name);
 
   M.Entry.CvRecord = layout(File, M.CvRecord);
   M.Entry.MiscRecord = layout(File, M.MiscRecord);
 }
 
 static void layout(BlobAllocator &File, ThreadListStream::entry_type &T) {

 }
 
 static Directory layout(BlobAllocator &File, Stream &S) {
   Directory Result;
   Result.Type = S.Type;
   Result.Location.RVA = File.tell();
   Optional<size_t> DataEnd;
   switch (S.Kind) {
+  case Stream::StreamKind::MemoryList:
+    DataEnd = layout(File, cast<MemoryListStream>(S));
+    break;
   case Stream::StreamKind::ModuleList:
     DataEnd = layout(File, cast<ModuleListStream>(S));
     break;
   case Stream::StreamKind::RawContent: {
     RawContentStream &Raw = cast<RawContentStream>(S);
     File.allocateCallback(Raw.Size, [&Raw](raw_ostream &OS) {
       Raw.Content.writeAsBinary(OS);
       assert(Raw.Content.binary_size() <= Raw.Size);
Error MinidumpYAML::writeAsBinary(StringRef Yaml, raw_ostream &OS) {
   writeAsBinary(Obj, OS);
   return Error::success();
 }
 
 Expected<std::unique_ptr<Stream>>
 Stream::create(const Directory &StreamDesc, const object::MinidumpFile &File) {
   StreamKind Kind = getKind(StreamDesc.Type);
   switch (Kind) {
+  case StreamKind::MemoryList: {
+    auto ExpectedList = File.getMemoryList();
+    if (!ExpectedList)
+      return ExpectedList.takeError();
+    std::vector<MemoryListStream::entry_type> Ranges;
+    for (const MemoryDescriptor &MD : *ExpectedList) {
+      auto ExpectedContent = File.getRawData(MD.Memory);
+      if (!ExpectedContent)
+        return ExpectedContent.takeError();
+      Ranges.push_back({MD, *ExpectedContent});
+    }
+    return llvm::make_unique<MemoryListStream>(std::move(Ranges));
+  }
   case StreamKind::ModuleList: {
     auto ExpectedList = File.getModuleList();
     if (!ExpectedList)
       return ExpectedList.takeError();
     std::vector<ModuleListStream::entry_type> Modules;
     for (const Module &M : *ExpectedList) {
       auto ExpectedName = File.getString(M.ModuleNameRVA);
       if (!ExpectedName)

test/tools/obj2yaml/basic-minidump.yaml

Modules:
           File Flags Mask:      0x28292A2B
           File Flags:           0x2C2D2E2F
           File OS:              0x30313233
           File Type:            0x34353637
           File Subtype:         0x38393A3B
           File Date High:       0x3C3D3E3F
           File Date Low:        0x40414243
         CodeView Record: '44454647'
-        Misc Record:     48494A4B
+        Misc Record:     '48494A4B'
       - Base of Image:   0x4C4D4E4F50515253
         Size of Image:   0x54555657
         Module Name:     libb.so
-        CodeView Record: 58595A5B
+        CodeView Record: '58595A5B'
   - Type:            ThreadList
     Threads:
       - Thread Id:         0x5C5D5E5F
         Priority Class:    0x60616263
         Environment Block: 0x6465666768696A6B
-        Context:         7C7D7E7F80818283
+        Context:           '7C7D7E7F80818283'
         Stack:
           Start of Memory Range: 0x6C6D6E6F70717273
-          Content:         7475767778797A7B
+          Content:               '7475767778797A7B'
+  - Type:            MemoryList
+    Memory Ranges:   
+      - Start of Memory Range: 0x7C7D7E7F80818283
+        Content:               '8485868788'
 ...
 
 # CHECK:      --- !minidump
 # CHECK-NEXT: Streams:
 # CHECK-NEXT:   - Type:            SystemInfo
 # CHECK-NEXT:     Processor Arch:  ARM64
 # CHECK-NEXT:     Platform ID:     Linux
 # CHECK-NEXT:     CSD Version:     Linux 3.13.0-91-generic

 # CHECK-NEXT:         CodeView Record: '44454647'
 # CHECK-NEXT:         Misc Record:     48494A4B
 # CHECK-NEXT:       - Base of Image:   0x4C4D4E4F50515253
 # CHECK-NEXT:         Size of Image:   0x54555657
 # CHECK-NEXT:         Module Name:     libb.so
 # CHECK-NEXT:         CodeView Record: 58595A5B
 # CHECK-NEXT:   - Type:            ThreadList
 # CHECK-NEXT:     Threads:
-# CHECK-NEXT:       - Thread Id:       0x5C5D5E5F
-# CHECK-NEXT:         Priority Class:  0x60616263
+# CHECK-NEXT:       - Thread Id:         0x5C5D5E5F
+# CHECK-NEXT:         Priority Class:    0x60616263
 # CHECK-NEXT:         Environment Block: 0x6465666768696A6B
-# CHECK-NEXT:         Context:         7C7D7E7F80818283
+# CHECK-NEXT:         Context:           7C7D7E7F80818283
 # CHECK-NEXT:         Stack:
 # CHECK-NEXT:           Start of Memory Range: 0x6C6D6E6F70717273
-# CHECK-NEXT:           Content:         7475767778797A7B
+# CHECK-NEXT:           Content:               7475767778797A7B
+# CHECK-NEXT:   - Type:            MemoryList
+# CHECK-NEXT:     Memory Ranges:   
+# CHECK-NEXT:       - Start of Memory Range: 0x7C7D7E7F80818283
+# CHECK-NEXT:         Content:               '8485868788'
 # CHECK-NEXT: ...

unittests/Object/MinidumpTest.cpp

for (ArrayRef<uint8_t> Data : {OneThread, PaddedThread}) {
     EXPECT_EQ(0x0403020100090807u, T.EnvironmentBlock);
     EXPECT_EQ(0x0201000908070605u, T.Stack.StartOfMemoryRange);
     EXPECT_EQ(0x06050403u, T.Stack.Memory.DataSize);
     EXPECT_EQ(0x00090807u, T.Stack.Memory.RVA);
     EXPECT_EQ(0x04030201u, T.Context.DataSize);
     EXPECT_EQ(0x08070605u, T.Context.RVA);
   }
 }
+
+TEST(MinidumpFile, getMemoryList) {
+  std::vector<uint8_t> OneRange{
+      // Header
+      'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+      1, 0, 0, 0,                           // NumberOfStreams,
+      32, 0, 0, 0,                          // StreamDirectoryRVA
+      0, 1, 2, 3, 4, 5, 6, 7,               // Checksum, TimeDateStamp
+      0, 0, 0, 0, 0, 0, 0, 0,               // Flags
+                                            // Stream Directory
+      5, 0, 0, 0, 20, 0, 0, 0,              // Type, DataSize,
+      44, 0, 0, 0,                          // RVA
+      // MemoryDescriptor
+      1, 0, 0, 0,             // NumberOfMemoryRanges
+      5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+      3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+  };
+  // Same as before, but with a padded memory list.
+  std::vector<uint8_t> PaddedRange{
+      // Header
+      'M', 'D', 'M', 'P', 0x93, 0xa7, 0, 0, // Signature, Version
+      1, 0, 0, 0,                           // NumberOfStreams,
+      32, 0, 0, 0,                          // StreamDirectoryRVA
+      0, 1, 2, 3, 4, 5, 6, 7,               // Checksum, TimeDateStamp
+      0, 0, 0, 0, 0, 0, 0, 0,               // Flags
+                                            // Stream Directory
+      5, 0, 0, 0, 24, 0, 0, 0,              // Type, DataSize,
+      44, 0, 0, 0,                          // RVA
+      // MemoryDescriptor
+      1, 0, 0, 0,             // NumberOfMemoryRanges
+      0, 0, 0, 0,             // Padding
+      5, 6, 7, 8, 9, 0, 1, 2, // StartOfMemoryRange
+      3, 4, 5, 6, 7, 8, 9, 0, // DataSize, RVA
+  };
+
+  for (ArrayRef<uint8_t> Data : {OneRange, PaddedRange}) {
+    auto ExpectedFile = create(Data);
+    ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+    const MinidumpFile &File = **ExpectedFile;
+    Expected<ArrayRef<MemoryDescriptor>> ExpectedRanges = File.getMemoryList();
+    ASSERT_THAT_EXPECTED(ExpectedRanges, Succeeded());
+    ASSERT_EQ(1u, ExpectedRanges->size());
+    const MemoryDescriptor &MD = ExpectedRanges.get()[0];
+    EXPECT_EQ(0x0201000908070605u, MD.StartOfMemoryRange);
+    EXPECT_EQ(0x06050403u, MD.Memory.DataSize);
+    EXPECT_EQ(0x00090807u, MD.Memory.RVA);
+  }
+}