MinidumpParser: use minidump parser in llvm/Object
authorPavel Labath <pavel@labath.sk>
Fri, 5 Apr 2019 07:56:39 +0000 (07:56 +0000)
committerPavel Labath <pavel@labath.sk>
Fri, 5 Apr 2019 07:56:39 +0000 (07:56 +0000)
This patch removes the lower layers of the minidump parsing code from
the MinidumpParser class, and replaces it with the minidump parser in
llvm.

Not all functionality is already avaiable in the llvm class, but it is
enough for us to be able to stop enumerating streams manually, and rely
on the minidump directory parsing code from the llvm class.

This also removes some checked-in binaries which were used to test error
handling in the parser, as the error handling is now done (and tested)
in llvm. Instead I just add one test that ensures we correctly propagate
the errors reported by the llvm parser. The input for this test can be
written in yaml instead of a checked-in binary.

llvm-svn: 357748

lldb/lit/Minidump/dump-all.test
lldb/lit/Minidump/fb-dump.test
lldb/source/Plugins/Process/minidump/CMakeLists.txt
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/source/Plugins/Process/minidump/MinidumpParser.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/unittests/Process/minidump/CMakeLists.txt
lldb/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp [deleted file]
lldb/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp [deleted file]
lldb/unittests/Process/minidump/MinidumpParserTest.cpp

index 427b488..efafad7 100644 (file)
 # CHECKDIR:      RVA        SIZE       TYPE       StreamType
 # CHECKDIR-NEXT: ---------- ---------- ---------- --------------------------
 # CHECKDIR-NEXT: 0x000000b0 0x00000038 0x00000007 SystemInfo
-# CHECKDIR-NEXT: 0x0000015d 0x0000001b 0x47670007 LinuxEnviron
-# CHECKDIR-NEXT: 0x00000190 0x000000bc 0x47670009 LinuxMaps
-# CHECKDIR-NEXT: 0x00000110 0x0000001a 0x47670004 LinuxProcStatus
-# CHECKDIR-NEXT: 0x0000024c 0x00000018 0x4767000b LinuxProcStat
-# CHECKDIR-NEXT: 0x00000142 0x0000001b 0x47670006 LinuxCMDLine
-# CHECKDIR-NEXT: 0x00000272 0x00000016 0x4767000d LinuxProcFD
-# CHECKDIR-NEXT: 0x00000178 0x00000018 0x47670008 LinuxAuxv
 # CHECKDIR-NEXT: 0x000000e8 0x00000018 0x0000000f MiscInfo
 # CHECKDIR-NEXT: 0x00000100 0x00000010 0x47670003 LinuxCPUInfo
+# CHECKDIR-NEXT: 0x00000110 0x0000001a 0x47670004 LinuxProcStatus
 # CHECKDIR-NEXT: 0x0000012a 0x00000018 0x47670005 LinuxLSBRelease
+# CHECKDIR-NEXT: 0x00000142 0x0000001b 0x47670006 LinuxCMDLine
+# CHECKDIR-NEXT: 0x0000015d 0x0000001b 0x47670007 LinuxEnviron
+# CHECKDIR-NEXT: 0x00000178 0x00000018 0x47670008 LinuxAuxv
+# CHECKDIR-NEXT: 0x00000190 0x000000bc 0x47670009 LinuxMaps
+# CHECKDIR-NEXT: 0x0000024c 0x00000018 0x4767000b LinuxProcStat
 # CHECKDIR-NEXT: 0x00000264 0x0000000e 0x4767000c LinuxProcUptime
+# CHECKDIR-NEXT: 0x00000272 0x00000016 0x4767000d LinuxProcFD
 
 # CHECKCPU:      /proc/cpuinfo:
 # CHECKCPU-NEXT: cpu info output
index c58d983..f493dfb 100644 (file)
 # RUN:   FileCheck --check-prefix=CHECK-LOGCAT %s
 # CHECK-DIR:      RVA        SIZE       TYPE       StreamType
 # CHECK-DIR-NEXT: ---------- ---------- ---------- --------------------------
+# CHECK-DIR-NEXT: 0x000000bc 0x00000038 0x00000007 SystemInfo
+# CHECK-DIR-NEXT: 0x000000f4 0x00000018 0x0000000f MiscInfo
 # CHECK-DIR-NEXT: 0x0000010c 0x00000013 0xfacecb00 FacebookDumpErrorLog
 # CHECK-DIR-NEXT: 0x0000011f 0x00000015 0xfacee000 FacebookThreadName
+# CHECK-DIR-NEXT: 0x00000134 0x00000010 0xface1ca7 FacebookLogcat
+# CHECK-DIR-NEXT: 0x00000144 0x00000017 0xfacecccc FacebookAppStateLog
 # CHECK-DIR-NEXT: 0x0000015b 0x00000016 0xfacedead FacebookAbortReason
