[Lldb-commits] [lldb] [lldb][windows] bound ReadProcessMemory (PR #200230)

[Charles Zablit via lldb-commits]

https://github.com/charles-zablit updated https://github.com/llvm/llvm-project/pull/200230

>From 94133771ea908de6b3f72695d50fb3c64f7e6778 Mon Sep 17 00:00:00 2001
From: Charles Zablit <c_zablit at apple.com>
Date: Thu, 28 May 2026 18:16:21 +0100
Subject: [PATCH] [lldb][windows] bound ReadProcessMemory

---
 .../Process/Windows/Common/DebuggerThread.cpp | 94 +++++++++++++------
 1 file changed, 66 insertions(+), 28 deletions(-)

diff --git a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
index 22a836f9cbed5..bbb8e6d5a6773 100644
--- a/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/DebuggerThread.cpp
@@ -614,6 +614,65 @@ static std::optional<std::string> GetFileNameByLoadAddress(HANDLE hProcess,
   return dos_path;
 }
 
+// Determine how many bytes can be read at `addr` in `hProcess` before crossing
+// out of the committed memory region containing it. Returns 0 if the address is
+// not within a committed region.
+static SIZE_T BytesReadableAt(HANDLE hProcess, LPCVOID addr) {
+  MEMORY_BASIC_INFORMATION mbi{};
+  if (!::VirtualQueryEx(hProcess, addr, &mbi, sizeof(mbi)))
+    return 0;
+  if (mbi.State != MEM_COMMIT)
+    return 0;
+  uintptr_t region_end =
+      reinterpret_cast<uintptr_t>(mbi.BaseAddress) + mbi.RegionSize;
+  uintptr_t a = reinterpret_cast<uintptr_t>(addr);
+  if (a >= region_end)
+    return 0;
+  return region_end - a;
+}
+
+static std::optional<std::string> ReadRemoteStringW(HANDLE hProcess,
+                                                    LPCVOID addr) {
+  SIZE_T to_read = std::min<SIZE_T>((MAX_PATH + 1) * sizeof(wchar_t),
+                                    BytesReadableAt(hProcess, addr));
+  to_read &= ~SIZE_T(1); // round down to a wchar_t boundary
+  if (to_read < sizeof(wchar_t))
+    return std::nullopt;
+
+  std::array<wchar_t, MAX_PATH + 1> buf{};
+  SIZE_T bytes_read = 0;
+  if (!::ReadProcessMemory(hProcess, addr, buf.data(), to_read, &bytes_read))
+    return std::nullopt;
+
+  size_t max_chars = bytes_read / sizeof(wchar_t);
+  size_t len = ::wcsnlen(buf.data(), max_chars);
+  if (len == 0 || len == max_chars) // no null terminator found
+    return std::nullopt;
+
+  std::string result;
+  llvm::convertWideToUTF8(std::wstring(buf.data(), len), result);
+  return result;
+}
+
+static std::optional<std::string> ReadRemoteStringA(HANDLE hProcess,
+                                                    LPCVOID addr) {
+  SIZE_T to_read =
+      std::min<SIZE_T>(MAX_PATH + 1, BytesReadableAt(hProcess, addr));
+  if (to_read == 0)
+    return std::nullopt;
+
+  std::array<char, MAX_PATH + 1> buf{};
+  SIZE_T bytes_read = 0;
+  if (!::ReadProcessMemory(hProcess, addr, buf.data(), to_read, &bytes_read))
+    return std::nullopt;
+
+  size_t len = ::strnlen(buf.data(), bytes_read);
+  if (len == 0 || len == bytes_read) // no null terminator found
+    return std::nullopt;
+
+  return std::string(buf.data(), len);
+}
+
 // Resolve the LOAD_DLL_DEBUG_INFO::lpImageName field.
 static std::optional<std::string>
 GetFileNameFromImageNameField(HANDLE hProcess,
@@ -621,37 +680,16 @@ GetFileNameFromImageNameField(HANDLE hProcess,
   if (info.lpImageName == nullptr)
     return std::nullopt;
 
-  LPVOID name_addr = nullptr;
+  LPVOID string_addr = nullptr;
   SIZE_T bytes_read = 0;
-  if (!::ReadProcessMemory(hProcess, info.lpImageName, &name_addr,
-                           sizeof(name_addr), &bytes_read) ||
-      bytes_read != sizeof(name_addr) || name_addr == nullptr)
+  if (!::ReadProcessMemory(hProcess, info.lpImageName, &string_addr,
+                           sizeof(string_addr), &bytes_read) ||
+      bytes_read != sizeof(string_addr))
     return std::nullopt;
 
-  if (info.fUnicode) {
-    std::array<wchar_t, MAX_PATH + 1> wbuf{};
-    if (!::ReadProcessMemory(hProcess, name_addr, wbuf.data(),
-                             wbuf.size() * sizeof(wchar_t), &bytes_read))
-      return std::nullopt;
-    if (wbuf[MAX_PATH] != L'\0')
-      return std::nullopt;
-    std::string path_utf8;
-    llvm::convertWideToUTF8(wbuf.data(), path_utf8);
-    if (path_utf8.empty())
-      return std::nullopt;
-    return path_utf8;
-  }
-
-  std::array<char, MAX_PATH + 1> abuf{};
-  if (!::ReadProcessMemory(hProcess, name_addr, abuf.data(), abuf.size(),
-                           &bytes_read))
-    return std::nullopt;
-  if (abuf[MAX_PATH] != '\0')
-    return std::nullopt;
-  std::string path(abuf.data());
-  if (path.empty())
-    return std::nullopt;
-  return path;
+  if (info.fUnicode)
+    return ReadRemoteStringW(hProcess, string_addr);
+  return ReadRemoteStringA(hProcess, string_addr);
 }
 
 DWORD