https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/104238
>From 8491aa073ae240eb5e31df12ac278c70fc9515e7 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Fri, 16 Aug 2024 17:03:37 +0400 Subject: [PATCH 1/4] [lldb] Removed gdbserver ports map from lldb-server Listen to gdbserver-port, accept the connection and run `lldb-server gdbserver --fd` on all platforms. Added acceptor_gdb and gdb_thread to lldb-platform.cpp SharedSocket has been moved to ConnectionFileDescriptorPosix. Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now. This is the part 2 of #101283. Fixes #97537, fixes #101475. --- lldb/docs/man/lldb-server.rst | 11 +- lldb/docs/resources/qemu-testing.rst | 19 +- .../posix/ConnectionFileDescriptorPosix.h | 26 ++ .../posix/ConnectionFileDescriptorPosix.cpp | 110 +++++- .../gdb-remote/GDBRemoteCommunication.cpp | 41 ++- .../gdb-remote/GDBRemoteCommunication.h | 8 +- .../GDBRemoteCommunicationServerPlatform.cpp | 287 +++++----------- .../GDBRemoteCommunicationServerPlatform.h | 83 +---- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../Shell/lldb-server/TestGdbserverPort.test | 4 - lldb/tools/lldb-server/lldb-gdbserver.cpp | 23 +- lldb/tools/lldb-server/lldb-platform.cpp | 324 ++++++++---------- .../Process/gdb-remote/CMakeLists.txt | 1 - .../Process/gdb-remote/PortMapTest.cpp | 115 ------- .../tools/lldb-server/tests/TestClient.h | 4 + 15 files changed, 450 insertions(+), 608 deletions(-) delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst index a67c00b305f6d2..31f5360df5e23e 100644 --- a/lldb/docs/man/lldb-server.rst +++ b/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> diff --git a/lldb/docs/resources/qemu-testing.rst b/lldb/docs/resources/qemu-testing.rst index 51a30b11717a87..e102f84a1d31f4 100644 --- a/lldb/docs/resources/qemu-testing.rst +++ b/lldb/docs/resources/qemu-testing.rst @@ -149,7 +149,6 @@ 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 @@ -157,22 +156,14 @@ 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. diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h index 35773d5907e913..08f486b92e0f07 100644 --- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h +++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h @@ -26,6 +26,32 @@ class Status; class Socket; class SocketAddress; +#ifdef _WIN32 +typedef lldb::pipe_t shared_fd_t; +#else +typedef NativeSocket shared_fd_t; +#endif + +class SharedSocket { +public: + static const shared_fd_t kInvalidFD; + + SharedSocket(Connection *conn, Status &error); + + shared_fd_t GetSendableFD() { return m_fd; } + + Status CompleteSending(lldb::pid_t child_pid); + + static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket); + +private: +#ifdef _WIN32 + Pipe m_socket_pipe; + NativeSocket m_socket; +#endif + shared_fd_t m_fd; +}; + class ConnectionFileDescriptor : public Connection { public: typedef llvm::function_ref<void(llvm::StringRef local_socket_id)> diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index fceeff08ed9d36..f90b01898aca7d 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -52,6 +52,94 @@ using namespace lldb; using namespace lldb_private; +#ifdef _WIN32 +const shared_fd_t SharedSocket::kInvalidFD = LLDB_INVALID_PIPE; +#else +const shared_fd_t SharedSocket::kInvalidFD = Socket::kInvalidSocketValue; +#endif + +SharedSocket::SharedSocket(Connection *conn, Status &error) { + m_fd = kInvalidFD; + + const Socket *socket = + static_cast<const Socket *>(conn->GetReadObject().get()); + if (socket == nullptr) { + error = Status("invalid conn socket"); + return; + } + +#ifdef _WIN32 + m_socket = socket->GetNativeSocket(); + + // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. + error = m_socket_pipe.CreateNew(true); + if (error.Fail()) + return; + + m_fd = m_socket_pipe.GetReadPipe(); +#else + m_fd = socket->GetNativeSocket(); + error = Status(); +#endif +} + +Status SharedSocket::CompleteSending(lldb::pid_t child_pid) { +#ifdef _WIN32 + // Transfer WSAPROTOCOL_INFO to the child process. + m_socket_pipe.CloseReadFileDescriptor(); + + WSAPROTOCOL_INFO protocol_info; + if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) == + SOCKET_ERROR) { + int last_error = ::WSAGetLastError(); + return Status("WSADuplicateSocket() failed, error: %d", last_error); + } + + size_t num_bytes; + Status error = + m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) + return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); +#endif + return Status(); +} + +Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) { +#ifdef _WIN32 + socket = Socket::kInvalidSocketValue; + // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. + WSAPROTOCOL_INFO protocol_info; + { + Pipe socket_pipe(fd, LLDB_INVALID_PIPE); + size_t num_bytes; + Status error = + socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) { + return Status( + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes", + num_bytes); + } + } + socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, + FROM_PROTOCOL_INFO, &protocol_info, 0, 0); + if (socket == INVALID_SOCKET) { + return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d", + ::WSAGetLastError()); + } + return Status(); +#else + socket = fd; + return Status(); +#endif +} + ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit) : Connection(), m_pipe(), m_mutex(), m_shutting_down(false), @@ -162,8 +250,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 +756,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; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 5d0a3e31d04374..075312016e0b22 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/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(); @@ -962,13 +966,14 @@ Status GDBRemoteCommunication::StartDebugserverProcess( if (url) 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()) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index 4e59bd5ec8be6b..bbff31de8aafd6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -20,6 +20,7 @@ #include "lldb/Core/Communication.h" #include "lldb/Host/Config.h" +#include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostThread.h" #include "lldb/Utility/Args.h" #include "lldb/Utility/Listener.h" @@ -146,6 +147,9 @@ 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( @@ -153,8 +157,8 @@ class GDBRemoteCommunication : public Communication { 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); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 65f1cc12ba307b..8230ecd713d771 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -44,79 +44,14 @@ using namespace lldb; using namespace lldb_private::process_gdb_remote; using namespace lldb_private; -GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port, - uint16_t max_port) { - assert(min_port); - for (; min_port < max_port; ++min_port) - m_port_map[min_port] = LLDB_INVALID_PROCESS_ID; -} - -void GDBRemoteCommunicationServerPlatform::PortMap::AllowPort(uint16_t port) { - assert(port); - // Do not modify existing mappings - m_port_map.insert({port, LLDB_INVALID_PROCESS_ID}); -} - -llvm::Expected<uint16_t> -GDBRemoteCommunicationServerPlatform::PortMap::GetNextAvailablePort() { - if (m_port_map.empty()) - return 0; // Bind to port zero and get a port, we didn't have any - // limitations - - for (auto &pair : m_port_map) { - if (pair.second == LLDB_INVALID_PROCESS_ID) { - pair.second = ~(lldb::pid_t)LLDB_INVALID_PROCESS_ID; - return pair.first; - } - } - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "No free port found in port map"); -} - -bool GDBRemoteCommunicationServerPlatform::PortMap::AssociatePortWithProcess( - uint16_t port, lldb::pid_t pid) { - auto pos = m_port_map.find(port); - if (pos != m_port_map.end()) { - pos->second = pid; - return true; - } - return false; -} - -bool GDBRemoteCommunicationServerPlatform::PortMap::FreePort(uint16_t port) { - std::map<uint16_t, lldb::pid_t>::iterator pos = m_port_map.find(port); - if (pos != m_port_map.end()) { - pos->second = LLDB_INVALID_PROCESS_ID; - return true; - } - return false; -} - -bool GDBRemoteCommunicationServerPlatform::PortMap::FreePortForProcess( - lldb::pid_t pid) { - if (!m_port_map.empty()) { - for (auto &pair : m_port_map) { - if (pair.second == pid) { - pair.second = LLDB_INVALID_PROCESS_ID; - return true; - } - } - } - return false; -} - -bool GDBRemoteCommunicationServerPlatform::PortMap::empty() const { - return m_port_map.empty(); -} +std::mutex GDBRemoteCommunicationServerPlatform::g_spawned_pids_mutex; +std::set<lldb::pid_t> GDBRemoteCommunicationServerPlatform::g_spawned_pids; // GDBRemoteCommunicationServerPlatform constructor GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( - const Socket::SocketProtocol socket_protocol, const char *socket_scheme) - : GDBRemoteCommunicationServerCommon(), - m_socket_protocol(socket_protocol), m_socket_scheme(socket_scheme), - m_spawned_pids_mutex(), m_port_map(), m_port_offset(0) { - m_pending_gdb_server.pid = LLDB_INVALID_PROCESS_ID; - m_pending_gdb_server.port = 0; + const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port) + : GDBRemoteCommunicationServerCommon(), m_socket_protocol(socket_protocol), + m_gdbserver_port(gdbserver_port), m_port_offset(0) { RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_qC, @@ -160,66 +95,58 @@ 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. + FileSpec debugserver_file_spec; + if (!GetDebugserverPath(nullptr, debugserver_file_spec)) + return Status("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("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(), + (int64_t)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, + __FUNCTION__, pid); } else { - if (*port > 0) - m_port_map.FreePort(*port); + LLDB_LOGF(log, + "GDBRemoteCommunicationServerPlatform::%s() " + "debugserver launch failed: %s", + __FUNCTION__, error.AsCString()); } return error; } @@ -250,27 +177,19 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer( } } + // Ignore client's hostname and the port. + lldb::pid_t debugserver_pid = LLDB_INVALID_PROCESS_ID; std::string socket_name; - Status error = - LaunchGDBServer(Args(), hostname, debugserver_pid, port, socket_name); - if (error.Fail()) { - LLDB_LOGF(log, - "GDBRemoteCommunicationServerPlatform::%s() debugserver " - "launch failed: %s", - __FUNCTION__, error.AsCString()); - return SendErrorResponse(9); - } - - LLDB_LOGF(log, - "GDBRemoteCommunicationServerPlatform::%s() debugserver " - "launched successfully as pid %" PRIu64, - __FUNCTION__, debugserver_pid); + Status error = LaunchGDBServer(Args(), debugserver_pid, socket_name, + SharedSocket::kInvalidFD); + if (error.Fail()) + return SendErrorResponse(9); // EBADF StreamGDBRemote response; - assert(port); + uint16_t gdbserver_port = socket_name.empty() ? m_gdbserver_port : 0; response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid, - *port + m_port_offset); + gdbserver_port + m_port_offset); if (!socket_name.empty()) { response.PutCString("socket_name:"); response.PutStringAsRawHex8(socket_name); @@ -290,13 +209,15 @@ GDBRemoteCommunicationServerPlatform::Handle_qQueryGDBServer( StringExtractorGDBRemote &packet) { namespace json = llvm::json; - if (m_pending_gdb_server.pid == LLDB_INVALID_PROCESS_ID) + if (!m_pending_gdb_server_socket_name) return SendErrorResponse(4); - json::Object server{{"port", m_pending_gdb_server.port}}; + json::Object server{{"port", m_pending_gdb_server_socket_name->empty() + ? m_gdbserver_port + : 0}}; - if (!m_pending_gdb_server.socket_name.empty()) - server.try_emplace("socket_name", m_pending_gdb_server.socket_name); + if (!m_pending_gdb_server_socket_name->empty()) + server.try_emplace("socket_name", *m_pending_gdb_server_socket_name); json::Array server_list; server_list.push_back(std::move(server)); @@ -317,28 +238,35 @@ GDBRemoteCommunicationServerPlatform::Handle_qKillSpawnedProcess( lldb::pid_t pid = packet.GetU64(LLDB_INVALID_PROCESS_ID); - // verify that we know anything about this pid. Scope for locker - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) { - // not a pid we know about - return SendErrorResponse(10); - } + // verify that we know anything about this pid. + if (SpawnedProcessFinished(pid)) { + // not a pid we know about + return SendErrorResponse(10); // ECHILD } // go ahead and attempt to kill the spawned process if (KillSpawnedProcess(pid)) return SendOKResponse(); else - return SendErrorResponse(11); + return SendErrorResponse(11); // EDEADLK +} + +void GDBRemoteCommunicationServerPlatform::AddSpawnedProcess(lldb::pid_t pid) { + std::lock_guard<std::mutex> guard(g_spawned_pids_mutex); + g_spawned_pids.insert(pid); +} + +bool GDBRemoteCommunicationServerPlatform::SpawnedProcessFinished( + lldb::pid_t pid) { + std::lock_guard<std::mutex> guard(g_spawned_pids_mutex); + return (g_spawned_pids.find(pid) == g_spawned_pids.end()); } bool GDBRemoteCommunicationServerPlatform::KillSpawnedProcess(lldb::pid_t pid) { // make sure we know about this process - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) - return false; + if (SpawnedProcessFinished(pid)) { + // it seems the process has been finished recently + return true; } // first try a SIGTERM (standard kill) @@ -346,46 +274,30 @@ bool GDBRemoteCommunicationServerPlatform::KillSpawnedProcess(lldb::pid_t pid) { // check if that worked for (size_t i = 0; i < 10; ++i) { - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) { - // it is now killed - return true; - } + if (SpawnedProcessFinished(pid)) { + // it is now killed + return true; } std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) - return true; - } + if (SpawnedProcessFinished(pid)) + return true; // the launched process still lives. Now try killing it again, this time // with an unblockable signal. Host::Kill(pid, SIGKILL); for (size_t i = 0; i < 10; ++i) { - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) { - // it is now killed - return true; - } + if (SpawnedProcessFinished(pid)) { + // it is now killed + return true; } std::this_thread::sleep_for(std::chrono::milliseconds(10)); } // check one more time after the final sleep - { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - if (m_spawned_pids.find(pid) == m_spawned_pids.end()) - return true; - } - - // no luck - the process still lives - return false; + return SpawnedProcessFinished(pid); } GDBRemoteCommunication::PacketResult @@ -519,10 +431,9 @@ GDBRemoteCommunicationServerPlatform::Handle_jSignalsInfo( } void GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped( - lldb::pid_t pid) { - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - m_port_map.FreePortForProcess(pid); - m_spawned_pids.erase(pid); + lldb::pid_t pid, int signal, int status) { + std::lock_guard<std::mutex> guard(g_spawned_pids_mutex); + g_spawned_pids.erase(pid); } Status GDBRemoteCommunicationServerPlatform::LaunchProcess() { @@ -533,9 +444,8 @@ Status GDBRemoteCommunicationServerPlatform::LaunchProcess() { // specify the process monitor if not already set. This should generally be // what happens since we need to reap started processes. if (!m_process_launch_info.GetMonitorProcessCallback()) - m_process_launch_info.SetMonitorProcessCallback(std::bind( - &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, this, - std::placeholders::_1)); + m_process_launch_info.SetMonitorProcessCallback( + &GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped); Status error = Host::LaunchProcess(m_process_launch_info); if (!error.Success()) { @@ -551,19 +461,12 @@ Status GDBRemoteCommunicationServerPlatform::LaunchProcess() { // add to list of spawned processes. On an lldb-gdbserver, we would expect // there to be only one. const auto pid = m_process_launch_info.GetProcessID(); - if (pid != LLDB_INVALID_PROCESS_ID) { - // add to spawned pids - std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); - m_spawned_pids.insert(pid); - } + if (pid != LLDB_INVALID_PROCESS_ID) + AddSpawnedProcess(pid); return error; } -void GDBRemoteCommunicationServerPlatform::SetPortMap(PortMap &&port_map) { - m_port_map = std::move(port_map); -} - const FileSpec &GDBRemoteCommunicationServerPlatform::GetDomainSocketDir() { static FileSpec g_domainsocket_dir; static llvm::once_flag g_once_flag; @@ -599,8 +502,6 @@ void GDBRemoteCommunicationServerPlatform::SetPortOffset(uint16_t port_offset) { } void GDBRemoteCommunicationServerPlatform::SetPendingGdbServer( - lldb::pid_t pid, uint16_t port, const std::string &socket_name) { - m_pending_gdb_server.pid = pid; - m_pending_gdb_server.port = port; - m_pending_gdb_server.socket_name = socket_name; + const std::string &socket_name) { + m_pending_gdb_server_socket_name = socket_name; } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h index 1853025466cf04..70d7425cf6097e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -15,6 +15,7 @@ #include <set> #include "GDBRemoteCommunicationServerCommon.h" +#include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/Socket.h" #include "llvm/Support/Error.h" @@ -25,90 +26,30 @@ namespace process_gdb_remote { class GDBRemoteCommunicationServerPlatform : public GDBRemoteCommunicationServerCommon { public: - class PortMap { - public: - // This class is used to restrict the range of ports that - // platform created debugserver/gdbserver processes will - // communicate on. - - // Construct an empty map, where empty means any port is allowed. - PortMap() = default; - - // Make a port map with a range of free ports - // from min_port to max_port-1. - PortMap(uint16_t min_port, uint16_t max_port); - - // Add a port to the map. If it is already in the map do not modify - // its mapping. (used ports remain used, new ports start as free) - void AllowPort(uint16_t port); - - // If we are using a port map where we can only use certain ports, - // get the next available port. - // - // If we are using a port map and we are out of ports, return an error. - // - // If we aren't using a port map, return 0 to indicate we should bind to - // port 0 and then figure out which port we used. - llvm::Expected<uint16_t> GetNextAvailablePort(); - - // Tie a port to a process ID. Returns false if the port is not in the port - // map. If the port is already in use it will be moved to the given pid. - // FIXME: This is and GetNextAvailablePort make create a race condition if - // the portmap is shared between processes. - bool AssociatePortWithProcess(uint16_t port, lldb::pid_t pid); - - // Free the given port. Returns false if the port is not in the map. - bool FreePort(uint16_t port); - - // Free the port associated with the given pid. Returns false if there is - // no port associated with the pid. - bool FreePortForProcess(lldb::pid_t pid); - - // Returns true if there are no ports in the map, regardless of the state - // of those ports. Meaning a map with 1 used port is not empty. - bool empty() const; - - private: - std::map<uint16_t, lldb::pid_t> m_port_map; - }; - GDBRemoteCommunicationServerPlatform( - const Socket::SocketProtocol socket_protocol, const char *socket_scheme); + const Socket::SocketProtocol socket_protocol, uint16_t gdbserver_port); ~GDBRemoteCommunicationServerPlatform() override; Status LaunchProcess() override; - // Set both ports to zero to let the platform automatically bind to - // a port chosen by the OS. - void SetPortMap(PortMap &&port_map); - void SetPortOffset(uint16_t port_offset); void SetInferiorArguments(const lldb_private::Args &args); - // Set port if you want to use a specific port number. - // Otherwise port will be set to the port that was chosen for you. - Status LaunchGDBServer(const lldb_private::Args &args, std::string hostname, - lldb::pid_t &pid, std::optional<uint16_t> &port, - std::string &socket_name); + Status LaunchGDBServer(const lldb_private::Args &args, lldb::pid_t &pid, + std::string &socket_name, shared_fd_t fd); - void SetPendingGdbServer(lldb::pid_t pid, uint16_t port, - const std::string &socket_name); + void SetPendingGdbServer(const std::string &socket_name); protected: const Socket::SocketProtocol m_socket_protocol; - const std::string m_socket_scheme; - std::recursive_mutex m_spawned_pids_mutex; - std::set<lldb::pid_t> m_spawned_pids; + static std::mutex g_spawned_pids_mutex; + static std::set<lldb::pid_t> g_spawned_pids; - PortMap m_port_map; + uint16_t m_gdbserver_port; uint16_t m_port_offset; - struct { - lldb::pid_t pid; - uint16_t port; - std::string socket_name; - } m_pending_gdb_server; + std::optional<std::string> m_pending_gdb_server_socket_name; PacketResult Handle_qLaunchGDBServer(StringExtractorGDBRemote &packet); @@ -129,9 +70,11 @@ class GDBRemoteCommunicationServerPlatform PacketResult Handle_jSignalsInfo(StringExtractorGDBRemote &packet); private: - bool KillSpawnedProcess(lldb::pid_t pid); + static bool KillSpawnedProcess(lldb::pid_t pid); + static bool SpawnedProcessFinished(lldb::pid_t pid); + static void AddSpawnedProcess(lldb::pid_t pid); - void DebugserverProcessReaped(lldb::pid_t pid); + static void DebugserverProcessReaped(lldb::pid_t pid, int signal, int status); static const FileSpec &GetDomainSocketDir(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index c7ce368ab41ce2..702aeccecc4702 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3433,7 +3433,7 @@ Status ProcessGDBRemote::LaunchAndConnectToDebugserver( } #endif - int communication_fd = -1; + shared_fd_t communication_fd = SharedSocket::kInvalidFD; #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION // Use a socketpair on non-Windows systems for security and performance // reasons. diff --git a/lldb/test/Shell/lldb-server/TestGdbserverPort.test b/lldb/test/Shell/lldb-server/TestGdbserverPort.test deleted file mode 100644 index 04facfec831cae..00000000000000 --- a/lldb/test/Shell/lldb-server/TestGdbserverPort.test +++ /dev/null @@ -1,4 +0,0 @@ -# Windows does not build lldb-server. -# UNSUPPORTED: system-windows -# RUN: %platformserver --server --listen :1234 --min-gdbserver-port 1234 --max-gdbserver-port 1234 2>&1 | FileCheck %s -# CHECK: error: --min-gdbserver-port (1234) is not lower than --max-gdbserver-port (1234) diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index 563284730bc705..3c45350c343bb6 100644 --- a/lldb/tools/lldb-server/lldb-gdbserver.cpp +++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -195,14 +195,21 @@ void ConnectToRemote(MainLoop &mainloop, bool reverse_connect, llvm::StringRef host_and_port, const char *const progname, const char *const subcommand, const char *const named_pipe_path, pipe_t unnamed_pipe, - int connection_fd) { + shared_fd_t connection_fd) { Status error; std::unique_ptr<Connection> connection_up; std::string url; - if (connection_fd != -1) { - url = llvm::formatv("fd://{0}", connection_fd).str(); + if (connection_fd != SharedSocket::kInvalidFD) { + NativeSocket fd; + Status error = SharedSocket::GetNativeSocket(connection_fd, fd); + if (error.Fail()) { + llvm::errs() << llvm::formatv("error: GetNativeSocket failed: {0}\n", + error.AsCString()); + exit(-1); + } + url = llvm::formatv("fd://{0}", (int64_t)fd).str(); // Create the connection. #if LLDB_ENABLE_POSIX && !defined _WIN32 @@ -338,7 +345,7 @@ int main_gdbserver(int argc, char *argv[]) { log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" lldb::pipe_t unnamed_pipe = LLDB_INVALID_PIPE; bool reverse_connect = false; - int connection_fd = -1; + shared_fd_t connection_fd = SharedSocket::kInvalidFD; // ProcessLaunchInfo launch_info; ProcessAttachInfo attach_info; @@ -404,10 +411,12 @@ int main_gdbserver(int argc, char *argv[]) { unnamed_pipe = (pipe_t)Arg; } if (Args.hasArg(OPT_fd)) { - if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), connection_fd)) { + int64_t fd; + if (!llvm::to_integer(Args.getLastArgValue(OPT_fd), fd)) { WithColor::error() << "invalid '--fd' argument\n" << HelpText; return 1; } + connection_fd = (shared_fd_t)fd; } if (!LLDBServerUtilities::SetupLogging( @@ -423,7 +432,7 @@ int main_gdbserver(int argc, char *argv[]) { for (const char *Val : Arg->getValues()) Inputs.push_back(Val); } - if (Inputs.empty() && connection_fd == -1) { + if (Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) { WithColor::error() << "no connection arguments\n" << HelpText; return 1; } @@ -432,7 +441,7 @@ int main_gdbserver(int argc, char *argv[]) { GDBRemoteCommunicationServerLLGS gdb_server(mainloop, manager); llvm::StringRef host_and_port; - if (!Inputs.empty()) { + if (!Inputs.empty() && connection_fd == SharedSocket::kInvalidFD) { host_and_port = Inputs.front(); Inputs.erase(Inputs.begin()); } diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 82a3a0d6b4e51c..da98e9ea805781 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -34,6 +34,7 @@ #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" #include "lldb/Host/Socket.h" +#include "lldb/Host/ThreadLauncher.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/LLDBLog.h" @@ -47,111 +48,10 @@ using namespace llvm; // option descriptors for getopt_long_only() -#ifdef _WIN32 -typedef pipe_t shared_fd_t; -const shared_fd_t kInvalidSharedFD = LLDB_INVALID_PIPE; -#else -typedef NativeSocket shared_fd_t; -const shared_fd_t kInvalidSharedFD = Socket::kInvalidSocketValue; -#endif - -class SharedSocket { -public: - SharedSocket(Connection *conn, Status &error) { - m_fd = kInvalidSharedFD; - - const Socket *socket = - static_cast<const Socket *>(conn->GetReadObject().get()); - if (socket == nullptr) { - error = Status("invalid conn socket"); - return; - } - -#ifdef _WIN32 - m_socket = socket->GetNativeSocket(); - - // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. - error = m_socket_pipe.CreateNew(true); - if (error.Fail()) - return; - - m_fd = m_socket_pipe.GetReadPipe(); -#else - m_fd = socket->GetNativeSocket(); - error = Status(); -#endif - } - - shared_fd_t GetSendableFD() { return m_fd; } - - Status CompleteSending(lldb::pid_t child_pid) { -#ifdef _WIN32 - // Transfer WSAPROTOCOL_INFO to the child process. - m_socket_pipe.CloseReadFileDescriptor(); - - WSAPROTOCOL_INFO protocol_info; - if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) == - SOCKET_ERROR) { - int last_error = ::WSAGetLastError(); - return Status("WSADuplicateSocket() failed, error: %d", last_error); - } - - size_t num_bytes; - Status error = - m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(10), num_bytes); - if (error.Fail()) - return error; - if (num_bytes != sizeof(protocol_info)) - return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); -#endif - return Status(); - } - - static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket) { -#ifdef _WIN32 - socket = Socket::kInvalidSocketValue; - // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. - WSAPROTOCOL_INFO protocol_info; - { - Pipe socket_pipe(fd, LLDB_INVALID_PIPE); - size_t num_bytes; - Status error = - socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(10), num_bytes); - if (error.Fail()) - return error; - if (num_bytes != sizeof(protocol_info)) { - return Status( - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes", - num_bytes); - } - } - socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, - FROM_PROTOCOL_INFO, &protocol_info, 0, 0); - if (socket == INVALID_SOCKET) { - return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d", - ::WSAGetLastError()); - } - return Status(); -#else - socket = fd; - return Status(); -#endif - } - -private: -#ifdef _WIN32 - Pipe m_socket_pipe; - NativeSocket m_socket; -#endif - shared_fd_t m_fd; -}; - static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; +static volatile bool g_listen_gdb = true; static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, @@ -227,13 +127,11 @@ static void client_handle(GDBRemoteCommunicationServerPlatform &platform, if (args.GetArgumentCount() > 0) { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - std::optional<uint16_t> port; std::string socket_name; - Status error = platform.LaunchGDBServer(args, - "", // hostname - pid, port, socket_name); + Status error = platform.LaunchGDBServer(args, pid, socket_name, + SharedSocket::kInvalidFD); if (error.Success()) - platform.SetPendingGdbServer(pid, *port, socket_name); + platform.SetPendingGdbServer(socket_name); else fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); } @@ -251,13 +149,7 @@ static void client_handle(GDBRemoteCommunicationServerPlatform &platform, printf("Disconnected.\n"); } -static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; -static std::mutex gdbserver_portmap_mutex; - -static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.FreePortForProcess(pid); -} +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {} static Status spawn_process(const char *progname, Connection *conn, uint16_t gdb_port, uint16_t port_offset, @@ -277,6 +169,11 @@ static Status spawn_process(const char *progname, Connection *conn, self_args.AppendArgument(llvm::StringRef("platform")); self_args.AppendArgument(llvm::StringRef("--child-platform-fd")); self_args.AppendArgument(llvm::to_string(shared_socket.GetSendableFD())); + + // Ignored in Windows + launch_info.AppendDuplicateFileAction((int)shared_socket.GetSendableFD(), + (int)shared_socket.GetSendableFD()); + if (gdb_port) { self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); self_args.AppendArgument(llvm::to_string(gdb_port)); @@ -330,11 +227,6 @@ static Status spawn_process(const char *progname, Connection *conn, LLDB_LOG(GetLog(LLDBLog::Platform), "lldb-platform launched '{0}', pid={1}", cmd, child_pid); - { - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.AssociatePortWithProcess(gdb_port, child_pid); - } - error = shared_socket.CompleteSending(child_pid); if (error.Fail()) { Host::Kill(child_pid, SIGTERM); @@ -344,6 +236,53 @@ static Status spawn_process(const char *progname, Connection *conn, return Status(); } +static thread_result_t +gdb_thread_proc(Acceptor *acceptor_gdb, uint16_t gdbserver_port, + const lldb_private::Args &inferior_arguments) { + lldb_private::Args args(inferior_arguments); + + Log *log = GetLog(LLDBLog::Platform); + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, + gdbserver_port); + while (g_listen_gdb) { + Connection *conn = nullptr; + Status error = + acceptor_gdb->Accept(/*children_inherit_accept_socket=*/false, conn); + if (error.Fail()) { + WithColor::error() << error.AsCString() << '\n'; + break; + } + printf("gdb connection established.\n"); + + { + SharedSocket shared_socket(conn, error); + if (error.Success()) { + lldb::pid_t child_pid = LLDB_INVALID_PROCESS_ID; + std::string socket_name; + error = platform.LaunchGDBServer(args, child_pid, socket_name, + shared_socket.GetSendableFD()); + if (error.Fail()) { + LLDB_LOGF(log, "gdbserver LaunchGDBServer failed: %s", + error.AsCString()); + } else if (child_pid != LLDB_INVALID_PROCESS_ID) { + error = shared_socket.CompleteSending(child_pid); + if (error.Success()) { + // Use inferior arguments once. + args.Clear(); + } else { + Host::Kill(child_pid, SIGTERM); + LLDB_LOGF(log, "gdbserver CompleteSending failed: %s", + error.AsCString()); + } + } + } else + LLDB_LOGF(log, "gdbserver SharedSocket failed: %s", error.AsCString()); + } + delete conn; + } + return {}; +} + // main int main_platform(int argc, char *argv[]) { const char *progname = argv[0]; @@ -363,10 +302,9 @@ int main_platform(int argc, char *argv[]) { StringRef log_channels; // e.g. "lldb process threads:gdb-remote default:linux all" - shared_fd_t fd = kInvalidSharedFD; + shared_fd_t fd = SharedSocket::kInvalidFD; - int min_gdbserver_port = 0; - int max_gdbserver_port = 0; + uint16_t gdbserver_port = 0; uint16_t port_offset = 0; FileSpec socket_file; @@ -441,11 +379,9 @@ int main_platform(int argc, char *argv[]) { break; } if (ch == 'P') - gdbserver_portmap.AllowPort(portnum); - else if (ch == 'm') - min_gdbserver_port = portnum; - else - max_gdbserver_port = portnum; + gdbserver_port = portnum; + else if (gdbserver_port == 0) + gdbserver_port = portnum; } break; case 2: { @@ -467,20 +403,8 @@ int main_platform(int argc, char *argv[]) { if (!LLDBServerUtilities::SetupLogging(log_file, log_channels, 0)) return -1; - // Make a port map for a port range that was specified. - if (min_gdbserver_port && min_gdbserver_port < max_gdbserver_port) { - gdbserver_portmap = GDBRemoteCommunicationServerPlatform::PortMap( - min_gdbserver_port, max_gdbserver_port); - } else if (min_gdbserver_port || max_gdbserver_port) { - WithColor::error() << llvm::formatv( - "--min-gdbserver-port ({0}) is not lower than " - "--max-gdbserver-port ({1})\n", - min_gdbserver_port, max_gdbserver_port); - option_error = 3; - } - // Print usage and exit if no listening port is specified. - if (listen_host_port.empty() && fd == kInvalidSharedFD) + if (listen_host_port.empty() && fd == SharedSocket::kInvalidFD) show_usage = true; if (show_usage || option_error) { @@ -494,7 +418,7 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); - if (fd != kInvalidSharedFD) { + if (fd != SharedSocket::kInvalidFD) { // Child process will handle the connection and exit. Log *log = GetLog(LLDBLog::Platform); if (!listen_host_port.empty()) { @@ -503,6 +427,12 @@ int main_platform(int argc, char *argv[]) { return socket_error; } + if (gdbserver_port == 0) { + LLDB_LOGF(log, "lldb-platform child: " + "--gdbserver-port is missing."); + return socket_error; + } + NativeSocket socket; error = SharedSocket::GetNativeSocket(fd, socket); if (error.Fail()) { @@ -512,10 +442,10 @@ int main_platform(int argc, char *argv[]) { Connection *conn = new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); - GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp"); + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, + gdbserver_port); if (port_offset > 0) platform.SetPortOffset(port_offset); - platform.SetPortMap(std::move(gdbserver_portmap)); platform.SetConnection(std::unique_ptr<Connection>(conn)); client_handle(platform, inferior_arguments); return 0; @@ -534,6 +464,64 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } + if (g_server && acceptor_up->GetSocketProtocol() != Socket::ProtocolTcp) { + fprintf(stderr, + "ambiguous parameters --server --listen %s\n" + "The protocol must be tcp for the server mode.", + listen_host_port.c_str()); + exit(socket_error); + } + + std::unique_ptr<Acceptor> acceptor_gdb; + HostThread gdb_thread; + if (acceptor_up->GetSocketProtocol() == Socket::ProtocolTcp) { + // Use the same host from listen_host_port for acceptor_gdb. + std::size_t port_pos = listen_host_port.rfind(":"); + if (port_pos == std::string::npos) + port_pos = 0; + else + ++port_pos; + std::string listen_gdb_port = llvm::formatv( + "{0}{1}", listen_host_port.substr(0, port_pos), gdbserver_port); + acceptor_gdb = Acceptor::Create(listen_gdb_port, + children_inherit_listen_socket, error); + if (error.Fail()) { + fprintf(stderr, "failed to create gdb acceptor: %s", error.AsCString()); + exit(socket_error); + } + + error = acceptor_gdb->Listen(backlog); + if (error.Fail()) { + printf("failed to listen gdb: %s\n", error.AsCString()); + exit(socket_error); + } + + if (gdbserver_port == 0) { + std::string str_gdbserver_port = acceptor_gdb->GetLocalSocketId(); + if (!llvm::to_integer(str_gdbserver_port, gdbserver_port)) { + printf("invalid gdb port: %s\n", str_gdbserver_port.c_str()); + exit(socket_error); + } + Log *log = GetLog(LLDBLog::Connection); + LLDB_LOG(log, "Listen to gdb port {0}", gdbserver_port); + } + assert(gdbserver_port); + + const std::string thread_name = llvm::formatv("gdb:{0}", gdbserver_port); + Acceptor *ptr_acceptor_gdb = acceptor_gdb.get(); + auto maybe_thread = ThreadLauncher::LaunchThread( + thread_name, [ptr_acceptor_gdb, gdbserver_port, inferior_arguments] { + return gdb_thread_proc(ptr_acceptor_gdb, gdbserver_port, + inferior_arguments); + }); + if (!maybe_thread) { + WithColor::error() << "failed to start gdb listen thread: " + << maybe_thread.takeError() << '\n'; + exit(socket_error); + } + gdb_thread = *maybe_thread; + } + error = acceptor_up->Listen(backlog); if (error.Fail()) { printf("failed to listen: %s\n", error.AsCString()); @@ -550,7 +538,7 @@ int main_platform(int argc, char *argv[]) { } GDBRemoteCommunicationServerPlatform platform( - acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); + acceptor_up->GetSocketProtocol(), gdbserver_port); if (port_offset > 0) platform.SetPortOffset(port_offset); @@ -565,32 +553,13 @@ int main_platform(int argc, char *argv[]) { printf("Connection established.\n"); if (g_server) { - std::optional<uint16_t> available_port; - { - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - auto port = gdbserver_portmap.GetNextAvailablePort(); - if (port) - available_port = *port; - else - llvm::consumeError(port.takeError()); - } - if (!available_port) { - fprintf(stderr, - "no available gdbserver port for connection - dropping...\n"); - } else { - error = spawn_process(progname, conn, *available_port, port_offset, - inferior_arguments, log_file, log_channels); - if (error.Fail()) { - { - - std::lock_guard<std::mutex> guard(gdbserver_portmap_mutex); - gdbserver_portmap.FreePort(*available_port); - } - LLDB_LOGF(GetLog(LLDBLog::Platform), "spawn_process failed: %s", - error.AsCString()); - WithColor::error() - << "spawn_process failed: " << error.AsCString() << "\n"; - } + error = spawn_process(progname, conn, gdbserver_port, port_offset, + inferior_arguments, log_file, log_channels); + if (error.Fail()) { + LLDB_LOGF(GetLog(LLDBLog::Platform), "spawn_process failed: %s", + error.AsCString()); + WithColor::error() << "spawn_process failed: " << error.AsCString() + << "\n"; } // Parent doesn't need a connection to the lldb client delete conn; @@ -601,15 +570,24 @@ int main_platform(int argc, char *argv[]) { // If not running as a server, this process will not accept // connections while a connection is active. acceptor_up.reset(); - - // When not running in server mode, use all available ports - platform.SetPortMap(std::move(gdbserver_portmap)); } platform.SetConnection(std::unique_ptr<Connection>(conn)); client_handle(platform, inferior_arguments); } while (g_server); + // FIXME: Make TCPSocket::CloseListenSockets() public and implement + // Acceptor::Close(). + /* + if (acceptor_gdb && gdb_thread.IsJoinable()) { + g_listen_gdb = false; + static_cast<TCPSocket *>(acceptor_gdb->m_listener_socket_up.get()) + ->CloseListenSockets(); + lldb::thread_result_t result; + gdb_thread.Join(&result); + } + */ + fprintf(stderr, "lldb-server exiting...\n"); return 0; diff --git a/lldb/unittests/Process/gdb-remote/CMakeLists.txt b/lldb/unittests/Process/gdb-remote/CMakeLists.txt index de14dc0169c13f..41bce6f31eafe2 100644 --- a/lldb/unittests/Process/gdb-remote/CMakeLists.txt +++ b/lldb/unittests/Process/gdb-remote/CMakeLists.txt @@ -5,7 +5,6 @@ add_lldb_unittest(ProcessGdbRemoteTests GDBRemoteCommunicationServerTest.cpp GDBRemoteCommunicationTest.cpp GDBRemoteTestUtils.cpp - PortMapTest.cpp LINK_LIBS lldbCore diff --git a/lldb/unittests/Process/gdb-remote/PortMapTest.cpp b/lldb/unittests/Process/gdb-remote/PortMapTest.cpp deleted file mode 100644 index 8671ebe0fd089b..00000000000000 --- a/lldb/unittests/Process/gdb-remote/PortMapTest.cpp +++ /dev/null @@ -1,115 +0,0 @@ -//===-- PortMapTest.cpp ---------------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "llvm/Testing/Support/Error.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h" - -using namespace lldb_private::process_gdb_remote; - -TEST(PortMapTest, Constructors) { - // Default construct to empty map - GDBRemoteCommunicationServerPlatform::PortMap p1; - ASSERT_TRUE(p1.empty()); - - // Empty means no restrictions, return 0 and bind to get a port - llvm::Expected<uint16_t> available_port = p1.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(0)); - - // Adding any port makes it not empty - p1.AllowPort(1); - ASSERT_FALSE(p1.empty()); - - // So we will return the added port this time - available_port = p1.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(1)); - - // Construct from a range of ports - GDBRemoteCommunicationServerPlatform::PortMap p2(1, 4); - ASSERT_FALSE(p2.empty()); - - // Use up all the ports - for (uint16_t expected = 1; expected < 4; ++expected) { - available_port = p2.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(expected)); - p2.AssociatePortWithProcess(*available_port, 1); - } - - // Now we fail since we're not an empty port map but all ports are used - available_port = p2.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::Failed()); -} - -TEST(PortMapTest, FreePort) { - GDBRemoteCommunicationServerPlatform::PortMap p(1, 4); - // Use up all the ports - for (uint16_t port = 1; port < 4; ++port) { - p.AssociatePortWithProcess(port, 1); - } - - llvm::Expected<uint16_t> available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::Failed()); - - // Can't free a port that isn't in the map - ASSERT_FALSE(p.FreePort(0)); - ASSERT_FALSE(p.FreePort(4)); - - // After freeing a port it becomes available - ASSERT_TRUE(p.FreePort(2)); - available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(2)); -} - -TEST(PortMapTest, FreePortForProcess) { - GDBRemoteCommunicationServerPlatform::PortMap p; - p.AllowPort(1); - p.AllowPort(2); - ASSERT_TRUE(p.AssociatePortWithProcess(1, 11)); - ASSERT_TRUE(p.AssociatePortWithProcess(2, 22)); - - // All ports have been used - llvm::Expected<uint16_t> available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::Failed()); - - // Can't free a port for a process that doesn't have any - ASSERT_FALSE(p.FreePortForProcess(33)); - - // You can move a used port to a new pid - ASSERT_TRUE(p.AssociatePortWithProcess(1, 99)); - - ASSERT_TRUE(p.FreePortForProcess(22)); - available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::Succeeded()); - ASSERT_EQ(2, *available_port); - - // proces 22 no longer has a port - ASSERT_FALSE(p.FreePortForProcess(22)); -} - -TEST(PortMapTest, AllowPort) { - GDBRemoteCommunicationServerPlatform::PortMap p; - - // Allow port 1 and tie it to process 11 - p.AllowPort(1); - ASSERT_TRUE(p.AssociatePortWithProcess(1, 11)); - - // Allowing it a second time shouldn't change existing mapping - p.AllowPort(1); - llvm::Expected<uint16_t> available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::Failed()); - - // A new port is marked as free when allowed - p.AllowPort(2); - available_port = p.GetNextAvailablePort(); - ASSERT_THAT_EXPECTED(available_port, llvm::HasValue(2)); - - // 11 should still be tied to port 1 - ASSERT_TRUE(p.FreePortForProcess(11)); -} diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.h b/lldb/unittests/tools/lldb-server/tests/TestClient.h index deb6802d0da707..5d1eacaf6198e8 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.h +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.h @@ -20,6 +20,10 @@ #include <optional> #include <string> +#ifdef SendMessage +#undef SendMessage +#endif + #if LLDB_SERVER_IS_DEBUGSERVER #define LLGS_TEST(x) DISABLED_ ## x #define DS_TEST(x) x >From 882f1494eede8fe2ad96be3d35a58b3a68a4aaa7 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Fri, 16 Aug 2024 17:08:03 +0400 Subject: [PATCH 2/4] Removed the flag g_listen_gdb. --- lldb/tools/lldb-server/lldb-platform.cpp | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index da98e9ea805781..ebaf44b505d9fc 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -51,7 +51,6 @@ using namespace llvm; static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; -static volatile bool g_listen_gdb = true; static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, @@ -244,7 +243,7 @@ gdb_thread_proc(Acceptor *acceptor_gdb, uint16_t gdbserver_port, Log *log = GetLog(LLDBLog::Platform); GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, gdbserver_port); - while (g_listen_gdb) { + while (true) { Connection *conn = nullptr; Status error = acceptor_gdb->Accept(/*children_inherit_accept_socket=*/false, conn); @@ -576,18 +575,6 @@ int main_platform(int argc, char *argv[]) { client_handle(platform, inferior_arguments); } while (g_server); - // FIXME: Make TCPSocket::CloseListenSockets() public and implement - // Acceptor::Close(). - /* - if (acceptor_gdb && gdb_thread.IsJoinable()) { - g_listen_gdb = false; - static_cast<TCPSocket *>(acceptor_gdb->m_listener_socket_up.get()) - ->CloseListenSockets(); - lldb::thread_result_t result; - gdb_thread.Join(&result); - } - */ - fprintf(stderr, "lldb-server exiting...\n"); return 0; >From d61a1c291665d5850cd99a75452873fee3e3b5b6 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Fri, 16 Aug 2024 17:10:33 +0400 Subject: [PATCH 3/4] Rebased after #104439. --- lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp | 4 ---- lldb/unittests/tools/lldb-server/tests/TestClient.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp index 66e92415911afb..34f8d23e94e4c9 100644 --- a/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp +++ b/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp @@ -14,10 +14,6 @@ using namespace llgs_tests; using namespace lldb_private; using namespace llvm; -#ifdef SendMessage -#undef SendMessage -#endif - // Disable this test on Windows as it appears to have a race condition // that causes lldb-server not to exit after the inferior hangs up. #if !defined(_WIN32) diff --git a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp index 2d7ce36bacfaa3..a6f2dc32c6d0c0 100644 --- a/lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ b/lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -26,10 +26,6 @@ using namespace lldb_private; using namespace llvm; using namespace llgs_tests; -#ifdef SendMessage -#undef SendMessage -#endif - TestClient::TestClient(std::unique_ptr<Connection> Conn) { SetConnection(std::move(Conn)); SetPacketTimeout(std::chrono::seconds(10)); >From 5ceb07673b8aacd3c130a6c122a979be40257076 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Sat, 17 Aug 2024 02:59:32 +0400 Subject: [PATCH 4/4] Refactored Acceptor to listen on 2 ports in the main thread. --- lldb/include/lldb/Host/Socket.h | 24 ++ lldb/include/lldb/Host/common/TCPSocket.h | 3 + .../posix/ConnectionFileDescriptorPosix.h | 26 -- lldb/source/Host/common/Socket.cpp | 76 +++++ lldb/source/Host/common/TCPSocket.cpp | 105 ++++--- .../posix/ConnectionFileDescriptorPosix.cpp | 88 ------ .../gdb-remote/GDBRemoteCommunication.cpp | 2 +- .../gdb-remote/GDBRemoteCommunication.h | 2 +- .../GDBRemoteCommunicationServerPlatform.h | 1 - .../TestPlatformLaunchGDBServer.py | 12 +- lldb/tools/lldb-server/Acceptor.cpp | 130 +++++---- lldb/tools/lldb-server/Acceptor.h | 27 +- lldb/tools/lldb-server/lldb-platform.cpp | 274 +++++++----------- 13 files changed, 385 insertions(+), 385 deletions(-) diff --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h index 573c881f727d8f..c9034faedbd843 100644 --- a/lldb/include/lldb/Host/Socket.h +++ b/lldb/include/lldb/Host/Socket.h @@ -19,6 +19,7 @@ #include "lldb/Utility/Status.h" #ifdef _WIN32 +#include "lldb/Host/Pipe.h" #include "lldb/Host/windows/windows.h" #include <winsock2.h> #include <ws2tcpip.h> @@ -32,12 +33,35 @@ namespace lldb_private { #if defined(_WIN32) typedef SOCKET NativeSocket; +typedef lldb::pipe_t shared_fd_t; #else typedef int NativeSocket; +typedef NativeSocket shared_fd_t; #endif +class Socket; class TCPSocket; class UDPSocket; +class SharedSocket { +public: + static const shared_fd_t kInvalidFD; + + SharedSocket(Socket *socket, Status &error); + + shared_fd_t GetSendableFD() { return m_fd; } + + Status CompleteSending(lldb::pid_t child_pid); + + static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket); + +private: +#ifdef _WIN32 + Pipe m_socket_pipe; + NativeSocket m_socket; +#endif + shared_fd_t m_fd; +}; + class Socket : public IOObject { public: enum SocketProtocol { diff --git a/lldb/include/lldb/Host/common/TCPSocket.h b/lldb/include/lldb/Host/common/TCPSocket.h index b782c9e6096c44..b45fdae00c6b43 100644 --- a/lldb/include/lldb/Host/common/TCPSocket.h +++ b/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; diff --git a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h index 08f486b92e0f07..35773d5907e913 100644 --- a/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h +++ b/lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h @@ -26,32 +26,6 @@ class Status; class Socket; class SocketAddress; -#ifdef _WIN32 -typedef lldb::pipe_t shared_fd_t; -#else -typedef NativeSocket shared_fd_t; -#endif - -class SharedSocket { -public: - static const shared_fd_t kInvalidFD; - - SharedSocket(Connection *conn, Status &error); - - shared_fd_t GetSendableFD() { return m_fd; } - - Status CompleteSending(lldb::pid_t child_pid); - - static Status GetNativeSocket(shared_fd_t fd, NativeSocket &socket); - -private: -#ifdef _WIN32 - Pipe m_socket_pipe; - NativeSocket m_socket; -#endif - shared_fd_t m_fd; -}; - class ConnectionFileDescriptor : public Connection { public: typedef llvm::function_ref<void(llvm::StringRef local_socket_id)> diff --git a/lldb/source/Host/common/Socket.cpp b/lldb/source/Host/common/Socket.cpp index 7364a12280cfdd..581a47a1f304b2 100644 --- a/lldb/source/Host/common/Socket.cpp +++ b/lldb/source/Host/common/Socket.cpp @@ -56,10 +56,12 @@ using namespace lldb_private; typedef const char *set_socket_option_arg_type; typedef char *get_socket_option_arg_type; const NativeSocket Socket::kInvalidSocketValue = INVALID_SOCKET; +const shared_fd_t SharedSocket::kInvalidFD = LLDB_INVALID_PIPE; #else // #if defined(_WIN32) typedef const void *set_socket_option_arg_type; typedef void *get_socket_option_arg_type; const NativeSocket Socket::kInvalidSocketValue = -1; +const shared_fd_t SharedSocket::kInvalidFD = Socket::kInvalidSocketValue; #endif // #if defined(_WIN32) static bool IsInterrupted() { @@ -70,6 +72,80 @@ static bool IsInterrupted() { #endif } +SharedSocket::SharedSocket(Socket *socket, Status &error) { +#ifdef _WIN32 + m_socket = socket->GetNativeSocket(); + m_fd = kInvalidFD; + + // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. + error = m_socket_pipe.CreateNew(true); + if (error.Fail()) + return; + + m_fd = m_socket_pipe.GetReadPipe(); +#else + m_fd = socket->GetNativeSocket(); + error = Status(); +#endif +} + +Status SharedSocket::CompleteSending(lldb::pid_t child_pid) { +#ifdef _WIN32 + // Transfer WSAPROTOCOL_INFO to the child process. + m_socket_pipe.CloseReadFileDescriptor(); + + WSAPROTOCOL_INFO protocol_info; + if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) == + SOCKET_ERROR) { + int last_error = ::WSAGetLastError(); + return Status("WSADuplicateSocket() failed, error: %d", last_error); + } + + size_t num_bytes; + Status error = + m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) + return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", + num_bytes); +#endif + return Status(); +} + +Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) { +#ifdef _WIN32 + socket = Socket::kInvalidSocketValue; + // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. + WSAPROTOCOL_INFO protocol_info; + { + Pipe socket_pipe(fd, LLDB_INVALID_PIPE); + size_t num_bytes; + Status error = + socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), + std::chrono::seconds(10), num_bytes); + if (error.Fail()) + return error; + if (num_bytes != sizeof(protocol_info)) { + return Status( + "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes", + num_bytes); + } + } + socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, + FROM_PROTOCOL_INFO, &protocol_info, 0, 0); + if (socket == INVALID_SOCKET) { + return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d", + ::WSAGetLastError()); + } + return Status(); +#else + socket = fd; + return Status(); +#endif +} + struct SocketScheme { const char *m_scheme; const Socket::SocketProtocol m_protocol; diff --git a/lldb/source/Host/common/TCPSocket.cpp b/lldb/source/Host/common/TCPSocket.cpp index df4737216ed3ac..e659b20bb0045e 100644 --- a/lldb/source/Host/common/TCPSocket.cpp +++ b/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()) { diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index f90b01898aca7d..4fdbe1963b8738 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -52,94 +52,6 @@ using namespace lldb; using namespace lldb_private; -#ifdef _WIN32 -const shared_fd_t SharedSocket::kInvalidFD = LLDB_INVALID_PIPE; -#else -const shared_fd_t SharedSocket::kInvalidFD = Socket::kInvalidSocketValue; -#endif - -SharedSocket::SharedSocket(Connection *conn, Status &error) { - m_fd = kInvalidFD; - - const Socket *socket = - static_cast<const Socket *>(conn->GetReadObject().get()); - if (socket == nullptr) { - error = Status("invalid conn socket"); - return; - } - -#ifdef _WIN32 - m_socket = socket->GetNativeSocket(); - - // Create a pipe to transfer WSAPROTOCOL_INFO to the child process. - error = m_socket_pipe.CreateNew(true); - if (error.Fail()) - return; - - m_fd = m_socket_pipe.GetReadPipe(); -#else - m_fd = socket->GetNativeSocket(); - error = Status(); -#endif -} - -Status SharedSocket::CompleteSending(lldb::pid_t child_pid) { -#ifdef _WIN32 - // Transfer WSAPROTOCOL_INFO to the child process. - m_socket_pipe.CloseReadFileDescriptor(); - - WSAPROTOCOL_INFO protocol_info; - if (::WSADuplicateSocket(m_socket, child_pid, &protocol_info) == - SOCKET_ERROR) { - int last_error = ::WSAGetLastError(); - return Status("WSADuplicateSocket() failed, error: %d", last_error); - } - - size_t num_bytes; - Status error = - m_socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(10), num_bytes); - if (error.Fail()) - return error; - if (num_bytes != sizeof(protocol_info)) - return Status("WriteWithTimeout(WSAPROTOCOL_INFO) failed: %d bytes", - num_bytes); -#endif - return Status(); -} - -Status SharedSocket::GetNativeSocket(shared_fd_t fd, NativeSocket &socket) { -#ifdef _WIN32 - socket = Socket::kInvalidSocketValue; - // Read WSAPROTOCOL_INFO from the parent process and create NativeSocket. - WSAPROTOCOL_INFO protocol_info; - { - Pipe socket_pipe(fd, LLDB_INVALID_PIPE); - size_t num_bytes; - Status error = - socket_pipe.ReadWithTimeout(&protocol_info, sizeof(protocol_info), - std::chrono::seconds(10), num_bytes); - if (error.Fail()) - return error; - if (num_bytes != sizeof(protocol_info)) { - return Status( - "socket_pipe.ReadWithTimeout(WSAPROTOCOL_INFO) failed: % d bytes", - num_bytes); - } - } - socket = ::WSASocket(FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO, - FROM_PROTOCOL_INFO, &protocol_info, 0, 0); - if (socket == INVALID_SOCKET) { - return Status("WSASocket(FROM_PROTOCOL_INFO) failed: error %d", - ::WSAGetLastError()); - } - return Status(); -#else - socket = fd; - return Status(); -#endif -} - ConnectionFileDescriptor::ConnectionFileDescriptor(bool child_processes_inherit) : Connection(), m_pipe(), m_mutex(), m_shutting_down(false), diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 075312016e0b22..4c3b29dac8e455 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -963,7 +963,7 @@ 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 != SharedSocket::kInvalidFD) { diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index bbff31de8aafd6..cd022821e6b1ce 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -20,8 +20,8 @@ #include "lldb/Core/Communication.h" #include "lldb/Host/Config.h" -#include "lldb/Host/ConnectionFileDescriptor.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" diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h index 70d7425cf6097e..f69ef633ac2139 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -15,7 +15,6 @@ #include <set> #include "GDBRemoteCommunicationServerCommon.h" -#include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/Socket.h" #include "llvm/Support/Error.h" diff --git a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py index ea849ab1fa236d..c365bc993e338d 100644 --- a/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py +++ b/lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py @@ -16,7 +16,8 @@ class TestPlatformProcessLaunchGDBServer(TestBase): NO_DEBUG_INFO_TESTCASE = True @skipIfRemote - # Windows doesn't use lldb-server and on Darwin we may be using debugserver. + # Windows cannot delete the executable while it is running. + # On Darwin we may be using debugserver. @skipUnlessPlatform(["linux"]) @add_test_categories(["lldb-server"]) def test_platform_process_launch_gdb_server(self): @@ -44,15 +45,16 @@ def test_platform_process_launch_gdb_server(self): 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) + # First connect to lldb-server which spawn a process to handle the connection. + # Then remove our new lldb-server so that when it tries to invoke itself as a + # gdbserver, it fails. + os.remove(new_lldb_server) + 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/tools/lldb-server/Acceptor.cpp b/lldb/tools/lldb-server/Acceptor.cpp index cf9a55c6c07da5..6397b230c0bc3b 100644 --- a/lldb/tools/lldb-server/Acceptor.cpp +++ b/lldb/tools/lldb-server/Acceptor.cpp @@ -23,76 +23,106 @@ using namespace lldb_private::lldb_server; using namespace llvm; Status Acceptor::Listen(int backlog) { - return m_listener_socket_up->Listen(StringRef(m_name), backlog); -} + if (m_socket_protocol == Socket::ProtocolTcp) { + // Update the url with the second port to listen on. + m_name += llvm::formatv(",{0}", m_gdbserver_port); + } -Status Acceptor::Accept(const bool child_processes_inherit, Connection *&conn) { - Socket *conn_socket = nullptr; - auto error = m_listener_socket_up->Accept(conn_socket); - if (error.Success()) - conn = new ConnectionFileDescriptor(conn_socket); + Status error = m_listener_socket_up->Listen(StringRef(m_name), backlog); + if (error.Fail()) + return error; + if (m_socket_protocol == Socket::ProtocolTcp) { + if (m_platform_port == 0 || m_gdbserver_port == 0) { + TCPSocket *tcp_socket = + static_cast<TCPSocket *>(m_listener_socket_up.get()); + std::set<uint16_t> ports = tcp_socket->GetLocalPortNumbers(); + std::set<uint16_t>::iterator port_i = ports.begin(); + while (m_platform_port == 0 || m_gdbserver_port == 0) { + if (port_i == ports.end()) + return Status("cannot resolve ports"); + if (m_platform_port == 0 && *port_i != m_gdbserver_port) { + m_platform_port = *port_i; + } else if (m_gdbserver_port == 0 && *port_i != m_platform_port) { + m_gdbserver_port = *port_i; + } + ++port_i; + } + } + assert(m_platform_port); + assert(m_gdbserver_port); + assert(m_platform_port != m_gdbserver_port); + } return error; } -Socket::SocketProtocol Acceptor::GetSocketProtocol() const { - return m_listener_socket_up->GetSocketProtocol(); +std::string Acceptor::GetLocalSocketId() const { + if (m_socket_protocol == Socket::ProtocolTcp) { + return (m_platform_port != 0) ? llvm::to_string(m_platform_port) : ""; + } else { + return m_name; + } } -const char *Acceptor::GetSocketScheme() const { - return Socket::FindSchemeByProtocol(GetSocketProtocol()); +Status Acceptor::Accept(Socket *&conn_socket, bool &is_gdbserver) { + conn_socket = nullptr; + Status error = m_listener_socket_up->Accept(conn_socket); + if (error.Fail()) + return error; + if (conn_socket->GetSocketProtocol() == Socket::ProtocolTcp) { + TCPSocket *tcp_socket = static_cast<TCPSocket *>(conn_socket); + uint16_t port = tcp_socket->GetLocalPortNumber(); + if (port == m_platform_port) + is_gdbserver = false; + else if (port == m_gdbserver_port) + is_gdbserver = true; + else { + delete conn_socket; + return Status("unexpected TCP socket port"); + } + } else { + is_gdbserver = false; + } + return Status(); } -std::string Acceptor::GetLocalSocketId() const { return m_local_socket_id(); } - -std::unique_ptr<Acceptor> Acceptor::Create(StringRef name, - const bool child_processes_inherit, - Status &error) { +Acceptor::Acceptor(StringRef name, uint16_t gdbserver_port, + const bool child_processes_inherit, Status &error) + : m_platform_port(0), m_gdbserver_port(gdbserver_port) { error.Clear(); - Socket::SocketProtocol socket_protocol = Socket::ProtocolUnixDomain; + m_socket_protocol = Socket::ProtocolUnixDomain; // Try to match socket name as URL - e.g., tcp://localhost:5555 if (std::optional<URI> res = URI::Parse(name)) { if (!Socket::FindProtocolByScheme(res->scheme.str().c_str(), - socket_protocol)) + m_socket_protocol)) { error.SetErrorStringWithFormat("Unknown protocol scheme \"%s\"", res->scheme.str().c_str()); - else - name = name.drop_front(res->scheme.size() + strlen("://")); + return; + } + name = name.drop_front(res->scheme.size() + strlen("://")); + if (m_socket_protocol == Socket::ProtocolTcp && res->port) { + m_platform_port = *(res->port); + } } else { // Try to match socket name as $host:port - e.g., localhost:5555 - if (!llvm::errorToBool(Socket::DecodeHostAndPort(name).takeError())) - socket_protocol = Socket::ProtocolTcp; - } - - if (error.Fail()) - return std::unique_ptr<Acceptor>(); - - std::unique_ptr<Socket> listener_socket_up = - Socket::Create(socket_protocol, child_processes_inherit, error); - - LocalSocketIdFunc local_socket_id; - if (error.Success()) { - if (listener_socket_up->GetSocketProtocol() == Socket::ProtocolTcp) { - TCPSocket *tcp_socket = - static_cast<TCPSocket *>(listener_socket_up.get()); - local_socket_id = [tcp_socket]() { - auto local_port = tcp_socket->GetLocalPortNumber(); - return (local_port != 0) ? llvm::to_string(local_port) : ""; - }; - } else { - const std::string socket_name = std::string(name); - local_socket_id = [socket_name]() { return socket_name; }; + llvm::Expected<Socket::HostAndPort> host_port = + Socket::DecodeHostAndPort(name); + if (!llvm::errorToBool(host_port.takeError())) { + m_socket_protocol = Socket::ProtocolTcp; + m_platform_port = host_port->port; } + } - return std::unique_ptr<Acceptor>( - new Acceptor(std::move(listener_socket_up), name, local_socket_id)); + if (m_socket_protocol == Socket::ProtocolTcp && m_platform_port != 0 && + m_platform_port == m_gdbserver_port) { + error.SetErrorStringWithFormat("The same ports \"%s\" and %u", + name.str().c_str(), m_gdbserver_port); + return; } - return std::unique_ptr<Acceptor>(); -} + m_name = name.str(); -Acceptor::Acceptor(std::unique_ptr<Socket> &&listener_socket, StringRef name, - const LocalSocketIdFunc &local_socket_id) - : m_listener_socket_up(std::move(listener_socket)), m_name(name.str()), - m_local_socket_id(local_socket_id) {} + m_listener_socket_up = + Socket::Create(m_socket_protocol, child_processes_inherit, error); +} diff --git a/lldb/tools/lldb-server/Acceptor.h b/lldb/tools/lldb-server/Acceptor.h index b441e92dcd22f3..bc7a1567dca13d 100644 --- a/lldb/tools/lldb-server/Acceptor.h +++ b/lldb/tools/lldb-server/Acceptor.h @@ -29,29 +29,28 @@ class Acceptor { Status Listen(int backlog); - Status Accept(const bool child_processes_inherit, Connection *&conn); + Status Accept(Socket *&conn_socket, bool &is_gdbserver); - static std::unique_ptr<Acceptor> Create(llvm::StringRef name, - const bool child_processes_inherit, - Status &error); + Acceptor(llvm::StringRef name, uint16_t gdbserver_port, + const bool child_processes_inherit, Status &error); - Socket::SocketProtocol GetSocketProtocol() const; + Socket::SocketProtocol GetSocketProtocol() const { + return m_socket_protocol; + }; - const char *GetSocketScheme() const; + uint16_t GetPlatformPort() const { return m_platform_port; }; + uint16_t GetGdbServerPort() const { return m_gdbserver_port; }; // Returns either TCP port number as string or domain socket path. // Empty string is returned in case of error. std::string GetLocalSocketId() const; private: - typedef std::function<std::string()> LocalSocketIdFunc; - - Acceptor(std::unique_ptr<Socket> &&listener_socket, llvm::StringRef name, - const LocalSocketIdFunc &local_socket_id); - - const std::unique_ptr<Socket> m_listener_socket_up; - const std::string m_name; - const LocalSocketIdFunc m_local_socket_id; + Socket::SocketProtocol m_socket_protocol; + uint16_t m_platform_port; + uint16_t m_gdbserver_port; + std::unique_ptr<Socket> m_listener_socket_up; + std::string m_name; }; } // namespace lldb_server diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index ebaf44b505d9fc..064223fe0f31a2 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -18,6 +18,7 @@ #if !defined(_WIN32) #include <sys/wait.h> #endif +#include <atomic> #include <fstream> #include <optional> @@ -34,7 +35,6 @@ #include "lldb/Host/HostGetOpt.h" #include "lldb/Host/OptionParser.h" #include "lldb/Host/Socket.h" -#include "lldb/Host/ThreadLauncher.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/LLDBLog.h" @@ -51,6 +51,9 @@ using namespace llvm; static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; +static std::atomic<bool> g_main_loop = true; +static std::atomic<lldb::pid_t> g_single_pid = LLDB_INVALID_PROCESS_ID; +static std::string g_break_conn; static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, @@ -119,44 +122,25 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } -static void client_handle(GDBRemoteCommunicationServerPlatform &platform, - const lldb_private::Args &args) { - if (!platform.IsConnected()) - return; - - if (args.GetArgumentCount() > 0) { - lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - std::string socket_name; - Status error = platform.LaunchGDBServer(args, pid, socket_name, - SharedSocket::kInvalidFD); - if (error.Success()) - platform.SetPendingGdbServer(socket_name); - else - fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); +static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) { + if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) { + // If not running as a server and the platform connection is closed + g_main_loop = false; + TCPSocket socket(/*should_close=*/true, /*child_processes_inherit=*/false); + Status error = socket.Connect(g_break_conn); + if (error.Fail()) + LLDB_LOGF(GetLog(LLDBLog::Platform), + "Cannot break the listening loop: %s", error.AsCString()); } - - bool interrupt = false; - bool done = false; - Status error; - while (!interrupt && !done) { - if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, - done) != - GDBRemoteCommunication::PacketResult::Success) - break; - } - - printf("Disconnected.\n"); } -static void spawn_process_reaped(lldb::pid_t pid, int signal, int status) {} - -static Status spawn_process(const char *progname, Connection *conn, +static Status spawn_process(const char *progname, Socket *conn_socket, uint16_t gdb_port, uint16_t port_offset, const lldb_private::Args &args, const std::string &log_file, const StringRef log_channels) { Status error; - SharedSocket shared_socket(conn, error); + SharedSocket shared_socket(conn_socket, error); if (error.Fail()) return error; @@ -232,54 +216,10 @@ static Status spawn_process(const char *progname, Connection *conn, return error; } - return Status(); -} - -static thread_result_t -gdb_thread_proc(Acceptor *acceptor_gdb, uint16_t gdbserver_port, - const lldb_private::Args &inferior_arguments) { - lldb_private::Args args(inferior_arguments); - - Log *log = GetLog(LLDBLog::Platform); - GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, - gdbserver_port); - while (true) { - Connection *conn = nullptr; - Status error = - acceptor_gdb->Accept(/*children_inherit_accept_socket=*/false, conn); - if (error.Fail()) { - WithColor::error() << error.AsCString() << '\n'; - break; - } - printf("gdb connection established.\n"); + if (!g_server) + g_single_pid = child_pid; - { - SharedSocket shared_socket(conn, error); - if (error.Success()) { - lldb::pid_t child_pid = LLDB_INVALID_PROCESS_ID; - std::string socket_name; - error = platform.LaunchGDBServer(args, child_pid, socket_name, - shared_socket.GetSendableFD()); - if (error.Fail()) { - LLDB_LOGF(log, "gdbserver LaunchGDBServer failed: %s", - error.AsCString()); - } else if (child_pid != LLDB_INVALID_PROCESS_ID) { - error = shared_socket.CompleteSending(child_pid); - if (error.Success()) { - // Use inferior arguments once. - args.Clear(); - } else { - Host::Kill(child_pid, SIGTERM); - LLDB_LOGF(log, "gdbserver CompleteSending failed: %s", - error.AsCString()); - } - } - } else - LLDB_LOGF(log, "gdbserver SharedSocket failed: %s", error.AsCString()); - } - delete conn; - } - return {}; + return Status(); } // main @@ -369,14 +309,8 @@ int main_platform(int argc, char *argv[]) { option_error = 2; break; } - if (portnum < LOW_PORT || portnum > HIGH_PORT) { - WithColor::error() << llvm::formatv( - "port number {0} is not in the " - "valid user port range of {1} - {2}\n", - portnum, LOW_PORT, HIGH_PORT); - option_error = 1; - break; - } + // Note the condition gdbserver_port > HIGH_PORT is valid in case of using + // --child-platform-fd. Check gdbserver_port later. if (ch == 'P') gdbserver_port = portnum; else if (gdbserver_port == 0) @@ -417,9 +351,10 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); + Log *log = GetLog(LLDBLog::Platform); + if (fd != SharedSocket::kInvalidFD) { // Child process will handle the connection and exit. - Log *log = GetLog(LLDBLog::Platform); if (!listen_host_port.empty()) { LLDB_LOGF(log, "lldb-platform child: " "ambiguous parameters --listen and --child-platform-fd"); @@ -439,25 +374,56 @@ int main_platform(int argc, char *argv[]) { return socket_error; } - Connection *conn = - new ConnectionFileDescriptor(new TCPSocket(socket, true, false)); GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, gdbserver_port); if (port_offset > 0) platform.SetPortOffset(port_offset); - platform.SetConnection(std::unique_ptr<Connection>(conn)); - client_handle(platform, inferior_arguments); + platform.SetConnection( + std::unique_ptr<Connection>(new ConnectionFileDescriptor( + new TCPSocket(socket, /*should_close=*/true, + /*child_processes_inherit=*/false)))); + if (platform.IsConnected()) { + if (inferior_arguments.GetArgumentCount() > 0) { + lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; + std::string socket_name; + Status error = platform.LaunchGDBServer( + inferior_arguments, pid, socket_name, SharedSocket::kInvalidFD); + if (error.Success()) + platform.SetPendingGdbServer(socket_name); + else + fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); + } + + bool interrupt = false; + bool done = false; + Status error; + while (!interrupt && !done) { + if (platform.GetPacketAndSendResponse(std::nullopt, error, interrupt, + done) != + GDBRemoteCommunication::PacketResult::Success) + break; + } + } + printf("Disconnected.\n"); return 0; } + if (gdbserver_port != 0 && + (gdbserver_port < LOW_PORT || gdbserver_port > HIGH_PORT)) { + WithColor::error() << llvm::formatv("port number {0} is not in the " + "valid user port range of {1} - {2}\n", + gdbserver_port, LOW_PORT, HIGH_PORT); + exit(1); + } + const bool children_inherit_listen_socket = false; // the test suite makes many connections in parallel, let's not miss any. // The highest this should get reasonably is a function of the number // of target CPUs. For now, let's just use 100. const int backlog = 100; - std::unique_ptr<Acceptor> acceptor_up(Acceptor::Create( - listen_host_port, children_inherit_listen_socket, error)); + std::unique_ptr<Acceptor> acceptor_up = std::make_unique<Acceptor>( + listen_host_port, gdbserver_port, children_inherit_listen_socket, error); if (error.Fail()) { fprintf(stderr, "failed to create acceptor: %s", error.AsCString()); exit(socket_error); @@ -471,56 +437,6 @@ int main_platform(int argc, char *argv[]) { exit(socket_error); } - std::unique_ptr<Acceptor> acceptor_gdb; - HostThread gdb_thread; - if (acceptor_up->GetSocketProtocol() == Socket::ProtocolTcp) { - // Use the same host from listen_host_port for acceptor_gdb. - std::size_t port_pos = listen_host_port.rfind(":"); - if (port_pos == std::string::npos) - port_pos = 0; - else - ++port_pos; - std::string listen_gdb_port = llvm::formatv( - "{0}{1}", listen_host_port.substr(0, port_pos), gdbserver_port); - acceptor_gdb = Acceptor::Create(listen_gdb_port, - children_inherit_listen_socket, error); - if (error.Fail()) { - fprintf(stderr, "failed to create gdb acceptor: %s", error.AsCString()); - exit(socket_error); - } - - error = acceptor_gdb->Listen(backlog); - if (error.Fail()) { - printf("failed to listen gdb: %s\n", error.AsCString()); - exit(socket_error); - } - - if (gdbserver_port == 0) { - std::string str_gdbserver_port = acceptor_gdb->GetLocalSocketId(); - if (!llvm::to_integer(str_gdbserver_port, gdbserver_port)) { - printf("invalid gdb port: %s\n", str_gdbserver_port.c_str()); - exit(socket_error); - } - Log *log = GetLog(LLDBLog::Connection); - LLDB_LOG(log, "Listen to gdb port {0}", gdbserver_port); - } - assert(gdbserver_port); - - const std::string thread_name = llvm::formatv("gdb:{0}", gdbserver_port); - Acceptor *ptr_acceptor_gdb = acceptor_gdb.get(); - auto maybe_thread = ThreadLauncher::LaunchThread( - thread_name, [ptr_acceptor_gdb, gdbserver_port, inferior_arguments] { - return gdb_thread_proc(ptr_acceptor_gdb, gdbserver_port, - inferior_arguments); - }); - if (!maybe_thread) { - WithColor::error() << "failed to start gdb listen thread: " - << maybe_thread.takeError() << '\n'; - exit(socket_error); - } - gdb_thread = *maybe_thread; - } - error = acceptor_up->Listen(backlog); if (error.Fail()) { printf("failed to listen: %s\n", error.AsCString()); @@ -536,44 +452,72 @@ int main_platform(int argc, char *argv[]) { } } + if (!g_server) { + g_break_conn = + llvm::formatv("127.0.0.1:{0}", acceptor_up->GetPlatformPort()); + } + gdbserver_port = acceptor_up->GetGdbServerPort(); + lldb_private::Args gdb_inferior_arguments(inferior_arguments); + GDBRemoteCommunicationServerPlatform platform( acceptor_up->GetSocketProtocol(), gdbserver_port); if (port_offset > 0) platform.SetPortOffset(port_offset); - do { - const bool children_inherit_accept_socket = true; - Connection *conn = nullptr; - error = acceptor_up->Accept(children_inherit_accept_socket, conn); + while (g_main_loop) { + bool is_gdbserver = false; + Socket *conn_socket = nullptr; + error = acceptor_up->Accept(conn_socket, is_gdbserver); if (error.Fail()) { WithColor::error() << error.AsCString() << '\n'; exit(socket_error); } - printf("Connection established.\n"); - if (g_server) { - error = spawn_process(progname, conn, gdbserver_port, port_offset, + if (!g_main_loop) + break; + + if (is_gdbserver) { + printf("Gdb connection established.\n"); + { + SharedSocket shared_socket(conn_socket, error); + if (error.Success()) { + lldb::pid_t child_pid = LLDB_INVALID_PROCESS_ID; + std::string socket_name; + error = platform.LaunchGDBServer(gdb_inferior_arguments, child_pid, + socket_name, + shared_socket.GetSendableFD()); + if (error.Success() && child_pid != LLDB_INVALID_PROCESS_ID) { + error = shared_socket.CompleteSending(child_pid); + if (error.Success()) { + // Use gdb inferior arguments once. + gdb_inferior_arguments.Clear(); + } else { + Host::Kill(child_pid, SIGTERM); + LLDB_LOGF(log, "gdbserver CompleteSending failed: %s", + error.AsCString()); + } + } + } else + LLDB_LOGF(log, "gdbserver SharedSocket failed: %s", + error.AsCString()); + } + } else if (g_server || g_single_pid == LLDB_INVALID_PROCESS_ID) { + // If not running as a server, this process will not accept + // connections while a connection is active. + + printf("Platform connection established.\n"); + error = spawn_process(progname, conn_socket, gdbserver_port, port_offset, inferior_arguments, log_file, log_channels); if (error.Fail()) { - LLDB_LOGF(GetLog(LLDBLog::Platform), "spawn_process failed: %s", - error.AsCString()); + LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString()); WithColor::error() << "spawn_process failed: " << error.AsCString() << "\n"; + if (!g_server) + g_main_loop = false; } - // Parent doesn't need a connection to the lldb client - delete conn; - - // Parent will continue to listen for new connections. - continue; - } else { - // If not running as a server, this process will not accept - // connections while a connection is active. - acceptor_up.reset(); } - - platform.SetConnection(std::unique_ptr<Connection>(conn)); - client_handle(platform, inferior_arguments); - } while (g_server); + delete conn_socket; + } fprintf(stderr, "lldb-server exiting...\n"); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