-# CHECK-DIR-NEXT: 0x000000bc 0x00000038 0x00000007 SystemInfo
-# CHECK-DIR-NEXT: 0x000001aa 0x00000005 0xfacecafb FacebookBuildID
-# CHECK-DIR-NEXT: 0x000001bc 0x00000019 0xfacecafd FacebookJavaStack
-# CHECK-DIR-NEXT: 0x000001ea 0x00000005 0xfacecaff FacebookUnwindSymbols
 # CHECK-DIR-NEXT: 0x00000171 0x00000039 0xfacecafa FacebookAppCustomData
-# CHECK-DIR-NEXT: 0x00000134 0x00000010 0xface1ca7 FacebookLogcat
-# CHECK-DIR-NEXT: 0x000000f4 0x00000018 0x0000000f MiscInfo
+# CHECK-DIR-NEXT: 0x000001aa 0x00000005 0xfacecafb FacebookBuildID
 # CHECK-DIR-NEXT: 0x000001af 0x0000000d 0xfacecafc FacebookAppVersionName
+# CHECK-DIR-NEXT: 0x000001bc 0x00000019 0xfacecafd FacebookJavaStack
 # CHECK-DIR-NEXT: 0x000001d5 0x00000015 0xfacecafe FacebookDalvikInfo
-# CHECK-DIR-NEXT: 0x00000144 0x00000017 0xfacecccc FacebookAppStateLog
+# CHECK-DIR-NEXT: 0x000001ea 0x00000005 0xfacecaff FacebookUnwindSymbols
 
 # CHECK-APPDATA:      Facebook App Data:
 # CHECK-APPDATA-NEXT: {"global": {"Fingerprint":"invalid device fingerprint"}}
index c90f4f0..afa3550 100644 (file)
@@ -16,5 +16,6 @@ add_lldb_library(lldbPluginProcessMinidump PLUGIN
     lldbPluginProcessElfCore
   LINK_COMPONENTS
     BinaryFormat
+    Object
     Support
   )
index 64af45f..eab0c03 100644 (file)
@@ -10,8 +10,8 @@
 #include "NtStructures.h"
 #include "RegisterContextMinidump_x86_32.h"
 
-#include "lldb/Utility/LLDBAssert.h"
 #include "Plugins/Process/Utility/LinuxProcMaps.h"
+#include "lldb/Utility/LLDBAssert.h"
 
 // C includes
 // C++ includes
 using namespace lldb_private;
 using namespace minidump;
 
