minidump: Refactor memory region computation code

[labath]

Summary

The goal of this refactor is to enable ProcessMinidump to take into
account the loaded modules and their sections when computing the
permissions of various ranges of memory, as discussed in D66638.

This patch moves some of the responsibility for computing the ranges
from MinidumpParser into ProcessMinidump. MinidumpParser still does the
parsing, but ProcessMinidump becomes responsible for answering the
actual queries about memory ranges. This will enable it (in a follow-up
patch) to augment the information obtained from the parser with data
obtained from actual object files.

The changes in the actual code are fairly straight-forward and just
involve moving code around. MinidumpParser::GetMemoryRegions is renamed
to BuildMemoryRegions to emphasize that it does no caching. The only new
thing is the additional bool flag returned from this function. This
indicates whether the returned regions describe all memory mapped into
the target process. Data obtained from /proc/maps and the MemoryInfoList
stream is considered to be exhaustive. Data obtained from Memory(64)List
is not. This will be used to determine whether we need to augment the
data or not.

This reshuffle means that it is no longer possible/easy to test some of
this code via unit tests, as constructing a ProcessMinidump instance is
hard. Instead, I update the unit tests to only test the parsing of the
actual data, and test the answering of queries through a lit test using
the "memory region" command. The patch also includes some tweaks to the
MemoryRegion class to make the unit tests easier to write.

[labath]

Actually, it looks like the "follow-up" patch will be more complicated than I had hoped for, and so I am starting to wonder whether we shouldn't just have the unwinder check for section permissions himself (which would be a one-liner). I'll need to investigate more, so you may want to hold off reviewing this.

[labath]

With some additional COFF tweaks (D69100, D69102), the next patch doesn't turn out that bad (though I'm still happy to accept "unwinder should just check section permissions instead of us translating them to memory regions" as an answer).

lldb/include/lldb/Target/MemoryRegionInfo.h

 
 namespace lldb_private {
 class MemoryRegionInfo {
 public:
   typedef Range<lldb::addr_t, lldb::addr_t> RangeType;
 
   enum OptionalBool { eDontKnow = -1, eNo = 0, eYes = 1 };
 
-  MemoryRegionInfo()
-      : m_range(), m_read(eDontKnow), m_write(eDontKnow), m_execute(eDontKnow),
-        m_mapped(eDontKnow), m_flash(eDontKnow), m_blocksize(0) {}
-
-  ~MemoryRegionInfo() {}
+  MemoryRegionInfo() = default;
+  MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write,
+                   OptionalBool execute, OptionalBool mapped, ConstString name,
+                   OptionalBool flash, lldb::offset_t blocksize)
+      : m_range(range), m_read(read), m_write(write), m_execute(execute),
+        m_mapped(mapped), m_name(name), m_flash(flash), m_blocksize(blocksize) {
+  }
 
   RangeType &GetRange() { return m_range; }
 
   void Clear() {
     m_range.Clear();
     m_read = m_write = m_execute = eDontKnow;
   }
 
void SetLLDBPermissions(uint32_t permissions) {
     m_read = (permissions & lldb::ePermissionsReadable) ? eYes : eNo;
     m_write = (permissions & lldb::ePermissionsWritable) ? eYes : eNo;
     m_execute = (permissions & lldb::ePermissionsExecutable) ? eYes : eNo;
   }
 
   bool operator==(const MemoryRegionInfo &rhs) const {
     return m_range == rhs.m_range && m_read == rhs.m_read &&
            m_write == rhs.m_write && m_execute == rhs.m_execute &&
-           m_mapped == rhs.m_mapped;
+           m_mapped == rhs.m_mapped && m_name == rhs.m_name &&
+           m_flash == rhs.m_flash && m_blocksize == rhs.m_blocksize;
   }
 
   bool operator!=(const MemoryRegionInfo &rhs) const { return !(*this == rhs); }
 
 protected:
   RangeType m_range;
-  OptionalBool m_read;
-  OptionalBool m_write;
-  OptionalBool m_execute;
-  OptionalBool m_mapped;
+  OptionalBool m_read = eDontKnow;
+  OptionalBool m_write = eDontKnow;
+  OptionalBool m_execute = eDontKnow;
+  OptionalBool m_mapped = eDontKnow;
   ConstString m_name;
-  OptionalBool m_flash;
-  lldb::offset_t m_blocksize;
+  OptionalBool m_flash = eDontKnow;
+  lldb::offset_t m_blocksize = 0;
 };
   
 inline bool operator<(const MemoryRegionInfo &lhs,
                       const MemoryRegionInfo &rhs) {
   return lhs.GetRange() < rhs.GetRange();
 }
 
 inline bool operator<(const MemoryRegionInfo &lhs, lldb::addr_t rhs) {
   return lhs.GetRange().GetRangeBase() < rhs;
 }
 
 inline bool operator<(lldb::addr_t lhs, const MemoryRegionInfo &rhs) {
   return lhs < rhs.GetRange().GetRangeBase();
 }
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+                              const MemoryRegionInfo &Info);
+
 // Forward-declarable wrapper.
 class MemoryRegionInfos : public std::vector<lldb_private::MemoryRegionInfo> {
 public:
   using std::vector<lldb_private::MemoryRegionInfo>::vector;
 };
 
 }
 

lldb/source/Plugins/Process/minidump/MinidumpParser.h

public:
   std::vector<const minidump::Module *> GetFilteredModuleList();
 
   const llvm::minidump::ExceptionStream *GetExceptionStream();
 
   llvm::Optional<Range> FindMemoryRange(lldb::addr_t addr);
 
   llvm::ArrayRef<uint8_t> GetMemory(lldb::addr_t addr, size_t size);
 
-  MemoryRegionInfo GetMemoryRegionInfo(lldb::addr_t load_addr);
-
-  const MemoryRegionInfos &GetMemoryRegions();
+  /// Returns a list of memory regions and a flag indicating whether the list is
+  /// complete (includes all regions mapped into the process memory).
+  std::pair<MemoryRegionInfos, bool> BuildMemoryRegions();
 
   static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
 
   llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }
 
 private:
   MinidumpParser(lldb::DataBufferSP data_sp,
                  std::unique_ptr<llvm::object::MinidumpFile> file);
 
-  MemoryRegionInfo FindMemoryRegion(lldb::addr_t load_addr) const;
-
 private:
   lldb::DataBufferSP m_data_sp;
   std::unique_ptr<llvm::object::MinidumpFile> m_file;
   ArchSpec m_arch;
-  MemoryRegionInfos m_regions;
-  bool m_parsed_regions = false;
 };
 
 } // end namespace minidump
 } // end namespace lldb_private
 #endif // liblldb_MinidumpParser_h_

lldb/source/Plugins/Process/minidump/MinidumpParser.cpp

for (const auto &memory_desc : memory64_list) {
     region.SetReadable(MemoryRegionInfo::eYes);
     region.SetMapped(MemoryRegionInfo::eYes);
     regions.push_back(region);
   }
   regions.shrink_to_fit();
   return !regions.empty();
 }
 
