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 <jalalo...@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@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 <jalalo...@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@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 <jalalo...@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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits