Fix native Windows adapter broken after detach (#1050)

Author: xusheng6

core/adapters/windowsnativeadapter.cpp

@@ -238,23 +238,6 @@ bool WindowsNativeAdapter::Detach()
 	// Wake up the debug thread if it's waiting
 	m_debugCondition.notify_one();
 
-	// Remove all breakpoints before detaching
-	{
-		std::lock_guard<std::mutex> lock(m_breakpointsMutex);
-		for (auto& bp : m_breakpoints)
-		{
-			if (bp.isActive)
-				RemoveBreakpointInternal(bp.address);
-		}
-		m_breakpoints.clear();
-	}
-
-	if (!DebugActiveProcessStop(m_processId))
-	{
-		LogError("Failed to detach from process: %d", GetLastError());
-		return false;
-	}
-
 	if (m_debugThread.joinable())
 		m_debugThread.join();
 
@@ -563,7 +546,16 @@ void WindowsNativeAdapter::DebugLoop()
 
 			if (m_shouldStop)
 			{
+				RemoveAllBreakpoints();
 				ContinueDebugEvent(debugEvent.dwProcessId, debugEvent.dwThreadId, DBG_CONTINUE);
+
+				// DebugActiveProcessStop must be called from the same thread that started debugging
+				if (!DebugActiveProcessStop(m_processId))
+				{
+					LogWarn("DebugActiveProcessStop failed (error %d) -- killing target", GetLastError());
+					TerminateProcess(m_processHandle, 1);
+				}
+
 				break;
 			}
 		}
@@ -588,6 +580,19 @@ void WindowsNativeAdapter::DebugLoop()
 		ContinueDebugEvent(debugEvent.dwProcessId, debugEvent.dwThreadId, continueStatus);
 	}
 
+	// If we exited the loop due to m_shouldStop while the target was running (not stopped at a
+	// breakpoint), we still need to detach. The stopped-at-breakpoint case is handled inside the loop.
+	if (m_shouldStop && m_activelyDebugging)
+	{
+		RemoveAllBreakpoints();
+
+		if (!DebugActiveProcessStop(m_processId))
+		{
+			LogWarn("DebugActiveProcessStop failed (error %d) -- killing target", GetLastError());
+			TerminateProcess(m_processHandle, 1);
+		}
+	}
+
 	m_activelyDebugging = false;
 }
 
@@ -1569,6 +1574,58 @@ bool WindowsNativeAdapter::RemoveBreakpoint(const ModuleNameAndOffset& breakpoin
 }
 
 
+void WindowsNativeAdapter::RemoveAllBreakpoints()
+{
+	// Remove software breakpoints
+	{
+		std::lock_guard<std::mutex> lock(m_breakpointsMutex);
+		for (auto& bp : m_breakpoints)
+		{
+			if (bp.isActive)
+				RemoveBreakpointInternal(bp.address);
+		}
+		m_breakpoints.clear();
+	}
+
+	// Remove hardware breakpoints from all threads
+	{
+		std::lock_guard<std::mutex> lock(m_hwBreakpointsMutex);
+		for (const auto& hwbp : m_hardwareBreakpoints)
+		{
+			if (hwbp.isActive)
+			{
+				for (auto& [tid, handle] : m_threads)
+				{
+					if (!handle)
+						continue;
+					if (m_isTargetWow64)
+					{
+						WOW64_CONTEXT ctx {};
+						ctx.ContextFlags = WOW64_CONTEXT_DEBUG_REGISTERS;
+						if (Wow64GetThreadContext(handle, &ctx))
+						{
+							if (ClearHardwareBreakpointInContext(ctx, hwbp.drIndex))
+								Wow64SetThreadContext(handle, &ctx);
+						}
+					}
+					else
+					{
+						CONTEXT ctx {};
+						ctx.ContextFlags = CONTEXT_DEBUG_REGISTERS;
+						if (GetThreadContext(handle, &ctx))
+						{
+							if (ClearHardwareBreakpointInContext(ctx, hwbp.drIndex))
+								SetThreadContext(handle, &ctx);
+						}
+					}
+				}
+			}
+		}
+		m_hardwareBreakpoints.clear();
+	}
+}
+
+
 bool WindowsNativeAdapter::RemoveBreakpointInternal(uint64_t address)
 {
 	// Find the breakpoint to get the original byte

core/adapters/windowsnativeadapter.h

@@ -154,6 +154,7 @@ namespace BinaryNinjaDebugger {
 		bool ApplyBreakpoint(uint64_t address, unsigned long id);
 		bool RemoveBreakpointInternal(uint64_t address);
 		void ApplyPendingBreakpoints();
+		void RemoveAllBreakpoints();
 		bool ApplyHardwareBreakpointsToThread(HANDLE threadHandle);
 		int FindFreeDebugRegister();
 		bool SetHardwareBreakpointInContext(CONTEXT& ctx, int drIndex, uint64_t address, DebugBreakpointType type, size_t size);