[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;
+  // This is our flag to control if we had a partial read
+  // if we read less than our expected number of bytes without
+  // getting an error, we should add those bytes and discountine
+  // trying to read.
+  bool partialReadEncountered = false;
+  Status error;
+  while (bytes_remaining > 0 && !partialReadEncountered) {
+    // 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 failed to read memory and got an error, we return and skip
+      // the rest of the region. We need to return a non-error status here
+      // because the caller can't differentiate between this skippable
+      // error, and an error appending data to the file, which is fatal.
+      return Status();
+    } else if (bytes_read_for_chunk != bytes_to_read) {
+      LLDB_LOGF(log,
+                "Memory region at: %" PRIx64 " partiall read %" PRIx64
+                " bytes out of %" PRIx64 " bytes.",
+                addr, bytes_read_for_chunk,
+                bytes_to_read - bytes_read_for_chunk);
+
+      // If we've read some bytes, we stop trying to read more and return
+      // this best effort attempt
+      partialReadEncountered = true;
+    }
+
+    // Write to the minidump file with the chunk potentially flushing to disk.
+    // this is the only place we want to return a true error, so that we can
+    // fail. If we get an error writing to disk we can't easily gaurauntee
+    // that we won't corrupt the minidump.
+    error = AddData(data_up->GetBytes(), bytes_read_for_chunk);
+    if (error.Fail())
+      return error;
+
----------------
Jlalond wrote:

Moved to the end of the function

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