Speadup memory regions handling and cleanup related code

[tatyana-krasnukha]

Summary

• replace unnecessary shared pointers with unique pointers
• reserve space before filling a vector with 'push_back' in a loop to avoid muptiple allocations and memory fragmentation (except Process::GetMemoryRegions, it is not possible to know the size in advance here)
• override GetMemoryRegions for the ProcessMinidump:

Process::GetMemoryRegions fills the list of regions by calling GetMemoryRegionInfo until it returns an error. But ProcessMinidump implementation GetMemoryRegionInfo parses whole list every time and searches requested range there. Now GetMemoryRegions does this work just once.

• use move semantic where it is possible (and desirable)
• fix size_t -> uint32_t truncation
• add missing constness

I had to change API slightly, but believe these changes doesn't break existing code that uses this API.
Profiling a sipmle program I got nice results - execution of GetMemoryRegions function took just 0.01% of total time against 0.17% for unchanged version.

[labath]

Woohoo for speedups.

However, I noticed that you are modifying the signatures of SB functions, which I don't believe will be acceptable for lldb.

include/lldb/API/SBMemoryRegionInfo.h

105

Generally, I think it's better to use values here instead of rvalue references, as they make clearer interfaces (a function which receives the a unique_ptr by value has to consume it whether it likes to or not, but in case of rvalue references, the object will survive the function call unless the function explicitly does something to it).

I don't believe this should matter for performance, as that's just copying the pointer value around.

source/Plugins/Process/minidump/MinidumpParser.cpp

404

Namespaces are not indented in llvm style.

408

llvm prefers static functions instead of ones in anonymous namespaces

[clayborg]

I am going to stop making comments as I have been working on a similar patch that does more than this one. I will post it today and we can decide which approach we want to use.

include/lldb/API/SBMemoryRegionInfoList.h

29

Revert. You can add functions, but never change them. Our API is stable.

31

This you can add, but std::vector implementations are quite efficient and I doubt this is really needed?

33

Revert to maintain API

35

"const" means nothing to our objects as they have shared pointers or unique pointers which never change.

37

ditto, and revert for API

include/lldb/Target/Process.h

2084

This was a a vector of shared pointers, seemingly for no good reason prior to this change, I would change this to be a vector of MemoryRegionInfo instances?

source/API/SBMemoryRegionInfo.cpp

23–24

Fine to add to API, but really no one outside LLDB will be able to do anything with this function (nor the one on the left) since MemoryRegionInfo is opaque as far as the public clients are concerned. It is there for internal LLDB convenience. So do we really need this? We can add a method that copies from an instance if needed. MemoryRegionInfo structs are very simple to copy. No need for heroics here.

23–27

Don't remove... API

39–44

Do this by instance instead of a move assignment operator with a unique ptr?

source/API/SBMemoryRegionInfoList.cpp

81

revert

85–88

Who needs this externally? I would rather not add this to the public API?

90

revert

110

remove const

115

revert

source/API/SBProcess.cpp

1367

Remove. "sb_region_info" already has an instance we can use.

1369

Change Process::GetMemoryRegionInfo() to use an "MemoryRegionInfo &region" instead of shared pointer or unique pointer?

1371

If above changes are made this is not needed.

1399

Just use the std::vector<MemoryRegionInfo> from MemoryRegionInfoListImpl inside the "sb_region_list" already?

1400

Change Process:: GetMemoryRegions() to take a "std::vector<MemoryRegionInfo>&region_list"?

1401

This doesn't need to be done if the above code is changed

1402–1403

This doesn't need to be done if the above code is changed to just pass the vector of regions from "sb_region_list"

source/Plugins/Process/minidump/MinidumpParser.h

91

switch to all internal GetMemoryRegions calls to just use "std::vector<lldb::MemoryRegionInfo>"?

[zturner]

include/lldb/API/SBMemoryRegionInfoList.h

31

std::vector is as efficient as it can be without additional hints from the user, but reserve is a very useful optimization in general.

[clayborg]

See my patch: https://reviews.llvm.org/D55522

Let me know what you think

[clayborg]

include/lldb/API/SBMemoryRegionInfoList.h

31

Agreed but this doesn't need to be in the public API as there is no real need.

[tatyana-krasnukha]

Thanks to all for comments!

As I wrote in inline comments, 'non-const' -> 'const' changes don't seem to break anything, I'd like to leave it here. Correct me, if I'm wrong.

The second question is whether it is allowed to change private member functions of SBXxx classes?

include/lldb/API/SBMemoryRegionInfoList.h

31

I'm not aware of the usual count of memory regions in dump files, but when I tested on a file with 140 regions the difference was significant
(if take into account all loops that do push_back without previous reserve).

35

Non-const argument version prevents passing r-value in this function. Another reason is readability.
Is this change braking API too?

include/lldb/Target/Process.h

2084

Hah, I feel stupid now)

source/API/SBMemoryRegionInfo.cpp

23–24

This is instead of private constructor with a raw pointer

23–27

Private member functions cannot be changed too?

[tatyana-krasnukha]

Replace vector of unique pointers with vector of values, revert some API changes, remove MinidumpParser changes.
I will put overridden GetMemoryRegions in a separate patch.

[tatyana-krasnukha]

Addressed more comments

[tatyana-krasnukha]

Minor change: replace using with typedef since old swig versions don't understand c++11's aliases.

[clayborg]

One quick change required to avoid STL in public headers.

include/lldb/lldb-forward.h

0

Remove due to this header being part of the public API.

1

After thinking about this a bit, we shouldn't put any STL stuff in headers that are part of the public API as we never want to expose any STL through our API. Can probably move this into MemoryRegionInfo.h, or just use "std::vector<lldb_private::MemoryRegionInfo>" everywhere as long as it is in lldb_private or the global namespace (we only export "lldb::*").

[tatyana-krasnukha]

Rid off of including std::vector at the cost of adding a wrapper class that can be forward declared.
Without this, it is not possible to do https://reviews.llvm.org/D55472#inline-491159

include/lldb/API/SBMemoryRegionInfo.h

 private:
   friend class SBProcess;
   friend class SBMemoryRegionInfoList;
 
   lldb_private::MemoryRegionInfo &ref();
 
   const lldb_private::MemoryRegionInfo &ref() const;
 
-  SBMemoryRegionInfo(const lldb_private::MemoryRegionInfo *lldb_object_ptr);
+  SBMemoryRegionInfo(lldb::MemoryRegionInfoUP &&lldb_object_up);
+
+  SBMemoryRegionInfo &operator=(lldb::MemoryRegionInfoUP &&lldb_object_up);
 
   lldb::MemoryRegionInfoUP m_opaque_ap;
 };
 
 } // namespace lldb
 
 #endif // LLDB_SBMemoryRegionInfo_h_

include/lldb/API/SBMemoryRegionInfoList.h

public:
   SBMemoryRegionInfoList();
 
   SBMemoryRegionInfoList(const lldb::SBMemoryRegionInfoList &rhs);
 
   const SBMemoryRegionInfoList &operator=(const SBMemoryRegionInfoList &rhs);
 
   ~SBMemoryRegionInfoList();
 
-  uint32_t GetSize() const;
+  size_t GetSize() const;
 
-  bool GetMemoryRegionAtIndex(uint32_t idx, SBMemoryRegionInfo &region_info);
+  void Reserve(size_t capacity);
 
-  void Append(lldb::SBMemoryRegionInfo &region);
+  bool GetMemoryRegionAtIndex(size_t idx, SBMemoryRegionInfo &region_info);
 
-  void Append(lldb::SBMemoryRegionInfoList &region_list);
+  void Append(const lldb::SBMemoryRegionInfo &region);
+
+  void Append(const lldb::SBMemoryRegionInfoList &region_list);
 
   void Clear();
 
 protected:
   const MemoryRegionInfoListImpl *operator->() const;
 
   const MemoryRegionInfoListImpl &operator*() const;
 
 private:
   std::unique_ptr<MemoryRegionInfoListImpl> m_opaque_ap;
 };
 
 } // namespace lldb
 
 #endif // LLDB_SBMemoryRegionInfoList_h_

include/lldb/Target/Process.h

public:
   /// @param[out] region_list
   ///     A vector to contain MemoryRegionInfo objects for all mapped
   ///     ranges.
   ///
   /// @return
   ///     An error value.
   //------------------------------------------------------------------
   virtual Status