-MemoryRegionInfo
-MinidumpParser::FindMemoryRegion(lldb::addr_t load_addr) const {
-  auto begin = m_regions.begin();
-  auto end = m_regions.end();
-  auto pos = std::lower_bound(begin, end, load_addr);
-  if (pos != end && pos->GetRange().Contains(load_addr))
-    return *pos;
-  
-  MemoryRegionInfo region;
-  if (pos == begin)
-    region.GetRange().SetRangeBase(0);
-  else {
-    auto prev = pos - 1;
-    if (prev->GetRange().Contains(load_addr))
-      return *prev;
-    region.GetRange().SetRangeBase(prev->GetRange().GetRangeEnd());
-  }
-  if (pos == end)
-    region.GetRange().SetRangeEnd(UINT64_MAX);
-  else
-    region.GetRange().SetRangeEnd(pos->GetRange().GetRangeBase());
-  region.SetReadable(MemoryRegionInfo::eNo);
-  region.SetWritable(MemoryRegionInfo::eNo);
-  region.SetExecutable(MemoryRegionInfo::eNo);
-  region.SetMapped(MemoryRegionInfo::eNo);
-  return region;
-}
-
-MemoryRegionInfo
-MinidumpParser::GetMemoryRegionInfo(lldb::addr_t load_addr) {
-  if (!m_parsed_regions)
-    GetMemoryRegions();
-  return FindMemoryRegion(load_addr);
-}
-
-const MemoryRegionInfos &MinidumpParser::GetMemoryRegions() {
-  if (!m_parsed_regions) {
-    m_parsed_regions = true;
-    // We haven't cached our memory regions yet we will create the region cache
-    // once. We create the region cache using the best source. We start with
-    // the linux maps since they are the most complete and have names for the
-    // regions. Next we try the MemoryInfoList since it has
-    // read/write/execute/map data, and then fall back to the MemoryList and
-    // Memory64List to just get a list of the memory that is mapped in this
-    // core file
-    if (!CreateRegionsCacheFromLinuxMaps(*this, m_regions))
-      if (!CreateRegionsCacheFromMemoryInfoList(*this, m_regions))
-        if (!CreateRegionsCacheFromMemoryList(*this, m_regions))
-          CreateRegionsCacheFromMemory64List(*this, m_regions);
-    llvm::sort(m_regions.begin(), m_regions.end());
-  }
-  return m_regions;
+std::pair<MemoryRegionInfos, bool> MinidumpParser::BuildMemoryRegions() {
+  // We create the region cache using the best source. We start with
+  MemoryRegionInfos result;
+  const auto &return_sorted = [&](bool is_complete) {
+    llvm::sort(result);
+    return std::make_pair(std::move(result), is_complete);
+  };
+  if (CreateRegionsCacheFromLinuxMaps(*this, result))
+    return return_sorted(true);
+  if (CreateRegionsCacheFromMemoryInfoList(*this, result))
+    return return_sorted(true);
+  if (CreateRegionsCacheFromMemoryList(*this, result))
+    return return_sorted(false);
+  CreateRegionsCacheFromMemory64List(*this, result);
+  return return_sorted(false);
 }
 
 #define ENUM_TO_CSTR(ST)                                                       \
   case StreamType::ST:                                                         \
     return #ST
 
 llvm::StringRef
 MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {

lldb/source/Plugins/Process/minidump/ProcessMinidump.h

 
 private:
   FileSpec m_core_file;
   lldb::DataBufferSP m_core_data;
   llvm::ArrayRef<minidump::Thread> m_thread_list;
   const minidump::ExceptionStream *m_active_exception;
   lldb::CommandObjectSP m_command_sp;
   bool m_is_wow64;
+  llvm::Optional<MemoryRegionInfos> m_memory_regions;
+
+  void BuildMemoryRegions();
 };
 
 } // namespace minidump
 } // namespace lldb_private
 
 #endif // liblldb_ProcessMinidump_h_

lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp

ArchSpec ProcessMinidump::GetArchitecture() {
 
   llvm::Triple triple;
   triple.setVendor(llvm::Triple::VendorType::UnknownVendor);
   triple.setArch(llvm::Triple::ArchType::x86);
   triple.setOS(llvm::Triple::OSType::Win32);
   return ArchSpec(triple);
 }
 
+void ProcessMinidump::BuildMemoryRegions() {
+  if (m_memory_regions)
+    return;
+  m_memory_regions.emplace();
+  bool is_complete;
+  std::tie(*m_memory_regions, is_complete) =
+      m_minidump_parser->BuildMemoryRegions();
+  // TODO: Use loaded modules to complete the region list.
+}
+
 Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
-                                            MemoryRegionInfo &range_info) {
-  range_info = m_minidump_parser->GetMemoryRegionInfo(load_addr);
+                                            MemoryRegionInfo &region) {
+  BuildMemoryRegions();
+  auto pos = llvm::upper_bound(*m_memory_regions, load_addr);
+  if (pos != m_memory_regions->begin() &&
+      std::prev(pos)->GetRange().Contains(load_addr)) {
+    region = *std::prev(pos);
+    return Status();
+  }
+
+  if (pos == m_memory_regions->begin())
+    region.GetRange().SetRangeBase(0);
+  else
+    region.GetRange().SetRangeBase(std::prev(pos)->GetRange().GetRangeEnd());
+
+  if (pos == m_memory_regions->end())
+    region.GetRange().SetRangeEnd(UINT64_MAX);
+  else
+    region.GetRange().SetRangeEnd(pos->GetRange().GetRangeBase());
+
+  region.SetReadable(MemoryRegionInfo::eNo);
+  region.SetWritable(MemoryRegionInfo::eNo);
+  region.SetExecutable(MemoryRegionInfo::eNo);
+  region.SetMapped(MemoryRegionInfo::eNo);
   return Status();
 }
 
