[Lldb-commits] [lldb] [lldb] Implement Process::ReadMemoryRanges (PR #163651)

[Jason Molenda via lldb-commits]

================
@@ -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
----------------
jasonmolenda wrote:

I'm unclear on the data store model.  This description sounds like calling ReadMemoryRanges({[0, 4], [4, 4], [8, 4]}, buffer) would need a buffer of at least 12 bytes.  But reading `Process::ReadMemoryRanges`, looks like it is using `buffer` as the store of a single memory range -- we could just as easily loop over the ranges, find the largest range, and create a local buffer instead of using an allocation from the caller.  I suppose we avoid the loop over the ranges.

If this description is correct -- the buffer provided by the caller is where *all* the results are stored, then the `Process::ReadMemoryRanges` implementation is wrong.  It's checking each individual memory range size against buffer size, but we need to check if the current read plus all previous reads overflows buffer.  And we need to read bytes into the buffer at the correct offset of each read.

https://github.com/llvm/llvm-project/pull/163651