[Lldb-commits] [lldb] dfbe3a7 - [lldb] Check that qLaunchGDBServer packet does not return an error

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2023-06-30T09:12:30Z
New Revision: dfbe3a79e20f1bc51a59ee858fabce792d59c9ae

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

LOG: [lldb] Check that qLaunchGDBServer packet does not return an error

While looking at https://github.com/llvm/llvm-project/issues/61955
I noticed that when we send qLaunchGDBServer we check that we got a response
but not what kind of response it was.

I think this was why the bug reporter saw:
(lldb) run
error: invalid host:port specification: '[192.168.64.2]'

The missing port is because we went down a path we only should have
chosen if the operation succeeded. Since we didn't check, we went ahead
with an empty port number.

To test this I've done the following:
* Make a temporary copy of lldb-server.
* Run that as a platform.
* Remove the copy.
* Attempt to create and run a target.

This fails because the running lldb-server will try to invoke itself
and it no longer exists.

Reviewed By: jasonmolenda

Differential Revision: https://reviews.llvm.org/D153513

Added: 
    lldb/test/API/commands/platform/launchgdbserver/Makefile
    lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
    lldb/test/API/commands/platform/launchgdbserver/main.c

Modified: 
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 18aa98bc046495..36e046d7466ebb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2590,6 +2590,9 @@ bool GDBRemoteCommunicationClient::LaunchGDBServer(
 
   if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
       PacketResult::Success) {
+    if (response.IsErrorResponse())
+      return false;
+
     llvm::StringRef name;
     llvm::StringRef value;
     while (response.GetNameColonValue(name, value)) {

diff  --git a/lldb/test/API/commands/platform/launchgdbserver/Makefile b/lldb/test/API/commands/platform/launchgdbserver/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/commands/platform/launchgdbserver/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
new file mode 100644
index 00000000000000..ea849ab1fa236d
--- /dev/null
+++ b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py
@@ -0,0 +1,58 @@
+""" Check that errors while handling qLaunchGDBServer are reported to the user.
+    Though this isn't a platform command in itself, the best way to test it is
+    from Python because we can juggle multiple processes more easily.
+"""
+
+import os
+import socket
+import shutil
+import lldbgdbserverutils
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPlatformProcessLaunchGDBServer(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIfRemote
+    # Windows doesn't use lldb-server and on Darwin we may be using debugserver.
+    @skipUnlessPlatform(["linux"])
+    @add_test_categories(["lldb-server"])
+    def test_platform_process_launch_gdb_server(self):
+        self.build()
+
+        hostname = socket.getaddrinfo("localhost", 0, proto=socket.IPPROTO_TCP)[0][4][0]
+        listen_url = "[%s]:0" % hostname
+
+        port_file = self.getBuildArtifact("port")
+        commandline_args = [
+            "platform",
+            "--listen",
+            listen_url,
+            "--socket-file",
+            port_file,
+            "--",
+            self.getBuildArtifact("a.out"),
+            "foo",
+        ]
+
+        # Run lldb-server from a new location.
+        new_lldb_server = self.getBuildArtifact("lldb-server")
+        shutil.copy(lldbgdbserverutils.get_lldb_server_exe(), new_lldb_server)
+
+        self.spawnSubprocess(new_lldb_server, commandline_args)
+        socket_id = lldbutil.wait_for_file_on_target(self, port_file)
+
+        # Remove our new lldb-server so that when it tries to invoke itself as a
+        # gdbserver, it fails.
+        os.remove(new_lldb_server)
+
+        new_platform = lldb.SBPlatform("remote-" + self.getPlatform())
+        self.dbg.SetSelectedPlatform(new_platform)
+
+        connect_url = "connect://[%s]:%s" % (hostname, socket_id)
+        self.runCmd("platform connect %s" % connect_url)
+
+        self.runCmd("target create {}".format(self.getBuildArtifact("a.out")))
+        self.expect("run", substrs=["unable to launch a GDB server on"], error=True)

diff  --git a/lldb/test/API/commands/platform/launchgdbserver/main.c b/lldb/test/API/commands/platform/launchgdbserver/main.c
new file mode 100644
index 00000000000000..76e8197013aabc
--- /dev/null
+++ b/lldb/test/API/commands/platform/launchgdbserver/main.c
@@ -0,0 +1 @@
+int main() { return 0; }