-  GetMemoryRegions(std::vector<lldb::MemoryRegionInfoSP> &region_list);
+  GetMemoryRegions(std::vector<lldb::MemoryRegionInfoUP> &region_list);
 
   virtual Status GetWatchpointSupportInfo(uint32_t &num) {
     Status error;
     num = 0;
     error.SetErrorString("Process::GetWatchpointSupportInfo() not supported");
     return error;
   }
 

include/lldb/lldb-forward.h

 //===-- lldb-forward.h ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 

 typedef std::unique_ptr<lldb_private::JITLoaderList> JITLoaderListUP;
 typedef std::shared_ptr<lldb_private::LanguageRuntime> LanguageRuntimeSP;
 typedef std::shared_ptr<lldb_private::SystemRuntime> SystemRuntimeSP;
 typedef std::unique_ptr<lldb_private::SystemRuntime> SystemRuntimeUP;
 typedef std::shared_ptr<lldb_private::LineTable> LineTableSP;
 typedef std::shared_ptr<lldb_private::Listener> ListenerSP;
 typedef std::weak_ptr<lldb_private::Listener> ListenerWP;
 typedef std::shared_ptr<lldb_private::MemoryHistory> MemoryHistorySP;
-typedef std::shared_ptr<lldb_private::MemoryRegionInfo> MemoryRegionInfoSP;
 typedef std::unique_ptr<lldb_private::MemoryRegionInfo> MemoryRegionInfoUP;
 typedef std::shared_ptr<lldb_private::Module> ModuleSP;
 typedef std::weak_ptr<lldb_private::Module> ModuleWP;
 typedef std::shared_ptr<lldb_private::ObjectFile> ObjectFileSP;
 typedef std::weak_ptr<lldb_private::ObjectFile> ObjectFileWP;
 typedef std::shared_ptr<lldb_private::ObjectFileJITDelegate>
     ObjectFileJITDelegateSP;
 typedef std::weak_ptr<lldb_private::ObjectFileJITDelegate>

source/API/SBMemoryRegionInfo.cpp

 #include "lldb/Utility/StreamString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBMemoryRegionInfo::SBMemoryRegionInfo()
     : m_opaque_ap(new MemoryRegionInfo()) {}
 
-SBMemoryRegionInfo::SBMemoryRegionInfo(const MemoryRegionInfo *lldb_object_ptr)
-    : m_opaque_ap(new MemoryRegionInfo()) {
-  if (lldb_object_ptr)
-    ref() = *lldb_object_ptr;
-}
+SBMemoryRegionInfo::SBMemoryRegionInfo(MemoryRegionInfoUP &&lldb_object_up)
+    : m_opaque_ap(std::move(lldb_object_up)) {}
 
 SBMemoryRegionInfo::SBMemoryRegionInfo(const SBMemoryRegionInfo &rhs)
     : m_opaque_ap(new MemoryRegionInfo()) {
   ref() = rhs.ref();
 }
 
 const SBMemoryRegionInfo &SBMemoryRegionInfo::
 operator=(const SBMemoryRegionInfo &rhs) {
   if (this != &rhs) {
     ref() = rhs.ref();
   }
   return *this;
 }
 
+SBMemoryRegionInfo &SBMemoryRegionInfo::
+operator=(MemoryRegionInfoUP &&lldb_object_up) {
+  m_opaque_ap = std::move(lldb_object_up);
+  return *this;
+}
+
 SBMemoryRegionInfo::~SBMemoryRegionInfo() {}
 
 void SBMemoryRegionInfo::Clear() { m_opaque_ap->Clear(); }
 
 bool SBMemoryRegionInfo::operator==(const SBMemoryRegionInfo &rhs) const {
   return ref() == rhs.ref();
 }
 

source/API/SBMemoryRegionInfoList.cpp

public:
 
   MemoryRegionInfoListImpl &operator=(const MemoryRegionInfoListImpl &rhs) {
     if (this == &rhs)
       return *this;
     m_regions = rhs.m_regions;
     return *this;
   }
 
