Skip to content

Conversation

labath
Copy link
Collaborator

@labath labath commented Sep 2, 2024

ReadProcessMemory will not perform the read if part of the memory is unreadable (and even though the API has a number_of_bytes_read argument). To make this work, I explicitly inspect the memory region being read and only read the accessible part.

ReadProcessMemory will not perform the read if part of the memory is
unreadable. To make this work, I explicitly inspect the memory region
being read and only read the accessible part.
@llvmbot
Copy link
Member

llvmbot commented Sep 2, 2024

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

ReadProcessMemory will not perform the read if part of the memory is unreadable (and even though the API has a number_of_bytes_read argument). To make this work, I explicitly inspect the memory region being read and only read the accessible part.


Full diff: https://github.com/llvm/llvm-project/pull/106981.diff

2 Files Affected:

  • (modified) lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp (+19-6)
  • (modified) lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py (-1)
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
index 04c7c0f826039d..9fc8077851d257 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
@@ -276,16 +276,29 @@ Status ProcessDebugger::ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
   LLDB_LOG(log, "attempting to read {0} bytes from address {1:x}", size,
            vm_addr);
 
-  HostProcess process = m_session_data->m_debugger->GetProcess();
+  lldb::process_t handle = m_session_data->m_debugger->GetProcess()
+                               .GetNativeProcess()
+                               .GetSystemHandle();
   void *addr = reinterpret_cast<void *>(vm_addr);
   SIZE_T num_of_bytes_read = 0;
-  if (!::ReadProcessMemory(process.GetNativeProcess().GetSystemHandle(), addr,
-                           buf, size, &num_of_bytes_read)) {
-    error = Status(GetLastError(), eErrorTypeWin32);
-    LLDB_LOG(log, "reading failed with error: {0}", error);
-  } else {
+  if (::ReadProcessMemory(handle, addr, buf, size, &num_of_bytes_read)) {
+    bytes_read = num_of_bytes_read;
+    return Status();
+  }
+  error = Status(GetLastError(), eErrorTypeWin32);
+  MemoryRegionInfo info;
+  if (GetMemoryRegionInfo(vm_addr, info).Fail() ||
+      info.GetMapped() != MemoryRegionInfo::OptionalBool::eYes)
+    return error;
+  size = info.GetRange().GetRangeEnd() - vm_addr;
+  LLDB_LOG(log, "retrying the read with size {0:x}", size);
+  if (::ReadProcessMemory(handle, addr, buf, size, &num_of_bytes_read)) {
+    LLDB_LOG(log, "success: read {0:x} bytes", num_of_bytes_read);
     bytes_read = num_of_bytes_read;
+    return Status();
   }
+  error = Status(GetLastError(), eErrorTypeWin32);
+  LLDB_LOG(log, "error: {0}", error);
   return error;
 }
 
diff --git a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
index 6ecb25b2e4db28..1c2c90d483ea3f 100644
--- a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
+++ b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py
@@ -50,7 +50,6 @@ def _prepare_inferior(self):
         ]
         self.assertEqual(len(self.positions), 5)
 
-    @expectedFailureWindows
     def test_memory_read(self):
         self._prepare_inferior()
 

@labath labath merged commit 04ed12c into llvm:main Sep 3, 2024
@labath labath deleted the read-win branch November 8, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants