[Lldb-commits] [lldb] [lldb] Clear stale error in Target::ReadMemory on file-cache fallback (PR #213451)
via lldb-commits
lldb-commits at lists.llvm.org
Sat Aug 1 07:48:35 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Yao Qi (qiyao)
<details>
<summary>Changes</summary>
Load the checked-in core `linux-aarch64-pac.core` from
`lldb/test/API/functionalities/postmortem/elf-core/` with its binary and
ask for a `char16_t *` summary at the start of `.text`:
```
(lldb) settings set target.max-string-summary-length 8
(lldb) expression -l c++ -- (char16_t *)0x400140
(char16_t *) $0 = 0x0000000000400140 unable to read data
(lldb) memory read -s1 -c16 0x400140
0x00400140: 3f 23 03 d5 ff 83 00 d1 fd 7b 01 a9 fd 43 00 91 ?#.......{...C..
```
`memory read` prints the very bytes the summary just claimed it could
not read. Both go through `Target::ReadMemory()`, which reuses a single
`Status &error` across the process read and the file-cache fallback at
the end of the function, and `Target::ReadMemoryFromFileCache()` only
ever sets that `Status`, it never clears it on success. So when the
process read fails outright and the fallback then satisfies the whole
request, `ReadMemory()` returns the correct bytes with the failed
process read's message still in `error`. `memory read` compares the
returned count against the requested length and is fine, but
`Target::ReadStringFromMemory()`, which `StringPrinter` uses for UTF-16
and UTF-32, checks `error.Success()` and gives up.
`SBTarget::ReadMemory()` hands the same stale `Status` to any scripted
client.
Core files reach this routinely. `ProcessMachCore` and `ProcessElfCore`
both report `IsAlive()`, so the process read is attempted and fails for
a read-only page that was not dumped into the core, and the fallback
then serves that page out of the binary on disk. In this core the
`PT_LOAD` covering `.text` has `p_filesz == 0`.
Clear `error` after the fallback, but only for a full read, that is
`bytes_read == dst_len`. `ObjectFile::ReadSectionData()` clamps a
request that overruns the section, so clearing unconditionally would
report success for a truncated read and leave the tail of `dst`
uninitialized. That is why the session lowers
`target.max-string-summary-length`: it keeps the request inside the
148-byte `.text`, whereas a larger request really is a short read and
still fails. Other exits that return a partial count without setting an
error are pre-existing.
With the fix:
```
(lldb) expression -l c++ -- (char16_t *)0x400140
(char16_t *) $0 = 0x0000000000400140 u"⌿픃菿턀篽꤁䏽"
```
---
Full diff: https://github.com/llvm/llvm-project/pull/213451.diff
4 Files Affected:
- (modified) lldb/source/Target/Target.cpp (+7-1)
- (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py (+7)
- (modified) lldb/unittests/Target/CMakeLists.txt (+1)
- (modified) lldb/unittests/Target/MemoryTest.cpp (+104)
``````````diff
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 148d7e0b30dbb..21aeb3ba1f446 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2209,7 +2209,13 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
if (!file_cache_read_buffer && resolved_addr.IsSectionOffset()) {
// If we didn't already try and read from the object file cache, then try
// it after failing to read from the process.
- return ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+ // ReadMemoryFromFileCache() only ever sets "error", so clear it to keep a
+ // failed process read from poisoning a successful read here. Only a full
+ // read counts: ReadSectionData() clamps, leaving the tail of "dst" unset.
+ bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+ if (bytes_read == dst_len)
+ error.Clear();
+ return bytes_read;
}
return 0;
}
diff --git a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
index f211ac4454209..2dfba0cd71da1 100644
--- a/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ b/lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -1238,6 +1238,13 @@ def test_read_only_cstring(self):
cstr = var.GetSummary()
self.assertEqual(cstr, '"_start"')
+ # Reading through the target falls back to the application binary too,
+ # and must not report the failed process read.
+ error = lldb.SBError()
+ addr = target.ResolveLoadAddress(var.GetValueAsUnsigned())
+ self.assertEqual(target.ReadMemory(addr, 7, error), b"_start\0")
+ self.assertSuccess(error)
+
@skipIfLLVMTargetMissing("X86")
@skipIfWindows
def test_linux_no_exe(self):
diff --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt
index bf08a8f015ba0..6f88a43f7ee7c 100644
--- a/lldb/unittests/Target/CMakeLists.txt
+++ b/lldb/unittests/Target/CMakeLists.txt
@@ -23,6 +23,7 @@ add_lldb_unittest(TargetTests
lldbHost
lldbPluginObjectFileBreakpad
lldbPluginObjectFileELF
+ lldbPluginObjectFileMachO
lldbPluginPlatformLinux
lldbPluginPlatformMacOSX
lldbPluginPlatformAndroid
diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp
index 9d04376b4fd5b..f1d804b90c317 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -7,10 +7,15 @@
//===----------------------------------------------------------------------===//
#include "lldb/Target/Memory.h"
+#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h"
#include "Plugins/Platform/MacOSX/PlatformMacOSX.h"
#include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "TestingSupport/TestUtilities.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/ABI.h"
@@ -18,6 +23,7 @@
#include "lldb/Target/Target.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBufferHeap.h"
+#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
#include <cstdint>
@@ -702,3 +708,101 @@ TEST_F(MemoryTest, TestReadMemoryRangesClearMetadata) {
ASSERT_EQ(read_results[0].size(), 1ull);
ASSERT_EQ(read_results[0][0], 0xf0); // The ABI masks with 0xf0.
}
+
+// The live process read fails outright, so Target::ReadMemory must fall all the
+// way through to the file-cache fallback at the end of the function, which
+// serves the bytes out of the module's (__DATA,__data) section. A full read
+// there must report success, and a short read must not.
+TEST_F(MemoryTest, TestReadMemoryClearsStaleError) {
+ SubsystemRAII<ObjectFileMachO> subsystems;
+
+ ArchSpec arch("x86_64-apple-macosx-");
+ Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch));
+
+ DebuggerSP debugger_sp = Debugger::CreateInstance();
+ ASSERT_TRUE(debugger_sp);
+
+ TargetSP target_sp = CreateTarget(debugger_sp, arch);
+ ASSERT_TRUE(target_sp);
+
+ ProcessSP process_sp = CreateProcess(target_sp);
+ ASSERT_TRUE(process_sp);
+
+ // The process can't produce a single byte, so the read must fail.
+ static_cast<DummyProcess *>(process_sp.get())->SetMaxReadSize(0);
+
+ auto expected_file = TestFile::fromYaml(R"(
+--- !mach-o
+FileHeader:
+ magic: 0xFEEDFACF
+ cputype: 0x1000007
+ cpusubtype: 0x3
+ filetype: 0x2
+ ncmds: 1
+ sizeofcmds: 152
+ flags: 0x200085
+ reserved: 0x0
+LoadCommands:
+ - cmd: LC_SEGMENT_64
+ cmdsize: 152
+ segname: __DATA
+ vmaddr: 0x100001000
+ vmsize: 0xC
+ fileoff: 0x1000
+ filesize: 0xC
+ maxprot: 3
+ initprot: 3
+ nsects: 1
+ flags: 0
+ Sections:
+ - sectname: __data
+ segname: __DATA
+ addr: 0x100001000
+ size: 12
+ offset: 0x1000
+ align: 0
+ reloff: 0x0
+ nreloc: 0
+ flags: 0x0
+ reserved1: 0x0
+ reserved2: 0x0
+ reserved3: 0x0
+ content: 68656C6C6F20776F726C6400
+...
+)");
+ // "expected_file" owns the buffer the Module reads through, so it has to
+ // outlive every ReadMemory() call below.
+ ASSERT_THAT_EXPECTED(expected_file, llvm::Succeeded());
+
+ ModuleSP module_sp = std::make_shared<Module>(expected_file->moduleSpec());
+ target_sp->GetImages().Append(module_sp, /*notify=*/false);
+
+ SectionList *sections = module_sp->GetSectionList();
+ ASSERT_TRUE(sections);
+ SectionSP section_sp = sections->FindSectionByName(ConstString("__data"));
+ ASSERT_TRUE(section_sp);
+ target_sp->SetSectionLoadAddress(section_sp, section_sp->GetFileAddress());
+
+ // force_live_memory = true skips the read-only file-cache fast path near the
+ // top of ReadMemory, and the section is writable so that path would reject
+ // it anyway. The fallback at the end is the only thing that can serve this.
+ Address addr;
+ ASSERT_TRUE(
+ target_sp->ResolveLoadAddress(section_sp->GetFileAddress(), addr));
+ char buf[5] = {};
+ Status error;
+ size_t bytes_read = target_sp->ReadMemory(addr, buf, sizeof(buf), error,
+ /*force_live_memory=*/true);
+ ASSERT_EQ(bytes_read, sizeof(buf));
+ EXPECT_TRUE(error.Success()) << error.AsCString();
+ EXPECT_EQ(llvm::StringRef(buf, sizeof(buf)), "hello");
+
+ // A short read must not be reported as success: ReadSectionData() clamps to
+ // the 12-byte section, so the tail of "big" would be left uninitialized.
+ char big[20] = {};
+ Status short_error;
+ EXPECT_EQ(target_sp->ReadMemory(addr, big, sizeof(big), short_error,
+ /*force_live_memory=*/true),
+ 12u);
+ EXPECT_TRUE(short_error.Fail());
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/213451
More information about the lldb-commits
mailing list