[Lldb-commits] [lldb] 29ce634 - [lldb] [Windows] Let LLGS TestClient tolerate async `O` output packets (#225521)

[via lldb-commits]

Author: Yao Qi
Date: 2026-09-23T14:04:19+01:00
New Revision: 29ce634abb29fc59a147e63e72426c343becf4f9

URL: https://github.com/llvm/llvm-project/commit/29ce634abb29fc59a147e63e72426c343becf4f9
DIFF: https://github.com/llvm/llvm-project/commit/29ce634abb29fc59a147e63e72426c343becf4f9.diff

LOG: [lldb] [Windows] Let LLGS TestClient tolerate async `O` output packets (#225521)

My PR (swiftlang/llvm-project#14269) has a pre-merge test failure on
Windows, and that lead me looking at test LLGSTest.cpp deeply

  ```
  Send Packet: vCont;c
  Read Packet: O1b5b3f32356c
  lldb-server exiting...
LLGSTest.cpp(30): error: Value of:
llvm::detail::TakeError(Client.ContinueAll())
  Expected: succeeded
    Actual: failed  (Unable to parse StopReply: Invalid packet)
  ```

`TestClient::SendMessage` read exactly one packet and had no handling
for `O`
(inferior stdout) packets, but inferior output arriving before a stop
reply
was parsed as the stop reply itself. `StopReply::create` accepts only
`T` and
`W`, so it failed.  The `O1b5b3f32356c` decodes to `ESC [ ? 2 5 l`, the
sequence that hides the cursor. Windows lldb-server gives the inferior a
ConPTY
created with `PSEUDOCONSOLE_INHERIT_CURSOR`, and ConPTY writes its own
VT
initialization bytes to the output pipe, which reach the client as an
`O` packet.

The fix is that `SendMessage` now uses the inherited
`SendPacketAndReceiveResponseWithOutputSupport`, which loops over
leading `O`
packets via `ReadPacketWithOutputSupport` and passes each payload to a
callback. `TestClient::Continue` had the same gap in the opposite order:
after a
`W` or `X` reply it read one raw packet and required
`ErrorDisconnected`, so an `O`
packet arriving before the socket closed failed the test. It now reads
through
`ReadPacketWithOutputSupport` as well.

The fix reverts 5032c6ea0803 ("[lldb-server] Disable a test on Windows
until it can be
fixed"), which disabled `LaunchModePreservesEnvironment` on Windows.

18b211cb1521 ("Disable stdin/stdout for environment_check inferior
process")
worked around the same root cause from a different source, where an
allocator
message on stderr under sanitizers reached the client the same way. That
commit noted the client "doesn't handle that unexpected output"; this
handles
it.

Assisted-by: claude

Added: 
    

Modified: 
    lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
    lldb/unittests/tools/lldb-server/tests/TestClient.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
index b438d7496d8810..af0a88d7f91350 100644
--- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -15,9 +15,6 @@ using namespace llgs_tests;
 using namespace lldb_private;
 using namespace llvm;
 
-// Disable this test on Windows as it appears to have a race condition
-// that causes lldb-server not to exit after the inferior hangs up.
-#if !defined(_WIN32)
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
   putenv(const_cast<char *>("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
@@ -33,7 +30,6 @@ TEST_F(TestBase, LaunchModePreservesEnvironment) {
       HasValue(testing::Property(&StopReply::getKind,
                                  WaitStatus{WaitStatus::Exit, 0})));
 }
-#endif
 
 TEST_F(TestBase, DS_TEST(DebugserverEnv)) {
   // Test that --env takes precedence over inherited environment variables.

diff  --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
index a459ae64254e89..fc5c82cb319776 100644
--- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -31,6 +31,10 @@ static std::chrono::seconds GetDefaultTimeout() {
   return std::chrono::seconds{10};
 }
 
+static void LogInferiorOutput(StringRef output) {
+  GTEST_LOG_(INFO) << "Inferior output: " << output.str();
+}
+
 TestClient::TestClient(std::unique_ptr<Connection> Conn) {
   SetConnection(std::move(Conn));
   SetPacketTimeout(GetDefaultTimeout());
@@ -207,7 +211,8 @@ Error TestClient::SendMessage(StringRef message, std::string &response_string,
                               PacketResult expected_result) {
   StringExtractorGDBRemote response;
   GTEST_LOG_(INFO) << "Send Packet: " << message.str();
-  PacketResult result = SendPacketAndWaitForResponse(message, response);
+  PacketResult result = SendPacketAndReceiveResponseWithOutputSupport(
+      message, response, std::chrono::seconds(0), LogInferiorOutput);
   response.GetEscapedBinaryData(response_string);
   GTEST_LOG_(INFO) << "Read Packet: " << response_string;
   if (result != expected_result)
@@ -275,7 +280,8 @@ Error TestClient::Continue(StringRef message) {
   m_stop_reply = std::move(*StopReplyOr);
   if (!isa<StopReplyStop>(m_stop_reply)) {
     StringExtractorGDBRemote R;
-    PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
+    PacketResult result = ReadPacketWithOutputSupport(R, GetPacketTimeout(),
+                                                      false, LogInferiorOutput);
     if (result != PacketResult::ErrorDisconnected) {
       return createStringErrorV("Expected connection close after sending {0}. "
                                 "Got {1}/{2} instead.",