[lldb] [lldb-gdbserver] Unify listen/connect code to use ConnectionFileDescriptor

Unify the listen and connect code inside lldb-server to use
ConnectionFileDescriptor uniformly rather than a mix of it and Acceptor.
This involves:

- adding a function to map legacy values of host:port parameter
  (including legacy server URLs) into CFD-style URLs

- adding a callback to return "local socket id" (i.e. UNIX socket path
  or TCP port number) between listen() and accept() calls in CFD

- adding a "unix-abstract-accept" scheme to CFD

As an additional advantage, this permits lldb-server to accept any URL
known to CFD including the new serial:// scheme.  Effectively,
lldb-server can now listen on the serial port.  Tests for connecting
over a pty are added to test that.

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

lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp

@@ -13,10 +13,10 @@
 #define _DARWIN_UNLIMITED_SELECT
 #endif
 
-#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/SocketAddress.h"
+#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
 #include "lldb/Utility/SelectHelper.h"
 #include "lldb/Utility/Timeout.h"
 
@@ -62,7 +62,7 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit)
 
 ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
     : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
-      m_waiting_for_accept(false), m_child_processes_inherit(false) {
+      m_child_processes_inherit(false) {
   m_io_sp =
       std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, owns_fd);
 
@@ -77,7 +77,7 @@ ConnectionFileDescriptor::ConnectionFileDescriptor(int fd, bool owns_fd)
 
 ConnectionFileDescriptor::ConnectionFileDescriptor(Socket *socket)
     : Connection(), m_pipe(), m_mutex(), m_shutting_down(false),
-      m_waiting_for_accept(false), m_child_processes_inherit(false) {
+      m_child_processes_inherit(false) {
   InitializeSocket(socket);
 }
 
@@ -124,6 +124,13 @@ bool ConnectionFileDescriptor::IsConnected() const {
 
 ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
                                                    Status *error_ptr) {
+  return Connect(path, nullptr, error_ptr);
+}
+
+ConnectionStatus
+ConnectionFileDescriptor::Connect(llvm::StringRef path,
+                                  socket_id_callback_type socket_id_callback,
+                                  Status *error_ptr) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_CONNECTION));
   LLDB_LOGF(log, "%p ConnectionFileDescriptor::Connect (url = '%s')",
@@ -143,10 +150,12 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
   if (!path.empty()) {
     auto method =
         llvm::StringSwitch<ConnectionStatus (ConnectionFileDescriptor::*)(
-            llvm::StringRef, Status *)>(scheme)
+            llvm::StringRef, socket_id_callback_type, Status *)>(scheme)
             .Case("listen", &ConnectionFileDescriptor::SocketListenAndAccept)
             .Cases("accept", "unix-accept",
                    &ConnectionFileDescriptor::NamedSocketAccept)
+            .Case("unix-abstract-accept",
+                  &ConnectionFileDescriptor::UnixAbstractSocketAccept)
             .Cases("connect", "tcp-connect",
                    &ConnectionFileDescriptor::ConnectTCP)
             .Case("udp", &ConnectionFileDescriptor::ConnectUDP)
@@ -161,7 +170,7 @@ ConnectionStatus ConnectionFileDescriptor::Connect(llvm::StringRef path,
             .Default(nullptr);
 
     if (method)
-      return (this->*method)(path, error_ptr);
+      return (this->*method)(path, socket_id_callback, error_ptr);
   }
 
   if (error_ptr)
@@ -498,7 +507,8 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
           // data from that pipe:
           char c;
 
-          ssize_t bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1);
+          ssize_t bytes_read =
+              llvm::sys::RetryAfterSignal(-1, ::read, pipe_fd, &c, 1);
           assert(bytes_read == 1);
           (void)bytes_read;
           switch (c) {
@@ -522,24 +532,36 @@ ConnectionFileDescriptor::BytesAvailable(const Timeout<std::micro> &timeout,
   return eConnectionStatusLostConnection;
 }
 
-ConnectionStatus
-ConnectionFileDescriptor::NamedSocketAccept(llvm::StringRef socket_name,
-                                            Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::NamedSocketAccept(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
+  Status error;
+  std::unique_ptr<Socket> listen_socket = Socket::Create(
+      Socket::ProtocolUnixDomain, m_child_processes_inherit, error);
   Socket *socket = nullptr;
-  Status error =
-      Socket::UnixDomainAccept(socket_name, m_child_processes_inherit, socket);
+
+  if (!error.Fail())
+    error = listen_socket->Listen(socket_name, 5);
+
+  if (!error.Fail()) {
+    socket_id_callback(socket_name);
+    error = listen_socket->Accept(socket);
+  }
+
+  if (!error.Fail()) {
+    m_io_sp.reset(socket);
+    m_uri.assign(socket_name.str());
+    return eConnectionStatusSuccess;
+  }
+
   if (error_ptr)
     *error_ptr = error;
-  m_io_sp.reset(socket);
-  if (error.Fail())
-    return eConnectionStatusError;
-  m_uri.assign(std::string(socket_name));
-  return eConnectionStatusSuccess;
+  return eConnectionStatusError;
 }
 
-ConnectionStatus
-ConnectionFileDescriptor::NamedSocketConnect(llvm::StringRef socket_name,
-                                             Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::NamedSocketConnect(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
   Socket *socket = nullptr;
   Status error =
       Socket::UnixDomainConnect(socket_name, m_child_processes_inherit, socket);
@@ -552,9 +574,37 @@ ConnectionFileDescriptor::NamedSocketConnect(llvm::StringRef socket_name,
   return eConnectionStatusSuccess;
 }
 
-lldb::ConnectionStatus
-ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name,
-                                                    Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketAccept(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
+  Status error;
+  std::unique_ptr<Socket> listen_socket = Socket::Create(
+      Socket::ProtocolUnixAbstract, m_child_processes_inherit, error);
+  Socket *socket = nullptr;
+
+  if (!error.Fail())
+    error = listen_socket->Listen(socket_name, 5);
+
+  if (!error.Fail())
+    socket_id_callback(socket_name);
+
+  if (!error.Fail())
+    error = listen_socket->Accept(socket);
+
+  if (!error.Fail()) {
+    m_io_sp.reset(socket);
+    m_uri.assign(socket_name.str());
+    return eConnectionStatusSuccess;
+  }
+
+  if (error_ptr)
+    *error_ptr = error;
+  return eConnectionStatusError;
+}
+
+lldb::ConnectionStatus ConnectionFileDescriptor::UnixAbstractSocketConnect(
+    llvm::StringRef socket_name, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
   Socket *socket = nullptr;
   Status error = Socket::UnixAbstractConnect(socket_name,
                                              m_child_processes_inherit, socket);
@@ -567,14 +617,13 @@ ConnectionFileDescriptor::UnixAbstractSocketConnect(llvm::StringRef socket_name,
   return eConnectionStatusSuccess;
 }
 
-ConnectionStatus
-ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
-                                                Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::SocketListenAndAccept(
+    llvm::StringRef s, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
   if (error_ptr)
     *error_ptr = Status();
   m_port_predicate.SetValue(0, eBroadcastNever);
 
-  m_waiting_for_accept = true;
   llvm::Expected<std::unique_ptr<TCPSocket>> listening_socket =
       Socket::TcpListen(s, m_child_processes_inherit, &m_port_predicate);
   if (!listening_socket) {
@@ -586,6 +635,8 @@ ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
     return eConnectionStatusError;
   }
 
+  uint16_t port = listening_socket.get()->GetLocalPortNumber();
+  socket_id_callback(std::to_string(port));
 
   Socket *accepted_socket;
   Status error = listening_socket.get()->Accept(accepted_socket);
@@ -598,8 +649,10 @@ ConnectionFileDescriptor::SocketListenAndAccept(llvm::StringRef s,
   return eConnectionStatusSuccess;
 }
 
-ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
-                                                      Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
+                                     socket_id_callback_type socket_id_callback,
+                                     Status *error_ptr) {
   if (error_ptr)
     *error_ptr = Status();
 
@@ -618,8 +671,10 @@ ConnectionStatus ConnectionFileDescriptor::ConnectTCP(llvm::StringRef s,
   return eConnectionStatusSuccess;
 }
 
-ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
-                                                      Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
+                                     socket_id_callback_type socket_id_callback,
+                                     Status *error_ptr) {
   if (error_ptr)
     *error_ptr = Status();
   llvm::Expected<std::unique_ptr<UDPSocket>> socket =
@@ -637,8 +692,10 @@ ConnectionStatus ConnectionFileDescriptor::ConnectUDP(llvm::StringRef s,
   return eConnectionStatusSuccess;
 }
 
-ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
-                                                     Status *error_ptr) {
+ConnectionStatus
+ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
+                                    socket_id_callback_type socket_id_callback,
+                                    Status *error_ptr) {
 #if LLDB_ENABLE_POSIX
   // Just passing a native file descriptor within this current process that
   // is already opened (possibly from a service or other source).
@@ -691,8 +748,9 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s,
   llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
 }
 
-ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s,
-                                                       Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::ConnectFile(
+    llvm::StringRef s, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
 #if LLDB_ENABLE_POSIX
   std::string addr_str = s.str();
   // file:///PATH
@@ -722,16 +780,15 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFile(llvm::StringRef s,
     llvm::sys::RetryAfterSignal(-1, ::tcsetattr, fd, TCSANOW, &options);
   }
 
-  m_io_sp =
-      std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
+  m_io_sp = std::make_shared<NativeFile>(fd, File::eOpenOptionReadWrite, true);
   return eConnectionStatusSuccess;
 #endif // LLDB_ENABLE_POSIX
   llvm_unreachable("this function should be only called w/ LLDB_ENABLE_POSIX");
 }
 
-ConnectionStatus
-ConnectionFileDescriptor::ConnectSerialPort(llvm::StringRef s,
-                                            Status *error_ptr) {
+ConnectionStatus ConnectionFileDescriptor::ConnectSerialPort(
+    llvm::StringRef s, socket_id_callback_type socket_id_callback,
+    Status *error_ptr) {
 #if LLDB_ENABLE_POSIX
   llvm::StringRef path, qs;
   // serial:///PATH?k1=v1&k2=v2...

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

@@ -25,6 +25,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/PosixApi.h"
+#include "lldb/Host/Socket.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
@@ -39,6 +40,7 @@
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UnimplementedError.h"
+#include "lldb/Utility/UriParser.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -3888,3 +3890,42 @@ void GDBRemoteCommunicationServerLLGS::SetEnabledExtensions(
   assert(!bool(flags & ~m_process_factory.GetSupportedExtensions()));
   process.SetEnabledExtensions(flags);
 }
+
+std::string
+lldb_private::process_gdb_remote::LLGSArgToURL(llvm::StringRef url_arg,
+                                               bool reverse_connect) {
+  // Try parsing the argument as URL.
+  if (llvm::Optional<URI> url = URI::Parse(url_arg)) {
+    if (reverse_connect)
+      return url_arg.str();
+
+    // Translate the scheme from LLGS notation to ConnectionFileDescriptor.
+    // If the scheme doesn't match any, pass it through to support using CFD
+    // schemes directly.
+    std::string new_url = llvm::StringSwitch<std::string>(url->scheme)
+                              .Case("tcp", "listen")
+                              .Case("unix", "unix-accept")
+                              .Case("unix-abstract", "unix-abstract-accept")
+                              .Default(url->scheme.str());
+    llvm::append_range(new_url, url_arg.substr(url->scheme.size()));
+    return new_url;
+  }
+
+  std::string host_port = url_arg.str();
+  // If host_and_port starts with ':', default the host to be "localhost" and
+  // expect the remainder to be the port.
+  if (url_arg.startswith(":"))
+    host_port.insert(0, "localhost");
+
+  std::string host_str;
+  std::string port_str;
+  uint16_t port;
+  // Try parsing the (preprocessed) argument as host:port pair.
+  if (!llvm::errorToBool(
+          Socket::DecodeHostAndPort(host_port, host_str, port_str, port)))
+    return (reverse_connect ? "connect://" : "listen://") + host_port;
+
+  // If none of the above applied, interpret the argument as UNIX socket path.
+  return (reverse_connect ? "unix-connect://" : "unix-accept://") +
+         url_arg.str();
+}

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

@@ -289,6 +289,8 @@ private:
   operator=(const GDBRemoteCommunicationServerLLGS &) = delete;
 };
 
+std::string LLGSArgToURL(llvm::StringRef url_arg, bool reverse_connect);
+
 } // namespace process_gdb_remote
 } // namespace lldb_private