[Lldb-commits] [lldb] [lldb] Correctly detach (rather than kill) when connecting with gdb-remote (PR #166869)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/166869

>From 322315b02a4360be97b730d371747e6f607cf554 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 6 Nov 2025 14:58:41 -0800
Subject: [PATCH 1/2] [lldb] Correctly detach (rather than kill) when
 connecting with gdb-remote

We weren't setting `m_should_detach` when going through the
`DoConnectRemote` code path. This meant that when you would attaches to
a remote process with gdb-remote [PORT] and use Ctrl+D, it would kill
the process instead of detach from it.

rdar://156111423
---
 lldb/source/Target/Process.cpp                |  1 +
 .../TestConnectRemoteDetach.py                | 64 +++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 42ce198a283da..69edea503002e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3256,6 +3256,7 @@ Status Process::ConnectRemote(llvm::StringRef remote_url) {
       if (state == eStateStopped || state == eStateCrashed) {
         // If we attached and actually have a process on the other end, then
         // this ended up being the equivalent of an attach.
+        SetShouldDetach(true);
         CompleteAttach();
 
         // This delays passing the stopped event to listeners till
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py b/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py
new file mode 100644
index 0000000000000..64679246ef933
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py
@@ -0,0 +1,64 @@
+"""
+Test that ConnectRemote sets ShouldDetach flag correctly.
+
+When connecting to a remote process that stops after connection,
+the process should be marked for detach (not kill) on destruction.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+from lldbsuite.test import lldbutil
+
+
+class TestConnectRemoteDetach(GDBRemoteTestBase):
+    """Test that ConnectRemote properly sets ShouldDetach flag."""
+
+    class StoppedResponder(MockGDBServerResponder):
+        """A responder that returns a stopped process."""
+
+        def qfThreadInfo(self):
+            return "m1"
+
+        def qsThreadInfo(self):
+            return "l"
+
+        def qC(self):
+            return "QC1"
+
+        def haltReason(self):
+            # Return that we're stopped
+            return "T05thread:1;"
+
+        def cont(self):
+            # Stay stopped
+            return "T05thread:1;"
+
+        def D(self):
+            # Detach packet - this is what we want to verify gets called
+            return "OK"
+
+    def test_connect_remote_sets_detach(self):
+        """Test that ConnectRemote to a stopped process sets ShouldDetach."""
+        self.server.responder = self.StoppedResponder()
+
+        target = self.createTarget("a.yaml")
+        process = self.connect(target)
+
+        # Wait for the process to be in stopped state after connecting.
+        # When ConnectRemote connects to a remote process that is stopped,
+        # it should call SetShouldDetach(true) before CompleteAttach().
+        lldbutil.expect_state_changes(
+            self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+        )
+
+        # Now destroy the process. Because ShouldDetach was set to true
+        # during ConnectRemote, this should send a 'D' (detach) packet
+        # rather than a 'k' (kill) packet when the process is destroyed.
+        process.Destroy()
+
+        # Verify that the detach packet was sent
+        # The 'D' packet is the detach command in GDB remote protocol
+        self.assertPacketLogReceived(["D"])

>From 2be14920ccf430e3de691fe336d6312a37b60394 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Fri, 7 Nov 2025 10:02:30 -0800
Subject: [PATCH 2/2] Make sure we don't get a k(ill) packet

---
 .../gdb_remote_client/TestConnectRemoteDetach.py         | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py b/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py
index 64679246ef933..4380455efc452 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py
@@ -37,9 +37,13 @@ def cont(self):
             return "T05thread:1;"
 
         def D(self):
-            # Detach packet - this is what we want to verify gets called
+            # Detach packet: this is what we want to verify gets called.
             return "OK"
 
+        def k(self):
+            # Kill packet: this is what we want to verify doesn't get called.
+            raise RuntimeError("should not receive k(ill) packet")
+
     def test_connect_remote_sets_detach(self):
         """Test that ConnectRemote to a stopped process sets ShouldDetach."""
         self.server.responder = self.StoppedResponder()
@@ -59,6 +63,5 @@ def test_connect_remote_sets_detach(self):
         # rather than a 'k' (kill) packet when the process is destroyed.
         process.Destroy()
 
-        # Verify that the detach packet was sent
-        # The 'D' packet is the detach command in GDB remote protocol
+        # Verify that the (D)etach packet was sent.
         self.assertPacketLogReceived(["D"])