[Lldb-commits] [lldb] [LLDB][Minidump]Update MinidumpFileBuilder to read and write in chunks (PR #129307)

[Jacob Lalonde via lldb-commits]

================
@@ -969,6 +969,83 @@ Status MinidumpFileBuilder::DumpDirectories() const {
   return error;
 }
 
+Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
+    const std::unique_ptr<lldb_private::DataBufferHeap> &data_up,
+    const lldb_private::CoreFileMemoryRange &range, uint64_t *bytes_read) {
+  if (!data_up)
+    return Status::FromErrorString("No buffer supplied to read memory.");
+
+  if (!bytes_read)
+    return Status::FromErrorString("Bytes read pointer cannot be null.");
+  Log *log = GetLog(LLDBLog::Object);
+  const lldb::addr_t addr = range.range.start();
+  const lldb::addr_t size = range.range.size();
+  // First we set the byte tally to 0, so if we do exit gracefully
+  // the caller doesn't think the random garbage on the stack is a
+  // success.
+  *bytes_read = 0;
+
+  uint64_t bytes_remaining = size;
+  Status error;
+  while (bytes_remaining > 0) {
+    // Get the next read chunk size as the minimum of the remaining bytes and
+    // the write chunk max size.
+    const size_t bytes_to_read =
+        std::min(bytes_remaining, data_up->GetByteSize());
+    const size_t bytes_read_for_chunk =
+        m_process_sp->ReadMemory(range.range.start() + *bytes_read,
+                                 data_up->GetBytes(), bytes_to_read, error);
+    if (error.Fail() || bytes_read_for_chunk == 0) {
+      LLDB_LOGF(log,
+                "Failed to read memory region at: %" PRIx64
+                ". Bytes read: %zu, error: %s",
+                addr, bytes_read_for_chunk, error.AsCString());
+      // If we've read nothing, and get an error or fail to read
+      // we can just give up early.
+      if (*bytes_read == 0)
+        return Status();
+
+      // If we've read some bytes, we stop trying to read more and return
+      // this best effort attempt
+      bytes_remaining = 0;
----------------
Jlalond wrote:

Hey @jeffreytan81, my thought process here is we managed to read any bytes, we should append those to the buffer and return bytes read up to the point.

So in my hypothetical we try to read 1000 bytes from an address, and only read 500. I still wanted to save off those 500. Are you against this proposal? It's why in error conditions other than `AddData()` I just set the loop to not continue.

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