-  uint32_t GetSize() { return m_regions.size(); }
+  size_t GetSize() const { return m_regions.size(); }
+
+  void Reserve(size_t capacity) { return m_regions.reserve(capacity); }
 
   void Append(const lldb::SBMemoryRegionInfo &sb_region) {
     m_regions.push_back(sb_region);
   }
 
   void Append(const MemoryRegionInfoListImpl &list) {
-    for (auto val : list.m_regions)
+    Reserve(GetSize() + list.GetSize());
+
+    for (const auto &val : list.m_regions)
       Append(val);
   }
 
   void Clear() { m_regions.clear(); }
 
-  bool GetMemoryRegionInfoAtIndex(uint32_t index,
+  bool GetMemoryRegionInfoAtIndex(size_t index,
                                   SBMemoryRegionInfo &region_info) {
     if (index >= GetSize())
       return false;
     region_info = m_regions[index];
     return true;
   }
 
 private:

 const SBMemoryRegionInfoList &SBMemoryRegionInfoList::
 operator=(const SBMemoryRegionInfoList &rhs) {
   if (this != &rhs) {
     *m_opaque_ap = *rhs.m_opaque_ap;
   }
   return *this;
 }
 
-uint32_t SBMemoryRegionInfoList::GetSize() const {
+size_t SBMemoryRegionInfoList::GetSize() const {
   return m_opaque_ap->GetSize();
 }
 
+void SBMemoryRegionInfoList::Reserve(size_t capacity) {
+  m_opaque_ap->Reserve(capacity);
+}
+
 bool SBMemoryRegionInfoList::GetMemoryRegionAtIndex(
-    uint32_t idx, SBMemoryRegionInfo &region_info) {
+    size_t idx, SBMemoryRegionInfo &region_info) {
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   bool result = m_opaque_ap->GetMemoryRegionInfoAtIndex(idx, region_info);
 
   if (log) {
     SBStream sstr;
     region_info.GetDescription(sstr);
     log->Printf("SBMemoryRegionInfoList::GetMemoryRegionAtIndex (this.ap=%p, "
                 "idx=%d) => SBMemoryRegionInfo (this.ap=%p, '%s')",
                 static_cast<void *>(m_opaque_ap.get()), idx,
                 static_cast<void *>(region_info.m_opaque_ap.get()),
                 sstr.GetData());
   }
 
   return result;
 }
 
 void SBMemoryRegionInfoList::Clear() { m_opaque_ap->Clear(); }
 
-void SBMemoryRegionInfoList::Append(SBMemoryRegionInfo &sb_region) {
+void SBMemoryRegionInfoList::Append(const SBMemoryRegionInfo &sb_region) {
   m_opaque_ap->Append(sb_region);
 }
 
-void SBMemoryRegionInfoList::Append(SBMemoryRegionInfoList &sb_region_list) {
+void
+SBMemoryRegionInfoList::Append(const SBMemoryRegionInfoList &sb_region_list) {
   m_opaque_ap->Append(*sb_region_list);
 }
 
 const MemoryRegionInfoListImpl *SBMemoryRegionInfoList::operator->() const {
   return m_opaque_ap.get();
 }
 
 const MemoryRegionInfoListImpl &SBMemoryRegionInfoList::operator*() const {
   assert(m_opaque_ap.get());
   return *m_opaque_ap.get();
 }

source/API/SBProcess.cpp

lldb::SBError SBProcess::SaveCore(const char *file_name) {
   return error;
 }
 
 lldb::SBError
 SBProcess::GetMemoryRegionInfo(lldb::addr_t load_addr,
                                SBMemoryRegionInfo &sb_region_info) {
   lldb::SBError sb_error;
   ProcessSP process_sp(GetSP());
-  MemoryRegionInfoSP region_info_sp =
-      std::make_shared<lldb_private::MemoryRegionInfo>();
   if (process_sp) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&process_sp->GetRunLock())) {
       std::lock_guard<std::recursive_mutex> guard(
           process_sp->GetTarget().GetAPIMutex());
+
+      MemoryRegionInfoUP region_info_up(new lldb_private::MemoryRegionInfo());
       sb_error.ref() =
-          process_sp->GetMemoryRegionInfo(load_addr, *region_info_sp);
+          process_sp->GetMemoryRegionInfo(load_addr, *region_info_up);
       if (sb_error.Success()) {
-        sb_region_info.ref() = *region_info_sp;
+        sb_region_info.m_opaque_ap = std::move(region_info_up);
       }
     } else {
       Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
       if (log)
         log->Printf(
             "SBProcess(%p)::GetMemoryRegionInfo() => error: process is running",
             static_cast<void *>(process_sp.get()));
       sb_error.SetErrorString("process is running");
     }
   } else {
     sb_error.SetErrorString("SBProcess is invalid");
   }
   return sb_error;
 }
 
 lldb::SBMemoryRegionInfoList SBProcess::GetMemoryRegions() {
-  lldb::SBError sb_error;
   lldb::SBMemoryRegionInfoList sb_region_list;
+
   ProcessSP process_sp(GetSP());
-  if (process_sp) {
-    Process::StopLocker stop_locker;
-    if (stop_locker.TryLock(&process_sp->GetRunLock())) {
-      std::lock_guard<std::recursive_mutex> guard(
-          process_sp->GetTarget().GetAPIMutex());
-      std::vector<MemoryRegionInfoSP> region_list;
-      sb_error.ref() = process_sp->GetMemoryRegions(region_list);
-      if (sb_error.Success()) {
-        std::vector<MemoryRegionInfoSP>::iterator end = region_list.end();
-        for (std::vector<MemoryRegionInfoSP>::iterator it = region_list.begin();
-             it != end; it++) {
-          SBMemoryRegionInfo sb_region_info(it->get());
-          sb_region_list.Append(sb_region_info);
-        }
-      }
-    } else {
-      Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
-      if (log)
-        log->Printf(
-            "SBProcess(%p)::GetMemoryRegionInfo() => error: process is running",
-            static_cast<void *>(process_sp.get()));
-      sb_error.SetErrorString("process is running");
-    }
-  } else {
-    sb_error.SetErrorString("SBProcess is invalid");
+  if (!process_sp)
+    return sb_region_list;
+
+
+    std::vector<MemoryRegionInfoUP> region_list;
+    if (process_sp->GetMemoryRegions(region_list).Success()) {
+      sb_region_list.Reserve(sb_region_list.GetSize() + region_list.size());
+      for (auto& region : region_list)
+        sb_region_list.Append(SBMemoryRegionInfo(std::move(region)));
   }
+
   return sb_region_list;
 }
 
 lldb::SBProcessInfo SBProcess::GetProcessInfo() {
   lldb::SBProcessInfo sb_proc_info;
   ProcessSP process_sp(GetSP());
   ProcessInstanceInfo proc_info;
   if (process_sp && process_sp->GetProcessInfo(proc_info)) {
     sb_proc_info.SetProcessInfo(proc_info);
   }
   return sb_proc_info;
 }

source/Plugins/Process/minidump/MinidumpParser.h

public:
   const MinidumpExceptionStream *GetExceptionStream();
 
   llvm::Optional<Range> FindMemoryRange(lldb::addr_t addr);
 
   llvm::ArrayRef<uint8_t> GetMemory(lldb::addr_t addr, size_t size);
 
   llvm::Optional<MemoryRegionInfo> GetMemoryRegionInfo(lldb::addr_t);
 
+  Status GetMemoryRegions(std::vector<lldb::MemoryRegionInfoUP> &region_list);
+
   // Perform consistency checks and initialize internal data structures
   Status Initialize();
 
 private:
   MinidumpParser(const lldb::DataBufferSP &data_buf_sp);
 
 private:
   lldb::DataBufferSP m_data_sp;
   llvm::DenseMap<uint32_t, MinidumpLocationDescriptor> m_directory_map;
   ArchSpec m_arch;
 };
 
 } // end namespace minidump
 } // end namespace lldb_private
 #endif // liblldb_MinidumpParser_h_

