Author: Felipe de Azevedo Piovezan Date: 2026-06-01T08:21:59Z New Revision: fb8cb1b3d0edd14ff0b40562200b827aca72598f
URL: https://github.com/llvm/llvm-project/commit/fb8cb1b3d0edd14ff0b40562200b827aca72598f DIFF: https://github.com/llvm/llvm-project/commit/fb8cb1b3d0edd14ff0b40562200b827aca72598f.diff LOG: [lldb] Strip pointer metadata in ReadMemoryRanges (#200398) The Process base class is generally responsible for fixing pointer metadata before delegating memory reads to concrete Process specializations. However, ReadMemoryRanges was a direct path into the derived classes, which made it so that pointer metadata was never stripped. This commit creates a non-virtual ReadMemoryRanges in Process, which clears pointer metadata, before delegating to the new virtual method DoReadMemoryRanges. This also allows, in the future, to plug into the memory cache system. Added: Modified: lldb/include/lldb/Target/Process.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/source/Target/Process.cpp lldb/unittests/Target/MemoryTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index e99b71399d01d..f68ea3b639e93 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1619,9 +1619,6 @@ class Process : public std::enable_shared_from_this<Process>, Status &error); /// Read from multiple memory ranges and write the results into buffer. - /// This calls ReadMemoryFromInferior multiple times, once per range, - /// bypassing the read cache. Process implementations that can perform this - /// operation more efficiently should override this. /// /// \param[in] ranges /// A collection of ranges (base address + size) to read from. @@ -1636,7 +1633,7 @@ class Process : public std::enable_shared_from_this<Process>, /// of the slice indicates how many bytes were read successfully. Partial /// reads are always performed from the start of the requested range, /// never from the middle or end. - virtual llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, llvm::MutableArrayRef<uint8_t> buffer); @@ -3047,6 +3044,13 @@ void PruneThreadPlans(); virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status &error) = 0; + /// Reads each range individually via ReadMemoryFromInferior, bypassing the + /// memory cache. Subclasses may override it to batch the reads more + /// efficiently. + virtual llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> + DoReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, + llvm::MutableArrayRef<uint8_t> buffer); + virtual void DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr, const uint8_t *buf, size_t size, AddressRanges &matches, size_t alignment, diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 109cdd8f8dede..f6eaf5851338b 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2890,11 +2890,11 @@ static uint64_t ComputeNumRangesMultiMemRead( } llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> -ProcessGDBRemote::ReadMemoryRanges( +ProcessGDBRemote::DoReadMemoryRanges( llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, llvm::MutableArrayRef<uint8_t> buffer) { if (!m_gdb_comm.GetMultiMemReadSupported()) - return Process::ReadMemoryRanges(ranges, buffer); + return Process::DoReadMemoryRanges(ranges, buffer); const llvm::ArrayRef<Range<lldb::addr_t, size_t>> original_ranges = ranges; llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> memory_regions; @@ -2903,7 +2903,7 @@ ProcessGDBRemote::ReadMemoryRanges( uint64_t num_ranges = ComputeNumRangesMultiMemRead(m_max_memory_size, ranges); if (num_ranges == 0) - return Process::ReadMemoryRanges(original_ranges, buffer); + return Process::DoReadMemoryRanges(original_ranges, buffer); auto ranges_for_request = ranges.take_front(num_ranges); ranges = ranges.drop_front(num_ranges); @@ -2913,7 +2913,7 @@ ProcessGDBRemote::ReadMemoryRanges( if (!response) { LLDB_LOG_ERROR(GetLog(GDBRLog::Process), response.takeError(), "MultiMemRead error response: {0}"); - return Process::ReadMemoryRanges(original_ranges, buffer); + return Process::DoReadMemoryRanges(original_ranges, buffer); } llvm::StringRef response_str = response->GetStringRef(); @@ -2922,7 +2922,7 @@ ProcessGDBRemote::ReadMemoryRanges( response_str, buffer, expected_num_ranges, memory_regions)) { LLDB_LOG_ERROR(GetLog(GDBRLog::Process), std::move(error), "MultiMemRead error parsing response: {0}"); - return Process::ReadMemoryRanges(original_ranges, buffer); + return Process::DoReadMemoryRanges(original_ranges, buffer); } } return memory_regions; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 921dc065ee560..0b71b304885c2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -139,11 +139,11 @@ class ProcessGDBRemote : public Process, size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size, Status &error) override; - /// Override of ReadMemoryRanges that uses MultiMemRead to optimize this - /// operation. + /// Override of DoReadMemoryRanges that uses MultiMemRead to perform this + /// operation in a single packet. llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> - ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, - llvm::MutableArrayRef<uint8_t> buf) override; + DoReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, + llvm::MutableArrayRef<uint8_t> buf) override; private: llvm::Expected<StringExtractorGDBRemote> diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 83c8f434e9a57..e77cb0b0835e1 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2072,6 +2072,17 @@ size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) { llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, llvm::MutableArrayRef<uint8_t> buffer) { + llvm::SmallVector<Range<lldb::addr_t, size_t>> fixed_ranges; + fixed_ranges.reserve(ranges.size()); + for (const Range<lldb::addr_t, size_t> &range : ranges) + fixed_ranges.emplace_back(FixAnyAddress(range.GetRangeBase()), + range.GetByteSize()); + return DoReadMemoryRanges(fixed_ranges, buffer); +} + +llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> +Process::DoReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, + llvm::MutableArrayRef<uint8_t> buffer) { auto total_ranges_len = llvm::sum_of( llvm::map_range(ranges, [](auto range) { return range.size; })); // If the buffer is not large enough, this is a programmer error. diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index 21045725be84a..e2a125e7e10a9 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -10,8 +10,10 @@ #include "Plugins/Platform/MacOSX/PlatformMacOSX.h" #include "Plugins/Platform/MacOSX/PlatformRemoteMacOSX.h" #include "lldb/Core/Debugger.h" +#include "lldb/Core/PluginManager.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" +#include "lldb/Target/ABI.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" #include "lldb/Utility/ArchSpec.h" @@ -23,17 +25,56 @@ using namespace lldb_private; using namespace lldb; namespace { +class MockABI : public ABI { +public: + // The only relevant method of this ABI: + lldb::addr_t FixAnyAddress(lldb::addr_t pc) override { + return pc & 0xf0ffffffffffffffULL; + } + + explicit MockABI(ProcessSP process_sp) + : ABI(std::move(process_sp), std::make_unique<llvm::MCRegisterInfo>()) {} + static ABISP CreateInstance(ProcessSP process_sp, const ArchSpec &) { + return std::make_shared<MockABI>(std::move(process_sp)); + } + llvm::StringRef GetPluginName() override { return "mock"; } + size_t GetRedZoneSize() const override { return 0; } + bool PrepareTrivialCall(Thread &, addr_t, addr_t, addr_t, + llvm::ArrayRef<addr_t>) const override { + return false; + } + bool GetArgumentValues(Thread &, ValueList &) const override { return false; } + Status SetReturnValueObject(StackFrameSP &, ValueObjectSP &) override { + return {}; + } + UnwindPlanSP CreateFunctionEntryUnwindPlan() override { return nullptr; } + UnwindPlanSP CreateDefaultUnwindPlan() override { return nullptr; } + bool RegisterIsVolatile(const RegisterInfo *) override { return false; } + bool CallFrameAddressIsValid(addr_t) override { return false; } + bool CodeAddressIsValid(addr_t) override { return false; } + void + AugmentRegisterInfo(std::vector<DynamicRegisterInfo::Register> &) override {} + +protected: + ValueObjectSP GetReturnValueObjectImpl(Thread &, + CompilerType &) const override { + return nullptr; + } +}; + class MemoryTest : public ::testing::Test { public: void SetUp() override { FileSystem::Initialize(); HostInfo::Initialize(); PlatformMacOSX::Initialize(); + PluginManager::RegisterPlugin("mock", "mock ABI", MockABI::CreateInstance); } void TearDown() override { PlatformMacOSX::Terminate(); HostInfo::Terminate(); FileSystem::Terminate(); + PluginManager::UnregisterPlugin(MockABI::CreateInstance); } }; @@ -616,3 +657,48 @@ TEST_F(MemoryTest, TestReadUnsignedIntegersFromMemory) { } } } + +// A process that, when asked to read memory from address X, returns the top +// byte of X. +class DummyMSBReaderProcess : public Process { +public: + // Only call this method with exactly one range. + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> + DoReadMemoryRanges(llvm::ArrayRef<Range<addr_t, size_t>> ranges, + llvm::MutableArrayRef<uint8_t> buffer) override { + buffer[0] = static_cast<uint8_t>(ranges[0].GetRangeBase() >> 56); + return {{buffer.take_front(1)}}; + } + // Boilerplate, nothing interesting below. + DummyMSBReaderProcess(TargetSP target_sp, ListenerSP listener_sp) + : Process(target_sp, listener_sp) {} + bool CanDebug(TargetSP, bool) override { return true; } + Status DoDestroy() override { return {}; } + void RefreshStateAfterStop() override {} + bool DoUpdateThreadList(ThreadList &, ThreadList &) override { return false; } + llvm::StringRef GetPluginName() override { return "Dummy"; } + size_t DoReadMemory(addr_t, void *, size_t, Status &) override { + llvm_unreachable("don't call this"); + } +}; + +TEST_F(MemoryTest, TestReadMemoryRangesClearMetadata) { + 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); + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = + std::make_shared<DummyMSBReaderProcess>(target_sp, listener_sp); + + llvm::SmallVector<uint8_t, 0> buffer(1024, 0); + llvm::SmallVector<Range<addr_t, size_t>> ranges = {{0xff0123456789abcd, 1}}; + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results = + process_sp->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), 1ull); + ASSERT_EQ(read_results[0].size(), 1ull); + ASSERT_EQ(read_results[0][0], 0xf0); // The ABI masks with 0xf0. +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
