[lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run lldb-server gdbserver --fd on all platforms.
Refactored Acceptor to listen on 2 ports in the main thread.

Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.

Author: slydiman

lldb/docs/man/lldb-server.rst

@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port <port>
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port <port>
-.. option:: --max-gdbserver-port <port>
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset <offset>
 

lldb/docs/resources/qemu-testing.rst

@@ -149,30 +149,21 @@ to the host (refer to QEMU's manuals for the specific options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is almost
 certainly not forwarded. An example of this is shown below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
-    --min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.

lldb/include/lldb/Host/common/TCPSocket.h

@@ -24,6 +24,9 @@ class TCPSocket : public Socket {
   // returns port number or 0 if error
   uint16_t GetLocalPortNumber() const;
 
+  // returns port numbers of all listening sockets
+  std::set<uint16_t> GetLocalPortNumbers() const;
+
   // returns ip address string or empty string if error
   std::string GetLocalIPAddress() const;
 

lldb/source/Host/common/TCPSocket.cpp

@@ -17,6 +17,7 @@
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/WindowsError.h"
@@ -94,6 +95,25 @@ uint16_t TCPSocket::GetLocalPortNumber() const {
   return 0;
 }
 
+// Return all the port numbers that is being used by the socket.
+std::set<uint16_t> TCPSocket::GetLocalPortNumbers() const {
+  std::set<uint16_t> ports;
+  if (m_socket != kInvalidSocketValue) {
+    SocketAddress sock_addr;
+    socklen_t sock_addr_len = sock_addr.GetMaxLength();
+    if (::getsockname(m_socket, sock_addr, &sock_addr_len) == 0)
+      ports.insert(sock_addr.GetPort());
+  } else if (!m_listen_sockets.empty()) {
+    SocketAddress sock_addr;
+    socklen_t sock_addr_len = sock_addr.GetMaxLength();
+    for (auto listen_socket : m_listen_sockets) {
+      if (::getsockname(listen_socket.first, sock_addr, &sock_addr_len) == 0)
+        ports.insert(sock_addr.GetPort());
+    }
+  }
+  return ports;
+}
+
 std::string TCPSocket::GetLocalIPAddress() const {
   // We bound to port zero, so we need to figure out which port we actually
   // bound to
@@ -196,49 +216,66 @@ Status TCPSocket::Listen(llvm::StringRef name, int backlog) {
   if (!host_port)
     return Status(host_port.takeError());
 
+  llvm::SmallVector<uint16_t, 2> ports;
+  ports.push_back(host_port->port);
+
+  llvm::SmallVector<llvm::StringRef, 2> extra_ports;
+  name.split(extra_ports, ',', -1, false);
+  if (extra_ports.size() > 1) {
+    for (auto i = extra_ports.begin() + 1; i != extra_ports.end(); ++i) {
+      uint16_t port;
+      if (!llvm::to_integer(*i, port, 10))
+        return Status("invalid extra port number %s", i->str().c_str());
+      ports.push_back(port);
+    }
+  }
+
   if (host_port->hostname == "*")
     host_port->hostname = "0.0.0.0";
   std::vector<SocketAddress> addresses = SocketAddress::GetAddressInfo(
       host_port->hostname.c_str(), nullptr, AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP);
   for (SocketAddress &address : addresses) {
-    int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
-                                  m_child_processes_inherit, error);
-    if (error.Fail() || fd < 0)
-      continue;
-
-    // enable local address reuse
-    int option_value = 1;
-    set_socket_option_arg_type option_value_p =
-        reinterpret_cast<set_socket_option_arg_type>(&option_value);
-    if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
-                     sizeof(option_value)) == -1) {
-      CLOSE_SOCKET(fd);
-      continue;
-    }
-
-    SocketAddress listen_address = address;
-    if(!listen_address.IsLocalhost())
-      listen_address.SetToAnyAddress(address.GetFamily(), host_port->port);
-    else
-      listen_address.SetPort(host_port->port);
+    for (size_t i = 0; i < ports.size(); ++i) {
+      int fd = Socket::CreateSocket(address.GetFamily(), kType, IPPROTO_TCP,
+                                    m_child_processes_inherit, error);
+      if (error.Fail() || fd < 0)
+        continue;
+
+      // enable local address reuse
+      int option_value = 1;
+      set_socket_option_arg_type option_value_p =
+          reinterpret_cast<set_socket_option_arg_type>(&option_value);
+      if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, option_value_p,
+                       sizeof(option_value)) == -1) {
+        CLOSE_SOCKET(fd);
+        continue;
+      }
 
-    int err =
-        ::bind(fd, &listen_address.sockaddr(), listen_address.GetLength());
-    if (err != -1)
-      err = ::listen(fd, backlog);
+      SocketAddress listen_address = address;
+      if (!listen_address.IsLocalhost())
+        listen_address.SetToAnyAddress(address.GetFamily(), ports[i]);
+      else
+        listen_address.SetPort(ports[i]);
+
+      int err =
+          ::bind(fd, &listen_address.sockaddr(), listen_address.GetLength());
+      if (err != -1)
+        err = ::listen(fd, backlog);
+
+      if (err == -1) {
+        error = GetLastSocketError();
+        CLOSE_SOCKET(fd);
+        continue;
+      }
 
-    if (err == -1) {
-      error = GetLastSocketError();
-      CLOSE_SOCKET(fd);
-      continue;
-    }
+      if (ports[i] == 0) {
+        socklen_t sa_len = address.GetLength();
+        if (getsockname(fd, &address.sockaddr(), &sa_len) == 0)
+          ports[i] = address.GetPort();
+      }
 
-    if (host_port->port == 0) {
-      socklen_t sa_len = address.GetLength();
-      if (getsockname(fd, &address.sockaddr(), &sa_len) == 0)
-        host_port->port = address.GetPort();
+      m_listen_sockets[fd] = address;
     }
-    m_listen_sockets[fd] = address;
   }
 
   if (m_listen_sockets.empty()) {

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

@@ -162,8 +162,10 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path,
             .Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket)
             .Case("unix-abstract-connect",
                   &ConnectionFileDescriptor::ConnectAbstractSocket)
-#if LLDB_ENABLE_POSIX
+#if LLDB_ENABLE_POSIX || defined(_WIN32)
             .Case("fd", &ConnectionFileDescriptor::ConnectFD)
+#endif
+#if LLDB_ENABLE_POSIX
             .Case("file", &ConnectionFileDescriptor::ConnectFile)
             .Case("serial", &ConnectionFileDescriptor::ConnectSerialPort)
 #endif
@@ -666,7 +668,23 @@ ConnectionStatus
 ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
                                     socket_id_callback_type socket_id_callback,
                                     Status *error_ptr) {
-#if LLDB_ENABLE_POSIX
+#ifdef _WIN32
+  int64_t fd = -1;
+  if (!s.getAsInteger(0, fd)) {
+    // Assume we own fd.
+    std::unique_ptr<TCPSocket> tcp_socket =
+        std::make_unique<TCPSocket>((NativeSocket)fd, true, false);
+    m_io_sp = std::move(tcp_socket);
+    m_uri = s.str();
+    return eConnectionStatusSuccess;
+  }
+
+  if (error_ptr)
+    error_ptr->SetErrorStringWithFormat("invalid file descriptor: \"%s\"",
+                                        s.str().c_str());
+  m_io_sp.reset();
+  return eConnectionStatusError;
+#elif LLDB_ENABLE_POSIX
   // Just passing a native file descriptor within this current process that
   // is already opened (possibly from a service or other source).
   int fd = -1;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

@@ -879,20 +879,12 @@ lldb::thread_result_t GDBRemoteCommunication::ListenThread() {
   return {};
 }
 
-Status GDBRemoteCommunication::StartDebugserverProcess(
-    const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
-    uint16_t *port, const Args *inferior_args, int pass_comm_fd) {
+bool GDBRemoteCommunication::GetDebugserverPath(
+    Platform *platform, FileSpec &debugserver_file_spec) {
   Log *log = GetLog(GDBRLog::Process);
-  LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
-            __FUNCTION__, url ? url : "<empty>", port ? *port : uint16_t(0));
-
-  Status error;
   // If we locate debugserver, keep that located version around
   static FileSpec g_debugserver_file_spec;
 
-  char debugserver_path[PATH_MAX];
-  FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
-
   Environment host_env = Host::GetEnvironment();
 
   // Always check to see if we have an environment override for the path to the
@@ -943,8 +935,20 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
       }
     }
   }