source/Plugins/Process/minidump/MinidumpParser.cpp

llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr,
 
   if (addr < range->start || offset >= range->range_ref.size())
     return {};
 
   const size_t overlap = std::min(size, range->range_ref.size() - offset);
   return range->range_ref.slice(offset, overlap);
 }
 
+namespace {
+  constexpr uint32_t StateMemFree =
+      static_cast<uint32_t>(MinidumpMemoryInfoState::MemFree);
+
+  MemoryRegionInfo MemoryInfoToRegionInfo(const MinidumpMemoryInfo &entry) {
+  const auto yes = MemoryRegionInfo::eYes;
+  const auto no = MemoryRegionInfo::eNo;
+
+  const auto head = entry.base_address;
+  const auto tail = head + entry.region_size;
+  MemoryRegionInfo info;
+
+  info.GetRange().SetRangeBase(head);
+  info.GetRange().SetRangeEnd(tail);
+
+  const uint32_t PageNoAccess =
+      static_cast<uint32_t>(MinidumpMemoryProtectionContants::PageNoAccess);
+  info.SetReadable((entry.protect & PageNoAccess) == 0 ? yes : no);
+
+  const uint32_t PageWritable =
+      static_cast<uint32_t>(MinidumpMemoryProtectionContants::PageWritable);
+  info.SetWritable((entry.protect & PageWritable) != 0 ? yes : no);
+
+  const uint32_t PageExecutable = static_cast<uint32_t>(
+      MinidumpMemoryProtectionContants::PageExecutable);
+  info.SetExecutable((entry.protect & PageExecutable) != 0 ? yes : no);
+  info.SetMapped((entry.state != StateMemFree) ? yes : no);
+
+  return info;
+}
+} // namespace
+
 llvm::Optional<MemoryRegionInfo>
 MinidumpParser::GetMemoryRegionInfo(lldb::addr_t load_addr) {
-  MemoryRegionInfo info;
   llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::MemoryInfoList);
   if (data.empty())
     return llvm::None;
 
   std::vector<const MinidumpMemoryInfo *> mem_info_list =
       MinidumpMemoryInfo::ParseMemoryInfoList(data);
   if (mem_info_list.empty())
     return llvm::None;
 
   const auto yes = MemoryRegionInfo::eYes;
   const auto no = MemoryRegionInfo::eNo;
 
   const MinidumpMemoryInfo *next_entry = nullptr;
