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

[Augusto Noronha via Phabricator via lldb-commits]

augusto2112 created this revision.
augusto2112 added reviewers: jasonmolenda, jingham.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101390

Files:
  lldb/source/Target/Target.cpp


Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1753,19 +1753,32 @@
   if (!resolved_addr.IsValid())
     resolved_addr = addr;
 
-  bool is_readonly = false;
+  auto deleter = [](void *x) { free(x); };
+
+  // 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<void, void (*)(void *)> file_cache_read_buffer(nullptr,
+                                                                 deleter);
+  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.reset(malloc(file_cache_bytes_read));
+          std::memcpy(file_cache_read_buffer.get(), dst, 
file_cache_bytes_read);
+        }
+      }
     }
   }
 
@@ -1804,7 +1817,14 @@
     }
   }
 
-  if (!is_readonly && resolved_addr.IsSectionOffset()) {
+  if (file_cache_read_buffer.get() && 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.get() && 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);



Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1753,19 +1753,32 @@
   if (!resolved_addr.IsValid())
     resolved_addr = addr;
 
-  bool is_readonly = false;
+  auto deleter = [](void *x) { free(x); };
+
+  // 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<void, void (*)(void *)> file_cache_read_buffer(nullptr,
+                                                                 deleter);
+  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.reset(malloc(file_cache_bytes_read));
+          std::memcpy(file_cache_read_buffer.get(), dst, file_cache_bytes_read);
+        }
+      }
     }
   }
 
@@ -1804,7 +1817,14 @@
     }
   }
 
-  if (!is_readonly && resolved_addr.IsSectionOffset()) {
+  if (file_cache_read_buffer.get() && 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.get() && 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);

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits