Skip to content

Commit 6cb1459

Browse files
JlalondJlalond
authored
[LLDB][Minidump] Fix ProcessMinidump::GetMemoryRegions to include 64b regions when /proc/pid maps are missing. (#101086)
This PR is in response to a bug my coworker @mbucko discovered where on MacOS Minidumps were being created where the 64b memory regions were readable, but were not being listed in `SBProcess.GetMemoryRegionList()`. This went unnoticed in #95312 due to all the linux testing including /proc/pid maps. On MacOS generated dumps (or any dump without access to /proc/pid) we would fail to properly map Memory Regions due to there being two independent methods for 32b and 64b mapping. In this PR I addressed this minor bug and merged the methods, but in order to add test coverage required additions to `obj2yaml` and `yaml2obj` which make up the bulk of this patch. Lastly, there are some non-required changes such as the addition of the `Memory64ListHeader` type, to make writing/reading the header section of the Memory64List easier.
1 parent e78156a commit 6cb1459

File tree

7 files changed

+119
-96
lines changed

7 files changed

+119
-96
lines changed

‎lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,15 +1014,17 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
10141014
// With a size of the number of ranges as a 32 bit num
10151015
// And then the size of all the ranges
10161016
error = AddDirectory(StreamType::MemoryList,
1017-
sizeof(llvm::support::ulittle32_t) +
1017+
sizeof(llvm::minidump::MemoryListHeader) +
10181018
descriptors.size() *
10191019
sizeof(llvm::minidump::MemoryDescriptor));
10201020
if (error.Fail())
10211021
return error;
10221022

1023+
llvm::minidump::MemoryListHeader list_header;
10231024
llvm::support::ulittle32_t memory_ranges_num =
10241025
static_cast<llvm::support::ulittle32_t>(descriptors.size());
1025-
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
1026+
list_header.NumberOfMemoryRanges = memory_ranges_num;
1027+
m_data.AppendData(&list_header, sizeof(llvm::minidump::MemoryListHeader));
10261028
// For 32b we can get away with writing off the descriptors after the data.
10271029
// This means no cleanup loop needed.
10281030
m_data.AppendData(descriptors.data(),
@@ -1044,9 +1046,10 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
10441046
if (error.Fail())
10451047
return error;
10461048

1049+
llvm::minidump::Memory64ListHeader list_header;
10471050
llvm::support::ulittle64_t memory_ranges_num =
10481051
static_cast<llvm::support::ulittle64_t>(ranges.size());
1049-
m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
1052+
list_header.NumberOfMemoryRanges = memory_ranges_num;
10501053
// Capture the starting offset for all the descriptors so we can clean them up
10511054
// if needed.
10521055
offset_t starting_offset =
@@ -1058,8 +1061,8 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
10581061
(ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
10591062
llvm::support::ulittle64_t memory_ranges_base_rva =
10601063
static_cast<llvm::support::ulittle64_t>(base_rva);
1061-
m_data.AppendData(&memory_ranges_base_rva,
1062-
sizeof(llvm::support::ulittle64_t));
1064+
list_header.BaseRVA = memory_ranges_base_rva;
1065+
m_data.AppendData(&list_header, sizeof(llvm::minidump::Memory64ListHeader));
10631066

10641067
bool cleanup_required = false;
10651068
std::vector<MemoryDescriptor_64> descriptors;

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

Lines changed: 45 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,6 @@ const minidump::ExceptionStream *MinidumpParser::GetExceptionStream() {
429429

430430
std::optional<minidump::Range>
431431
MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
432-
llvm::ArrayRef<uint8_t> data64 = GetStream(StreamType::Memory64List);
433432
Log *log = GetLog(LLDBLog::Modules);
434433

435434
auto ExpectedMemory = GetMinidumpFile().getMemoryList();
@@ -457,33 +456,17 @@ MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
457456
}
458457
}
459458

460-
// Some Minidumps have a Memory64ListStream that captures all the heap memory
461-
// (full-memory Minidumps). We can't exactly use the same loop as above,
462-
// because the Minidump uses slightly different data structures to describe
463-
// those
464-
465-
if (!data64.empty()) {
466-
llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
467-
uint64_t base_rva;
468-
std::tie(memory64_list, base_rva) =
469-
MinidumpMemoryDescriptor64::ParseMemory64List(data64);
470-
471-
if (memory64_list.empty())
472-
return std::nullopt;
473-
474-
for (const auto &memory_desc64 : memory64_list) {
475-
const lldb::addr_t range_start = memory_desc64.start_of_memory_range;
476-
const size_t range_size = memory_desc64.data_size;
477-
478-
if (base_rva + range_size > GetData().size())
479-
return std::nullopt;
480-
481-
if (range_start <= addr && addr < range_start + range_size) {
482-
return minidump::Range(range_start,
483-
GetData().slice(base_rva, range_size));
459+
if (!GetStream(StreamType::Memory64List).empty()) {
460+
llvm::Error err = llvm::Error::success();
461+
for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
462+
if (memory_desc.first.StartOfMemoryRange <= addr
463+
&& addr < memory_desc.first.StartOfMemoryRange + memory_desc.first.DataSize) {
464+
return minidump::Range(memory_desc.first.StartOfMemoryRange, memory_desc.second);
484465
}
485-
base_rva += range_size;
486466
}
467+
468+
if (err)
469+
LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
487470
}
488471

489472
return std::nullopt;
@@ -512,6 +495,11 @@ llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr,
512495
return range->range_ref.slice(offset, overlap);
513496
}
514497

498+
llvm::iterator_range<FallibleMemory64Iterator> MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
499+
llvm::ErrorAsOutParameter ErrAsOutParam(&err);
500+
return m_file->getMemory64List(err);
501+
}
502+
515503
static bool
516504
CreateRegionsCacheFromMemoryInfoList(MinidumpParser &parser,
517505
std::vector<MemoryRegionInfo> &regions) {
@@ -553,53 +541,44 @@ static bool
553541
CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
554542
std::vector<MemoryRegionInfo> &regions) {
555543
Log *log = GetLog(LLDBLog::Modules);
544+
// Cache the expected memory32 into an optional
545+
// because it is possible to just have a memory64 list
556546
auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList();
557547
if (!ExpectedMemory) {
558548
LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
559549
"Failed to read memory list: {0}");
560-
return false;
561-
}
562-
regions.reserve(ExpectedMemory->size());
563-
for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
564-
if (memory_desc.Memory.DataSize == 0)
565-
continue;
566-
MemoryRegionInfo region;
567-
region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
568-
region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
569-
region.SetReadable(MemoryRegionInfo::eYes);
570-
region.SetMapped(MemoryRegionInfo::eYes);
571-
regions.push_back(region);
550+
} else {
551+
for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
552+
if (memory_desc.Memory.DataSize == 0)
553+
continue;
554+
MemoryRegionInfo region;
555+
region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
556+
region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
557+
region.SetReadable(MemoryRegionInfo::eYes);
558+
region.SetMapped(MemoryRegionInfo::eYes);
559+
regions.push_back(region);
560+
}
572561
}
573-
regions.shrink_to_fit();
574-
return !regions.empty();
575-
}
576-
577-
static bool
578-
CreateRegionsCacheFromMemory64List(MinidumpParser &parser,
579-
std::vector<MemoryRegionInfo> &regions) {
580-
llvm::ArrayRef<uint8_t> data =
581-
parser.GetStream(StreamType::Memory64List);
582-
if (data.empty())
583-
return false;
584-
llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
585-
uint64_t base_rva;
586-
std::tie(memory64_list, base_rva) =
587-
MinidumpMemoryDescriptor64::ParseMemory64List(data);
588562

589-
if (memory64_list.empty())
590-
return false;
563+
if (!parser.GetStream(StreamType::Memory64List).empty()) {
564+
llvm::Error err = llvm::Error::success();
565+
for (const auto &memory_desc : parser.GetMemory64Iterator(err)) {
566+
if (memory_desc.first.DataSize == 0)
567+
continue;
568+
MemoryRegionInfo region;
569+
region.GetRange().SetRangeBase(memory_desc.first.StartOfMemoryRange);
570+
region.GetRange().SetByteSize(memory_desc.first.DataSize);
571+
region.SetReadable(MemoryRegionInfo::eYes);
572+
region.SetMapped(MemoryRegionInfo::eYes);
573+
regions.push_back(region);
574+
}
591575

592-
regions.reserve(memory64_list.size());
593-
for (const auto &memory_desc : memory64_list) {
594-
if (memory_desc.data_size == 0)
595-
continue;
596-
MemoryRegionInfo region;
597-
region.GetRange().SetRangeBase(memory_desc.start_of_memory_range);
598-
region.GetRange().SetByteSize(memory_desc.data_size);
599-
region.SetReadable(MemoryRegionInfo::eYes);
600-
region.SetMapped(MemoryRegionInfo::eYes);
601-
regions.push_back(region);
576+
if (err) {
577+
LLDB_LOG_ERROR(log, std::move(err), "Failed to read memory64 list: {0}");
578+
return false;
579+
}
602580
}
581+
603582
regions.shrink_to_fit();
604583
return !regions.empty();
605584
}
@@ -620,9 +599,7 @@ std::pair<MemoryRegionInfos, bool> MinidumpParser::BuildMemoryRegions() {
620599
return return_sorted(true);
621600
if (CreateRegionsCacheFromMemoryInfoList(*this, result))
622601
return return_sorted(true);
623-
if (CreateRegionsCacheFromMemoryList(*this, result))
624-
return return_sorted(false);
625-
CreateRegionsCacheFromMemory64List(*this, result);
602+
CreateRegionsCacheFromMemoryList(*this, result);
626603
return return_sorted(false);
627604
}
628605

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ struct Range {
4747
}
4848
};
4949

