[Lldb-commits] [lldb] r357748 - MinidumpParser: use minidump parser in llvm/Object

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Apr 5 00:56:39 PDT 2019


Author: labath
Date: Fri Apr  5 00:56:39 2019
New Revision: 357748

URL: http://llvm.org/viewvc/llvm-project?rev=357748&view=rev
Log:
MinidumpParser: use minidump parser in llvm/Object

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.

Removed:
    lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
    lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
Modified:
    lldb/trunk/lit/Minidump/dump-all.test
    lldb/trunk/lit/Minidump/fb-dump.test
    lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt
    lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
    lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
    lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
    lldb/trunk/unittests/Process/minidump/CMakeLists.txt
    lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

Modified: lldb/trunk/lit/Minidump/dump-all.test
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/dump-all.test?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/lit/Minidump/dump-all.test (original)
+++ lldb/trunk/lit/Minidump/dump-all.test Fri Apr  5 00:56:39 2019
@@ -40,17 +40,17 @@
 # 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

Modified: lldb/trunk/lit/Minidump/fb-dump.test
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Minidump/fb-dump.test?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/lit/Minidump/fb-dump.test (original)
+++ lldb/trunk/lit/Minidump/fb-dump.test Fri Apr  5 00:56:39 2019
@@ -57,19 +57,19 @@
 # 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"}}

Modified: lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/Process/minidump/CMakeLists.txt Fri Apr  5 00:56:39 2019
@@ -16,5 +16,6 @@ add_lldb_library(lldbPluginProcessMinidu
     lldbPluginProcessElfCore
   LINK_COMPONENTS
     BinaryFormat
+    Object
     Support
   )

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Fri Apr  5 00:56:39 2019
@@ -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
@@ -23,11 +23,6 @@
 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
 
 llvm::Expected<MinidumpParser>
 MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
-  if (data_sp->GetByteSize() < sizeof(Header))
-    return stringError("Buffer too small.");
+  auto ExpectedFile = llvm::object::MinidumpFile::create(
+      llvm::MemoryBufferRef(toStringRef(data_sp->GetData()), "minidump"));
+  if (!ExpectedFile)
+    return ExpectedFile.takeError();
 
-  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);
-    }
-  }
-
-  // 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) {

Modified: lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Fri Apr  5 00:56:39 2019
@@ -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;

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Fri Apr  5 00:56:39 2019
@@ -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,

Modified: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/CMakeLists.txt?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt Fri Apr  5 00:56:39 2019
@@ -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

Removed: lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/bad_duplicate_streams.dmp?rev=357747&view=auto
==============================================================================
Binary file - no diff available.

Removed: lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/Inputs/bad_overlapping_streams.dmp?rev=357747&view=auto
==============================================================================
Binary file - no diff available.

Modified: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp?rev=357748&r1=357747&r2=357748&view=diff
==============================================================================
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp (original)
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Fri Apr  5 00:56:39 2019
@@ -14,10 +14,12 @@
 #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, GetMemoryList
   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,




More information about the lldb-commits mailing list