-static llvm::Error stringError(llvm::StringRef Err) {
-  return llvm::make_error<llvm::StringError>(Err,
-                                             llvm::inconvertibleErrorCode());
-}
-
 const Header *ParseHeader(llvm::ArrayRef<uint8_t> &data) {
   const Header *header = nullptr;
   Status error = consumeObject(data, header);
@@ -45,117 +40,26 @@ const Header *ParseHeader(llvm::ArrayRef<uint8_t> &data) {
 
 llvm::Expected<MinidumpParser>
 MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
-  if (data_sp->GetByteSize() < sizeof(Header))
-    return stringError("Buffer too small.");
-
-  llvm::ArrayRef<uint8_t> header_data(data_sp->GetBytes(),
-                                      sizeof(Header));
-  const Header *header = ParseHeader(header_data);
-  if (!header)
-    return stringError("invalid minidump: can't parse the header");
-
-  // A minidump without at least one stream is clearly ill-formed
-  if (header->NumberOfStreams == 0)
-    return stringError("invalid minidump: no streams present");
-
-  struct FileRange {
-    uint32_t offset = 0;
-    uint32_t size = 0;
-
-    FileRange(uint32_t offset, uint32_t size) : offset(offset), size(size) {}
-    uint32_t end() const { return offset + size; }
-  };
-
-  const uint32_t file_size = data_sp->GetByteSize();
-
-  // Build a global minidump file map, checking for:
-  // - overlapping streams/data structures
-  // - truncation (streams pointing past the end of file)
-  std::vector<FileRange> minidump_map;
-
-  minidump_map.emplace_back(0, sizeof(Header));
-
-  // Add the directory entries to the file map
-  FileRange directory_range(header->StreamDirectoryRVA,
-                            header->NumberOfStreams * sizeof(Directory));
-  if (directory_range.end() > file_size)
-    return stringError("invalid minidump: truncated streams directory");
-  minidump_map.push_back(directory_range);
-
-  llvm::DenseMap<StreamType, LocationDescriptor> directory_map;
-
-  // Parse stream directory entries
-  llvm::ArrayRef<uint8_t> directory_data(
-      data_sp->GetBytes() + directory_range.offset, directory_range.size);
-  for (uint32_t i = 0; i < header->NumberOfStreams; ++i) {
-    const Directory *directory_entry = nullptr;
-    Status error = consumeObject(directory_data, directory_entry);
-    if (error.Fail())
-      return error.ToError();
-    if (directory_entry->Type == StreamType::Unused) {
-      // Ignore dummy streams (technically ill-formed, but a number of
-      // existing minidumps seem to contain such streams)
-      if (directory_entry->Location.DataSize == 0)
-        continue;
-      return stringError("invalid minidump: bad stream type");
-    }
-    // Update the streams map, checking for duplicate stream types
-    if (!directory_map
-             .insert({directory_entry->Type, directory_entry->Location})
-             .second)
-      return stringError("invalid minidump: duplicate stream type");
-
-    // Ignore the zero-length streams for layout checks
-    if (directory_entry->Location.DataSize != 0) {
-      minidump_map.emplace_back(directory_entry->Location.RVA,
-                                directory_entry->Location.DataSize);
-    }
-  }
+  auto ExpectedFile = llvm::object::MinidumpFile::create(
+      llvm::MemoryBufferRef(toStringRef(data_sp->GetData()), "minidump"));
+  if (!ExpectedFile)
+    return ExpectedFile.takeError();
 
-  // Sort the file map ranges by start offset
-  llvm::sort(minidump_map.begin(), minidump_map.end(),
-             [](const FileRange &a, const FileRange &b) {
-               return a.offset < b.offset;
-             });
-
-  // Check for overlapping streams/data structures
-  for (size_t i = 1; i < minidump_map.size(); ++i) {
-    const auto &prev_range = minidump_map[i - 1];
-    if (prev_range.end() > minidump_map[i].offset)
-      return stringError("invalid minidump: overlapping streams");
-  }
-
-  // Check for streams past the end of file
-  const auto &last_range = minidump_map.back();
-  if (last_range.end() > file_size)
-    return stringError("invalid minidump: truncated stream");
-
-  return MinidumpParser(std::move(data_sp), std::move(directory_map));
+  return MinidumpParser(data_sp, std::move(*ExpectedFile));
 }
 
-MinidumpParser::MinidumpParser(
-    lldb::DataBufferSP data_sp,
-    llvm::DenseMap<StreamType, LocationDescriptor> directory_map)
-    : m_data_sp(std::move(data_sp)), m_directory_map(std::move(directory_map)) {
-}
+MinidumpParser::MinidumpParser(lldb::DataBufferSP data_sp,
+                               std::unique_ptr<llvm::object::MinidumpFile> file)
+    : m_data_sp(std::move(data_sp)), m_file(std::move(file)) {}
 
 llvm::ArrayRef<uint8_t> MinidumpParser::GetData() {
   return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
                                  m_data_sp->GetByteSize());
 }
 
-llvm::ArrayRef<uint8_t>
-MinidumpParser::GetStream(StreamType stream_type) {
-  auto iter = m_directory_map.find(stream_type);
-  if (iter == m_directory_map.end())
-    return {};
-
-  // check if there is enough data
-  if (iter->second.RVA + iter->second.DataSize > m_data_sp->GetByteSize())
-    return {};
-
-  return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes() + iter->second.RVA,
-                                 iter->second.DataSize);
+llvm::ArrayRef<uint8_t> MinidumpParser::GetStream(StreamType stream_type) {
+  return m_file->getRawStream(stream_type)
+      .getValueOr(llvm::ArrayRef<uint8_t>());
 }
 
 llvm::Optional<std::string> MinidumpParser::GetMinidumpString(uint32_t rva) {
index 7720772..07d7d07 100644 (file)
@@ -21,6 +21,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Object/Minidump.h"
 
 // C includes
 
@@ -93,20 +94,17 @@ public:
 
   static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
 
-  const llvm::DenseMap<StreamType, LocationDescriptor> &
-  GetDirectoryMap() const {
-    return m_directory_map;
-  }
+  llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }
 
 private:
   MinidumpParser(lldb::DataBufferSP data_sp,
-                 llvm::DenseMap<StreamType, LocationDescriptor> directory_map);
+                 std::unique_ptr<llvm::object::MinidumpFile> file);
 
   MemoryRegionInfo FindMemoryRegion(lldb::addr_t load_addr) const;
 
 private:
   lldb::DataBufferSP m_data_sp;
-  llvm::DenseMap<StreamType, LocationDescriptor> m_directory_map;
+  std::unique_ptr<llvm::object::MinidumpFile> m_file;
   ArchSpec m_arch;
   MemoryRegionInfos m_regions;
   bool m_parsed_regions = false;
