[LLDB] Process minidump better error messages #149206

Jlalond · 2025-07-16T22:33:39Z

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.

…at explain why the memory can't be materialized

llvmbot · 2025-07-16T22:34:10Z

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

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. This call path is used by MinidumpYAML and so I created a new method GetExpectedMemory that GetMemory now uses but converts the expected back into an optional.

There were also no tests for memory read and process Minidump so I added one.

Full diff: https://github.com/llvm/llvm-project/pull/149206.diff

4 Files Affected:

(modified) lldb/source/Plugins/Process/minidump/MinidumpParser.cpp (+16-2)
(modified) lldb/source/Plugins/Process/minidump/MinidumpParser.h (+1)
(modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp (+5-3)
(added) lldb/test/Shell/Minidump/missing-memory-region.yaml (+42)

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index ef691b77193ce..edc9cd8ba245f 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -480,9 +480,22 @@ void MinidumpParser::PopulateMemoryRanges() {
 
 llvm::ArrayRef<uint8_t> MinidumpParser::GetMemory(lldb::addr_t addr,
                                                   size_t size) {
+  llvm::Expected<llvm::ArrayRef<uint8_t>> expected_memory = GetExpectedMemory(addr, size);
+  if (!expected_memory) {
+    llvm::consumeError(expected_memory.takeError());
+    return {};
+  }
+
+  return *expected_memory;
+}
+
+llvm::Expected<llvm::ArrayRef<uint8_t>> MinidumpParser::GetExpectedMemory(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 +504,8 @@ 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..46135a6fe44a3 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -105,6 +105,7 @@ 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>> GetExpectedMemory(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..c4f43689c1ed0 100644
--- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -322,12 +322,14 @@ 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->GetExpectedMemory(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 :        ''
+...

github-actions · 2025-07-16T22:36:09Z

✅ With the latest revision this PR passed the C/C++ code formatter.

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

qxy11

LGTM with a small nit

qxy11 · 2025-07-16T23:22:24Z

lldb/unittests/Process/minidump/MinidumpParserTest.cpp

+      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)"));


nit: maybe we can also add a test for the "Address xxx is not in range xxx" error case?

Not sure how plausible this'll be. That's mostly a safety check.

const size_t offset = addr - range->start;

Ideally we'll never return an entry unless it's in the range. (Not sure if we even need the check anymore)

JDevlieghere · 2025-07-16T23:35:05Z

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

-  auto teb_mem = GetMemory(td.EnvironmentBlock, sizeof(TEB64));
+  auto teb_mem_maybe = GetMemory(td.EnvironmentBlock, sizeof(TEB64));
+  if (!teb_mem_maybe) {
+    llvm::consumeError(teb_mem_maybe.takeError());


Should we use LLDB_LOG_ERROR here? Same question below.

I personally don't have a lot of context on if this should be an error (WOW even when I was at MSFT wasn't a common talking point). But I'm going to make the change

Jlalond added 2 commits July 16, 2025 10:22

Create new API for expected so we can return better error messages th…

e5edc85

…at explain why the memory can't be materialized

Add shell test for new error message

74a07f2

Jlalond requested review from dmpots, jeffreytan81 and qxy11 July 16, 2025 22:33

Jlalond requested a review from JDevlieghere as a code owner July 16, 2025 22:33

llvmbot added the lldb label Jul 16, 2025

Run clang format

3ff5c27

JDevlieghere reviewed Jul 16, 2025

View reviewed changes

lldb/source/Plugins/Process/minidump/MinidumpParser.h Outdated Show resolved Hide resolved

Swap over all of GetMemory

95ef690

qxy11 approved these changes Jul 16, 2025

View reviewed changes

JDevlieghere reviewed Jul 16, 2025

View reviewed changes

format test/add LLDB_LOG_ERROR

ffc83c9

Jlalond force-pushed the process-minidump-better-error-messages branch from 6fab7d0 to ffc83c9 Compare July 17, 2025 00:35

JDevlieghere approved these changes Jul 17, 2025

View reviewed changes

Jlalond merged commit a6fb3b3 into llvm:main Jul 17, 2025
9 checks passed

tru mentioned this pull request Jul 17, 2025

Use Parallel xz for test-suite sources. #149389

Merged

Jlalond mentioned this pull request Jul 17, 2025

[LLDB] Process minidump is in memory check command #149401

Open

This was referenced Jul 23, 2025

test abhinavgaba/llvm-project#2

Closed

Add dataFence plugin interface abhinavgaba/llvm-project#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LLDB] Process minidump better error messages #149206

[LLDB] Process minidump better error messages #149206

Uh oh!

Jlalond commented Jul 16, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 16, 2025

Uh oh!

github-actions bot commented Jul 16, 2025 •

edited

Loading

Uh oh!

Uh oh!

qxy11 left a comment

Uh oh!

qxy11 Jul 16, 2025

Uh oh!

Jlalond Jul 16, 2025

Uh oh!

JDevlieghere Jul 16, 2025

Uh oh!

Jlalond Jul 16, 2025

Uh oh!

Uh oh!

Uh oh!

[LLDB] Process minidump better error messages #149206

[LLDB] Process minidump better error messages #149206

Uh oh!

Conversation

Jlalond commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 16, 2025

Uh oh!

github-actions bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

qxy11 left a comment

Choose a reason for hiding this comment

Uh oh!

qxy11 Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

JDevlieghere Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Jlalond Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jlalond commented Jul 16, 2025 •

edited

Loading

github-actions bot commented Jul 16, 2025 •

edited

Loading