[Lldb-commits] [PATCH] D62715: [NativeProcessLinux] Reuse memory read by process_vm_readv before calling ptrace

[António Afonso via Phabricator via lldb-commits]

aadsm created this revision.
aadsm added reviewers: clayborg, xiaobai, labath.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I'm putting this up as discussed per D62503 <https://reviews.llvm.org/D62503>.
Today when process_vm_readv fails to read the entire range we fallback to ptrace and try to read the same range.
However, process_vm_readv does partial reads so there's no need to read again what has already been read by process_vm_readv.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62715

Files:
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp


Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===================================================================
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1441,6 +1441,7 @@
 
 Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                       size_t &bytes_read) {
+  bytes_read = 0;
   if (ProcessVmReadvSupported()) {
     // The process_vm_readv path is about 50 times faster than ptrace api. We
     // want to use this syscall if it is supported.
@@ -1453,8 +1454,21 @@
     remote_iov.iov_base = reinterpret_cast<void *>(addr);
     remote_iov.iov_len = size;
 
-    bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
-    const bool success = bytes_read == size;
+    auto vm_bytes_read =
+        process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
+    const bool success = vm_bytes_read == size;
+    if (vm_bytes_read > 0) {
+      bytes_read = vm_bytes_read;
+      // When process_vm_readv is not able to read all the data we fallback to
+      // ptrace. However, if process_vm_readv didn't fail (vm_bytes_read != -1)
+      // and was able to some bytes avoid re-reading them with ptrace by
+      // updating the addr and size to point to the rest of the data.
+      if (!success) {
+        addr = reinterpret_cast<addr_t>(reinterpret_cast<char *>(addr) +
+                                        vm_bytes_read);
+        size -= vm_bytes_read;
+      }
+    }
 
     Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
     LLDB_LOG(log,
@@ -1468,14 +1482,14 @@
     // api.
   }
 
-  unsigned char *dst = static_cast<unsigned char *>(buf);
+  unsigned char *dst = static_cast<unsigned char *>(buf) + bytes_read;
   size_t remainder;
   long data;
 
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_MEMORY));
   LLDB_LOG(log, "addr = {0}, buf = {1}, size = {2}", addr, buf, size);
 
-  for (bytes_read = 0; bytes_read < size; bytes_read += remainder) {
+  for (; bytes_read < size; bytes_read += remainder) {
     Status error = NativeProcessLinux::PtraceWrapper(
         PTRACE_PEEKDATA, GetID(), (void *)addr, nullptr, 0, &data);
     if (error.Fail())


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62715.202355.patch
Type: text/x-patch
Size: 2290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190531/abc5aa95/attachment.bin>