+  return debugserver_exists;
+}
 
-  if (debugserver_exists) {
+Status GDBRemoteCommunication::StartDebugserverProcess(
+    const char *url, Platform *platform, ProcessLaunchInfo &launch_info,
+    uint16_t *port, const Args *inferior_args, shared_fd_t pass_comm_fd) {
+  Log *log = GetLog(GDBRLog::Process);
+  LLDB_LOGF(log, "GDBRemoteCommunication::%s(url=%s, port=%" PRIu16 ")",
+            __FUNCTION__, url ? url : "<empty>", port ? *port : uint16_t(0));
+
+  Status error;
+  FileSpec &debugserver_file_spec = launch_info.GetExecutableFile();
+  if (GetDebugserverPath(platform, debugserver_file_spec)) {
+    char debugserver_path[PATH_MAX];
     debugserver_file_spec.GetPath(debugserver_path, sizeof(debugserver_path));
 
     Args &debugserver_args = launch_info.GetArguments();
@@ -959,16 +963,17 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 #endif
 
     // If a url is supplied then use it
-    if (url)
+    if (url && url[0])
       debugserver_args.AppendArgument(llvm::StringRef(url));
 
-    if (pass_comm_fd >= 0) {
+    if (pass_comm_fd != SharedSocket::kInvalidFD) {
       StreamString fd_arg;
-      fd_arg.Printf("--fd=%i", pass_comm_fd);
+      fd_arg.Printf("--fd=%" PRIi64, (int64_t)pass_comm_fd);
       debugserver_args.AppendArgument(fd_arg.GetString());
       // Send "pass_comm_fd" down to the inferior so it can use it to
-      // communicate back with this process
-      launch_info.AppendDuplicateFileAction(pass_comm_fd, pass_comm_fd);
+      // communicate back with this process. Ignored on Windows.
+      launch_info.AppendDuplicateFileAction((int)pass_comm_fd,
+                                            (int)pass_comm_fd);
     }
 
     // use native registers, not the GDB registers
@@ -988,7 +993,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
     // port is null when debug server should listen on domain socket - we're
     // not interested in port value but rather waiting for debug server to
     // become available.
-    if (pass_comm_fd == -1) {
+    if (pass_comm_fd == SharedSocket::kInvalidFD) {
       if (url) {
 // Create a temporary file to get the stdout/stderr and redirect the output of
 // the command into this file. We will later read this file if all goes well
@@ -1059,6 +1064,8 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
         }
       }
     }
+
+    Environment host_env = Host::GetEnvironment();
     std::string env_debugserver_log_file =
         host_env.lookup("LLDB_DEBUGSERVER_LOG_FILE");
     if (!env_debugserver_log_file.empty()) {
@@ -1132,7 +1139,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
 
     if (error.Success() &&
         (launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) &&
-        pass_comm_fd == -1) {
+        pass_comm_fd == SharedSocket::kInvalidFD) {
       if (named_pipe_path.size() > 0) {
         error = socket_pipe.OpenAsReader(named_pipe_path, false);
         if (error.Fail())

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h

@@ -21,6 +21,7 @@
 #include "lldb/Core/Communication.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/HostThread.h"
+#include "lldb/Host/Socket.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Predicate.h"
@@ -146,15 +147,18 @@ class GDBRemoteCommunication : public Communication {
 
   std::chrono::seconds GetPacketTimeout() const { return m_packet_timeout; }
 
+  // Get the debugserver path and check that it exist.
+  bool GetDebugserverPath(Platform *platform, FileSpec &debugserver_file_spec);
+
   // Start a debugserver instance on the current host using the
   // supplied connection URL.
   Status StartDebugserverProcess(
       const char *url,
       Platform *platform, // If non nullptr, then check with the platform for
                           // the GDB server binary if it can't be located
       ProcessLaunchInfo &launch_info, uint16_t *port, const Args *inferior_args,
-      int pass_comm_fd); // Communication file descriptor to pass during
-                         // fork/exec to avoid having to connect/accept
+      shared_fd_t pass_comm_fd); // Communication file descriptor to pass during
+                                 // fork/exec to avoid having to connect/accept
 
   void DumpHistory(Stream &strm);