https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/104238
>From 5732bda3558a0d55f2f7b9d1a3722d11458a2360 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Wed, 14 Aug 2024 22:36:52 +0400 Subject: [PATCH] [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 | 290 ++++++---------- .../GDBRemoteCommunicationServerPlatform.h | 83 +---- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../Shell/lldb-server/TestGdbserverPort.test | 4 - lldb/tools/lldb-server/Acceptor.cpp | 6 +- lldb/tools/lldb-server/Acceptor.h | 3 + lldb/tools/lldb-server/lldb-gdbserver.cpp | 23 +- lldb/tools/lldb-server/lldb-platform.cpp | 326 ++++++++---------- .../Process/gdb-remote/CMakeLists.txt | 1 - .../Process/gdb-remote/PortMapTest.cpp | 115 ------ .../tools/lldb-server/tests/TestClient.h | 4 + 17 files changed, 461 insertions(+), 611 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..5c51c9afbe76df 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -40,83 +40,20 @@ #include "lldb/Utility/StringExtractorGDBRemote.h" +#include "../tools/lldb-server/Acceptor.h" + 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 +97,59 @@ 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 << lldb_server::Acceptor::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 +180,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 +212,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 +241,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 +277,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 +434,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 +447,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 +464,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 +505,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 6f9c2cc1e4b4e8..e5a5876cd60dcc 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3430,7 +3430,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/Acceptor.cpp b/lldb/tools/lldb-server/Acceptor.cpp index 2037f1e0f62b9a..4227c9151d5783 100644 --- a/lldb/tools/lldb-server/Acceptor.cpp +++ b/lldb/tools/lldb-server/Acceptor.cpp @@ -47,14 +47,16 @@ bool FindProtocolByScheme(const char *scheme, return false; } -const char *FindSchemeByProtocol(const Socket::SocketProtocol protocol) { +} // namespace + +const char * +Acceptor::FindSchemeByProtocol(const Socket::SocketProtocol protocol) { for (auto s : socket_schemes) { if (s.m_protocol == protocol) return s.m_scheme; } return nullptr; } -} Status Acceptor::Listen(int backlog) { return m_listener_socket_up->Listen(StringRef(m_name), backlog); diff --git a/lldb/tools/lldb-server/Acceptor.h b/lldb/tools/lldb-server/Acceptor.h index b441e92dcd22f3..fc3f48ad063db5 100644 --- a/lldb/tools/lldb-server/Acceptor.h +++ b/lldb/tools/lldb-server/Acceptor.h @@ -39,6 +39,9 @@ class Acceptor { const char *GetSocketScheme() const; + static const char * + FindSchemeByProtocol(const Socket::SocketProtocol protocol); + // Returns either TCP port number as string or domain socket path. // Empty string is returned in case of error. std::string GetLocalSocketId() const; 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..81125aa066d32e 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}, @@ -173,7 +73,7 @@ static struct option g_long_options[] = { #define HIGH_PORT (IPPORT_HIFIRSTAUTO) #else #define LOW_PORT (1024u) -#define HIGH_PORT (49151u) +#define HIGH_PORT (65535u) #endif #if !defined(_WIN32) @@ -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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
