🍒 [lldb][Windows] ignore loader breakpoints in system modules (#208233)

lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.cpp

@@ -19,16 +19,81 @@
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Utility/FileSpec.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Error.h"
 
 #include "DebuggerThread.h"
 #include "ExceptionRecord.h"
 #include "ProcessWindowsLog.h"
 
+#include <string>
+#include <string_view>
+
 using namespace lldb;
 using namespace lldb_private;
 
+static void NormalizeWindowsPathSeparators(std::string &s) {
+  for (char &c : s)
+    if (c == '/')
+      c = '\\';
+}
+
+bool ProcessDebugger::IsSystemDLL(llvm::StringRef path) {
+  if (path.empty())
+    return false;
+
+  static const std::string windows_prefix = []() {
+    std::string prefix;
+    wchar_t buf[MAX_PATH];
+    UINT len = ::GetWindowsDirectoryW(buf, MAX_PATH);
+    if (len == 0 || len >= MAX_PATH)
+      return prefix;
+    llvm::convertWideToUTF8(std::wstring_view(buf, len), prefix);
+    NormalizeWindowsPathSeparators(prefix);
+    if (!prefix.empty() && prefix.back() != '\\')
+      prefix += '\\';
+    return prefix;
+  }();
+
+  if (windows_prefix.empty())
+    return false;
+
+  std::string normalized = path.str();
+  NormalizeWindowsPathSeparators(normalized);
+  return llvm::StringRef(normalized).starts_with_insensitive(windows_prefix);
+}
+
+bool ProcessDebugger::IsSystemModuleAddress(lldb::addr_t addr) {
+  if (!m_session_data || !m_session_data->m_debugger)
+    return false;
+  lldb::process_t handle = m_session_data->m_debugger->GetProcess()
+                               .GetNativeProcess()
+                               .GetSystemHandle();
+  if (handle == nullptr || handle == LLDB_INVALID_PROCESS)
+    return false;
+
+  MEMORY_BASIC_INFORMATION mbi = {};
+  if (::VirtualQueryEx(handle, reinterpret_cast<LPCVOID>(addr), &mbi,
+                       sizeof(mbi)) != sizeof(mbi))
+    return false;
+  if (mbi.AllocationBase == nullptr)
+    return false;
+
+  // A truncated path still carries the leading directory, which is all
+  // IsSystemDLL() inspects. MAX_PATH is enough.
+  wchar_t module_path[MAX_PATH];
+  DWORD len = ::GetModuleFileNameExW(
+      handle, reinterpret_cast<HMODULE>(mbi.AllocationBase), module_path,
+      MAX_PATH);
+  if (len == 0)
+    return false;
+
+  std::string path_utf8;
+  llvm::convertWideToUTF8(std::wstring_view(module_path, len), path_utf8);
+  return IsSystemDLL(path_utf8);
+}
+
 static DWORD ConvertLldbToWinApiProtect(uint32_t protect) {
   // We also can process a read / write permissions here, but if the debugger
   // will make later a write into the allocated memory, it will fail. To get

lldb/source/Plugins/Process/Windows/Common/ProcessDebugger.h

@@ -14,6 +14,10 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorExtras.h"
 #include "llvm/Support/Mutex.h"
 
 #include "ForwardDecl.h"
@@ -62,6 +66,10 @@ class ProcessDebugger {
   virtual void OnDebugString(const std::string &string);
   virtual void OnDebuggerError(const Status &error, uint32_t type);
 
+  static bool IsSystemDLL(llvm::StringRef path);
+
+  bool IsSystemModuleAddress(lldb::addr_t addr);
+
 protected:
   Status DetachProcess();
 

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

@@ -13,6 +13,7 @@
 #include <psapi.h>
 
 #include "lldb/Breakpoint/Watchpoint.h"
+#include "lldb/Core/Address.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
@@ -225,6 +226,9 @@ ProcessWindows::DoAttachToProcessWithID(lldb::pid_t pid,
   Status error = AttachProcess(pid, attach_info, delegate);
   if (error.Success())
     SetID(GetDebuggedProcessId());
+
+  m_expecting_loader_int3 = true;
+
   return error;
 }
 
@@ -291,8 +295,12 @@ Status ProcessWindows::DoDestroy() {
 
 Status ProcessWindows::DoHalt(bool &caused_stop) {
   StateType state = GetPrivateState();
-  if (state != eStateStopped)
-    return HaltProcess(caused_stop);
+  if (state != eStateStopped) {
+    m_pending_halt = true;
+    Status error = HaltProcess(caused_stop);
+    if (error.Fail() || !caused_stop)
+      m_pending_halt = false;
+  }
   caused_stop = false;
   return Status();
 }
@@ -772,7 +780,22 @@ ProcessWindows::OnDebugException(bool first_chance,
 
   ExceptionResult result = ExceptionResult::SendToApplication;
   switch (record.GetExceptionCode()) {
-  case EXCEPTION_BREAKPOINT:
+  case EXCEPTION_BREAKPOINT: {
+    const lldb::addr_t bp_addr = record.GetExceptionAddress();
+    if (m_pending_halt) {
+      m_pending_halt = false;
+    } else if (m_expecting_loader_int3 && first_chance &&
+               m_session_data->m_initial_stop_received &&
+               !GetBreakpointSiteList().FindByAddress(bp_addr) &&
+               IsSystemModuleAddress(bp_addr)) {
+      m_expecting_loader_int3 = false;
+      LLDB_LOG(log,
+               "Skipping expected loader breakpoint at address {0:x} in a "
+               "system module.",
+               bp_addr);
+      return ExceptionResult::MaskException;
+    }
+
     // Handle breakpoints at the first chance.
     result = ExceptionResult::BreakInDebugger;
 
@@ -795,6 +818,7 @@ ProcessWindows::OnDebugException(bool first_chance,
     DrainProcessStdout();
     SetPrivateState(eStateStopped);
     break;
+  }
   case EXCEPTION_SINGLE_STEP:
     result = ExceptionResult::BreakInDebugger;
     DrainProcessStdout();

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h

@@ -123,6 +123,8 @@ class ProcessWindows : public Process, public ProcessDebugger {
   std::map<lldb::break_id_t, WatchpointInfo> m_watchpoints;
   std::vector<lldb::break_id_t> m_watchpoint_ids;
   std::shared_ptr<PTY> m_pty;
+  bool m_pending_halt = false;
+  bool m_expecting_loader_int3 = false;
 };
 } // namespace lldb_private
 

lldb/test/API/attach/Makefile

@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

lldb/test/API/attach/TestWindowsAttachBreakpoint.py

@@ -0,0 +1,160 @@
+"""
+Test that lldb ignores the Windows loader breakpoint when attaching to a
+process, but still stops at a genuine breakpoint instruction (an int3) that
+lives in the program's own code.
+"""
+
+import ctypes
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class WindowsAttachLoaderBreakpointTestCase(TestBase):
+    def create_suspended_process(self, exe):
+        """Create ``exe`` suspended and return its (pid, hProcess, hThread).
+
+        This mirrors what the lldb-dap runInTerminal launcher does on Windows:
+        the debuggee is started with CREATE_SUSPENDED so lldb can attach while
+        the process is still being initialized by the loader. The launcher then
+        resumes the main thread once the debugger has attached.
+        """
+        from ctypes import wintypes
+
+        CREATE_SUSPENDED = 0x00000004
+
+        class STARTUPINFOW(ctypes.Structure):
+            _fields_ = [
+                ("cb", wintypes.DWORD),
+                ("lpReserved", wintypes.LPWSTR),
+                ("lpDesktop", wintypes.LPWSTR),
+                ("lpTitle", wintypes.LPWSTR),
+                ("dwX", wintypes.DWORD),
+                ("dwY", wintypes.DWORD),
+                ("dwXSize", wintypes.DWORD),
+                ("dwYSize", wintypes.DWORD),
+                ("dwXCountChars", wintypes.DWORD),
+                ("dwYCountChars", wintypes.DWORD),
+                ("dwFillAttribute", wintypes.DWORD),
+                ("dwFlags", wintypes.DWORD),
+                ("wShowWindow", wintypes.WORD),
+                ("cbReserved2", wintypes.WORD),
+                ("lpReserved2", ctypes.POINTER(ctypes.c_byte)),
+                ("hStdInput", wintypes.HANDLE),
+                ("hStdOutput", wintypes.HANDLE),
+                ("hStdError", wintypes.HANDLE),
+            ]
+
+        class PROCESS_INFORMATION(ctypes.Structure):
+            _fields_ = [
+                ("hProcess", wintypes.HANDLE),
+                ("hThread", wintypes.HANDLE),
+                ("dwProcessId", wintypes.DWORD),
+                ("dwThreadId", wintypes.DWORD),
+            ]
+
+        kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
+        kernel32.CreateProcessW.argtypes = [
+            wintypes.LPCWSTR,
+            wintypes.LPWSTR,
+            ctypes.c_void_p,
+            ctypes.c_void_p,
+            wintypes.BOOL,
+            wintypes.DWORD,
+            ctypes.c_void_p,
+            wintypes.LPCWSTR,
+            ctypes.POINTER(STARTUPINFOW),
+            ctypes.POINTER(PROCESS_INFORMATION),
+        ]
+        kernel32.CreateProcessW.restype = wintypes.BOOL
+
+        startupinfo = STARTUPINFOW()
+        startupinfo.cb = ctypes.sizeof(STARTUPINFOW)
+        process_information = PROCESS_INFORMATION()
+
+        # CreateProcessW may modify the command-line buffer, so it must be
+        # writable.
+        command_line = ctypes.create_unicode_buffer('"{}"'.format(exe))
+
+        if not kernel32.CreateProcessW(
+            exe,
+            command_line,
+            None,
+            None,
+            False,
+            CREATE_SUSPENDED,
+            None,
+            None,
+            ctypes.byref(startupinfo),
+            ctypes.byref(process_information),
+        ):
+            raise OSError(ctypes.get_last_error(), "CreateProcessW failed")
+
+        return (
+            process_information.dwProcessId,
+            process_information.hProcess,
+            process_information.hThread,
+        )
+
+    @skipUnlessWindows
+    def test_attach_ignores_loader_breakpoint(self):
+        """
+        lldb must not report the loader's int3 (raised in a system module while
+        attaching) as a user-visible stop, but must still stop at the
+        __builtin_debugtrap() in the program's own code.
+        """
+        self.build()
+        exe = self.getBuildArtifact("a.out")
+
+        pid, hProcess, hThread = self.create_suspended_process(exe)
+
+        kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
+        kernel32.ResumeThread.argtypes = [ctypes.c_void_p]
+        kernel32.ResumeThread.restype = ctypes.c_uint
+
+        def cleanup():
+            kernel32.TerminateProcess(hProcess, 0)
+            kernel32.CloseHandle(hThread)
+            kernel32.CloseHandle(hProcess)
+
+        self.addTearDownHook(cleanup)
+
+        self.dbg.SetAsync(False)
+
+        # Attach while the process is suspended and still being initialized.
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        error = lldb.SBError()
+        process = target.AttachToProcessWithID(self.dbg.GetListener(), pid, error)
+        self.assertSuccess(error, "attach to the suspended process")
+        self.assertState(process.GetState(), lldb.eStateStopped)
+
+        self.assertNotEqual(
+            kernel32.ResumeThread(hThread), 0xFFFFFFFF, "ResumeThread failed"
+        )
+
+        # Continuing must run past the loader breakpoint (skipped because it is
+        # raised in a system module) and stop at the program's own
+        # __builtin_debugtrap().
+        process.Continue()
+        self.assertState(process.GetState(), lldb.eStateStopped)
+
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonException)
+        self.assertIsNotNone(
+            thread, "the process should stop at the user __builtin_debugtrap()"
+        )
+        function_name = thread.GetFrameAtIndex(0).GetFunctionName()
+        self.assertIn(
+            "main",
+            function_name,
+            "expected to stop at the program's __builtin_debugtrap(), but "
+            "stopped in '{}' (a spurious loader breakpoint in a system module "
+            "was not skipped)".format(function_name),
+        )
+
+        process.Continue()
+        self.assertState(process.GetState(), lldb.eStateExited)
+        self.assertEqual(process.GetExitStatus(), 0)

lldb/test/API/attach/main.c

@@ -0,0 +1,4 @@
+int main(int argc, char *argv[]) {
+  __builtin_debugtrap();
+  return 0;
+}