-Status ProcessMinidump::GetMemoryRegions(
-    lldb_private::MemoryRegionInfos &region_list) {
-  region_list = m_minidump_parser->GetMemoryRegions();
+Status ProcessMinidump::GetMemoryRegions(MemoryRegionInfos &region_list) {
+  BuildMemoryRegions();
+  region_list = *m_memory_regions;
   return Status();
 }
 
 void ProcessMinidump::Clear() { Process::m_thread_list.Clear(); }
 
 bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
                                        ThreadList &new_thread_list) {
   for (const minidump::Thread &thread : m_thread_list) {

lldb/source/Target/CMakeLists.txt

add_lldb_library(lldbTarget
   JITLoader.cpp
   JITLoaderList.cpp
   InstrumentationRuntime.cpp
   InstrumentationRuntimeStopInfo.cpp
   Language.cpp
   LanguageRuntime.cpp
   Memory.cpp
   MemoryHistory.cpp
+  MemoryRegionInfo.cpp
   ModuleCache.cpp
   OperatingSystem.cpp
   PathMappingList.cpp
   Platform.cpp
   Process.cpp
   Queue.cpp
   QueueItem.cpp
   QueueList.cpp

lldb/source/Target/MemoryRegionInfo.cpp

+//===-- MemoryRegionInfo.cpp ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Target/MemoryRegionInfo.h"
+
+llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &OS,
+                                            const MemoryRegionInfo &Info) {
+  return OS << llvm::formatv("MemoryRegionInfo([{0}, {1}), {2}, {3}, {4}, {5}, "
+                             "`{6}`, {7}, {8})",
+                             Info.GetRange().GetRangeBase(),
+                             Info.GetRange().GetRangeEnd(), Info.GetReadable(),
+                             Info.GetWritable(), Info.GetExecutable(),
+                             Info.GetMapped(), Info.GetName(), Info.GetFlash(),
+                             Info.GetBlocksize());
+}

lldb/test/Shell/Minidump/memory-region.yaml

+# Check that memory region parsing works correctly, with a particular emphasis
+# on the boundary conditions.
+
+# RUN: yaml2obj %s > %t
+# RUN: %lldb -b -c %t \
+# RUN:   -o "memory region 0" -o "memory region 0xd9000" \
+# RUN:   -o "memory region 0xd9001" -o "memory region 0xdafff" \
+# RUN:   -o "memory region 0xdb000" -o "memory region 0xdd000" | FileCheck %s
+
+# CHECK-LABEL: (lldb) memory region 0
+# CHECK: [0x0000000000000000-0x00000000000d9000) ---
+# CHECK-LABEL: (lldb) memory region 0xd9000
+# CHECK: [0x00000000000d9000-0x00000000000db000) r-x /system/bin/app_process
+# CHECK-LABEL: (lldb) memory region 0xd9001
+# CHECK: [0x00000000000d9000-0x00000000000db000) r-x /system/bin/app_process
+# CHECK-LABEL: (lldb) memory region 0xdafff
+# CHECK: [0x00000000000d9000-0x00000000000db000) r-x /system/bin/app_process
+# CHECK-LABEL: (lldb) memory region 0xdb000
+# CHECK: [0x00000000000db000-0x00000000000dc000) ---
+# CHECK-LABEL: (lldb) memory region 0xdd000
+# CHECK: [0x00000000000dd000-0xffffffffffffffff) ---
+
+--- !minidump
+Streams:         
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Platform ID:     Linux
+    CPU:             
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            LinuxProcStatus
+    Text:             |
+      Name: nonexisting-module
+      State:  t (tracing stop)
+      Tgid: 29939
+      Ngid: 0
+      Pid:  29939
+      PPid: 29370
+      TracerPid:  29940
+      Uid:  1001  1001  1001  1001
+      Gid:  1001  1001  1001  1001
+
+  - Type:            LinuxMaps
+    Text:             |
+      000d9000-000db000 r-xp 00000000 b3:04 227        /system/bin/app_process
+      000dc000-000dd000 rw-p 00000000 00:00 0
+
+...

lldb/unittests/Process/minidump/MinidumpParserTest.cpp

TEST_F(MinidumpParserTest, FindMemoryRangeWithFullMemoryMinidump) {
   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
   EXPECT_FALSE(parser->FindMemoryRange(0x7ffe0000 + 4096).hasValue());
 }
 
-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()) {
-  SCOPED_TRACE(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());
-  EXPECT_EQ(write, range_info.GetWritable());
-  EXPECT_EQ(exec, range_info.GetExecutable());
-  EXPECT_EQ(mapped, range_info.GetMapped());
-  EXPECT_EQ(name, range_info.GetName());
-}
-
-// Same as above function where addr == start
-void check_region(MinidumpParser &parser, lldb::addr_t start, lldb::addr_t end,
-                  MemoryRegionInfo::OptionalBool read,
-                  MemoryRegionInfo::OptionalBool write,
-                  MemoryRegionInfo::OptionalBool exec,
-                  MemoryRegionInfo::OptionalBool mapped,
-                  ConstString name = ConstString()) {
-  check_region(parser, start, start, end, read, write, exec, mapped, name);
-}
-
-
 constexpr auto yes = MemoryRegionInfo::eYes;
 constexpr auto no = MemoryRegionInfo::eNo;
 constexpr auto unknown = MemoryRegionInfo::eDontKnow;
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfo) {
   ASSERT_THAT_ERROR(SetUpFromYaml(R"(
 --- !minidump
 Streams:
   - Type:            MemoryInfoList
     Memory Ranges:
       - Base Address:    0x0000000000000000
         Allocation Protect: [  ]
         Region Size:     0x0000000000010000
         State:           [ MEM_FREE ]
         Protect:         [ PAGE_NO_ACCESS ]
         Type:            [  ]
       - Base Address:    0x0000000000010000
         Allocation Protect: [ PAGE_READ_WRITE ]
-        Region Size:     0x0000000000010000
-        State:           [ MEM_COMMIT ]
-        Type:            [ MEM_MAPPED ]
-      - Base Address:    0x0000000000020000
-        Allocation Protect: [ PAGE_READ_WRITE ]
-        Region Size:     0x0000000000010000
-        State:           [ MEM_COMMIT ]
-        Type:            [ MEM_MAPPED ]
-      - Base Address:    0x0000000000030000
-        Allocation Protect: [ PAGE_READ_WRITE ]
-        Region Size:     0x0000000000001000
+        Region Size:     0x0000000000021000
         State:           [ MEM_COMMIT ]
         Type:            [ MEM_MAPPED ]
       - Base Address:    0x0000000000040000
         Allocation Protect: [ PAGE_EXECUTE_WRITE_COPY ]
         Region Size:     0x0000000000001000
         State:           [ MEM_COMMIT ]
         Protect:         [ PAGE_READ_ONLY ]
         Type:            [ MEM_IMAGE ]
       - Base Address:    0x000000007FFE0000
         Allocation Protect: [ PAGE_READ_ONLY ]
         Region Size:     0x0000000000001000
         State:           [ MEM_COMMIT ]
         Type:            [ MEM_PRIVATE ]
       - Base Address:    0x000000007FFE1000
         Allocation Base: 0x000000007FFE0000
         Allocation Protect: [ PAGE_READ_ONLY ]
         Region Size:     0x000000000000F000
         State:           [ MEM_RESERVE ]
         Protect:         [ PAGE_NO_ACCESS ]
         Type:            [ MEM_PRIVATE ]
 ...
 )"),
                     llvm::Succeeded());
 
-  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);
-
-  // 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);
+  EXPECT_THAT(
+      parser->BuildMemoryRegions(),
+      testing::Pair(testing::ElementsAre(
+                        MemoryRegionInfo({0x0, 0x10000}, no, no, no, no,
+                                         ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x10000, 0x21000}, yes, yes, no, yes,
+                                         ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x40000, 0x1000}, yes, no, no, yes,
+                                         ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x7ffe0000, 0x1000}, yes, no, no, yes,
+                                         ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x7ffe1000, 0xf000}, no, no, no, yes,
+                                         ConstString(), unknown, 0)),
+                    true));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemoryList) {
   ASSERT_THAT_ERROR(SetUpFromYaml(R"(
 --- !minidump
 Streams:
   - Type:            MemoryList
     Memory Ranges:
       - Start of Memory Range: 0x0000000000001000
         Content:         '31313131313131313131313131313131'
       - Start of Memory Range: 0x0000000000002000
         Content:         '3333333333333333333333333333333333333333333333333333333333333333'
 ...
 )"),
                     llvm::Succeeded());
+
   // Test we can get memory regions from the MINIDUMP_MEMORY_LIST stream when
   // we don't have a MemoryInfoListStream.
 
-  // 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);
+  EXPECT_THAT(
+      parser->BuildMemoryRegions(),
+      testing::Pair(testing::ElementsAre(
+                        MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
+                                         yes, ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
+                                         yes, ConstString(), unknown, 0)),
+                    false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoFromMemory64List) {
   SetUpData("regions-memlist64.dmp");
+
   // Test we can get memory regions from the MINIDUMP_MEMORY64_LIST stream when
   // we don't have a MemoryInfoListStream.
-
-  // 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);
+  EXPECT_THAT(
+      parser->BuildMemoryRegions(),
+      testing::Pair(testing::ElementsAre(
+                        MemoryRegionInfo({0x1000, 0x10}, yes, unknown, unknown,
+                                         yes, ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x2000, 0x20}, yes, unknown, unknown,
+                                         yes, ConstString(), unknown, 0)),
+                    false));
 }
 
 TEST_F(MinidumpParserTest, GetMemoryRegionInfoLinuxMaps) {
   ASSERT_THAT_ERROR(SetUpFromYaml(R"(
 --- !minidump
 Streams:
   - Type:            LinuxMaps
     Text:             |
       400d9000-400db000 r-xp 00000000 b3:04 227        /system/bin/app_process
       400db000-400dc000 r--p 00001000 b3:04 227        /system/bin/app_process
       400dc000-400dd000 rw-p 00000000 00:00 0
-      400dd000-400ec000 r-xp 00000000 b3:04 300        /system/bin/linker
       400ec000-400ed000 r--p 00000000 00:00 0
-      400ed000-400ee000 r--p 0000f000 b3:04 300        /system/bin/linker
       400ee000-400ef000 rw-p 00010000 b3:04 300        /system/bin/linker
-      400ef000-400fb000 rw-p 00000000 00:00 0
-      400fb000-400fc000 r-xp 00000000 b3:04 1096       /system/lib/liblog.so
       400fc000-400fd000 rwxp 00001000 b3:04 1096       /system/lib/liblog.so
-      400fd000-400ff000 r-xp 00002000 b3:04 1096       /system/lib/liblog.so
-      400ff000-40100000 r--p 00003000 b3:04 1096       /system/lib/liblog.so
-      40100000-40101000 rw-p 00004000 b3:04 1096       /system/lib/liblog.so
-      40101000-40122000 r-xp 00000000 b3:04 955        /system/lib/libc.so
-      40122000-40123000 rwxp 00021000 b3:04 955        /system/lib/libc.so
-      40123000-40167000 r-xp 00022000 b3:04 955        /system/lib/libc.so
-      40167000-40169000 r--p 00065000 b3:04 955        /system/lib/libc.so
-      40169000-4016b000 rw-p 00067000 b3:04 955        /system/lib/libc.so
-      4016b000-40176000 rw-p 00000000 00:00 0
 
 ...
 )"),
                     llvm::Succeeded());
   // Test we can get memory regions from the linux /proc/<pid>/maps stream when
   // we don't have a MemoryInfoListStream.
-
-  // Test addres before the first entry comes back with nothing mapped up
-  // to first valid region info
-  ConstString a("/system/bin/app_process");
-  ConstString b("/system/bin/linker");
-  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);
+  ConstString app_process("/system/bin/app_process");
+  ConstString linker("/system/bin/linker");
+  ConstString liblog("/system/lib/liblog.so");
+  EXPECT_THAT(
+      parser->BuildMemoryRegions(),
+      testing::Pair(testing::ElementsAre(
+                        MemoryRegionInfo({0x400d9000, 0x2000}, yes, no, yes,
+                                         yes, app_process, unknown, 0),
+                        MemoryRegionInfo({0x400db000, 0x1000}, yes, no, no, yes,
+                                         app_process, unknown, 0),
+                        MemoryRegionInfo({0x400dc000, 0x1000}, yes, yes, no,
+                                         yes, ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x400ec000, 0x1000}, yes, no, no, yes,
+                                         ConstString(), unknown, 0),
+                        MemoryRegionInfo({0x400ee000, 0x1000}, yes, yes, no,
+                                         yes, linker, unknown, 0),
+                        MemoryRegionInfo({0x400fc000, 0x1000}, yes, yes, yes,
+                                         yes, liblog, unknown, 0)),
+                    true));
 }
 
 // Windows Minidump tests
 TEST_F(MinidumpParserTest, GetArchitectureWindows) {
   ASSERT_THAT_ERROR(SetUpFromYaml(R"(
 --- !minidump
 Streams:
   - Type:            SystemInfo