[Lldb-commits] [lldb] [LLDB][Minidump] Fix ProcessMinidump::GetMemoryRegions to include 64b regions when /proc/pid maps are missing. (PR #101086)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 16 08:07:39 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101086
>From 2860a75fdc243f55d1e675068f9d120fb43cb21d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 26 Jul 2024 14:14:42 -0700
Subject: [PATCH 1/3] Remove 64b specific method and create Cache from both
memory 32 and memory 64.
Add Mem64List to Obj2Yaml and Yaml2Minidump
Fix yaml2obj minidump emission where I was passing a stack variable by reference. Change CreateRegionsCache to not double evaluate Expected<T> as it was causing unchecked to flip back to true instead slotting into an optional.
Uncomment python tests"
Run C++ and Python formatters.
Revert unchanged files.
Modify test yaml to show support for multiple mem64 entries
Remove llvm related components because they were changed in a different patch
---
.../Minidump/MinidumpFileBuilder.cpp | 13 ++--
.../Process/minidump/MinidumpParser.cpp | 58 +++++++++---------
.../minidump-new/TestMiniDumpNew.py | 19 ++++++
.../minidump-new/linux-x86_64_mem64.dmp | Bin 0 -> 860 bytes
.../minidump-new/linux-x86_64_mem64.yaml | 40 ++++++++++++
5 files changed, 96 insertions(+), 34 deletions(-)
create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index de212c6b20da7e..b941748bd7ccbb 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1020,15 +1020,17 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
// With a size of the number of ranges as a 32 bit num
// And then the size of all the ranges
error = AddDirectory(StreamType::MemoryList,
- sizeof(llvm::support::ulittle32_t) +
+ sizeof(llvm::minidump::MemoryListHeader) +
descriptors.size() *
sizeof(llvm::minidump::MemoryDescriptor));
if (error.Fail())
return error;
+ llvm::minidump::MemoryListHeader list_header;
llvm::support::ulittle32_t memory_ranges_num =
static_cast<llvm::support::ulittle32_t>(descriptors.size());
- m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
+ list_header.NumberOfMemoryRanges = memory_ranges_num;
+ m_data.AppendData(&list_header, sizeof(llvm::minidump::MemoryListHeader));
// For 32b we can get away with writing off the descriptors after the data.
// This means no cleanup loop needed.
m_data.AppendData(descriptors.data(),
@@ -1050,9 +1052,10 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
if (error.Fail())
return error;
+ llvm::minidump::Memory64ListHeader list_header;
llvm::support::ulittle64_t memory_ranges_num =
static_cast<llvm::support::ulittle64_t>(ranges.size());
- m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
+ list_header.NumberOfMemoryRanges = memory_ranges_num;
// Capture the starting offset for all the descriptors so we can clean them up
// if needed.
offset_t starting_offset =
@@ -1064,8 +1067,8 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
(ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
llvm::support::ulittle64_t memory_ranges_base_rva =
static_cast<llvm::support::ulittle64_t>(base_rva);
- m_data.AppendData(&memory_ranges_base_rva,
- sizeof(llvm::support::ulittle64_t));
+ list_header.BaseRVA = memory_ranges_base_rva;
+ m_data.AppendData(&list_header, sizeof(llvm::minidump::Memory64ListHeader));
bool cleanup_required = false;
std::vector<MemoryDescriptor_64> descriptors;
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index be9fae938e2276..7e19acd3d08347 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -553,43 +553,45 @@ static bool
CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
std::vector<MemoryRegionInfo> ®ions) {
Log *log = GetLog(LLDBLog::Modules);
+ // Cache the expected memory32 into an optional
+ // because double checking the expected triggers the unchecked warning.
+ std::optional<llvm::ArrayRef<MemoryDescriptor>> memory32_list;
auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList();
if (!ExpectedMemory) {
LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
"Failed to read memory list: {0}");
- return false;
- }
- regions.reserve(ExpectedMemory->size());
- for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
- if (memory_desc.Memory.DataSize == 0)
- continue;
- MemoryRegionInfo region;
- region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
- region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
- region.SetReadable(MemoryRegionInfo::eYes);
- region.SetMapped(MemoryRegionInfo::eYes);
- regions.push_back(region);
+ } else {
+ memory32_list = *ExpectedMemory;
}
- regions.shrink_to_fit();
- return !regions.empty();
-}
-static bool
-CreateRegionsCacheFromMemory64List(MinidumpParser &parser,
- std::vector<MemoryRegionInfo> ®ions) {
+ size_t num_regions = memory32_list ? memory32_list->size() : 0;
+
llvm::ArrayRef<uint8_t> data =
parser.GetStream(StreamType::Memory64List);
- if (data.empty())
- return false;
+
llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
- uint64_t base_rva;
- std::tie(memory64_list, base_rva) =
- MinidumpMemoryDescriptor64::ParseMemory64List(data);
+ if (!data.empty()) {
+ uint64_t base_rva;
+ std::tie(memory64_list, base_rva) =
+ MinidumpMemoryDescriptor64::ParseMemory64List(data);
- if (memory64_list.empty())
- return false;
+ num_regions += memory64_list.size();
+ }
+
+ regions.reserve(num_regions);
+ if (memory32_list) {
+ for (const MemoryDescriptor &memory_desc : *memory32_list) {
+ if (memory_desc.Memory.DataSize == 0)
+ continue;
+ MemoryRegionInfo region;
+ region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
+ region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
+ region.SetReadable(MemoryRegionInfo::eYes);
+ region.SetMapped(MemoryRegionInfo::eYes);
+ regions.push_back(region);
+ }
+ }
- regions.reserve(memory64_list.size());
for (const auto &memory_desc : memory64_list) {
if (memory_desc.data_size == 0)
continue;
@@ -620,9 +622,7 @@ std::pair<MemoryRegionInfos, bool> MinidumpParser::BuildMemoryRegions() {
return return_sorted(true);
if (CreateRegionsCacheFromMemoryInfoList(*this, result))
return return_sorted(true);
- if (CreateRegionsCacheFromMemoryList(*this, result))
- return return_sorted(false);
- CreateRegionsCacheFromMemory64List(*this, result);
+ CreateRegionsCacheFromMemoryList(*this, result);
return return_sorted(false);
}
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 91fd2439492b54..2de3e36b507341 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -491,3 +491,22 @@ def test_minidump_sysroot(self):
spec_dir_norm = os.path.normcase(module.GetFileSpec().GetDirectory())
exe_dir_norm = os.path.normcase(exe_dir)
self.assertEqual(spec_dir_norm, exe_dir_norm)
+
+ def test_minidump_memory64list(self):
+ """Test that lldb can read from the memory64list in a minidump."""
+ self.process_from_yaml("linux-x86_64_mem64.yaml")
+
+ region_count = 3
+ region_info_list = self.process.GetMemoryRegions()
+ self.assertEqual(region_info_list.GetSize(), region_count)
+
+ region = lldb.SBMemoryRegionInfo()
+ self.assertTrue(region_info_list.GetMemoryRegionAtIndex(0, region))
+ self.assertEqual(region.GetRegionBase(), 0x7FFF12A84030)
+ self.assertTrue(region.GetRegionEnd(), 0x7FFF12A84030 + 0x2FD0)
+ self.assertTrue(region_info_list.GetMemoryRegionAtIndex(1, region))
+ self.assertEqual(region.GetRegionBase(), 0x00007fff12a87000)
+ self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87000 + 0x00000018)
+ self.assertTrue(region_info_list.GetMemoryRegionAtIndex(2, region))
+ self.assertEqual(region.GetRegionBase(), 0x00007fff12a87018)
+ self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87018 + 0x00000400)
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
new file mode 100644
index 0000000000000000000000000000000000000000..a1069ef45ad3543bcc2862fa8699b239c2fefa8e
GIT binary patch
literal 860
zcmeZu at eP=~oPmLffq_8*h|vKvP{0I;Er6I4h&$LA7;J!oj6nA8X&OKhKga<}UjP~o
zqDckp3<6*+|AByk0YpLospb$;zrkUJ(EoZus>qiQxUf8W7Y)rP-({mZDG7lJ|1VOk
XVGIm6(&az?t7l*|fTp7h`VdnAg=Zxs
literal 0
HcmV?d00001
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
new file mode 100644
index 00000000000000..4a290355a863a1
--- /dev/null
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
@@ -0,0 +1,40 @@
+--- !minidump
+Streams:
+ - Type: SystemInfo
+ Processor Arch: AMD64
+ Processor Level: 6
+ Processor Revision: 15876
+ Number of Processors: 40
+ Platform ID: Linux
+ CSD Version: 'Linux 3.13.0-91-generic'
+ CPU:
+ Vendor ID: GenuineIntel
+ Version Info: 0x00000000
+ Feature Info: 0x00000000
+ - Type: LinuxProcStatus
+ Text: |
+ Name: linux-x86_64
+ State: t (tracing stop)
+ Tgid: 29917
+ Ngid: 0
+ Pid: 29917
+ PPid: 29370
+ TracerPid: 29918
+ Uid: 1001 1001 1001 1001
+ Gid: 1001 1001 1001 1001
+ - Type: ThreadList
+ Threads:
+ - Thread Id: 0x2896BB
+ Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000700100000000000FFFFFFFF0000FFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B040A812FF7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000050D0A75BBA7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+ Stack:
+ Start of Memory Range: 0x0
+ Content: ''
+ - Type: Memory64List
+ Memory Ranges:
+ - Start of memory range: 0x7FFF12A84030
+ Data Size: 0x2FD0
+ - Start of memory range: 0x00007fff12a87000
+ Data Size: 0x00000018
+ - Start of memory range: 0x00007fff12a87018
+ Data Size: 0x00000400
+...
>From aab60b78229a26114a2e919039b6eb89fa56c352 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 15 Aug 2024 16:49:27 -0700
Subject: [PATCH 2/3] Migrate everything to the new 64b memory iterator
---
.../Process/minidump/MinidumpParser.cpp | 88 ++++++++----------
.../Plugins/Process/minidump/MinidumpParser.h | 4 +
.../Process/minidump/MinidumpTypes.cpp | 20 ----
.../Plugins/Process/minidump/MinidumpTypes.h | 3 -
.../minidump-new/linux-x86_64_mem64.dmp | Bin 860 -> 0 bytes
.../minidump-new/linux-x86_64_mem64.yaml | 9 +-
6 files changed, 48 insertions(+), 76 deletions(-)
delete mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 7e19acd3d08347..afe8b134d283fb 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -429,7 +429,6 @@ const minidump::ExceptionStream *MinidumpParser::GetExceptionStream() {
std::optional<minidump::Range>
MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
- llvm::ArrayRef<uint8_t> data64 = GetStream(StreamType::Memory64List);
Log *log = GetLog(LLDBLog::Modules);
auto ExpectedMemory = GetMinidumpFile().getMemoryList();
@@ -457,34 +456,24 @@ MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
}
}
- // Some Minidumps have a Memory64ListStream that captures all the heap memory
- // (full-memory Minidumps). We can't exactly use the same loop as above,
- // because the Minidump uses slightly different data structures to describe
- // those
-
- if (!data64.empty()) {
- llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
- uint64_t base_rva;
- std::tie(memory64_list, base_rva) =
- MinidumpMemoryDescriptor64::ParseMemory64List(data64);
-
- if (memory64_list.empty())
- return std::nullopt;
-
- for (const auto &memory_desc64 : memory64_list) {
- const lldb::addr_t range_start = memory_desc64.start_of_memory_range;
- const size_t range_size = memory_desc64.data_size;
-
- if (base_rva + range_size > GetData().size())
- return std::nullopt;
-
- if (range_start <= addr && addr < range_start + range_size) {
- return minidump::Range(range_start,
- GetData().slice(base_rva, range_size));
+ if (!GetStream(StreamType::Memory64List).empty()) {
+ llvm::Error err = llvm::Error::success();
+ for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
+ // Explicit error check so we can return from within
+ if (memory_desc.first.StartOfMemoryRange <= addr
+ && addr < memory_desc.first.StartOfMemoryRange + memory_desc.first.DataSize
+ && !err) {
+ return minidump::Range(memory_desc.first.StartOfMemoryRange, memory_desc.second);
}
- base_rva += range_size;
}
+
+ if (err)
+ // Without std::move(err) fails with
+ // error: call to deleted constructor of '::llvm::Error'
+ LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
}
+
+
return std::nullopt;
}
@@ -512,6 +501,11 @@ 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::ErrorAsOutParameter ErrAsOutParam(&err);
+ return m_file->getMemory64List(err);
+}
+
static bool
CreateRegionsCacheFromMemoryInfoList(MinidumpParser &parser,
std::vector<MemoryRegionInfo> ®ions) {
@@ -564,21 +558,6 @@ CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
memory32_list = *ExpectedMemory;
}
- size_t num_regions = memory32_list ? memory32_list->size() : 0;
-
- llvm::ArrayRef<uint8_t> data =
- parser.GetStream(StreamType::Memory64List);
-
- llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
- if (!data.empty()) {
- uint64_t base_rva;
- std::tie(memory64_list, base_rva) =
- MinidumpMemoryDescriptor64::ParseMemory64List(data);
-
- num_regions += memory64_list.size();
- }
-
- regions.reserve(num_regions);
if (memory32_list) {
for (const MemoryDescriptor &memory_desc : *memory32_list) {
if (memory_desc.Memory.DataSize == 0)
@@ -592,16 +571,25 @@ CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
}
}
- for (const auto &memory_desc : memory64_list) {
- if (memory_desc.data_size == 0)
- continue;
- MemoryRegionInfo region;
- region.GetRange().SetRangeBase(memory_desc.start_of_memory_range);
- region.GetRange().SetByteSize(memory_desc.data_size);
- region.SetReadable(MemoryRegionInfo::eYes);
- region.SetMapped(MemoryRegionInfo::eYes);
- regions.push_back(region);
+ if (!parser.GetStream(StreamType::Memory64List).empty()) {
+ llvm::Error err = llvm::Error::success();
+ for (const auto &memory_desc : parser.GetMemory64Iterator(err)) {
+ if (memory_desc.first.DataSize == 0)
+ continue;
+ MemoryRegionInfo region;
+ region.GetRange().SetRangeBase(memory_desc.first.StartOfMemoryRange);
+ region.GetRange().SetByteSize(memory_desc.first.DataSize);
+ region.SetReadable(MemoryRegionInfo::eYes);
+ region.SetMapped(MemoryRegionInfo::eYes);
+ regions.push_back(region);
+ }
+
+ if (err) {
+ LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
+ return false;
+ }
}
+
regions.shrink_to_fit();
return !regions.empty();
}
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index 050ba086f46f54..222c0ef47fb853 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -47,6 +47,8 @@ struct Range {
}
};
+using FallibleMemory64Iterator = llvm::object::MinidumpFile::FallibleMemory64Iterator;
+
class MinidumpParser {
public:
static llvm::Expected<MinidumpParser>
@@ -92,6 +94,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);
+
static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
index 5b919828428fae..45dd2272aef041 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp
@@ -57,23 +57,3 @@ LinuxProcStatus::Parse(llvm::ArrayRef<uint8_t> &data) {
}
lldb::pid_t LinuxProcStatus::GetPid() const { return pid; }
-
-std::pair<llvm::ArrayRef<MinidumpMemoryDescriptor64>, uint64_t>
-MinidumpMemoryDescriptor64::ParseMemory64List(llvm::ArrayRef<uint8_t> &data) {
- const llvm::support::ulittle64_t *mem_ranges_count;
- Status error = consumeObject(data, mem_ranges_count);
- if (error.Fail() ||
- *mem_ranges_count * sizeof(MinidumpMemoryDescriptor64) > data.size())
- return {};
-
- const llvm::support::ulittle64_t *base_rva;
- error = consumeObject(data, base_rva);
- if (error.Fail())
- return {};
-
- return std::make_pair(
- llvm::ArrayRef(
- reinterpret_cast<const MinidumpMemoryDescriptor64 *>(data.data()),
- *mem_ranges_count),
- *base_rva);
-}
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h
index fe99abf9e24ed9..9a9f1cc1578336 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpTypes.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpTypes.h
@@ -62,9 +62,6 @@ Status consumeObject(llvm::ArrayRef<uint8_t> &Buffer, const T *&Object) {
struct MinidumpMemoryDescriptor64 {
llvm::support::ulittle64_t start_of_memory_range;
llvm::support::ulittle64_t data_size;
-
- static std::pair<llvm::ArrayRef<MinidumpMemoryDescriptor64>, uint64_t>
- ParseMemory64List(llvm::ArrayRef<uint8_t> &data);
};
static_assert(sizeof(MinidumpMemoryDescriptor64) == 16,
"sizeof MinidumpMemoryDescriptor64 is not correct!");
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
deleted file mode 100644
index a1069ef45ad3543bcc2862fa8699b239c2fefa8e..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001
literal 860
zcmeZu at eP=~oPmLffq_8*h|vKvP{0I;Er6I4h&$LA7;J!oj6nA8X&OKhKga<}UjP~o
zqDckp3<6*+|AByk0YpLospb$;zrkUJ(EoZus>qiQxUf8W7Y)rP-({mZDG7lJ|1VOk
XVGIm6(&az?t7l*|fTp7h`VdnAg=Zxs
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
index 4a290355a863a1..df3c6477ae50a0 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
@@ -31,10 +31,13 @@ Streams:
Content: ''
- Type: Memory64List
Memory Ranges:
- - Start of memory range: 0x7FFF12A84030
+ - Start of Memory Range: 0x7FFF12A84030
Data Size: 0x2FD0
- - Start of memory range: 0x00007fff12a87000
+ Content : ''
+ - Start of Memory Range: 0x00007fff12a87000
Data Size: 0x00000018
- - Start of memory range: 0x00007fff12a87018
+ Content : ''
+ - Start of Memory Range: 0x00007fff12a87018
Data Size: 0x00000400
+ Content : ''
...
>From 32e008c9d3efe3fd74a0091b2db4153046f7b694 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 16 Aug 2024 08:04:19 -0700
Subject: [PATCH 3/3] Remove comments that Pavel mentioned, and remove
redundant error checks
---
.../Plugins/Process/minidump/MinidumpParser.cpp | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index afe8b134d283fb..c099c28a620ecf 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -459,21 +459,15 @@ MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
if (!GetStream(StreamType::Memory64List).empty()) {
llvm::Error err = llvm::Error::success();
for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
- // Explicit error check so we can return from within
if (memory_desc.first.StartOfMemoryRange <= addr
- && addr < memory_desc.first.StartOfMemoryRange + memory_desc.first.DataSize
- && !err) {
+ && addr < memory_desc.first.StartOfMemoryRange + memory_desc.first.DataSize) {
return minidump::Range(memory_desc.first.StartOfMemoryRange, memory_desc.second);
}
}
if (err)
- // Without std::move(err) fails with
- // error: call to deleted constructor of '::llvm::Error'
LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
}
-
-
return std::nullopt;
}
@@ -548,18 +542,13 @@ CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
std::vector<MemoryRegionInfo> ®ions) {
Log *log = GetLog(LLDBLog::Modules);
// Cache the expected memory32 into an optional
- // because double checking the expected triggers the unchecked warning.
- std::optional<llvm::ArrayRef<MemoryDescriptor>> memory32_list;
+ // because it is possible to just have a memory64 list
auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList();
if (!ExpectedMemory) {
LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
"Failed to read memory list: {0}");
} else {
- memory32_list = *ExpectedMemory;
- }
-
- if (memory32_list) {
- for (const MemoryDescriptor &memory_desc : *memory32_list) {
+ for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
if (memory_desc.Memory.DataSize == 0)
continue;
MemoryRegionInfo region;
More information about the lldb-commits
mailing list