[Lldb-commits] [lldb] a96a3f1 - [lldb][Minidump Parser] Implement a range data vector for minidump memory ranges (#136040)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 17 18:37:19 PDT 2025
Author: Jacob Lalonde
Date: 2025-06-17T18:37:15-07:00
New Revision: a96a3f1b26baa8e5ee0abbac629f02566b7e9d1c
URL: https://github.com/llvm/llvm-project/commit/a96a3f1b26baa8e5ee0abbac629f02566b7e9d1c
DIFF: https://github.com/llvm/llvm-project/commit/a96a3f1b26baa8e5ee0abbac629f02566b7e9d1c.diff
LOG: [lldb][Minidump Parser] Implement a range data vector for minidump memory ranges (#136040)
Recently I was debugging a Minidump with a few thousand ranges, and came
across the (now deleted) comment:
```
// I don't have a sense of how frequently this is called or how many memory
// ranges a Minidump typically has, so I'm not sure if searching for the
// appropriate range linearly each time is stupid. Perhaps we should build
// an index for faster lookups.
```
blaming this comment, it's 9 years old! Much overdue for this simple fix
with a range data vector.
I had to add a default constructor to Range in order to implement the
RangeDataVector, but otherwise this just a replacement of look up logic.
Added:
Modified:
lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
lldb/source/Plugins/Process/minidump/MinidumpParser.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 94c0a5f11e435..ef691b77193ce 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -20,8 +20,8 @@
#include <algorithm>
#include <map>
#include <optional>
-#include <vector>
#include <utility>
+#include <vector>
using namespace lldb_private;
using namespace minidump;
@@ -75,8 +75,7 @@ UUID MinidumpParser::GetModuleUUID(const minidump::Module *module) {
if (GetArchitecture().GetTriple().isOSBinFormatELF()) {
if (pdb70_uuid->Age != 0)
return UUID(pdb70_uuid, sizeof(*pdb70_uuid));
- return UUID(&pdb70_uuid->Uuid,
- sizeof(pdb70_uuid->Uuid));
+ return UUID(&pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
}
return UUID(*pdb70_uuid);
} else if (cv_signature == CvSignature::ElfBuildId)
@@ -429,62 +428,65 @@ MinidumpParser::GetExceptionStreams() {
std::optional<minidump::Range>
MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
- Log *log = GetLog(LLDBLog::Modules);
+ if (m_memory_ranges.IsEmpty())
+ PopulateMemoryRanges();
+
+ const MemoryRangeVector::Entry *entry =
+ m_memory_ranges.FindEntryThatContains(addr);
+ if (!entry)
+ return std::nullopt;
+ return entry->data;
+}
+
+void MinidumpParser::PopulateMemoryRanges() {
+ Log *log = GetLog(LLDBLog::Modules);
auto ExpectedMemory = GetMinidumpFile().getMemoryList();
- if (!ExpectedMemory) {
- LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
- "Failed to read memory list: {0}");
- } else {
+ if (ExpectedMemory) {
for (const auto &memory_desc : *ExpectedMemory) {
const LocationDescriptor &loc_desc = memory_desc.Memory;
const lldb::addr_t range_start = memory_desc.StartOfMemoryRange;
const size_t range_size = loc_desc.DataSize;
-
- if (loc_desc.RVA + loc_desc.DataSize > GetData().size())
- return std::nullopt;
-
- if (range_start <= addr && addr < range_start + range_size) {
- auto ExpectedSlice = GetMinidumpFile().getRawData(loc_desc);
- if (!ExpectedSlice) {
- LLDB_LOG_ERROR(log, ExpectedSlice.takeError(),
- "Failed to get memory slice: {0}");
- return std::nullopt;
- }
- return minidump::Range(range_start, *ExpectedSlice);
+ auto ExpectedSlice = GetMinidumpFile().getRawData(loc_desc);
+ if (!ExpectedSlice) {
+ LLDB_LOG_ERROR(log, ExpectedSlice.takeError(),
+ "Failed to get memory slice: {0}");
+ continue;
}
+ m_memory_ranges.Append(MemoryRangeVector::Entry(
+ range_start, range_size,
+ minidump::Range(range_start, *ExpectedSlice)));
}
+ } else {
+ LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
+ "Failed to read memory list: {0}");
}
if (!GetStream(StreamType::Memory64List).empty()) {
llvm::Error err = llvm::Error::success();
- for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
- if (memory_desc.first.StartOfMemoryRange <= addr
- && addr < memory_desc.first.StartOfMemoryRange + memory_desc.first.DataSize) {
- return minidump::Range(memory_desc.first.StartOfMemoryRange, memory_desc.second);
- }
+ for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
+ m_memory_ranges.Append(MemoryRangeVector::Entry(
+ memory_desc.first.StartOfMemoryRange, memory_desc.first.DataSize,
+ minidump::Range(memory_desc.first.StartOfMemoryRange,
+ memory_desc.second)));
}
if (err)
LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
}
- return std::nullopt;
+ m_memory_ranges.Sort();
}
llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr,
size_t size) {
- // I don't have a sense of how frequently this is called or how many memory
- // ranges a Minidump typically has, so I'm not sure if searching for the
- // appropriate range linearly each time is stupid. Perhaps we should build
- // an index for faster lookups.
std::optional<minidump::Range> range = FindMemoryRange(addr);
if (!range)
return {};
// There's at least some overlap between the beginning of the desired range
- // (addr) and the current range. Figure out where the overlap begins and how
- // much overlap there is.
+ // (addr) and the current range. Figure out where the overlap begins and
+ // how much overlap there is.
const size_t offset = addr - range->start;
@@ -495,7 +497,8 @@ llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr,
return range->range_ref.slice(offset, overlap);
}
-llvm::iterator_range<FallibleMemory64Iterator> MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
+llvm::iterator_range<FallibleMemory64Iterator>
+MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
llvm::ErrorAsOutParameter ErrAsOutParam(&err);
return m_file->getMemory64List(err);
}
@@ -607,8 +610,7 @@ std::pair<MemoryRegionInfos, bool> MinidumpParser::BuildMemoryRegions() {
case StreamType::ST: \
return #ST
-llvm::StringRef
-MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
+llvm::StringRef MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
switch (stream_type) {
ENUM_TO_CSTR(Unused);
ENUM_TO_CSTR(ThreadList);
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index 2c5e6f19ff9a1..14599f8d572aa 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -17,6 +17,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/UUID.h"
+#include "lldb/Utility/RangeMap.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/StringRef.h"
@@ -35,6 +36,9 @@ namespace minidump {
// Describes a range of memory captured in the Minidump
struct Range {
+ // Default constructor required for range data vector
+ // but unusued.
+ Range() = default;
lldb::addr_t start; // virtual address of the beginning of the range
// range_ref - absolute pointer to the first byte of the range and size
llvm::ArrayRef<uint8_t> range_ref;
@@ -45,9 +49,18 @@ struct Range {
friend bool operator==(const Range &lhs, const Range &rhs) {
return lhs.start == rhs.start && lhs.range_ref == rhs.range_ref;
}
+
+ friend bool operator<(const Range &lhs, const Range &rhs) {
+ if (lhs.start == rhs.start)
+ return lhs.range_ref.size() < rhs.range_ref.size();
+ return lhs.start < rhs.start;
+ }
};
-using FallibleMemory64Iterator = llvm::object::MinidumpFile::FallibleMemory64Iterator;
+using MemoryRangeVector =
+ lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, minidump::Range>;
+using FallibleMemory64Iterator =
+ llvm::object::MinidumpFile::FallibleMemory64Iterator;
using ExceptionStreamsIterator =
llvm::object::MinidumpFile::ExceptionStreamsIterator;
@@ -97,7 +110,8 @@ class MinidumpParser {
/// complete (includes all regions mapped into the process memory).
std::pair<MemoryRegionInfos, bool> BuildMemoryRegions();
- llvm::iterator_range<FallibleMemory64Iterator> GetMemory64Iterator(llvm::Error &err);
+ llvm::iterator_range<FallibleMemory64Iterator>
+ GetMemory64Iterator(llvm::Error &err);
static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
@@ -109,10 +123,11 @@ class MinidumpParser {
private:
MinidumpParser(lldb::DataBufferSP data_sp,
std::unique_ptr<llvm::object::MinidumpFile> file);
-
+ void PopulateMemoryRanges();
lldb::DataBufferSP m_data_sp;
std::unique_ptr<llvm::object::MinidumpFile> m_file;
ArchSpec m_arch;
+ MemoryRangeVector m_memory_ranges;
};
} // end namespace minidump
More information about the lldb-commits
mailing list