index 4d16f82..d6ab58c 100644 (file)
@@ -718,10 +718,12 @@ public:
     if (DumpDirectory()) {
       s.Printf("RVA        SIZE       TYPE       StreamType\n");
       s.Printf("---------- ---------- ---------- --------------------------\n");
-      for (const auto &pair: minidump.GetDirectoryMap())
-        s.Printf("0x%8.8x 0x%8.8x 0x%8.8x %s\n", (uint32_t)pair.second.RVA,
-                 (uint32_t)pair.second.DataSize, (unsigned)pair.first,
-                 MinidumpParser::GetStreamTypeAsString(pair.first).data());
+      for (const auto &stream_desc : minidump.GetMinidumpFile().streams())
+        s.Printf(
+            "0x%8.8x 0x%8.8x 0x%8.8x %s\n", (uint32_t)stream_desc.Location.RVA,
+            (uint32_t)stream_desc.Location.DataSize,
+            (unsigned)(StreamType)stream_desc.Type,
+            MinidumpParser::GetStreamTypeAsString(stream_desc.Type).data());
       s.Printf("\n");
     }
     auto DumpTextStream = [&](StreamType stream_type,
index 9e3a18e..9cbd7a9 100644 (file)
@@ -11,12 +11,11 @@ add_lldb_unittest(LLDBMinidumpTests
     lldbUtilityHelpers
     LLVMTestingSupport
   LINK_COMPONENTS
+    ObjectYAML
     Support
   )
 
 set(test_inputs
-   bad_duplicate_streams.dmp
-   bad_overlapping_streams.dmp
    fizzbuzz_no_heap.dmp
    fizzbuzz_wow64.dmp
    linux-i386.dmp
diff --git a/lldb/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp b/lldb/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
deleted file mode 100644 (file)
index d9be8e2..0000000
Binary files a/lldb/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp and /dev/null differ
diff --git a/lldb/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp b/lldb/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
deleted file mode 100644 (file)
index f131ae7..0000000
Binary files a/lldb/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp and /dev/null differ
index ba727e0..a49d0c2 100644 (file)
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/FileSpec.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ObjectYAML/MinidumpYAML.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -49,18 +51,28 @@ public:
     ASSERT_GT(parser->GetData().size(), 0UL);
   }
 
-  void InvalidMinidump(const char *minidump_filename, uint64_t load_size) {
-    std::string filename = GetInputFilePath(minidump_filename);
-    auto BufferPtr =
-        FileSystem::Instance().CreateDataBuffer(filename, load_size, 0);
-    ASSERT_NE(BufferPtr, nullptr);
-
-    EXPECT_THAT_EXPECTED(MinidumpParser::Create(BufferPtr), llvm::Failed());
-  }
-
   llvm::Optional<MinidumpParser> parser;
 };
 
+TEST_F(MinidumpParserTest, InvalidMinidump) {
+  std::string duplicate_streams;
+  llvm::raw_string_ostream os(duplicate_streams);
+  ASSERT_THAT_ERROR(llvm::MinidumpYAML::writeAsBinary(R"(
+--- !minidump
+Streams:         
+  - Type:            LinuxAuxv
+    Content:         DEADBEEFBAADF00D
+  - Type:            LinuxAuxv
+    Content:         DEADBEEFBAADF00D
+  )",
+                                                      os),
+                    llvm::Succeeded());
+  os.flush();
+  auto data_buffer_sp = std::make_shared<DataBufferHeap>(
+      duplicate_streams.data(), duplicate_streams.size());
+  ASSERT_THAT_EXPECTED(MinidumpParser::Create(data_buffer_sp), llvm::Failed());
+}
+
 TEST_F(MinidumpParserTest, GetThreadsAndGetThreadContext) {
   SetUpData("linux-x86_64.dmp");
   llvm::ArrayRef<MinidumpThread> thread_list;
@@ -146,17 +158,6 @@ TEST_F(MinidumpParserTest, GetMemoryListPadded) {
   EXPECT_EQ((lldb::addr_t)0x8010, mem->start);
 }
 
-TEST_F(MinidumpParserTest, TruncatedMinidumps) {
-  InvalidMinidump("linux-x86_64.dmp", 32);
-  InvalidMinidump("linux-x86_64.dmp", 100);
-  InvalidMinidump("linux-x86_64.dmp", 20 * 1024);
-}
-
-TEST_F(MinidumpParserTest, IllFormedMinidumps) {
-  InvalidMinidump("bad_duplicate_streams.dmp", -1);
-  InvalidMinidump("bad_overlapping_streams.dmp", -1);
-}
-
 TEST_F(MinidumpParserTest, GetArchitecture) {
   SetUpData("linux-x86_64.dmp");
   ASSERT_EQ(llvm::Triple::ArchType::x86_64,