-  for (const auto &entry : mem_info_list) {
+  for (const auto entry : mem_info_list) {
     const auto head = entry->base_address;
     const auto tail = head + entry->region_size;
 
     if (head <= load_addr && load_addr < tail) {
-      info.GetRange().SetRangeBase(
-          (entry->state != uint32_t(MinidumpMemoryInfoState::MemFree))
-              ? head
-              : load_addr);
-      info.GetRange().SetRangeEnd(tail);
-
-      const uint32_t PageNoAccess =
-          static_cast<uint32_t>(MinidumpMemoryProtectionContants::PageNoAccess);
-      info.SetReadable((entry->protect & PageNoAccess) == 0 ? yes : no);
-
-      const uint32_t PageWritable =
-          static_cast<uint32_t>(MinidumpMemoryProtectionContants::PageWritable);
-      info.SetWritable((entry->protect & PageWritable) != 0 ? yes : no);
-
-      const uint32_t PageExecutable = static_cast<uint32_t>(
-          MinidumpMemoryProtectionContants::PageExecutable);
-      info.SetExecutable((entry->protect & PageExecutable) != 0 ? yes : no);
-
-      const uint32_t MemFree =
-          static_cast<uint32_t>(MinidumpMemoryInfoState::MemFree);
-      info.SetMapped((entry->state != MemFree) ? yes : no);
-
+      MemoryRegionInfo info = MemoryInfoToRegionInfo(*entry);
+      if (entry->state == StateMemFree)
+        info.GetRange().SetRangeBase(load_addr);
       return info;
     } else if (head > load_addr &&
                (next_entry == nullptr || head < next_entry->base_address)) {
       // In case there is no region containing load_addr keep track of the
       // nearest region after load_addr so we can return the distance to it.
       next_entry = entry;
     }
   }
 
   // No containing region found. Create an unmapped region that extends to the
   // next region or LLDB_INVALID_ADDRESS
+  MemoryRegionInfo info;
   info.GetRange().SetRangeBase(load_addr);
   info.GetRange().SetRangeEnd((next_entry != nullptr) ? next_entry->base_address
                                                       : LLDB_INVALID_ADDRESS);
   info.SetReadable(no);
   info.SetWritable(no);
   info.SetExecutable(no);
   info.SetMapped(no);
 
   // Note that the memory info list doesn't seem to contain ranges in kernel
   // space, so if you're walking a stack that has kernel frames, the stack may
   // appear truncated.
   return info;
 }
 
