[Lldb-commits] [lldb] 6c82b8a - Change Target::ReadMemory to ensure the amount of memory read from the file-cache is the amount requested.

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2021-05-11T13:07:11-03:00
New Revision: 6c82b8a378a6f59e94a81d91225db4fabf6e2bff

URL: https://github.com/llvm/llvm-project/commit/6c82b8a378a6f59e94a81d91225db4fabf6e2bff
DIFF: https://github.com/llvm/llvm-project/commit/6c82b8a378a6f59e94a81d91225db4fabf6e2bff.diff

LOG: Change Target::ReadMemory to ensure the amount of memory read from the file-cache is the amount requested.

This change ensures that if for whatever reason we read less bytes than expected (for example, when trying to read memory that spans multiple sections), we try reading from the live process as well.

Reviewed By: jasonmolenda

Differential Revision: https://reviews.llvm.org/D101390

Added: 
    

Modified: 
    lldb/source/Target/Target.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index f41a1528053be..d879c0f6221df 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1758,19 +1758,30 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
   if (!resolved_addr.IsValid())
     resolved_addr = addr;
 
-  bool is_readonly = false;
+  // If we read from the file cache but can't get as many bytes as requested,
+  // we keep the result around in this buffer, in case this result is the
+  // best we can do.
+  std::unique_ptr<uint8_t[]> file_cache_read_buffer;
+  size_t file_cache_bytes_read = 0;
+
   // Read from file cache if read-only section.
   if (!force_live_memory && resolved_addr.IsSectionOffset()) {
     SectionSP section_sp(resolved_addr.GetSection());
     if (section_sp) {
       auto permissions = Flags(section_sp->GetPermissions());
-      is_readonly = !permissions.Test(ePermissionsWritable) &&
-                    permissions.Test(ePermissionsReadable);
-    }
-    if (is_readonly) {
-      bytes_read = ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
-      if (bytes_read > 0)
-        return bytes_read;
+      bool is_readonly = !permissions.Test(ePermissionsWritable) &&
+                         permissions.Test(ePermissionsReadable);
+      if (is_readonly) {
+        file_cache_bytes_read =
+            ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
+        if (file_cache_bytes_read == dst_len)
+          return file_cache_bytes_read;
+        else if (file_cache_bytes_read > 0) {
+          file_cache_read_buffer =
+              std::make_unique<uint8_t[]>(file_cache_bytes_read);
+          std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+        }
+      }
     }
   }
 
@@ -1809,7 +1820,14 @@ size_t Target::ReadMemory(const Address &addr, void *dst, size_t dst_len,
     }
   }
 
-  if (!is_readonly && resolved_addr.IsSectionOffset()) {
+  if (file_cache_read_buffer && file_cache_bytes_read > 0) {
+    // Reading from the process failed. If we've previously succeeded in reading
+    // something from the file cache, then copy that over and return that.
+    std::memcpy(dst, file_cache_read_buffer.get(), file_cache_bytes_read);
+    return file_cache_bytes_read;
+  }
+
+  if (!file_cache_read_buffer && resolved_addr.IsSectionOffset()) {
     // If we didn't already try and read from the object file cache, then try
     // it after failing to read from the process.
     return ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);