[lldb] Correctly detach (rather than kill) when connecting with gdb-remote

[JDevlieghere]

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

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

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

---

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

2 Files Affected:

• (modified) lldb/source/Target/Process.cpp (+1)
• (added) lldb/test/API/functionalities/gdb_remote_client/TestConnectRemoteDetach.py (+64)

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"])

[DavidSpickett]

LGTM

[DavidSpickett]

Also check that no k was received. Since it's cheap to do and might catch a silly mistake.