diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index dc75d98acea70..8f5892e16cedf 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1571,6 +1571,28 @@ class Process : public std::enable_shared_from_this, virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, 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. + /// + /// \param[out] buffer + /// A buffer where the read memory will be written to. It must be at least + /// as long as the sum of the sizes of each range. + /// + /// \return + /// A vector of MutableArrayRef, where each MutableArrayRef is a slice of + /// the input buffer into which the memory contents were copied. The size + /// 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> + ReadMemoryRanges(llvm::ArrayRef> ranges, + llvm::MutableArrayRef buffer); + /// Read of memory from a process. /// /// This function has the same semantics of ReadMemory except that it diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp index 6d8f41aef1ffc..7ce4cbf4c61a4 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp @@ -279,22 +279,23 @@ ClassDescriptorV2::ReadMethods(llvm::ArrayRef addresses, const size_t num_methods = addresses.size(); llvm::SmallVector buffer(num_methods * size, 0); - llvm::DenseSet failed_indices; - for (auto [idx, addr] : llvm::enumerate(addresses)) { - Status error; - process->ReadMemory(addr, buffer.data() + idx * size, size, error); - if (error.Fail()) - failed_indices.insert(idx); - } + llvm::SmallVector> mem_ranges = + llvm::to_vector(llvm::map_range(llvm::seq(num_methods), [&](size_t idx) { + return Range(addresses[idx], size); + })); + + llvm::SmallVector> read_results = + process->ReadMemoryRanges(mem_ranges, buffer); llvm::SmallVector methods; methods.reserve(num_methods); - for (auto [idx, addr] : llvm::enumerate(addresses)) { - if (failed_indices.contains(idx)) + for (auto [addr, memory] : llvm::zip(addresses, read_results)) { + // Ignore partial reads. + if (memory.size() != size) continue; - DataExtractor extractor(buffer.data() + idx * size, size, - process->GetByteOrder(), + + DataExtractor extractor(memory.data(), size, process->GetByteOrder(), process->GetAddressByteSize()); methods.push_back(method_t()); methods.back().Read(extractor, process, addr, relative_selector_base_addr, diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 3176852f0b724..fb9e7eb5ed1bd 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -1971,6 +1971,49 @@ size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) { } } +llvm::SmallVector> +Process::ReadMemoryRanges(llvm::ArrayRef> ranges, + llvm::MutableArrayRef 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. + // In production builds, gracefully fail by returning a length of 0 for all + // ranges. + assert(buffer.size() >= total_ranges_len && "provided buffer is too short"); + if (buffer.size() < total_ranges_len) { + llvm::MutableArrayRef empty; + return {ranges.size(), empty}; + } + + llvm::SmallVector> results; + + // While `buffer` has space, take the next requested range and read + // memory into a `buffer` piece, then slice it to remove the used memory. + for (auto [addr, range_len] : ranges) { + Status status; + size_t num_bytes_read = + ReadMemoryFromInferior(addr, buffer.data(), range_len, status); + // FIXME: ReadMemoryFromInferior promises to return 0 in case of errors, but + // it doesn't; it never checks for errors. + if (status.Fail()) + num_bytes_read = 0; + + assert(num_bytes_read <= range_len && "read more than requested bytes"); + if (num_bytes_read > range_len) { + // In production builds, gracefully fail by returning length zero for this + // range. + results.emplace_back(); + continue; + } + + results.push_back(buffer.take_front(num_bytes_read)); + // Slice buffer to remove the used memory. + buffer = buffer.drop_front(num_bytes_read); + } + + return results; +} + void Process::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/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index 4a96730e00464..f7b4e97b1f64a 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -17,6 +17,7 @@ #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/DataBufferHeap.h" #include "gtest/gtest.h" +#include using namespace lldb_private; using namespace lldb; @@ -225,3 +226,144 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) { // instead of using an // old cache } + +/// A process class that, when asked to read memory from some address X, returns +/// the least significant byte of X. +class DummyReaderProcess : public Process { +public: + // If true, `DoReadMemory` will not return all requested bytes. + // It's not possible to control exactly how many bytes will be read, because + // Process::ReadMemoryFromInferior tries to fulfill the entire request by + // reading smaller chunks until it gets nothing back. + bool read_less_than_requested = false; + bool read_more_than_requested = false; + + size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) override { + if (read_less_than_requested && size > 0) + size--; + if (read_more_than_requested) + size *= 2; + uint8_t *buffer = static_cast(buf); + for (size_t addr = vm_addr; addr < vm_addr + size; addr++) + buffer[addr - vm_addr] = static_cast(addr); // LSB of addr. + return size; + } + // Boilerplate, nothing interesting below. + DummyReaderProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp) + : Process(target_sp, listener_sp) {} + bool CanDebug(lldb::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"; } +}; + +TEST_F(MemoryTest, TestReadMemoryRanges) { + 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(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + { + llvm::SmallVector buffer(1024, 0); + // Read 8 ranges of 128 bytes with arbitrary base addresses. + llvm::SmallVector> ranges = { + {0x12345, 128}, {0x11112222, 128}, {0x77777777, 128}, + {0xffaabbccdd, 128}, {0x0, 128}, {0x4242424242, 128}, + {0x17171717, 128}, {0x99999, 128}}; + + llvm::SmallVector> read_results = + process_sp->ReadMemoryRanges(ranges, buffer); + + for (auto [range, memory] : llvm::zip(ranges, read_results)) { + ASSERT_EQ(memory.size(), 128u); + addr_t range_base = range.GetRangeBase(); + for (auto [idx, byte] : llvm::enumerate(memory)) + ASSERT_EQ(byte, static_cast(range_base + idx)); + } + } + + auto &dummy_process = static_cast(*process_sp); + dummy_process.read_less_than_requested = true; + { + llvm::SmallVector buffer(1024, 0); + llvm::SmallVector> ranges = { + {0x12345, 128}, {0x11112222, 128}, {0x77777777, 128}}; + llvm::SmallVector> read_results = + dummy_process.ReadMemoryRanges(ranges, buffer); + for (auto [range, memory] : llvm::zip(ranges, read_results)) { + ASSERT_LT(memory.size(), 128u); + addr_t range_base = range.GetRangeBase(); + for (auto [idx, byte] : llvm::enumerate(memory)) + ASSERT_EQ(byte, static_cast(range_base + idx)); + } + } +} + +using MemoryDeathTest = MemoryTest; + +TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) { + 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(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + auto &dummy_process = static_cast(*process_sp); + dummy_process.read_more_than_requested = true; + llvm::SmallVector buffer(1024, 0); + llvm::SmallVector> ranges = {{0x12345, 128}}; + + llvm::SmallVector> read_results; + ASSERT_DEBUG_DEATH( + { read_results = process_sp->ReadMemoryRanges(ranges, buffer); }, + "read more than requested bytes"); +#ifdef NDEBUG + // With asserts off, the read should return empty ranges. + ASSERT_EQ(read_results.size(), 1u); + ASSERT_TRUE(read_results[0].empty()); +#endif +} + +TEST_F(MemoryDeathTest, TestReadMemoryRangesWithShortBuffer) { + 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(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + llvm::SmallVector short_buffer(10, 0); + llvm::SmallVector> ranges = {{0x12345, 128}, + {0x11, 128}}; + llvm::SmallVector> read_results; + ASSERT_DEBUG_DEATH( + { read_results = process_sp->ReadMemoryRanges(ranges, short_buffer); }, + "provided buffer is too short"); +#ifdef NDEBUG + // With asserts off, the read should return empty ranges. + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef result : read_results) + ASSERT_TRUE(result.empty()); +#endif +}