https://github.com/slydiman updated https://github.com/llvm/llvm-project/pull/104238
>From 18b8a835a3182e5be5f0dd848977333d9b8a26fa Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Fri, 30 Aug 2024 01:08:24 +0400 Subject: [PATCH 1/2] [lldb] Removed gdbserver ports map from lldb-server Listen to gdbserver-port, accept the connection and run lldb-server gdbserver --fd on all platforms. 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.cpp | 21 +- .../gdb-remote/GDBRemoteCommunication.cpp | 45 ++- .../gdb-remote/GDBRemoteCommunication.h | 8 +- .../GDBRemoteCommunicationServerPlatform.cpp | 287 +++++---------- .../GDBRemoteCommunicationServerPlatform.h | 82 +---- .../Process/gdb-remote/ProcessGDBRemote.cpp | 2 +- .../TestPlatformLaunchGDBServer.py | 12 +- .../Shell/lldb-server/TestGdbserverPort.test | 4 - lldb/tools/lldb-server/Acceptor.cpp | 98 ----- lldb/tools/lldb-server/Acceptor.h | 60 ---- lldb/tools/lldb-server/CMakeLists.txt | 1 - lldb/tools/lldb-server/lldb-gdbserver.cpp | 23 +- lldb/tools/lldb-server/lldb-platform.cpp | 335 ++++++++++++------ .../Process/gdb-remote/CMakeLists.txt | 1 - .../Process/gdb-remote/PortMapTest.cpp | 115 ------ .../tools/lldb-server/tests/LLGSTest.cpp | 4 - .../tools/lldb-server/tests/TestClient.cpp | 4 - .../tools/lldb-server/tests/TestClient.h | 4 + 20 files changed, 415 insertions(+), 721 deletions(-) delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp delete mode 100644 lldb/tools/lldb-server/Acceptor.h 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/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 6a40f66be39b18..0c9672ea6aa753 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -162,8 +162,10 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path, .Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket) .Case("unix-abstract-connect", &ConnectionFileDescriptor::ConnectAbstractSocket) -#if LLDB_ENABLE_POSIX +#if LLDB_ENABLE_POSIX || defined(_WIN32) .Case("fd", &ConnectionFileDescriptor::ConnectFD) +#endif +#if LLDB_ENABLE_POSIX .Case("file", &ConnectionFileDescriptor::ConnectFile) .Case("serial", &ConnectionFileDescriptor::ConnectSerialPort) #endif @@ -667,7 +669,22 @@ 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 = Status::FromErrorStringWithFormat( + "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 50fa11e916c5f5..514fd8891674bf 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(); @@ -959,16 +963,19 @@ Status GDBRemoteCommunication::StartDebugserverProcess( #endif // If a url is supplied then use it - if (url) + if (url && url[0]) debugserver_args.AppendArgument(llvm::StringRef(url)); - if (pass_comm_fd >= 0) { + if (pass_comm_fd != SharedSocket::kInvalidFD) { StreamString fd_arg; - fd_arg.Printf("--fd=%i", pass_comm_fd); + fd_arg.Printf("--fd=%" PRIi64, (int64_t)pass_comm_fd); debugserver_args.AppendArgument(fd_arg.GetString()); // Send "pass_comm_fd" down to the inferior so it can use it to - // communicate back with this process - launch_info.AppendDuplicateFileAction(pass_comm_fd, pass_comm_fd); + // communicate back with this process. Ignored on Windows. +#ifndef _WIN32 + launch_info.AppendDuplicateFileAction((int)pass_comm_fd, + (int)pass_comm_fd); +#endif } // use native registers, not the GDB registers @@ -988,7 +995,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 +1066,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 +1141,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..cd022821e6b1ce 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -21,6 +21,7 @@ #include "lldb/Core/Communication.h" #include "lldb/Host/Config.h" #include "lldb/Host/HostThread.h" +#include "lldb/Host/Socket.h" #include "lldb/Utility/Args.h" #include "lldb/Utility/Listener.h" #include "lldb/Utility/Predicate.h" @@ -146,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 30e782e3be1846..820bbd4d92ec22 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::FromErrorString("unable to locate debugserver"); + return Status(); + } - // ignore the hostname send from the remote end, just use the ip address that - // we're currently communicating with as the hostname + // debugserver does not accept the URL scheme prefix. +#if !defined(__APPLE__) + url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://"; +#endif + socket_name = GetDomainSocketPath("gdbserver").GetPath(); + url << socket_name; + } else { + if (m_socket_protocol != Socket::ProtocolTcp) + return Status::FromErrorString("protocol must be tcp"); + } // Spawn a debugserver and try to get the port it listens to. ProcessLaunchInfo debugserver_launch_info; - if (hostname.empty()) - hostname = "127.0.0.1"; - Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(), - *port); + LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), + (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..f69ef633ac2139 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -25,90 +25,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 +69,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 65b1b3a0f26863..5eaf9ce2a302aa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3447,7 +3447,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/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/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 deleted file mode 100644 index fc692a847d9e0e..00000000000000 --- a/lldb/tools/lldb-server/Acceptor.cpp +++ /dev/null @@ -1,98 +0,0 @@ -//===-- Acceptor.cpp --------------------------------------------*- C++ -*-===// -// -// 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 "Acceptor.h" - -#include "llvm/ADT/StringRef.h" -#include "llvm/Support/ScopedPrinter.h" - -#include "lldb/Host/ConnectionFileDescriptor.h" -#include "lldb/Host/common/TCPSocket.h" -#include "lldb/Utility/StreamString.h" -#include "lldb/Utility/UriParser.h" -#include <optional> - -using namespace lldb; -using namespace lldb_private; -using namespace lldb_private::lldb_server; -using namespace llvm; - -Status Acceptor::Listen(int backlog) { - return m_listener_socket_up->Listen(StringRef(m_name), backlog); -} - -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); - - return error; -} - -Socket::SocketProtocol Acceptor::GetSocketProtocol() const { - return m_listener_socket_up->GetSocketProtocol(); -} - -const char *Acceptor::GetSocketScheme() const { - return Socket::FindSchemeByProtocol(GetSocketProtocol()); -} - -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) { - error.Clear(); - - Socket::SocketProtocol 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)) - error = Status::FromErrorStringWithFormat( - "Unknown protocol scheme \"%s\"", res->scheme.str().c_str()); - else - name = name.drop_front(res->scheme.size() + strlen("://")); - } 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; }; - } - - return std::unique_ptr<Acceptor>( - new Acceptor(std::move(listener_socket_up), name, local_socket_id)); - } - - return std::unique_ptr<Acceptor>(); -} - -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) {} diff --git a/lldb/tools/lldb-server/Acceptor.h b/lldb/tools/lldb-server/Acceptor.h deleted file mode 100644 index b441e92dcd22f3..00000000000000 --- a/lldb/tools/lldb-server/Acceptor.h +++ /dev/null @@ -1,60 +0,0 @@ -//===-- Acceptor.h ----------------------------------------------*- C++ -*-===// -// -// 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 -// -//===----------------------------------------------------------------------===// -#ifndef LLDB_TOOLS_LLDB_SERVER_ACCEPTOR_H -#define LLDB_TOOLS_LLDB_SERVER_ACCEPTOR_H - -#include "lldb/Host/Socket.h" -#include "lldb/Utility/Connection.h" -#include "lldb/Utility/Status.h" - -#include <functional> -#include <memory> -#include <string> - -namespace llvm { -class StringRef; -} - -namespace lldb_private { -namespace lldb_server { - -class Acceptor { -public: - virtual ~Acceptor() = default; - - Status Listen(int backlog); - - Status Accept(const bool child_processes_inherit, Connection *&conn); - - static std::unique_ptr<Acceptor> Create(llvm::StringRef name, - const bool child_processes_inherit, - Status &error); - - Socket::SocketProtocol GetSocketProtocol() const; - - const char *GetSocketScheme() const; - - // 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; -}; - -} // namespace lldb_server -} // namespace lldb_private - -#endif // LLDB_TOOLS_LLDB_SERVER_ACCEPTOR_H diff --git a/lldb/tools/lldb-server/CMakeLists.txt b/lldb/tools/lldb-server/CMakeLists.txt index 9030ed709a647a..e6d88a8579885c 100644 --- a/lldb/tools/lldb-server/CMakeLists.txt +++ b/lldb/tools/lldb-server/CMakeLists.txt @@ -37,7 +37,6 @@ if(APPLE_EMBEDDED) endif() add_lldb_tool(lldb-server - Acceptor.cpp lldb-gdbserver.cpp lldb-platform.cpp lldb-server.cpp diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index 563284730bc705..a95f456804f786 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; + 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 c021bc7e30194d..2eb0e264d5bdd1 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> @@ -26,18 +27,19 @@ #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" -#include "Acceptor.h" #include "LLDBServerUtilities.h" #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h" #include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h" #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/HostGetOpt.h" +#include "lldb/Host/MainLoop.h" #include "lldb/Host/OptionParser.h" #include "lldb/Host/Socket.h" #include "lldb/Host/common/TCPSocket.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" +#include "lldb/Utility/UriParser.h" using namespace lldb; using namespace lldb_private; @@ -50,6 +52,9 @@ using namespace llvm; static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; +static std::atomic<lldb::pid_t> g_single_pid = LLDB_INVALID_PROCESS_ID; +static std::atomic<bool> g_terminate = false; +static std::unique_ptr<MainLoop> g_main_loop(new MainLoop()); static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, @@ -126,13 +131,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()); } @@ -150,12 +153,17 @@ 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); + if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid) { + // If not running as a server and the platform connection is closed + if (!g_terminate) { + g_terminate = true; + if (g_main_loop) { + g_main_loop->AddPendingCallback( + [](MainLoopBase &loop) { loop.RequestTermination(); }); + } + } + } } static Status spawn_process(const char *progname, const Socket *conn_socket, @@ -176,6 +184,10 @@ static Status spawn_process(const char *progname, const Socket *conn_socket, self_args.AppendArgument(llvm::StringRef("platform")); self_args.AppendArgument(llvm::StringRef("--child-platform-fd")); self_args.AppendArgument(llvm::to_string(shared_socket.GetSendableFD())); +#ifndef _WIN32 + launch_info.AppendDuplicateFileAction((int)shared_socket.GetSendableFD(), + (int)shared_socket.GetSendableFD()); +#endif if (gdb_port) { self_args.AppendArgument(llvm::StringRef("--gdbserver-port")); self_args.AppendArgument(llvm::to_string(gdb_port)); @@ -229,17 +241,15 @@ static Status spawn_process(const char *progname, const Socket *conn_socket, 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); return error; } + if (!g_server) + g_single_pid = child_pid; + return Status(); } @@ -264,8 +274,7 @@ int main_platform(int argc, char *argv[]) { 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; @@ -331,20 +340,12 @@ 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_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: { @@ -366,18 +367,6 @@ 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 == SharedSocket::kInvalidFD) show_usage = true; @@ -393,15 +382,22 @@ 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"); 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()) { @@ -409,10 +405,10 @@ int main_platform(int argc, char *argv[]) { return socket_error; } - 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>(new ConnectionFileDescriptor( new TCPSocket(socket, /*should_close=*/true, @@ -421,27 +417,106 @@ int main_platform(int argc, char *argv[]) { return 0; } - const bool children_inherit_listen_socket = false; + 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); + return 1; + } + // 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)); - if (error.Fail()) { - fprintf(stderr, "failed to create acceptor: %s", error.AsCString()); - exit(socket_error); + std::unique_ptr<Socket> sock_named; + std::unique_ptr<TCPSocket> sock_platform; + std::unique_ptr<TCPSocket> sock_gdb; + std::string hostname; + uint16_t platform_port = 0; + Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain; + + // Try to match socket name as URL - e.g., tcp://localhost:5555 + if (std::optional<URI> uri = URI::Parse(listen_host_port)) { + if (!Socket::FindProtocolByScheme(uri->scheme.str().c_str(), protocol)) { + fprintf(stderr, "Unknown protocol scheme \"%s\".", + uri->scheme.str().c_str()); + return socket_error; + } + if (protocol == Socket::ProtocolTcp) { + hostname = uri->hostname; + if (uri->port) { + platform_port = *(uri->port); + } + } else + hostname = listen_host_port.substr(uri->scheme.size() + strlen("://")); + } else { + // Try to match socket name as $host:port - e.g., localhost:5555 + llvm::Expected<Socket::HostAndPort> host_port = + Socket::DecodeHostAndPort(listen_host_port); + if (!llvm::errorToBool(host_port.takeError())) { + protocol = Socket::ProtocolTcp; + hostname = host_port->hostname; + platform_port = host_port->port; + } else + hostname = listen_host_port; } - error = acceptor_up->Listen(backlog); - if (error.Fail()) { - printf("failed to listen: %s\n", error.AsCString()); - exit(socket_error); + std::string socket_id; + if (protocol == Socket::ProtocolTcp) { + if (platform_port != 0 && platform_port == gdbserver_port) { + fprintf(stderr, "The same platform and gdb ports %u.", platform_port); + return socket_error; + } + + sock_platform = std::make_unique<TCPSocket>( + /*should_close=*/true, /*child_processes_inherit=*/false); + error = sock_platform->Listen( + llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog); + if (error.Fail()) { + printf("Failed to listen platform: %s\n", error.AsCString()); + return socket_error; + } + if (platform_port == 0) + platform_port = sock_platform->GetLocalPortNumber(); + if (platform_port != 0) + socket_id = llvm::to_string(platform_port); + + sock_gdb = std::make_unique<TCPSocket>(/*should_close=*/true, + /*child_processes_inherit=*/false); + error = sock_gdb->Listen( + llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog); + if (error.Fail()) { + printf("Failed to listen gdb: %s\n", error.AsCString()); + return socket_error; + } + if (gdbserver_port == 0) + gdbserver_port = sock_gdb->GetLocalPortNumber(); + } else { + if (g_server) { + fprintf(stderr, + "Ambiguous parameters --server --listen %s\n" + "The protocol must be tcp for the server mode.", + listen_host_port.c_str()); + return socket_error; + } + sock_named = + Socket::Create(protocol, /*child_processes_inherit=*/false, error); + if (error.Fail()) { + fprintf(stderr, "Failed to create socket: %s", error.AsCString()); + return socket_error; + } + error = sock_named->Listen(hostname, backlog); + if (error.Fail()) { + printf("Failed to listen: %s\n", error.AsCString()); + return socket_error; + } + socket_id = hostname; } + if (socket_file) { - error = - save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file); + error = save_socket_id_to_file(socket_id, socket_file); if (error.Fail()) { fprintf(stderr, "failed to write socket id to %s: %s\n", socket_file.GetPath().c_str(), error.AsCString()); @@ -449,69 +524,103 @@ int main_platform(int argc, char *argv[]) { } } - GDBRemoteCommunicationServerPlatform platform( - acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); + lldb_private::Args gdb_inferior_arguments(inferior_arguments); + GDBRemoteCommunicationServerPlatform platform(protocol, 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); - if (error.Fail()) { - WithColor::error() << error.AsCString() << '\n'; - exit(socket_error); + if (protocol == Socket::ProtocolTcp) { + llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> platform_handles = + sock_platform->Accept( + *g_main_loop, + [progname, gdbserver_port, port_offset, inferior_arguments, + log_file, log_channels](std::unique_ptr<TCPSocket> sock_up) { + // If not running as a server, this process will not accept + // connections while a connection is active. + if (g_server || g_single_pid == LLDB_INVALID_PROCESS_ID) { + printf("Platform connection established.\n"); + Status error = spawn_process( + progname, sock_up.get(), gdbserver_port, port_offset, + inferior_arguments, log_file, log_channels); + if (error.Fail()) { + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString()); + WithColor::error() + << "spawn_process failed: " << error.AsCString() << "\n"; + if (!g_server) { + g_terminate = true; + g_main_loop->AddPendingCallback( + [](MainLoopBase &loop) { loop.RequestTermination(); }); + } + } + } + sock_up.reset(); + }); + if (!platform_handles) { + printf("Failed to accept platform: %s\n", + llvm::toString(platform_handles.takeError()).c_str()); + return socket_error; } - 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 { - const Socket *conn_socket = - static_cast<const Socket *>(conn->GetReadObject().get()); - error = - spawn_process(progname, conn_socket, *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"; - } - } - // 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(); - - // When not running in server mode, use all available ports - platform.SetPortMap(std::move(gdbserver_portmap)); + llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> gdb_handles = + sock_gdb->Accept(*g_main_loop, [&platform, &gdb_inferior_arguments]( + std::unique_ptr<TCPSocket> sock_up) { + printf("Gdb connection established.\n"); + Status error; + Log *log = GetLog(LLDBLog::Platform); + SharedSocket shared_socket(sock_up.get(), 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()); + + sock_up.reset(); + }); + if (!gdb_handles) { + printf("Failed to accept gdb: %s\n", + llvm::toString(gdb_handles.takeError()).c_str()); + return socket_error; } - platform.SetConnection(std::unique_ptr<Connection>(conn)); + do { + g_main_loop->Run(); + } while (!g_terminate); + + gdb_handles->clear(); + platform_handles->clear(); + + g_main_loop.reset(); + sock_gdb.reset(); + sock_platform.reset(); + } else { + Socket *conn_up = nullptr; + error = sock_named->Accept(conn_up); + if (error.Fail()) { + printf("Failed to accept: %s\n", error.AsCString()); + return socket_error; + } + printf("Connection established.\n"); + platform.SetConnection( + std::unique_ptr<Connection>(new ConnectionFileDescriptor(conn_up))); client_handle(platform, inferior_arguments); - } while (g_server); + sock_named.reset(); + } fprintf(stderr, "lldb-server exiting...\n"); 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/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)); 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 aedb0edc01ce03086ed30728b8160bb6b7e9cf33 Mon Sep 17 00:00:00 2001 From: Dmitry Vasilyev <dvassil...@accesssoftek.com> Date: Wed, 4 Sep 2024 20:52:51 +0400 Subject: [PATCH 2/2] Updated as recommended. --- .../posix/ConnectionFileDescriptorPosix.cpp | 21 +- .../gdb-remote/GDBRemoteCommunication.cpp | 8 +- .../gdb-remote/GDBRemoteCommunication.h | 2 +- .../GDBRemoteCommunicationServerPlatform.cpp | 6 +- lldb/tools/lldb-server/lldb-gdbserver.cpp | 16 +- lldb/tools/lldb-server/lldb-platform.cpp | 351 +++++++++--------- 6 files changed, 203 insertions(+), 201 deletions(-) diff --git a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp index 0c9672ea6aa753..6a40f66be39b18 100644 --- a/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp +++ b/lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp @@ -162,10 +162,8 @@ ConnectionFileDescriptor::Connect(llvm::StringRef path, .Case("unix-connect", &ConnectionFileDescriptor::ConnectNamedSocket) .Case("unix-abstract-connect", &ConnectionFileDescriptor::ConnectAbstractSocket) -#if LLDB_ENABLE_POSIX || defined(_WIN32) - .Case("fd", &ConnectionFileDescriptor::ConnectFD) -#endif #if LLDB_ENABLE_POSIX + .Case("fd", &ConnectionFileDescriptor::ConnectFD) .Case("file", &ConnectionFileDescriptor::ConnectFile) .Case("serial", &ConnectionFileDescriptor::ConnectSerialPort) #endif @@ -669,22 +667,7 @@ ConnectionStatus ConnectionFileDescriptor::ConnectFD(llvm::StringRef s, socket_id_callback_type socket_id_callback, Status *error_ptr) { -#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 = Status::FromErrorStringWithFormat( - "invalid file descriptor: \"%s\"", s.str().c_str()); - m_io_sp.reset(); - return eConnectionStatusError; -#elif LLDB_ENABLE_POSIX +#if LLDB_ENABLE_POSIX // Just passing a native file descriptor within this current process that // is already opened (possibly from a service or other source). int fd = -1; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 514fd8891674bf..4196d6b7eaae81 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -879,11 +879,11 @@ lldb::thread_result_t GDBRemoteCommunication::ListenThread() { return {}; } -bool GDBRemoteCommunication::GetDebugserverPath( - Platform *platform, FileSpec &debugserver_file_spec) { +FileSpec GDBRemoteCommunication::GetDebugserverPath(Platform *platform) { Log *log = GetLog(GDBRLog::Process); // If we locate debugserver, keep that located version around static FileSpec g_debugserver_file_spec; + FileSpec debugserver_file_spec; Environment host_env = Host::GetEnvironment(); @@ -935,7 +935,7 @@ bool GDBRemoteCommunication::GetDebugserverPath( } } } - return debugserver_exists; + return debugserver_file_spec; } Status GDBRemoteCommunication::StartDebugserverProcess( @@ -947,7 +947,7 @@ Status GDBRemoteCommunication::StartDebugserverProcess( Status error; FileSpec &debugserver_file_spec = launch_info.GetExecutableFile(); - if (GetDebugserverPath(platform, debugserver_file_spec)) { + if (debugserver_file_spec = GetDebugserverPath(platform)) { char debugserver_path[PATH_MAX]; debugserver_file_spec.GetPath(debugserver_path, sizeof(debugserver_path)); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index cd022821e6b1ce..754797ba7f5045 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -148,7 +148,7 @@ 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); + FileSpec GetDebugserverPath(Platform *platform); // Start a debugserver instance on the current host using the // supplied connection URL. diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index 820bbd4d92ec22..67228ef275ba4f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -102,8 +102,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( 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)) + if (!GetDebugserverPath(nullptr)) return Status::FromErrorString("unable to locate debugserver"); return Status(); } @@ -122,8 +121,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( // Spawn a debugserver and try to get the port it listens to. ProcessLaunchInfo debugserver_launch_info; Log *log = GetLog(LLDBLog::Platform); - LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), - (int64_t)fd); + LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd); // Do not run in a new session so that it can not linger after the platform // closes. diff --git a/lldb/tools/lldb-server/lldb-gdbserver.cpp b/lldb/tools/lldb-server/lldb-gdbserver.cpp index a95f456804f786..3b80e922be73a9 100644 --- a/lldb/tools/lldb-server/lldb-gdbserver.cpp +++ b/lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -24,8 +24,8 @@ #include "lldb/Host/ConnectionFileDescriptor.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/Pipe.h" -#include "lldb/Host/Socket.h" #include "lldb/Host/common/NativeProcessProtocol.h" +#include "lldb/Host/common/TCPSocket.h" #include "lldb/Target/Process.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Status.h" @@ -202,18 +202,24 @@ void ConnectToRemote(MainLoop &mainloop, std::string url; if (connection_fd != SharedSocket::kInvalidFD) { - NativeSocket fd; - error = SharedSocket::GetNativeSocket(connection_fd, fd); +#ifdef _WIN32 + NativeSocket sockfd; + error = SharedSocket::GetNativeSocket(connection_fd, sockfd); 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(); + connection_up = + std::unique_ptr<Connection>(new ConnectionFileDescriptor(new TCPSocket( + sockfd, /*should_close=*/true, /*child_processes_inherit=*/false))); +#else + url = llvm::formatv("fd://{0}", connection_fd).str(); // Create the connection. -#if LLDB_ENABLE_POSIX && !defined _WIN32 +#if LLDB_ENABLE_POSIX ::fcntl(connection_fd, F_SETFD, FD_CLOEXEC); +#endif #endif } else if (!host_and_port.empty()) { llvm::Expected<std::string> url_exp = diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 2eb0e264d5bdd1..ab374fc2b329aa 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -47,15 +47,18 @@ using namespace lldb_private::lldb_server; using namespace lldb_private::process_gdb_remote; using namespace llvm; -// option descriptors for getopt_long_only() - +// 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. +static const int backlog = 100; +static const int socket_error = -1; static int g_debug = 0; static int g_verbose = 0; static int g_server = 0; static std::atomic<lldb::pid_t> g_single_pid = LLDB_INVALID_PROCESS_ID; -static std::atomic<bool> g_terminate = false; -static std::unique_ptr<MainLoop> g_main_loop(new MainLoop()); +static std::unique_ptr<MainLoop> g_main_loop; +// option descriptors for getopt_long_only() static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, {"verbose", no_argument, &g_verbose, 1}, @@ -153,16 +156,13 @@ static void client_handle(GDBRemoteCommunicationServerPlatform &platform, printf("Disconnected.\n"); } +// This callback is called from the MonitorChildProcessThread. 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 - if (!g_terminate) { - g_terminate = true; - if (g_main_loop) { - g_main_loop->AddPendingCallback( - [](MainLoopBase &loop) { loop.RequestTermination(); }); - } - } + // If not running as a server and the platform connection is closed + if (g_single_pid != LLDB_INVALID_PROCESS_ID && g_single_pid == pid && + g_main_loop) { + g_main_loop->AddPendingCallback( + [](MainLoopBase &loop) { loop.RequestTermination(); }); } } @@ -253,6 +253,159 @@ static Status spawn_process(const char *progname, const Socket *conn_socket, return Status(); } +static int platform_tcp(const char *progname, std::string &hostname, + uint16_t platform_port, uint16_t gdb_port, + uint16_t port_offset, FileSpec &socket_file, + const lldb_private::Args &args, + const std::string &log_file, + const StringRef log_channels) { + std::unique_ptr<TCPSocket> sock_platform = std::make_unique<TCPSocket>( + /*should_close=*/true, /*child_processes_inherit=*/false); + Status error = sock_platform->Listen( + llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog); + if (error.Fail()) { + printf("Failed to listen platform: %s\n", error.AsCString()); + return socket_error; + } + if (platform_port == 0) + platform_port = sock_platform->GetLocalPortNumber(); + + if (socket_file) { + error = save_socket_id_to_file( + platform_port ? llvm::to_string(platform_port) : "", socket_file); + if (error.Fail()) { + fprintf(stderr, "failed to write socket id to %s: %s\n", + socket_file.GetPath().c_str(), error.AsCString()); + return 1; + } + } + + std::unique_ptr<TCPSocket> sock_gdb = std::make_unique<TCPSocket>( + /*should_close=*/true, /*child_processes_inherit=*/false); + error = sock_gdb->Listen(llvm::formatv("{0}:{1}", hostname, gdb_port).str(), + backlog); + if (error.Fail()) { + printf("Failed to listen gdb: %s\n", error.AsCString()); + return socket_error; + } + if (gdb_port == 0) + gdb_port = sock_gdb->GetLocalPortNumber(); + + lldb_private::Args gdb_inferior_arguments(args); + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, gdb_port); + if (port_offset > 0) + platform.SetPortOffset(port_offset); + + g_main_loop.reset(new MainLoop()); + { + llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> platform_handles = + sock_platform->Accept( + *g_main_loop, [progname, gdb_port, port_offset, args, log_file, + log_channels](std::unique_ptr<TCPSocket> sock_up) { + // If not running as a server, this process will not accept + // connections while a connection is active. + if (g_server || g_single_pid == LLDB_INVALID_PROCESS_ID) { + printf("Connection established.\n"); + Status error = + spawn_process(progname, sock_up.get(), gdb_port, + port_offset, args, log_file, log_channels); + if (error.Fail()) { + Log *log = GetLog(LLDBLog::Platform); + LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString()); + WithColor::error() + << "spawn_process failed: " << error.AsCString() << "\n"; + if (!g_server) { + g_main_loop->AddPendingCallback( + [](MainLoopBase &loop) { loop.RequestTermination(); }); + } + } + } + }); + if (!platform_handles) { + printf("Failed to accept platform: %s\n", + llvm::toString(platform_handles.takeError()).c_str()); + return socket_error; + } + + llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> gdb_handles = + sock_gdb->Accept(*g_main_loop, [&platform, &gdb_inferior_arguments]( + std::unique_ptr<TCPSocket> sock_up) { + Log *log = GetLog(LLDBLog::Platform); + Status error; + SharedSocket shared_socket(sock_up.get(), error); + if (error.Fail()) { + LLDB_LOGF(log, "gdbserver SharedSocket failed: %s", + error.AsCString()); + return; + } + 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.Fail()) { + Host::Kill(child_pid, SIGTERM); + LLDB_LOGF(log, "gdbserver CompleteSending failed: %s", + error.AsCString()); + return; + } + // Use gdb inferior arguments once. + gdb_inferior_arguments.Clear(); + } + }); + if (!gdb_handles) { + printf("Failed to accept gdb: %s\n", + llvm::toString(gdb_handles.takeError()).c_str()); + return socket_error; + } + + g_main_loop->Run(); + } + return 0; +} + +static int platform_named(Socket::SocketProtocol protocol, std::string &name, + FileSpec &socket_file, + const lldb_private::Args &args) { + Status error; + std::unique_ptr<Socket> sock_named = + Socket::Create(protocol, /*child_processes_inherit=*/false, error); + if (error.Fail()) { + fprintf(stderr, "Failed to create socket: %s", error.AsCString()); + return socket_error; + } + error = sock_named->Listen(name, backlog); + if (error.Fail()) { + printf("Failed to listen: %s\n", error.AsCString()); + return socket_error; + } + + if (socket_file) { + error = save_socket_id_to_file(name, socket_file); + if (error.Fail()) { + fprintf(stderr, "failed to write socket id to %s: %s\n", + socket_file.GetPath().c_str(), error.AsCString()); + return 1; + } + } + + Socket *conn_up = nullptr; + error = sock_named->Accept(conn_up); + if (error.Fail()) { + printf("Failed to accept: %s\n", error.AsCString()); + return socket_error; + } + printf("Connection established.\n"); + + GDBRemoteCommunicationServerPlatform platform(protocol, 0); + platform.SetConnection( + std::unique_ptr<Connection>(new ConnectionFileDescriptor(conn_up))); + client_handle(platform, args); + return 0; +} + // main int main_platform(int argc, char *argv[]) { const char *progname = argv[0]; @@ -280,7 +433,6 @@ int main_platform(int argc, char *argv[]) { FileSpec socket_file; bool show_usage = false; int option_error = 0; - int socket_error = -1; std::string short_options(OptionParser::GetShortOptionString(g_long_options)); @@ -382,10 +534,9 @@ 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"); @@ -425,17 +576,9 @@ int main_platform(int argc, char *argv[]) { return 1; } - // 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<Socket> sock_named; - std::unique_ptr<TCPSocket> sock_platform; - std::unique_ptr<TCPSocket> sock_gdb; + Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain; std::string hostname; uint16_t platform_port = 0; - Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain; // Try to match socket name as URL - e.g., tcp://localhost:5555 if (std::optional<URI> uri = URI::Parse(listen_host_port)) { @@ -463,166 +606,38 @@ int main_platform(int argc, char *argv[]) { hostname = listen_host_port; } - std::string socket_id; + int res; if (protocol == Socket::ProtocolTcp) { if (platform_port != 0 && platform_port == gdbserver_port) { fprintf(stderr, "The same platform and gdb ports %u.", platform_port); return socket_error; } - - sock_platform = std::make_unique<TCPSocket>( - /*should_close=*/true, /*child_processes_inherit=*/false); - error = sock_platform->Listen( - llvm::formatv("{0}:{1}", hostname, platform_port).str(), backlog); - if (error.Fail()) { - printf("Failed to listen platform: %s\n", error.AsCString()); + res = platform_tcp(progname, hostname, platform_port, gdbserver_port, + port_offset, socket_file, inferior_arguments, log_file, + log_channels); + } else { + if (gdbserver_port) { + fprintf(stderr, + "--gdbserver-port %u is redundant for non-tcp protocol %s.", + gdbserver_port, Socket::FindSchemeByProtocol(protocol)); return socket_error; } - if (platform_port == 0) - platform_port = sock_platform->GetLocalPortNumber(); - if (platform_port != 0) - socket_id = llvm::to_string(platform_port); - - sock_gdb = std::make_unique<TCPSocket>(/*should_close=*/true, - /*child_processes_inherit=*/false); - error = sock_gdb->Listen( - llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(), backlog); - if (error.Fail()) { - printf("Failed to listen gdb: %s\n", error.AsCString()); + if (port_offset) { + fprintf(stderr, "--port-offset %u is redundant for non-tcp protocol %s.", + port_offset, Socket::FindSchemeByProtocol(protocol)); return socket_error; } - if (gdbserver_port == 0) - gdbserver_port = sock_gdb->GetLocalPortNumber(); - } else { if (g_server) { fprintf(stderr, - "Ambiguous parameters --server --listen %s\n" + "Ambiguous parameters --server --listen %s.\n" "The protocol must be tcp for the server mode.", listen_host_port.c_str()); return socket_error; } - sock_named = - Socket::Create(protocol, /*child_processes_inherit=*/false, error); - if (error.Fail()) { - fprintf(stderr, "Failed to create socket: %s", error.AsCString()); - return socket_error; - } - error = sock_named->Listen(hostname, backlog); - if (error.Fail()) { - printf("Failed to listen: %s\n", error.AsCString()); - return socket_error; - } - socket_id = hostname; - } - - if (socket_file) { - error = save_socket_id_to_file(socket_id, socket_file); - if (error.Fail()) { - fprintf(stderr, "failed to write socket id to %s: %s\n", - socket_file.GetPath().c_str(), error.AsCString()); - return 1; - } - } - - lldb_private::Args gdb_inferior_arguments(inferior_arguments); - GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port); - if (port_offset > 0) - platform.SetPortOffset(port_offset); - - if (protocol == Socket::ProtocolTcp) { - llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> platform_handles = - sock_platform->Accept( - *g_main_loop, - [progname, gdbserver_port, port_offset, inferior_arguments, - log_file, log_channels](std::unique_ptr<TCPSocket> sock_up) { - // If not running as a server, this process will not accept - // connections while a connection is active. - if (g_server || g_single_pid == LLDB_INVALID_PROCESS_ID) { - printf("Platform connection established.\n"); - Status error = spawn_process( - progname, sock_up.get(), gdbserver_port, port_offset, - inferior_arguments, log_file, log_channels); - if (error.Fail()) { - Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "spawn_process failed: %s", error.AsCString()); - WithColor::error() - << "spawn_process failed: " << error.AsCString() << "\n"; - if (!g_server) { - g_terminate = true; - g_main_loop->AddPendingCallback( - [](MainLoopBase &loop) { loop.RequestTermination(); }); - } - } - } - sock_up.reset(); - }); - if (!platform_handles) { - printf("Failed to accept platform: %s\n", - llvm::toString(platform_handles.takeError()).c_str()); - return socket_error; - } - - llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> gdb_handles = - sock_gdb->Accept(*g_main_loop, [&platform, &gdb_inferior_arguments]( - std::unique_ptr<TCPSocket> sock_up) { - printf("Gdb connection established.\n"); - Status error; - Log *log = GetLog(LLDBLog::Platform); - SharedSocket shared_socket(sock_up.get(), 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()); - - sock_up.reset(); - }); - if (!gdb_handles) { - printf("Failed to accept gdb: %s\n", - llvm::toString(gdb_handles.takeError()).c_str()); - return socket_error; - } - - do { - g_main_loop->Run(); - } while (!g_terminate); - - gdb_handles->clear(); - platform_handles->clear(); - - g_main_loop.reset(); - sock_gdb.reset(); - sock_platform.reset(); - } else { - Socket *conn_up = nullptr; - error = sock_named->Accept(conn_up); - if (error.Fail()) { - printf("Failed to accept: %s\n", error.AsCString()); - return socket_error; - } - printf("Connection established.\n"); - platform.SetConnection( - std::unique_ptr<Connection>(new ConnectionFileDescriptor(conn_up))); - client_handle(platform, inferior_arguments); - sock_named.reset(); + res = platform_named(protocol, hostname, socket_file, inferior_arguments); } fprintf(stderr, "lldb-server exiting...\n"); - return 0; + return res; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