+Status MinidumpParser::GetMemoryRegions(
+    std::vector<lldb::MemoryRegionInfoUP> &region_list) {
+  llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::MemoryInfoList);
+  if (data.empty())
+    return Status("No memory info data");
+
+  std::vector<const MinidumpMemoryInfo *> mem_info_list =
+      MinidumpMemoryInfo::ParseMemoryInfoList(data);
+  if (mem_info_list.empty())
+    return Status("Failed to parse memory info data");
+
+  region_list.reserve(mem_info_list.size());
+  for (const auto entry : mem_info_list) {
+    if (entry->state != StateMemFree)
+      region_list.emplace_back(
+          new MemoryRegionInfo(MemoryInfoToRegionInfo(*entry)));
+  }
+
+  return Status();
+}
+
 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);

source/Plugins/Process/minidump/MinidumpTypes.cpp

MinidumpMemoryInfo::ParseMemoryInfoList(llvm::ArrayRef<uint8_t> &data) {
 
   data = data.drop_front(header->size_of_header -
                          sizeof(MinidumpMemoryInfoListHeader));
 
   if (header->size_of_entry * header->num_of_entries > data.size())
     return {};
 
   std::vector<const MinidumpMemoryInfo *> result;
+  result.reserve(header->num_of_entries);
+
   for (uint64_t i = 0; i < header->num_of_entries; ++i) {
     result.push_back(reinterpret_cast<const MinidumpMemoryInfo *>(
         data.data() + i * header->size_of_entry));
   }
 
   return result;
 }

source/Plugins/Process/minidump/ProcessMinidump.h

public:
   size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                       Status &error) override;
 
   ArchSpec GetArchitecture();
 
   Status GetMemoryRegionInfo(lldb::addr_t load_addr,
                              MemoryRegionInfo &range_info) override;
 
+  Status
+  GetMemoryRegions(std::vector<lldb::MemoryRegionInfoUP> &region_list) override;
+
   bool GetProcessInfo(ProcessInstanceInfo &info) override;
 
   Status WillResume() override {
     Status error;
     error.SetErrorStringWithFormat(
         "error: %s does not support resuming processes",
         GetPluginName().GetCString());
     return error;

source/Plugins/Process/minidump/ProcessMinidump.cpp

Status ProcessMinidump::GetMemoryRegionInfo(lldb::addr_t load_addr,
   if (!info) {
     error.SetErrorString("No valid MemoryRegionInfo found!");
     return error;
   }
   range_info = info.getValue();
   return error;
 }
 
+Status ProcessMinidump::GetMemoryRegions(
+    std::vector<lldb::MemoryRegionInfoUP> &region_list) {
+  region_list.clear();
+  return m_minidump_parser.GetMemoryRegions(region_list);
+}
+
 void ProcessMinidump::Clear() { Process::m_thread_list.Clear(); }
 
 bool ProcessMinidump::UpdateThreadList(ThreadList &old_thread_list,
                                        ThreadList &new_thread_list) {
   uint32_t num_threads = 0;
   if (m_thread_list.size() > 0)
     num_threads = m_thread_list.size();
 

source/Target/Process.cpp

if (next_branch_insn_address.IsValid() &&
       retval = next_branch_insn_address;
     }
   }
 
   return retval;
 }
 
 Status
-Process::GetMemoryRegions(std::vector<lldb::MemoryRegionInfoSP> &region_list) {
+Process::GetMemoryRegions(std::vector<lldb::MemoryRegionInfoUP> &region_list) {
 
   Status error;
 
   lldb::addr_t range_end = 0;
 
   region_list.clear();
   do {
-    lldb::MemoryRegionInfoSP region_info(new lldb_private::MemoryRegionInfo());
+    lldb::MemoryRegionInfoUP region_info(new lldb_private::MemoryRegionInfo());
     error = GetMemoryRegionInfo(range_end, *region_info);
     // GetMemoryRegionInfo should only return an error if it is unimplemented.
     if (error.Fail()) {
       region_list.clear();
       break;
     }
 
     range_end = region_info->GetRange().GetRangeEnd();
     if (region_info->GetMapped() == MemoryRegionInfo::eYes) {
-      region_list.push_back(region_info);
+      region_list.push_back(std::move(region_info));
     }
   } while (range_end != LLDB_INVALID_ADDRESS);
 
   return error;
 }
 
 Status
 Process::ConfigureStructuredData(const ConstString &type_name,