[Lldb-commits] [lldb] r354668 - Avoid two-stage initialization of MinidumpParser

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Fri Feb 22 05:36:01 PST 2019


Author: labath
Date: Fri Feb 22 05:36:01 2019
New Revision: 354668

URL: http://llvm.org/viewvc/llvm-project?rev=354668&view=rev
Log:
Avoid two-stage initialization of MinidumpParser

remove the Initialize function, move the things that can fail into the
static factory function. The factory function now returns
Expected<Parser> instead of Optional<Parser> so that it can give a
reason why creation failed.

Modified:
    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/source/Plugins/Process/minidump/ProcessMinidump.h
    lldb/trunk/unittests/Process/minidump/CMakeLists.txt
    lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp

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=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.cpp Fri Feb 22 05:36:01 2019
@@ -23,16 +23,106 @@
 using namespace lldb_private;
 using namespace minidump;
 
-llvm::Optional<MinidumpParser>
-MinidumpParser::Create(const lldb::DataBufferSP &data_buf_sp) {
-  if (data_buf_sp->GetByteSize() < sizeof(MinidumpHeader)) {
-    return llvm::None;
+static llvm::Error stringError(llvm::StringRef Err) {
+  return llvm::make_error<llvm::StringError>(Err,
+                                             llvm::inconvertibleErrorCode());
+}
+
+llvm::Expected<MinidumpParser>
+MinidumpParser::Create(const lldb::DataBufferSP &data_sp) {
+  if (data_sp->GetByteSize() < sizeof(MinidumpHeader))
+    return stringError("Buffer too small.");
+
+  llvm::ArrayRef<uint8_t> header_data(data_sp->GetBytes(),
+                                      sizeof(MinidumpHeader));
+  const MinidumpHeader *header = MinidumpHeader::Parse(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->streams_count == 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(MinidumpHeader));
+
+  // Add the directory entries to the file map
+  FileRange directory_range(header->stream_directory_rva,
+                            header->streams_count * sizeof(MinidumpDirectory));
+  if (directory_range.end() > file_size)
+    return stringError("invalid minidump: truncated streams directory");
+  minidump_map.push_back(directory_range);
+
+  llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> 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->streams_count; ++i) {
+    const MinidumpDirectory *directory_entry = nullptr;
+    Status error = consumeObject(directory_data, directory_entry);
+    if (error.Fail())
+      return error.ToError();
+    if (directory_entry->stream_type == 0) {
+      // Ignore dummy streams (technically ill-formed, but a number of
+      // existing minidumps seem to contain such streams)
+      if (directory_entry->location.data_size == 0)
+        continue;
+      return stringError("invalid minidump: bad stream type");
+    }
+    // Update the streams map, checking for duplicate stream types
+    if (!directory_map
+             .insert({directory_entry->stream_type, directory_entry->location})
+             .second)
+      return stringError("invalid minidump: duplicate stream type");
+
+    // Ignore the zero-length streams for layout checks
+    if (directory_entry->location.data_size != 0) {
+      minidump_map.emplace_back(directory_entry->location.rva,
+                                directory_entry->location.data_size);
+    }
   }
-  return MinidumpParser(data_buf_sp);
+
+  // 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));
 }
 
-MinidumpParser::MinidumpParser(const lldb::DataBufferSP &data_buf_sp)
-    : m_data_sp(data_buf_sp) {}
+MinidumpParser::MinidumpParser(
+    lldb::DataBufferSP data_sp,
+    llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map)
+    : m_data_sp(std::move(data_sp)), m_directory_map(std::move(directory_map)) {
+}
 
 llvm::ArrayRef<uint8_t> MinidumpParser::GetData() {
   return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes(),
@@ -563,112 +653,6 @@ const MemoryRegionInfos &MinidumpParser:
   return m_regions;
 }
 
-Status MinidumpParser::Initialize() {
-  Status error;
-
-  lldbassert(m_directory_map.empty());
-
-  llvm::ArrayRef<uint8_t> header_data(m_data_sp->GetBytes(),
-                                      sizeof(MinidumpHeader));
-  const MinidumpHeader *header = MinidumpHeader::Parse(header_data);
-  if (header == nullptr) {
-    error.SetErrorString("invalid minidump: can't parse the header");
-    return error;
-  }
-
-  // A minidump without at least one stream is clearly ill-formed
-  if (header->streams_count == 0) {
-    error.SetErrorString("invalid minidump: no streams present");
-    return error;
-  }
-
-  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 = m_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;
-
-  // Add the minidump header to the file map
-  if (sizeof(MinidumpHeader) > file_size) {
-    error.SetErrorString("invalid minidump: truncated header");
-    return error;
-  }
-  minidump_map.emplace_back( 0, sizeof(MinidumpHeader) );
-
-  // Add the directory entries to the file map
-  FileRange directory_range(header->stream_directory_rva,
-                            header->streams_count *
-                                sizeof(MinidumpDirectory));
-  if (directory_range.end() > file_size) {
-    error.SetErrorString("invalid minidump: truncated streams directory");
-    return error;
-  }
-  minidump_map.push_back(directory_range);
-
-  // Parse stream directory entries
-  llvm::ArrayRef<uint8_t> directory_data(
-      m_data_sp->GetBytes() + directory_range.offset, directory_range.size);
-  for (uint32_t i = 0; i < header->streams_count; ++i) {
-    const MinidumpDirectory *directory_entry = nullptr;
-    error = consumeObject(directory_data, directory_entry);
-    if (error.Fail())
-      return error;
-    if (directory_entry->stream_type == 0) {
-      // Ignore dummy streams (technically ill-formed, but a number of
-      // existing minidumps seem to contain such streams)
-      if (directory_entry->location.data_size == 0)
-        continue;
-      error.SetErrorString("invalid minidump: bad stream type");
-      return error;
-    }
-    // Update the streams map, checking for duplicate stream types
-    if (!m_directory_map
-             .insert({directory_entry->stream_type, directory_entry->location})
-             .second) {
-      error.SetErrorString("invalid minidump: duplicate stream type");
-      return error;
-    }
-    // Ignore the zero-length streams for layout checks
-    if (directory_entry->location.data_size != 0) {
-      minidump_map.emplace_back(directory_entry->location.rva,
-                                directory_entry->location.data_size);
-    }
-  }
-
-  // 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) {
-      error.SetErrorString("invalid minidump: overlapping streams");
-      return error;
-    }
-  }
-
-  // Check for streams past the end of file
-  const auto &last_range = minidump_map.back();
-  if (last_range.end() > file_size) {
-    error.SetErrorString("invalid minidump: truncated stream");
-    return error;
-  }
-
-  return error;
-}
-
 #define ENUM_TO_CSTR(ST) case (uint32_t)MinidumpStreamType::ST: return #ST
 
 llvm::StringRef

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=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/MinidumpParser.h Fri Feb 22 05:36:01 2019
@@ -44,7 +44,7 @@ struct Range {
 
 class MinidumpParser {
 public:
-  static llvm::Optional<MinidumpParser>
+  static llvm::Expected<MinidumpParser>
   Create(const lldb::DataBufferSP &data_buf_sp);
 
   llvm::ArrayRef<uint8_t> GetData();
@@ -92,9 +92,6 @@ public:
 
   const MemoryRegionInfos &GetMemoryRegions();
 
-  // Perform consistency checks and initialize internal data structures
-  Status Initialize();
-
   static llvm::StringRef GetStreamTypeAsString(uint32_t stream_type);
 
   const llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> &
@@ -103,7 +100,9 @@ public:
   }
 
 private:
-  MinidumpParser(const lldb::DataBufferSP &data_buf_sp);
+  MinidumpParser(
+      lldb::DataBufferSP data_sp,
+      llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> directory_map);
 
   MemoryRegionInfo FindMemoryRegion(lldb::addr_t load_addr) const;
 

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=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.cpp Fri Feb 22 05:36:01 2019
@@ -123,13 +123,8 @@ lldb::ProcessSP ProcessMinidump::CreateI
   if (!AllData)
     return nullptr;
 
-  auto minidump_parser = MinidumpParser::Create(AllData);
-  // check if the parser object is valid
-  if (!minidump_parser)
-    return nullptr;
-
   return std::make_shared<ProcessMinidump>(target_sp, listener_sp, *crash_file,
-                                           minidump_parser.getValue());
+                                           std::move(AllData));
 }
 
 bool ProcessMinidump::CanDebug(lldb::TargetSP target_sp,
@@ -140,9 +135,9 @@ bool ProcessMinidump::CanDebug(lldb::Tar
 ProcessMinidump::ProcessMinidump(lldb::TargetSP target_sp,
                                  lldb::ListenerSP listener_sp,
                                  const FileSpec &core_file,
-                                 MinidumpParser minidump_parser)
-    : Process(target_sp, listener_sp), m_minidump_parser(minidump_parser),
-      m_core_file(core_file), m_is_wow64(false) {}
+                                 DataBufferSP core_data)
+    : Process(target_sp, listener_sp), m_core_file(core_file),
+      m_core_data(std::move(core_data)), m_is_wow64(false) {}
 
 ProcessMinidump::~ProcessMinidump() {
   Clear();
@@ -168,12 +163,12 @@ void ProcessMinidump::Terminate() {
 }
 
 Status ProcessMinidump::DoLoadCore() {
-  Status error;
+  auto expected_parser = MinidumpParser::Create(m_core_data);
+  if (!expected_parser)
+    return Status(expected_parser.takeError());
+  m_minidump_parser = std::move(*expected_parser);
 
-  // Minidump parser initialization & consistency checks
-  error = m_minidump_parser.Initialize();
-  if (error.Fail())
-    return error;
+  Status error;
 
   // Do we support the minidump's architecture?
   ArchSpec arch = GetArchitecture();
@@ -192,11 +187,11 @@ Status ProcessMinidump::DoLoadCore() {
   }
   GetTarget().SetArchitecture(arch, true /*set_platform*/);
 
-  m_thread_list = m_minidump_parser.GetThreads();
-  m_active_exception = m_minidump_parser.GetExceptionStream();
+  m_thread_list = m_minidump_parser->GetThreads();
+  m_active_exception = m_minidump_parser->GetExceptionStream();
   ReadModuleList();
 
-  llvm::Optional<lldb::pid_t> pid = m_minidump_parser.GetPid();
+  llvm::Optional<lldb::pid_t> pid = m_minidump_parser->GetPid();
   if (!pid) {
     error.SetErrorString("failed to parse PID");
     return error;
@@ -267,7 +262,7 @@ size_t ProcessMinidump::ReadMemory(lldb:
 size_t ProcessMinidump::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                      Status &error) {
 
-  llvm::ArrayRef<uint8_t> mem = m_minidump_parser.GetMemory(addr, size);
+  llvm::ArrayRef<uint8_t> mem = m_minidump_parser->GetMemory(addr, size);
   if (mem.empty()) {
     error.SetErrorString("could not parse memory info");
     return 0;
@@ -279,7 +274,7 @@ size_t ProcessMinidump::DoReadMemory(lld
 
 ArchSpec ProcessMinidump::GetArchitecture() {
   if (!m_is_wow64) {
-    return m_minidump_parser.GetArchitecture();
+    return m_minidump_parser->GetArchitecture();
   }
 
   llvm::Triple triple;
@@ -291,13 +286,13 @@ ArchSpec ProcessMinidump::GetArchitectur
 
 Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
                                             MemoryRegionInfo &range_info) {
-  range_info = m_minidump_parser.GetMemoryRegionInfo(load_addr);
+  range_info = m_minidump_parser->GetMemoryRegionInfo(load_addr);
   return Status();
 }
 
 Status ProcessMinidump::GetMemoryRegions(
     lldb_private::MemoryRegionInfos &region_list) {
-  region_list = m_minidump_parser.GetMemoryRegions();
+  region_list = m_minidump_parser->GetMemoryRegions();
   return Status();
 }
 
@@ -316,9 +311,9 @@ bool ProcessMinidump::UpdateThreadList(T
 
     llvm::ArrayRef<uint8_t> context;
     if (!m_is_wow64)
-      context = m_minidump_parser.GetThreadContext(context_location);
+      context = m_minidump_parser->GetThreadContext(context_location);
     else
-      context = m_minidump_parser.GetThreadContextWow64(thread);
+      context = m_minidump_parser->GetThreadContextWow64(thread);
 
     lldb::ThreadSP thread_sp(new ThreadMinidump(*this, thread, context));
     new_thread_list.AddThread(thread_sp);
@@ -328,11 +323,11 @@ bool ProcessMinidump::UpdateThreadList(T
 
 void ProcessMinidump::ReadModuleList() {
   std::vector<const MinidumpModule *> filtered_modules =
-      m_minidump_parser.GetFilteredModuleList();
+      m_minidump_parser->GetFilteredModuleList();
 
   for (auto module : filtered_modules) {
     llvm::Optional<std::string> name =
-        m_minidump_parser.GetMinidumpString(module->module_name_rva);
+        m_minidump_parser->GetMinidumpString(module->module_name_rva);
 
     if (!name)
       continue;
@@ -353,7 +348,7 @@ void ProcessMinidump::ReadModuleList() {
       m_is_wow64 = true;
     }
 
-    const auto uuid = m_minidump_parser.GetModuleUUID(module);
+    const auto uuid = m_minidump_parser->GetModuleUUID(module);
     auto file_spec = FileSpec(name.getValue(), GetArchitecture().GetTriple());
     FileSystem::Instance().Resolve(file_spec);
     ModuleSpec module_spec(file_spec, uuid);
@@ -666,7 +661,7 @@ public:
         m_interpreter.GetExecutionContext().GetProcessPtr());
     result.SetStatus(eReturnStatusSuccessFinishResult);
     Stream &s = result.GetOutputStream();
-    MinidumpParser &minidump = process->m_minidump_parser;
+    MinidumpParser &minidump = *process->m_minidump_parser;
     if (DumpDirectory()) {
       s.Printf("RVA        SIZE       TYPE       MinidumpStreamType\n");
       s.Printf("---------- ---------- ---------- --------------------------\n");

Modified: lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h?rev=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h (original)
+++ lldb/trunk/source/Plugins/Process/minidump/ProcessMinidump.h Fri Feb 22 05:36:01 2019
@@ -41,7 +41,7 @@ public:
   static const char *GetPluginDescriptionStatic();
 
   ProcessMinidump(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                  const FileSpec &core_file, MinidumpParser minidump_parser);
+                  const FileSpec &core_file, lldb::DataBufferSP code_data);
 
   ~ProcessMinidump() override;
 
@@ -92,7 +92,7 @@ public:
     return error;
   }
 
-  MinidumpParser m_minidump_parser;
+  llvm::Optional<MinidumpParser> m_minidump_parser;
 
 protected:
   void Clear();
@@ -106,6 +106,7 @@ protected:
 
 private:
   FileSpec m_core_file;
+  lldb::DataBufferSP m_core_data;
   llvm::ArrayRef<MinidumpThread> m_thread_list;
   const MinidumpExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;

Modified: lldb/trunk/unittests/Process/minidump/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/CMakeLists.txt?rev=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/unittests/Process/minidump/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Process/minidump/CMakeLists.txt Fri Feb 22 05:36:01 2019
@@ -9,6 +9,7 @@ add_lldb_unittest(LLDBMinidumpTests
     lldbPluginProcessUtility
     lldbPluginProcessMinidump
     lldbUtilityHelpers
+    LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )

Modified: lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp?rev=354668&r1=354667&r2=354668&view=diff
==============================================================================
--- lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp (original)
+++ lldb/trunk/unittests/Process/minidump/MinidumpParserTest.cpp Fri Feb 22 05:36:01 2019
@@ -21,6 +21,7 @@
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 // C includes
@@ -41,13 +42,11 @@ public:
     std::string filename = GetInputFilePath(minidump_filename);
     auto BufferPtr = FileSystem::Instance().CreateDataBuffer(filename, -1, 0);
     ASSERT_NE(BufferPtr, nullptr);
-    llvm::Optional<MinidumpParser> optional_parser =
+    llvm::Expected<MinidumpParser> expected_parser =
         MinidumpParser::Create(BufferPtr);
-    ASSERT_TRUE(optional_parser.hasValue());
-    parser.reset(new MinidumpParser(optional_parser.getValue()));
+    ASSERT_THAT_EXPECTED(expected_parser, llvm::Succeeded());
+    parser = std::move(*expected_parser);
     ASSERT_GT(parser->GetData().size(), 0UL);
-    auto result = parser->Initialize();
-    ASSERT_TRUE(result.Success()) << result.AsCString();
   }
 
   void InvalidMinidump(const char *minidump_filename, uint64_t load_size) {
@@ -56,16 +55,10 @@ public:
         FileSystem::Instance().CreateDataBuffer(filename, load_size, 0);
     ASSERT_NE(BufferPtr, nullptr);
 
-    llvm::Optional<MinidumpParser> optional_parser =
-        MinidumpParser::Create(BufferPtr);
-    ASSERT_TRUE(optional_parser.hasValue());
-    parser.reset(new MinidumpParser(optional_parser.getValue()));
-    ASSERT_GT(parser->GetData().size(), 0UL);
-    auto result = parser->Initialize();
-    ASSERT_TRUE(result.Fail());
+    EXPECT_THAT_EXPECTED(MinidumpParser::Create(BufferPtr), llvm::Failed());
   }
 
-  std::unique_ptr<MinidumpParser> parser;
+  llvm::Optional<MinidumpParser> parser;
 };
 
 TEST_F(MinidumpParserTest, GetThreadsAndGetThreadContext) {
@@ -246,10 +239,9 @@ TEST_F(MinidumpParserTest, GetExceptionS
   ASSERT_EQ(11UL, exception_stream->exception_record.exception_code);
 }
 
-void check_mem_range_exists(std::unique_ptr<MinidumpParser> &parser,
-                            const uint64_t range_start,
+void check_mem_range_exists(MinidumpParser &parser, const uint64_t range_start,
                             const uint64_t range_size) {
-  llvm::Optional<minidump::Range> range = parser->FindMemoryRange(range_start);
+  llvm::Optional<minidump::Range> range = parser.FindMemoryRange(range_start);
   ASSERT_TRUE(range.hasValue()) << "There is no range containing this address";
   EXPECT_EQ(range_start, range->start);
   EXPECT_EQ(range_start + range_size, range->start + range->range_ref.size());
@@ -263,10 +255,10 @@ TEST_F(MinidumpParserTest, FindMemoryRan
   EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
   EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
 
-  check_mem_range_exists(parser, 0x401d46, 256);
+  check_mem_range_exists(*parser, 0x401d46, 256);
   EXPECT_FALSE(parser->FindMemoryRange(0x401d46 + 256).hasValue());
 
-  check_mem_range_exists(parser, 0x7ffceb34a000, 12288);
+  check_mem_range_exists(*parser, 0x7ffceb34a000, 12288);
   EXPECT_FALSE(parser->FindMemoryRange(0x7ffceb34a000 + 12288).hasValue());
 }
 
@@ -288,22 +280,21 @@ TEST_F(MinidumpParserTest, FindMemoryRan
   // There are a lot of ranges in the file, just testing with some of them
   EXPECT_FALSE(parser->FindMemoryRange(0x00).hasValue());
   EXPECT_FALSE(parser->FindMemoryRange(0x2a).hasValue());
-  check_mem_range_exists(parser, 0x10000, 65536); // first range
-  check_mem_range_exists(parser, 0x40000, 4096);
+  check_mem_range_exists(*parser, 0x10000, 65536); // first range
+  check_mem_range_exists(*parser, 0x40000, 4096);
   EXPECT_FALSE(parser->FindMemoryRange(0x40000 + 4096).hasValue());
-  check_mem_range_exists(parser, 0x77c12000, 8192);
-  check_mem_range_exists(parser, 0x7ffe0000, 4096); // last range
+  check_mem_range_exists(*parser, 0x77c12000, 8192);
+  check_mem_range_exists(*parser, 0x7ffe0000, 4096); // last range
   EXPECT_FALSE(parser->FindMemoryRange(0x7ffe0000 + 4096).hasValue());
 }
 
-void check_region(std::unique_ptr<MinidumpParser> &parser,
-                  lldb::addr_t addr, lldb::addr_t start, lldb::addr_t end,
-                  MemoryRegionInfo::OptionalBool read,
+void check_region(MinidumpParser &parser, lldb::addr_t addr, lldb::addr_t start,
+                  lldb::addr_t end, MemoryRegionInfo::OptionalBool read,
                   MemoryRegionInfo::OptionalBool write,
                   MemoryRegionInfo::OptionalBool exec,
                   MemoryRegionInfo::OptionalBool mapped,
                   ConstString name = ConstString()) {
-  auto range_info = parser->GetMemoryRegionInfo(addr);
+  auto range_info = parser.GetMemoryRegionInfo(addr);
   EXPECT_EQ(start, range_info.GetRange().GetRangeBase());
   EXPECT_EQ(end, range_info.GetRange().GetRangeEnd());
   EXPECT_EQ(read, range_info.GetReadable());
@@ -314,8 +305,7 @@ void check_region(std::unique_ptr<Minidu
 }
 
 // Same as above function where addr == start
-void check_region(std::unique_ptr<MinidumpParser> &parser,
-                  lldb::addr_t start, lldb::addr_t end,
+void check_region(MinidumpParser &parser, lldb::addr_t start, lldb::addr_t end,
                   MemoryRegionInfo::OptionalBool read,
                   MemoryRegionInfo::OptionalBool write,
                   MemoryRegionInfo::OptionalBool exec,
@@ -332,21 +322,21 @@ constexpr auto unknown = MemoryRegionInf
 TEST_F(MinidumpParserTest, GetMemoryRegionInfo) {
   SetUpData("fizzbuzz_wow64.dmp");
 
-  check_region(parser, 0x00000000, 0x00010000, no, no, no, no);
-  check_region(parser, 0x00010000, 0x00020000, yes, yes, no, yes);
-  check_region(parser, 0x00020000, 0x00030000, yes, yes, no, yes);
-  check_region(parser, 0x00030000, 0x00031000, yes, yes, no, yes);
-  check_region(parser, 0x00031000, 0x00040000, no, no, no, no);
-  check_region(parser, 0x00040000, 0x00041000, yes, no, no, yes);
+  check_region(*parser, 0x00000000, 0x00010000, no, no, no, no);
+  check_region(*parser, 0x00010000, 0x00020000, yes, yes, no, yes);
+  check_region(*parser, 0x00020000, 0x00030000, yes, yes, no, yes);
+  check_region(*parser, 0x00030000, 0x00031000, yes, yes, no, yes);
+  check_region(*parser, 0x00031000, 0x00040000, no, no, no, no);
+  check_region(*parser, 0x00040000, 0x00041000, yes, no, no, yes);
 
   // Check addresses contained inside ranges
-  check_region(parser, 0x00000001, 0x00000000, 0x00010000, no, no, no, no);
-  check_region(parser, 0x0000ffff, 0x00000000, 0x00010000, no, no, no, no);
-  check_region(parser, 0x00010001, 0x00010000, 0x00020000, yes, yes, no, yes);
-  check_region(parser, 0x0001ffff, 0x00010000, 0x00020000, yes, yes, no, yes);
+  check_region(*parser, 0x00000001, 0x00000000, 0x00010000, no, no, no, no);
+  check_region(*parser, 0x0000ffff, 0x00000000, 0x00010000, no, no, no, no);
+  check_region(*parser, 0x00010001, 0x00010000, 0x00020000, yes, yes, no, yes);
+  check_region(*parser, 0x0001ffff, 0x00010000, 0x00020000, yes, yes, no, yes);
 
   // Test that an address after the last entry maps to rest of the memory space
-  check_region(parser, 0x7fff0000, 0x7fff0000, UINT64_MAX, no, no, no, no);
+  check_region(*parser, 0x7fff0000, 0x7fff0000, UINT64_MAX, no, no, no, no);
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemoryList) {
@@ -356,11 +346,11 @@ TEST_F(MinidumpParserTest, GetMemoryRegi
 
   // Test addres before the first entry comes back with nothing mapped up
   // to first valid region info
-  check_region(parser, 0x00000000, 0x00001000, no, no, no, no);
-  check_region(parser, 0x00001000, 0x00001010, yes, unknown, unknown, yes);
-  check_region(parser, 0x00001010, 0x00002000, no, no, no, no);
-  check_region(parser, 0x00002000, 0x00002020, yes, unknown, unknown, yes);
-  check_region(parser, 0x00002020, UINT64_MAX, no, no, no, no);
+  check_region(*parser, 0x00000000, 0x00001000, no, no, no, no);
+  check_region(*parser, 0x00001000, 0x00001010, yes, unknown, unknown, yes);
+  check_region(*parser, 0x00001010, 0x00002000, no, no, no, no);
+  check_region(*parser, 0x00002000, 0x00002020, yes, unknown, unknown, yes);
+  check_region(*parser, 0x00002020, UINT64_MAX, no, no, no, no);
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemory64List) {
@@ -370,11 +360,11 @@ TEST_F(MinidumpParserTest, GetMemoryRegi
 
   // Test addres before the first entry comes back with nothing mapped up
   // to first valid region info
-  check_region(parser, 0x00000000, 0x00001000, no, no, no, no);
-  check_region(parser, 0x00001000, 0x00001010, yes, unknown, unknown, yes);
-  check_region(parser, 0x00001010, 0x00002000, no, no, no, no);
-  check_region(parser, 0x00002000, 0x00002020, yes, unknown, unknown, yes);
-  check_region(parser, 0x00002020, UINT64_MAX, no, no, no, no);
+  check_region(*parser, 0x00000000, 0x00001000, no, no, no, no);
+  check_region(*parser, 0x00001000, 0x00001010, yes, unknown, unknown, yes);
+  check_region(*parser, 0x00001010, 0x00002000, no, no, no, no);
+  check_region(*parser, 0x00002000, 0x00002020, yes, unknown, unknown, yes);
+  check_region(*parser, 0x00002020, UINT64_MAX, no, no, no, no);
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoLinuxMaps) {
@@ -389,27 +379,27 @@ TEST_F(MinidumpParserTest, GetMemoryRegi
   ConstString c("/system/lib/liblog.so");
   ConstString d("/system/lib/libc.so");
   ConstString n;
-  check_region(parser, 0x00000000, 0x400d9000, no , no , no , no , n);
-  check_region(parser, 0x400d9000, 0x400db000, yes, no , yes, yes, a);
-  check_region(parser, 0x400db000, 0x400dc000, yes, no , no , yes, a);
-  check_region(parser, 0x400dc000, 0x400dd000, yes, yes, no , yes, n);
-  check_region(parser, 0x400dd000, 0x400ec000, yes, no , yes, yes, b);
-  check_region(parser, 0x400ec000, 0x400ed000, yes, no , no , yes, n);
-  check_region(parser, 0x400ed000, 0x400ee000, yes, no , no , yes, b);
-  check_region(parser, 0x400ee000, 0x400ef000, yes, yes, no , yes, b);
-  check_region(parser, 0x400ef000, 0x400fb000, yes, yes, no , yes, n);
-  check_region(parser, 0x400fb000, 0x400fc000, yes, no , yes, yes, c);
-  check_region(parser, 0x400fc000, 0x400fd000, yes, yes, yes, yes, c);
-  check_region(parser, 0x400fd000, 0x400ff000, yes, no , yes, yes, c);
-  check_region(parser, 0x400ff000, 0x40100000, yes, no , no , yes, c);
-  check_region(parser, 0x40100000, 0x40101000, yes, yes, no , yes, c);
-  check_region(parser, 0x40101000, 0x40122000, yes, no , yes, yes, d);
-  check_region(parser, 0x40122000, 0x40123000, yes, yes, yes, yes, d);
-  check_region(parser, 0x40123000, 0x40167000, yes, no , yes, yes, d);
-  check_region(parser, 0x40167000, 0x40169000, yes, no , no , yes, d);
-  check_region(parser, 0x40169000, 0x4016b000, yes, yes, no , yes, d);
-  check_region(parser, 0x4016b000, 0x40176000, yes, yes, no , yes, n);
-  check_region(parser, 0x40176000, UINT64_MAX, no , no , no , no , n);
+  check_region(*parser, 0x00000000, 0x400d9000, no, no, no, no, n);
+  check_region(*parser, 0x400d9000, 0x400db000, yes, no, yes, yes, a);
+  check_region(*parser, 0x400db000, 0x400dc000, yes, no, no, yes, a);
+  check_region(*parser, 0x400dc000, 0x400dd000, yes, yes, no, yes, n);
+  check_region(*parser, 0x400dd000, 0x400ec000, yes, no, yes, yes, b);
+  check_region(*parser, 0x400ec000, 0x400ed000, yes, no, no, yes, n);
+  check_region(*parser, 0x400ed000, 0x400ee000, yes, no, no, yes, b);
+  check_region(*parser, 0x400ee000, 0x400ef000, yes, yes, no, yes, b);
+  check_region(*parser, 0x400ef000, 0x400fb000, yes, yes, no, yes, n);
+  check_region(*parser, 0x400fb000, 0x400fc000, yes, no, yes, yes, c);
+  check_region(*parser, 0x400fc000, 0x400fd000, yes, yes, yes, yes, c);
+  check_region(*parser, 0x400fd000, 0x400ff000, yes, no, yes, yes, c);
+  check_region(*parser, 0x400ff000, 0x40100000, yes, no, no, yes, c);
+  check_region(*parser, 0x40100000, 0x40101000, yes, yes, no, yes, c);
+  check_region(*parser, 0x40101000, 0x40122000, yes, no, yes, yes, d);
+  check_region(*parser, 0x40122000, 0x40123000, yes, yes, yes, yes, d);
+  check_region(*parser, 0x40123000, 0x40167000, yes, no, yes, yes, d);
+  check_region(*parser, 0x40167000, 0x40169000, yes, no, no, yes, d);
+  check_region(*parser, 0x40169000, 0x4016b000, yes, yes, no, yes, d);
+  check_region(*parser, 0x4016b000, 0x40176000, yes, yes, no, yes, n);
+  check_region(*parser, 0x40176000, UINT64_MAX, no, no, no, no, n);
 }
 
 // Windows Minidump tests




More information about the lldb-commits mailing list