[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

[Dmitry Vasilyev via lldb-commits]

================
@@ -160,66 +95,56 @@ GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
     default;
 
 Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
-    const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid,
-    std::optional<uint16_t> &port, std::string &socket_name) {
-  if (!port) {
-    llvm::Expected<uint16_t> available_port = m_port_map.GetNextAvailablePort();
-    if (available_port)
-      port = *available_port;
-    else
-      return Status(available_port.takeError());
-  }
-
-  // Spawn a new thread to accept the port that gets bound after binding to
-  // port 0 (zero).
+    const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name,
+    shared_fd_t fd) {
+  std::ostringstream url;
+  if (fd == SharedSocket::kInvalidFD) {
+    if (m_socket_protocol == Socket::ProtocolTcp) {
+      // Just check that GDBServer exists. GDBServer must be launched after
+      // accepting the connection.
+      if (!GetDebugserverPath(nullptr))
+        return Status::FromErrorString("unable to locate debugserver");
+      return Status();
+    }
 
-  // ignore the hostname send from the remote end, just use the ip address that
-  // we're currently communicating with as the hostname
+    // debugserver does not accept the URL scheme prefix.
+#if !defined(__APPLE__)
+    url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://";
+#endif
+    socket_name = GetDomainSocketPath("gdbserver").GetPath();
+    url << socket_name;
+  } else {
+    if (m_socket_protocol != Socket::ProtocolTcp)
+      return Status::FromErrorString("protocol must be tcp");
+  }
 
   // Spawn a debugserver and try to get the port it listens to.
   ProcessLaunchInfo debugserver_launch_info;
-  if (hostname.empty())
-    hostname = "127.0.0.1";
-
   Log *log = GetLog(LLDBLog::Platform);
-  LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(),
-            *port);
+  LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd);
 
   // Do not run in a new session so that it can not linger after the platform
   // closes.
   debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
   debugserver_launch_info.SetMonitorProcessCallback(
-      std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
-                this, std::placeholders::_1));
-
-  std::ostringstream url;
-// debugserver does not accept the URL scheme prefix.
-#if !defined(__APPLE__)
-  url << m_socket_scheme << "://";
-#endif
-  uint16_t *port_ptr = &*port;
-  if (m_socket_protocol == Socket::ProtocolTcp) {
-    std::string platform_uri = GetConnection()->GetURI();
-    std::optional<URI> parsed_uri = URI::Parse(platform_uri);
-    url << '[' << parsed_uri->hostname.str() << "]:" << *port;
-  } else {
-    socket_name = GetDomainSocketPath("gdbserver").GetPath();
-    url << socket_name;
-    port_ptr = nullptr;
-  }
+      &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped);
 
   Status error = StartDebugserverProcess(
-      url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args, -1);
+      url.str().c_str(), nullptr, debugserver_launch_info, nullptr, &args, fd);
 
-  pid = debugserver_launch_info.GetProcessID();
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-    std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex);
-    m_spawned_pids.insert(pid);
-    if (*port > 0)
-      m_port_map.AssociatePortWithProcess(*port, pid);
+  if (error.Success()) {
+    pid = debugserver_launch_info.GetProcessID();
+    if (pid != LLDB_INVALID_PROCESS_ID)
+      AddSpawnedProcess(pid);
+    LLDB_LOGF(log,
+              "GDBRemoteCommunicationServerPlatform::%s() "
+              "debugserver launched successfully as pid %" PRIu64,
----------------
slydiman wrote:

Exactly. 
A non-error Status mean the process was launched. But don't add LLDB_INVALID_PROCESS_ID to the list. 
We can add assert() here but I don't like it.
```
  if (error.Success()) {
    pid = debugserver_launch_info.GetProcessID();
    if (pid != LLDB_INVALID_PROCESS_ID)
      AddSpawnedProcess(pid);
    LLDB_LOGF(log,
              "GDBRemoteCommunicationServerPlatform::%s() "
              "debugserver launched successfully as pid %" PRIu64,
              __FUNCTION__, pid);
  } else {
    LLDB_LOGF(log,
              "GDBRemoteCommunicationServerPlatform::%s() "
              "debugserver launch failed: %s",
              __FUNCTION__, error.AsCString());
  }
```

https://github.com/llvm/llvm-project/pull/104238