llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Felipe de Azevedo Piovezan (felipepiovezan)
<details>
<summary>Changes</summary>
This commit introduces a base-class implementation for a method that reads
memory from multiple ranges at once. This implementation simply calls the
underlying `ReadMemoryFromInferior` method on each requested range,
intentionally bypassing the memory caching mechanism (though this may be easily
changed in the future).
`Process` implementations that can be perform this operation more efficiently -
e.g. with the MultiMemPacket described in [1] - are expected to override this
method.
As an example, this commit changes AppleObjCClassDescriptorV2 to use the new
API.
Note about the API
------------------
In the RFC, we discussed having the API return some kind of class
`ReadMemoryRangesResult`. However, while writing such a class, it became clear
that it was merely wrapping a vector, without providing anything useful. For
example, this class:
```
struct ReadMemoryRangesResult {
ReadMemoryRangesResult(
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges)
: ranges(std::move(ranges)) {}
llvm::ArrayRef<llvm::MutableArrayRef<uint8_t>> getRanges() const {
return ranges;
}
private:
llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> ranges;
};
```
As can be seen in the added test and in the added use-case
(AppleObjCClassDescriptorV2), users of this API will just iterate over the
vector of memory buffers. So they want a return type that can be iterated over,
and the vector seems more natural than creating a new class and defining
iterators for it.
Likewise, in the RFC, we discussed wrapping the result into an `Expected`. Upon
experimenting with the code, this feels like it limits what the API is able to
do as the base class implementation never needs to fail the entire result, it's
the individual reads that may fail and this is expressed through a zero-length
result. Any derived classes overriding `ReadMemoryRanges` should also never
produce a top level failure: if they did, they can just fall back to the base
class implementation, which would produce a better result.
[1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/
---
Full diff: https://github.com/llvm/llvm-project/pull/163651.diff
4 Files Affected:
- (modified) lldb/include/lldb/Target/Process.h (+19)
- (modified)
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
(+12-11)
- (modified) lldb/source/Target/Process.cpp (+28)
- (modified) lldb/unittests/Target/MemoryTest.cpp (+62)
``````````diff
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index dc75d98acea70..7a260323b5a3d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1571,6 +1571,25 @@ class Process : public
std::enable_shared_from_this<Process>,
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.
+ ///
+ /// \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<llvm::MutableArrayRef<uint8_t>>
+ ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
+ llvm::MutableArrayRef<uint8_t> 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<lldb::addr_t> addresses,
const size_t num_methods = addresses.size();
llvm::SmallVector<uint8_t, 0> buffer(num_methods * size, 0);
- llvm::DenseSet<uint32_t> 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<Range<addr_t, size_t>> mem_ranges =
+ llvm::to_vector(llvm::map_range(llvm::seq(num_methods), [&](size_t idx) {
+ return Range<addr_t, size_t>(addresses[idx], size);
+ }));
+
+ llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+ process->ReadMemoryRanges(mem_ranges, buffer);
llvm::SmallVector<method_t, 0> 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..9692a7fece7e5 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1971,6 +1971,34 @@ 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<llvm::MutableArrayRef<uint8_t>> results;
+
+ for (auto [addr, len] : ranges) {
+ // This is either a programmer error, or a protocol violation.
+ // In production builds, gracefully fail.
+ assert(buffer.size() >= len);
+ if (buffer.size() < len) {
+ results.push_back(buffer.take_front(0));
+ continue;
+ }
+
+ Status status;
+ size_t num_bytes_read =
+ ReadMemoryFromInferior(addr, buffer.data(), 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;
+ results.push_back(buffer.take_front(num_bytes_read));
+ 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..1317dd27b256e 100644
--- a/lldb/unittests/Target/MemoryTest.cpp
+++ b/lldb/unittests/Target/MemoryTest.cpp
@@ -17,6 +17,8 @@
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "gtest/gtest.h"
+#include <cstdint>
+#include <random>
using namespace lldb_private;
using namespace lldb;
@@ -225,3 +227,63 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) {
// instead of using an
// old cache
}
+
+/// A process class that reads `lower_byte(address)` for each `address` it
+/// reads.
+class DummyReaderProcess : public Process {
+public:
+ size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+ Status &error) override {
+ uint8_t *buffer = static_cast<uint8_t*>(buf);
+ for(size_t addr = vm_addr; addr < vm_addr + size; addr++)
+ buffer[addr - vm_addr] = 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<DummyReaderProcess>(target_sp, listener_sp);
+ ASSERT_TRUE(process_sp);
+
+ DummyProcess *process = static_cast<DummyProcess *>(process_sp.get());
+ process->SetMaxReadSize(1024);
+
+ llvm::SmallVector<uint8_t, 0> buffer(1024, 0);
+
+ // Read 8 ranges of 128 bytes, starting at random addresses
+ std::mt19937 rng(42);
+ std::uniform_int_distribution<addr_t> distribution(1, 100000);
+ llvm::SmallVector<Range<addr_t, size_t>> ranges;
+ for (unsigned i = 0; i < 1024; i += 128)
+ ranges.emplace_back(distribution(rng), 128);
+
+ llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results =
+ process->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<uint8_t>(range_base + idx));
+ }
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/163651
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