50+
using FallibleMemory64Iterator = llvm::object::MinidumpFile::FallibleMemory64Iterator;
51+
5052
class MinidumpParser {
5153
public:
5254
static llvm::Expected<MinidumpParser>
@@ -92,6 +94,8 @@ class MinidumpParser {
9294
/// complete (includes all regions mapped into the process memory).
9395
std::pair<MemoryRegionInfos, bool> BuildMemoryRegions();
9496

97+
llvm::iterator_range<FallibleMemory64Iterator> GetMemory64Iterator(llvm::Error &err);
98+
9599
static llvm::StringRef GetStreamTypeAsString(StreamType stream_type);
96100

97101
llvm::object::MinidumpFile &GetMinidumpFile() { return *m_file; }

‎lldb/source/Plugins/Process/minidump/MinidumpTypes.cpp‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,3 @@ LinuxProcStatus::Parse(llvm::ArrayRef<uint8_t> &data) {
5757
}
5858

5959
lldb::pid_t LinuxProcStatus::GetPid() const { return pid; }
60-
61-
std::pair<llvm::ArrayRef<MinidumpMemoryDescriptor64>, uint64_t>
62-
MinidumpMemoryDescriptor64::ParseMemory64List(llvm::ArrayRef<uint8_t> &data) {
63-
const llvm::support::ulittle64_t *mem_ranges_count;
64-
Status error = consumeObject(data, mem_ranges_count);
65-
if (error.Fail() ||
66-
*mem_ranges_count * sizeof(MinidumpMemoryDescriptor64) > data.size())
67-
return {};
68-
69-
const llvm::support::ulittle64_t *base_rva;
70-
error = consumeObject(data, base_rva);
71-
if (error.Fail())
72-
return {};
73-
74-
return std::make_pair(
75-
llvm::ArrayRef(
76-
reinterpret_cast<const MinidumpMemoryDescriptor64 *>(data.data()),
77-
*mem_ranges_count),
78-
*base_rva);
79-
}

‎lldb/source/Plugins/Process/minidump/MinidumpTypes.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,6 @@ Status consumeObject(llvm::ArrayRef<uint8_t> &Buffer, const T *&Object) {
6262
struct MinidumpMemoryDescriptor64 {
6363
llvm::support::ulittle64_t start_of_memory_range;
6464
llvm::support::ulittle64_t data_size;
65-
66-
static std::pair<llvm::ArrayRef<MinidumpMemoryDescriptor64>, uint64_t>
67-
ParseMemory64List(llvm::ArrayRef<uint8_t> &data);
6865
};
6966
static_assert(sizeof(MinidumpMemoryDescriptor64) == 16,
7067
"sizeof MinidumpMemoryDescriptor64 is not correct!");

‎lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,3 +491,22 @@ def test_minidump_sysroot(self):
491491
spec_dir_norm = os.path.normcase(module.GetFileSpec().GetDirectory())
492492
exe_dir_norm = os.path.normcase(exe_dir)
493493
self.assertEqual(spec_dir_norm, exe_dir_norm)
494+
495+
def test_minidump_memory64list(self):
496+
"""Test that lldb can read from the memory64list in a minidump."""
497+
self.process_from_yaml("linux-x86_64_mem64.yaml")
498+
499+
region_count = 3
500+
region_info_list = self.process.GetMemoryRegions()
501+
self.assertEqual(region_info_list.GetSize(), region_count)
502+
503+
region = lldb.SBMemoryRegionInfo()
504+
self.assertTrue(region_info_list.GetMemoryRegionAtIndex(0, region))
505+
self.assertEqual(region.GetRegionBase(), 0x7FFF12A84030)
506+
self.assertTrue(region.GetRegionEnd(), 0x7FFF12A84030 + 0x2FD0)
507+
self.assertTrue(region_info_list.GetMemoryRegionAtIndex(1, region))
508+
self.assertEqual(region.GetRegionBase(), 0x00007fff12a87000)
509+
self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87000 + 0x00000018)
510+
self.assertTrue(region_info_list.GetMemoryRegionAtIndex(2, region))
511+
self.assertEqual(region.GetRegionBase(), 0x00007fff12a87018)
512+
self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87018 + 0x00000400)

‎lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--- !minidump
2+
Streams:
3+
- Type: SystemInfo
4+
Processor Arch: AMD64
5+
Processor Level: 6
6+
Processor Revision: 15876
7+
Number of Processors: 40
8+
Platform ID: Linux
9+
CSD Version: 'Linux 3.13.0-91-generic'
10+
CPU:
11+
Vendor ID: GenuineIntel
12+
Version Info: 0x00000000
13+
Feature Info: 0x00000000
14+
- Type: LinuxProcStatus
15+
Text: |
16+
Name: linux-x86_64
17+
State: t (tracing stop)
18+
Tgid: 29917
19+
Ngid: 0
20+
Pid: 29917
21+
PPid: 29370
22+
TracerPid: 29918
23+
Uid: 1001 1001 1001 1001
24+
Gid: 1001 1001 1001 1001
25+
- Type: ThreadList
26+
Threads:
27+
- Thread Id: 0x2896BB
28+
Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000700100000000000FFFFFFFF0000FFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B040A812FF7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000050D0A75BBA7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
29+
Stack:
30+
Start of Memory Range: 0x0
31+
Content: ''
32+
- Type: Memory64List
33+
Memory Ranges:
34+
- Start of Memory Range: 0x7FFF12A84030
35+
Data Size: 0x2FD0
36+
Content : ''
37+
- Start of Memory Range: 0x00007fff12a87000
38+
Data Size: 0x00000018
39+
Content : ''
40+
- Start of Memory Range: 0x00007fff12a87018
41+
Data Size: 0x00000400
42+
Content : ''
43+
...

0 commit comments

Comments
 (0)