Author: Jacob Lalonde Date: 2025-07-17T09:18:23-07:00 New Revision: a6fb3b3c18fd48a2eaaa8c969edbc013b9276a09
URL: https://github.com/llvm/llvm-project/commit/a6fb3b3c18fd48a2eaaa8c969edbc013b9276a09 DIFF: https://github.com/llvm/llvm-project/commit/a6fb3b3c18fd48a2eaaa8c969edbc013b9276a09.diff LOG: [LLDB] Process minidump better error messages (#149206) Prior, Process Minidump would return ``` Status::FromErrorString("could not parse memory info"); ``` For any unsuccessful memory read, with no differentiation between an error in LLDB and the data simply not being present. This lead to a lot of user confusion and overall pretty terrible user experience. To fix this I've refactored the APIs so we can pass an error back in an llvm expected. There were also no shell tests for memory read and process Minidump so I added one. Added: lldb/test/Shell/Minidump/missing-memory-region.yaml Modified: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp lldb/source/Plugins/Process/minidump/MinidumpParser.h lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp lldb/unittests/Process/minidump/MinidumpParserTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp index ef691b77193ce..58ebb7be11994 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp @@ -108,13 +108,21 @@ MinidumpParser::GetThreadContext(const minidump::Thread &td) { llvm::ArrayRef<uint8_t> MinidumpParser::GetThreadContextWow64(const minidump::Thread &td) { + Log *log = GetLog(LLDBLog::Process); // On Windows, a 32-bit process can run on a 64-bit machine under WOW64. If // the minidump was captured with a 64-bit debugger, then the CONTEXT we just // grabbed from the mini_dump_thread is the one for the 64-bit "native" // process rather than the 32-bit "guest" process we care about. In this // case, we can get the 32-bit CONTEXT from the TEB (Thread Environment // Block) of the 64-bit process. - auto teb_mem = GetMemory(td.EnvironmentBlock, sizeof(TEB64)); + auto teb_mem_maybe = GetMemory(td.EnvironmentBlock, sizeof(TEB64)); + if (!teb_mem_maybe) { + LLDB_LOG_ERROR(log, teb_mem_maybe.takeError(), + "Failed to read Thread Environment Block: {0}"); + return {}; + } + + auto teb_mem = *teb_mem_maybe; if (teb_mem.empty()) return {}; @@ -126,8 +134,16 @@ MinidumpParser::GetThreadContextWow64(const minidump::Thread &td) { // Slot 1 of the thread-local storage in the 64-bit TEB points to a structure // that includes the 32-bit CONTEXT (after a ULONG). See: // https://msdn.microsoft.com/en-us/library/ms681670.aspx - auto context = + auto context_maybe = GetMemory(wow64teb->tls_slots[1] + 4, sizeof(MinidumpContext_x86_32)); + if (!context_maybe) { + LLDB_LOG_ERROR(log, context_maybe.takeError(), + "Failed to read WOW Thread Context: {0}"); + return {}; + } + + auto context = *context_maybe; + if (context.size() < sizeof(MinidumpContext_x86_32)) return {}; @@ -478,11 +494,13 @@ void MinidumpParser::PopulateMemoryRanges() { m_memory_ranges.Sort(); } -llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr, - size_t size) { +llvm::Expected<llvm::ArrayRef<uint8_t>> +MinidumpParser::GetMemory(lldb::addr_t addr, size_t size) { std::optional<minidump::Range> range = FindMemoryRange(addr); if (!range) - return {}; + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "No memory range found for address (0x%" PRIx64 ")", addr); // 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 @@ -491,7 +509,11 @@ llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr, const size_t offset = addr - range->start; if (addr < range->start || offset >= range->range_ref.size()) - return {}; + return llvm::createStringError( + llvm::inconvertibleErrorCode(), + "Address (0x%" PRIx64 ") is not in range [0x%" PRIx64 " - 0x%" PRIx64 + ")", + addr, range->start, range->start + range->range_ref.size()); const size_t overlap = std::min(size, range->range_ref.size() - offset); return range->range_ref.slice(offset, overlap); diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h b/lldb/source/Plugins/Process/minidump/MinidumpParser.h index 14599f8d572aa..3b7d33daca717 100644 --- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h +++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h @@ -104,7 +104,8 @@ class MinidumpParser { std::optional<Range> FindMemoryRange(lldb::addr_t addr); - llvm::ArrayRef<uint8_t> GetMemory(lldb::addr_t addr, size_t size); + llvm::Expected<llvm::ArrayRef<uint8_t>> GetMemory(lldb::addr_t addr, + size_t size); /// Returns a list of memory regions and a flag indicating whether the list is /// complete (includes all regions mapped into the process memory). diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index ef3c00e2857df..17a421a722743 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -322,12 +322,15 @@ size_t ProcessMinidump::ReadMemory(lldb::addr_t addr, void *buf, size_t size, size_t ProcessMinidump::DoReadMemory(lldb::addr_t addr, void *buf, size_t size, Status &error) { - llvm::ArrayRef<uint8_t> mem = m_minidump_parser->GetMemory(addr, size); - if (mem.empty()) { - error = Status::FromErrorString("could not parse memory info"); + llvm::Expected<llvm::ArrayRef<uint8_t>> mem_maybe = + m_minidump_parser->GetMemory(addr, size); + if (!mem_maybe) { + error = Status::FromError(mem_maybe.takeError()); return 0; } + llvm::ArrayRef<uint8_t> mem = *mem_maybe; + std::memcpy(buf, mem.data(), mem.size()); return mem.size(); } diff --git a/lldb/test/Shell/Minidump/missing-memory-region.yaml b/lldb/test/Shell/Minidump/missing-memory-region.yaml new file mode 100644 index 0000000000000..1784cacfaf1ba --- /dev/null +++ b/lldb/test/Shell/Minidump/missing-memory-region.yaml @@ -0,0 +1,42 @@ +# Check that looking up a memory region not present in the Minidump fails +# even if it's in the /proc/<pid>/maps file. + +# RUN: yaml2obj %s -o %t +# RUN: %lldb -c %t -o "memory read 0x5000" 2>&1 | FileCheck %s + +# CHECK-LABEL: (lldb) memory read 0x5000 +# CHECK-NEXT: error: No memory range found for address (0x5000) + +--- !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 #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64' + CPU: + Vendor ID: GenuineIntel + Version Info: 0x00000000 + Feature Info: 0x00000000 + - Type: LinuxProcStatus + Text: | + Name: test-yaml + Umask: 0002 + State: t (tracing stop) + Pid: 8567 + - Type: LinuxMaps + Text: | + 0x1000-0x1100 r-xp 00000000 00:00 0 + 0x2000-0x2200 rw-p 00000000 00:00 0 + 0x4000-0x6000 rw-- 00000000 00:00 0 + - Type: Memory64List + Memory Ranges: + - Start of Memory Range: 0x1000 + Data Size: 0x100 + Content : '' + - Start of Memory Range: 0x2000 + Data Size: 0x200 + Content : '' +... diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp index ee31c8e63644b..44f653c6fa135 100644 --- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp +++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp @@ -308,16 +308,19 @@ TEST_F(MinidumpParserTest, GetMemory) { )"), llvm::Succeeded()); - EXPECT_EQ((llvm::ArrayRef<uint8_t>{0x54}), parser->GetMemory(0x401d46, 1)); - EXPECT_EQ((llvm::ArrayRef<uint8_t>{0x54, 0x21}), - parser->GetMemory(0x401d46, 4)); - - EXPECT_EQ((llvm::ArrayRef<uint8_t>{0xc8, 0x4d, 0x04, 0xbc, 0xe9}), - parser->GetMemory(0x7ffceb34a000, 5)); - EXPECT_EQ((llvm::ArrayRef<uint8_t>{0xc8, 0x4d, 0x04}), - parser->GetMemory(0x7ffceb34a000, 3)); - - EXPECT_EQ(llvm::ArrayRef<uint8_t>(), parser->GetMemory(0x500000, 512)); + EXPECT_THAT_EXPECTED(parser->GetMemory(0x401d46, 1), + llvm::HasValue(llvm::ArrayRef<uint8_t>{0x54})); + EXPECT_THAT_EXPECTED(parser->GetMemory(0x401d46, 4), + llvm::HasValue(llvm::ArrayRef<uint8_t>{0x54, 0x21})); + EXPECT_THAT_EXPECTED( + parser->GetMemory(0x7ffceb34a000, 5), + llvm::HasValue(llvm::ArrayRef<uint8_t>{0xc8, 0x4d, 0x04, 0xbc, 0xe9})); + EXPECT_THAT_EXPECTED( + parser->GetMemory(0x7ffceb34a000, 3), + llvm::HasValue(llvm::ArrayRef<uint8_t>{0xc8, 0x4d, 0x04})); + EXPECT_THAT_EXPECTED( + parser->GetMemory(0x500000, 512), + llvm::FailedWithMessage("No memory range found for address (0x500000)")); } TEST_F(MinidumpParserTest, FindMemoryRangeWithFullMemoryMinidump) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
