[Lldb-commits] [lldb] [lldb-dap][windows] fix TestDAP_restart_console tests (PR #198573)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Charles Zablit (charles-zablit)

<details>
<summary>Changes</summary>

a614cd391a40 introduced a regression. It assumed that the launcher's `ResumeThread` was sufficient to start the debuggee.

That's incorrect: on Windows the inferior is held by two independent mechanisms after attach:
- pending Win32 debug events from the attach. The debuggee is created with `CREATE_SUSPENDED`. This requires a `ContinueDebugEvent` to release.
- the OS suspend count (handled by the launcher's `ResumeThread`), which are regular breakpoints.

The fix is to run `Continue()` in async mode on Windows by resetting scope_sync_mode first.

This is a follow up to https://github.com/llvm/llvm-project/pull/198764.

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


4 Files Affected:

- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_stdio_redirection_and_console.py (-2) 
- (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py (-2) 
- (modified) lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py (-1) 
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+12-13) 


``````````diff
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_stdio_redirection_and_console.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_stdio_redirection_and_console.py
index f4512a5c7bb63..9dcc4df77042f 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_stdio_redirection_and_console.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_stdio_redirection_and_console.py
@@ -7,7 +7,6 @@
     skipIf,
     skipIfBuildType,
     no_match,
-    skipIfWindows,
 )
 import lldbdap_testcase
 import tempfile
@@ -19,7 +18,6 @@ class TestDAP_launch_stdio_redirection_and_console(lldbdap_testcase.DAPTestCaseB
     """
 
     @skipIfAsan
-    @skipIfWindows  # https://github.com/llvm/llvm-project/issues/198763
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     @skipIfBuildType(["debug"])
     def test(self):
diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py
index c80a3cdc10d95..068fc5146c73b 100644
--- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py
+++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_console.py
@@ -12,7 +12,6 @@
 @skipIfBuildType(["debug"])
 class TestDAP_restart_console(lldbdap_testcase.DAPTestCaseBase):
     @skipIfAsan
-    @expectedFailureWindows
     @skipIf(oslist=["linux"], archs=["arm$"])  # Always times out on buildbot
     def test_basic_functionality(self):
         """
@@ -61,7 +60,6 @@ def test_basic_functionality(self):
         self.continue_to_exit()
 
     @skipIfAsan
-    @expectedFailureWindows
     @skipIf(oslist=["linux"], archs=["arm$"])  # Always times out on buildbot
     def test_stopOnEntry(self):
         """
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
index 429051c9cf9f9..4e09efadfdcdc 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
@@ -66,7 +66,6 @@ def read_pipe_message(pipe):
 
 
 @skipIfBuildType(["debug"])
- at skipIfWindows  # https://github.com/llvm/llvm-project/issues/198763
 class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase):
     SHARED_BUILD_TESTCASE = False
 
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index e44e09e4741e4..cb21c47385a7c 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -139,21 +139,20 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
   std::future<lldb::SBError> did_attach_message_success =
       comm_channel.NotifyDidAttach();
 
-// We just attached to the runInTerminal launcher, which was waiting to be
-// attached. We now resume it, so it can receive the didAttach notification
-// and then perform the exec. Upon continuing, the debugger will stop the
-// process right in the middle of the exec. To the user, what we are doing is
-// transparent, as they will only be able to see the process since the exec,
-// completely unaware of the preparatory work.
-//
-// On Windows, the debuggee itself is waiting to be attached to. There is no
-// need to continue.
-#ifndef _WIN32
+#ifdef _WIN32
+  // On Windows the target was created with CREATE_SUSPENDED. The launcher
+  // will call ResumeThread after reading didAttach, but we still must process
+  // the initial debug events from the attach for the target to actually run.
+  // Use async Continue() so we don't block while the suspend count is still 1.
+  scope_sync_mode.reset();
+  dap.target.GetProcess().Continue();
+#else
+  // On Linux the launcher and target are the same process. Continue() resumes
+  // the launcher so it can read didAttach and execvp into the target, stopping
+  // again at the new program's entry.
   dap.target.GetProcess().Continue();
-#endif
-
-  // Return the debugger to its prior async state.
   scope_sync_mode.reset();
+#endif
 
   // If sending the notification failed, the launcher should be dead by now and
   // the async didAttach notification should have an error message, so we

``````````

</details>


https://github.com/llvm/llvm-project/pull/198573