Author: Dmitry Vasilyev Date: 2024-10-03T21:00:47+04:00 New Revision: 2e89312419c5f7875c947fcf79ea0446cf65a287
URL: https://github.com/llvm/llvm-project/commit/2e89312419c5f7875c947fcf79ea0446cf65a287 DIFF: https://github.com/llvm/llvm-project/commit/2e89312419c5f7875c947fcf79ea0446cf65a287.diff LOG: [lldb] Removed gdbserver ports map from lldb-server (#104238) 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. Added: Modified: lldb/docs/man/lldb-server.rst lldb/docs/resources/qemu-testing.rst lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h lldb/test/API/commands/platform/launchgdbserver/TestPlatformLaunchGDBServer.py lldb/tools/lldb-server/CMakeLists.txt lldb/tools/lldb-server/lldb-gdbserver.cpp lldb/tools/lldb-server/lldb-platform.cpp lldb/unittests/Process/gdb-remote/CMakeLists.txt Removed: lldb/test/Shell/lldb-server/TestGdbserverPort.test lldb/tools/lldb-server/Acceptor.cpp lldb/tools/lldb-server/Acceptor.h 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..da6b2c71818427 100644 --- a/lldb/docs/man/lldb-server.rst +++ b/lldb/docs/man/lldb-server.rst @@ -147,21 +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. - -.. option:: --port-offset <offset> - - Add the specified offset to port numbers returned by server. This is useful - if the server is running behind a firewall, and a range of ports is redirected - to it with an offset. + Define a port to be used for gdb-server connections. This port is used for + multiple connections. EXAMPLES -------- 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/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index c60a83d351ba09..89fdfa74bc025c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -44,79 +44,11 @@ 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(); -} - // 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) { RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_qC, @@ -160,67 +92,55 @@ 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::FromError(available_port.takeError()); - } - - // Spawn a new thread to accept the port that gets bound after binding to - // port 0 (zero). + const lldb_private::Args &args, lldb::pid_t &pid, std::string &socket_name, + shared_fd_t fd) { + std::ostringstream url; + if (fd == SharedSocket::kInvalidFD) { + if (m_socket_protocol == Socket::ProtocolTcp) { + // Just check that GDBServer exists. GDBServer must be launched after + // accepting the connection. + if (!GetDebugserverPath(nullptr)) + return Status::FromErrorString("unable to locate debugserver"); + return Status(); + } - // ignore the hostname send from the remote end, just use the ip address that - // we're currently communicating with as the hostname + // debugserver does not accept the URL scheme prefix. +#if !defined(__APPLE__) + url << Socket::FindSchemeByProtocol(m_socket_protocol) << "://"; +#endif + socket_name = GetDomainSocketPath("gdbserver").GetPath(); + url << socket_name; + } else { + if (m_socket_protocol != Socket::ProtocolTcp) + return Status::FromErrorString("protocol must be tcp"); + } // Spawn a debugserver and try to get the port it listens to. ProcessLaunchInfo debugserver_launch_info; - if (hostname.empty()) - hostname = "127.0.0.1"; - Log *log = GetLog(LLDBLog::Platform); - LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(), - *port); + LLDB_LOG(log, "Launching debugserver url='{0}', fd={1}...", url.str(), fd); // Do not run in a new session so that it can not linger after the platform // closes. debugserver_launch_info.SetLaunchInSeparateProcessGroup(false); debugserver_launch_info.SetMonitorProcessCallback( - std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped, - this, std::placeholders::_1)); - - std::ostringstream url; -// debugserver does not accept the URL scheme prefix. -#if !defined(__APPLE__) - url << m_socket_scheme << "://"; -#endif - uint16_t *port_ptr = &*port; - if (m_socket_protocol == Socket::ProtocolTcp) { - std::string platform_uri = GetConnection()->GetURI(); - std::optional<URI> parsed_uri = URI::Parse(platform_uri); - url << '[' << parsed_uri->hostname.str() << "]:" << *port; - } else { - socket_name = GetDomainSocketPath("gdbserver").GetPath(); - url << socket_name; - port_ptr = nullptr; - } + [](lldb::pid_t, int, int) {}); - Status error = StartDebugserverProcess(url.str().c_str(), nullptr, - debugserver_launch_info, port_ptr, - &args, SharedSocket::kInvalidFD); + Status error = StartDebugserverProcess( + 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(); + 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; } @@ -251,27 +171,18 @@ 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); - response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid, - *port + m_port_offset); + uint16_t gdbserver_port = socket_name.empty() ? m_gdbserver_port : 0; + response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid, gdbserver_port); if (!socket_name.empty()) { response.PutCString("socket_name:"); response.PutStringAsRawHex8(socket_name); @@ -291,13 +202,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)); @@ -318,13 +231,10 @@ 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 (!SpawnedProcessIsRunning(pid)) { + // not a pid we know about + return SendErrorResponse(10); } // go ahead and attempt to kill the spawned process @@ -334,12 +244,23 @@ GDBRemoteCommunicationServerPlatform::Handle_qKillSpawnedProcess( return SendErrorResponse(11); } +void GDBRemoteCommunicationServerPlatform::AddSpawnedProcess(lldb::pid_t pid) { + assert(pid != LLDB_INVALID_PROCESS_ID); + std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); + m_spawned_pids.insert(pid); +} + +bool GDBRemoteCommunicationServerPlatform::SpawnedProcessIsRunning( + lldb::pid_t pid) { + std::lock_guard<std::recursive_mutex> guard(m_spawned_pids_mutex); + return (m_spawned_pids.find(pid) != m_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 (!SpawnedProcessIsRunning(pid)) { + // it seems the process has been finished recently + return true; } // first try a SIGTERM (standard kill) @@ -347,46 +268,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 (!SpawnedProcessIsRunning(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 (!SpawnedProcessIsRunning(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 (!SpawnedProcessIsRunning(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 !SpawnedProcessIsRunning(pid); } GDBRemoteCommunication::PacketResult @@ -522,7 +427,6 @@ 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); } @@ -552,19 +456,11 @@ 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); - } + 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; @@ -595,13 +491,7 @@ GDBRemoteCommunicationServerPlatform::GetDomainSocketPath(const char *prefix) { return FileSpec(socket_path.c_str()); } -void GDBRemoteCommunicationServerPlatform::SetPortOffset(uint16_t port_offset) { - m_port_offset = 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..beee61ea8048e0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -25,90 +25,27 @@ 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; - PortMap m_port_map; - uint16_t m_port_offset; - struct { - lldb::pid_t pid; - uint16_t port; - std::string socket_name; - } m_pending_gdb_server; + uint16_t m_gdbserver_port; + std::optional<std::string> m_pending_gdb_server_socket_name; PacketResult Handle_qLaunchGDBServer(StringExtractorGDBRemote &packet); @@ -130,6 +67,8 @@ class GDBRemoteCommunicationServerPlatform private: bool KillSpawnedProcess(lldb::pid_t pid); + bool SpawnedProcessIsRunning(lldb::pid_t pid); + void AddSpawnedProcess(lldb::pid_t pid); void DebugserverProcessReaped(lldb::pid_t pid); 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 d7503112697ef5..bc93c837a107bd 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,10 +202,21 @@ void ConnectToRemote(MainLoop &mainloop, std::string url; if (connection_fd != SharedSocket::kInvalidFD) { +#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); + } + 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 ::fcntl(connection_fd, F_SETFD, FD_CLOEXEC); #endif } else if (!host_and_port.empty()) { diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index c021bc7e30194d..d1f925685dff91 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -26,18 +26,22 @@ #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" +#if LLDB_ENABLE_POSIX +#include "lldb/Host/posix/DomainSocket.h" +#endif #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; @@ -45,19 +49,22 @@ 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; +// option descriptors for getopt_long_only() static struct option g_long_options[] = { {"debug", no_argument, &g_debug, 1}, {"verbose", no_argument, &g_verbose, 1}, {"log-file", required_argument, nullptr, 'l'}, {"log-channels", required_argument, nullptr, 'c'}, {"listen", required_argument, nullptr, 'L'}, - {"port-offset", required_argument, nullptr, 'p'}, {"gdbserver-port", required_argument, nullptr, 'P'}, {"min-gdbserver-port", required_argument, nullptr, 'm'}, {"max-gdbserver-port", required_argument, nullptr, 'M'}, @@ -98,6 +105,55 @@ static void display_usage(const char *progname, const char *subcommand) { exit(0); } +static Status parse_listen_host_port(Socket::SocketProtocol &protocol, + const std::string &listen_host_port, + std::string &address, + uint16_t &platform_port, + std::string &gdb_address, + const uint16_t gdbserver_port) { + std::string hostname; + // 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)) { + return Status::FromErrorStringWithFormat( + "Unknown protocol scheme \"%s\".", uri->scheme.str().c_str()); + } + if (protocol == Socket::ProtocolTcp) { + hostname = uri->hostname; + if (uri->port) { + platform_port = *(uri->port); + } + } else + address = 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 + address = listen_host_port; + } + + if (protocol == Socket::ProtocolTcp) { + if (platform_port != 0 && platform_port == gdbserver_port) { + return Status::FromErrorStringWithFormat( + "The same platform and gdb ports %u.", platform_port); + } + address = llvm::formatv("{0}:{1}", hostname, platform_port).str(); + gdb_address = llvm::formatv("{0}:{1}", hostname, gdbserver_port).str(); + } else { + if (gdbserver_port) { + return Status::FromErrorStringWithFormat( + "--gdbserver-port %u is redundant for non-tcp protocol %s.", + gdbserver_port, Socket::FindSchemeByProtocol(protocol)); + } + } + return Status(); +} + static Status save_socket_id_to_file(const std::string &socket_id, const FileSpec &file_spec) { FileSpec temp_file_spec(file_spec.GetDirectory().GetStringRef()); @@ -119,6 +175,59 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static Status ListenGdbConnectionsIfNeeded( + const Socket::SocketProtocol protocol, std::unique_ptr<TCPSocket> &gdb_sock, + const std::string &gdb_address, uint16_t &gdbserver_port) { + if (protocol != Socket::ProtocolTcp) + return Status(); + + gdb_sock = std::make_unique<TCPSocket>( + /*should_close=*/true, /*child_processes_inherit=*/false); + Status error = gdb_sock->Listen(gdb_address, backlog); + if (error.Fail()) + return error; + + if (gdbserver_port == 0) + gdbserver_port = gdb_sock->GetLocalPortNumber(); + + return Status(); +} + +static llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> +AcceptGdbConnectionsIfNeeded(const Socket::SocketProtocol protocol, + std::unique_ptr<TCPSocket> &gdb_sock, + MainLoop &main_loop, const uint16_t gdbserver_port, + const lldb_private::Args &args) { + if (protocol != Socket::ProtocolTcp) + return std::vector<MainLoopBase::ReadHandleUP>(); + + return gdb_sock->Accept(main_loop, [gdbserver_port, + &args](std::unique_ptr<Socket> 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; + GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, + gdbserver_port); + error = platform.LaunchGDBServer(args, 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; + } + } + }); +} + static void client_handle(GDBRemoteCommunicationServerPlatform &platform, const lldb_private::Args &args) { if (!platform.IsConnected()) @@ -126,13 +235,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,19 +257,11 @@ 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 Status spawn_process(const char *progname, const Socket *conn_socket, - uint16_t gdb_port, uint16_t port_offset, - const lldb_private::Args &args, + uint16_t gdb_port, const lldb_private::Args &args, const std::string &log_file, - const StringRef log_channels) { + const StringRef log_channels, MainLoop &main_loop, + std::promise<void> &child_exited) { Status error; SharedSocket shared_socket(conn_socket, error); if (error.Fail()) @@ -176,14 +275,14 @@ 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)); } - if (port_offset > 0) { - self_args.AppendArgument(llvm::StringRef("--port-offset")); - self_args.AppendArgument(llvm::to_string(port_offset)); - } if (!log_file.empty()) { self_args.AppendArgument(llvm::StringRef("--log-file")); self_args.AppendArgument(log_file); @@ -198,7 +297,16 @@ static Status spawn_process(const char *progname, const Socket *conn_socket, } launch_info.SetLaunchInSeparateProcessGroup(false); - launch_info.SetMonitorProcessCallback(&spawn_process_reaped); + + if (g_server) + launch_info.SetMonitorProcessCallback([](lldb::pid_t, int, int) {}); + else + launch_info.SetMonitorProcessCallback( + [&child_exited, &main_loop](lldb::pid_t, int, int) { + main_loop.AddPendingCallback( + [](MainLoopBase &loop) { loop.RequestTermination(); }); + child_exited.set_value(); + }); // Copy the current environment. launch_info.GetEnvironment() = Host::GetEnvironment(); @@ -229,11 +337,6 @@ 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); @@ -264,14 +367,11 @@ 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 port_offset = 0; + uint16_t gdbserver_port = 0; FileSpec socket_file; bool show_usage = false; int option_error = 0; - int socket_error = -1; std::string short_options(OptionParser::GetShortOptionString(g_long_options)); @@ -307,21 +407,6 @@ int main_platform(int argc, char *argv[]) { socket_file.SetFile(optarg, FileSpec::Style::native); break; - case 'p': { - if (!llvm::to_integer(optarg, port_offset)) { - WithColor::error() << "invalid port offset string " << optarg << "\n"; - option_error = 4; - break; - } - if (port_offset < LOW_PORT || port_offset > HIGH_PORT) { - WithColor::error() << llvm::formatv( - "port offset {0} is not in the " - "valid user port range of {1} - {2}\n", - port_offset, LOW_PORT, HIGH_PORT); - option_error = 5; - } - } break; - case 'P': case 'm': case 'M': { @@ -331,20 +416,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 +443,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,55 +458,82 @@ int main_platform(int argc, char *argv[]) { lldb_private::Args inferior_arguments; inferior_arguments.SetArguments(argc, const_cast<const char **>(argv)); + Socket::SocketProtocol protocol = Socket::ProtocolUnixDomain; + if (fd != SharedSocket::kInvalidFD) { // Child process will handle the connection and exit. + if (gdbserver_port) + protocol = Socket::ProtocolTcp; + 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; - } - NativeSocket socket; - error = SharedSocket::GetNativeSocket(fd, socket); + NativeSocket sockfd; + error = SharedSocket::GetNativeSocket(fd, sockfd); if (error.Fail()) { LLDB_LOGF(log, "lldb-platform child: %s", error.AsCString()); return socket_error; } - GDBRemoteCommunicationServerPlatform platform(Socket::ProtocolTcp, "tcp"); - if (port_offset > 0) - platform.SetPortOffset(port_offset); - platform.SetPortMap(std::move(gdbserver_portmap)); + GDBRemoteCommunicationServerPlatform platform(protocol, gdbserver_port); + Socket *socket; + if (protocol == Socket::ProtocolTcp) + socket = new TCPSocket(sockfd, /*should_close=*/true, + /*child_processes_inherit=*/false); + else { +#if LLDB_ENABLE_POSIX + socket = new DomainSocket(sockfd, /*should_close=*/true, + /*child_processes_inherit=*/false); +#else + WithColor::error() << "lldb-platform child: Unix domain sockets are not " + "supported on this platform."; + return socket_error; +#endif + } platform.SetConnection( - std::unique_ptr<Connection>(new ConnectionFileDescriptor( - new TCPSocket(socket, /*should_close=*/true, - /*child_processes_inherit=*/false)))); + std::unique_ptr<Connection>(new ConnectionFileDescriptor(socket))); client_handle(platform, inferior_arguments); return 0; } - const bool children_inherit_listen_socket = false; - // the test suite makes many connections in parallel, let's not miss any. - // The highest this should get reasonably is a function of the number - // of target CPUs. For now, let's just use 100. - const int backlog = 100; + 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; + } - std::unique_ptr<Acceptor> acceptor_up(Acceptor::Create( - listen_host_port, children_inherit_listen_socket, error)); + std::string address; + std::string gdb_address; + uint16_t platform_port = 0; + error = parse_listen_host_port(protocol, listen_host_port, address, + platform_port, gdb_address, gdbserver_port); if (error.Fail()) { - fprintf(stderr, "failed to create acceptor: %s", error.AsCString()); - exit(socket_error); + printf("Failed to parse listen address: %s\n", error.AsCString()); + return socket_error; } - error = acceptor_up->Listen(backlog); + std::unique_ptr<Socket> platform_sock = + Socket::Create(protocol, /*child_processes_inherit=*/false, error); if (error.Fail()) { - printf("failed to listen: %s\n", error.AsCString()); - exit(socket_error); + printf("Failed to create platform socket: %s\n", error.AsCString()); + return socket_error; } + error = platform_sock->Listen(address, backlog); + if (error.Fail()) { + printf("Failed to listen platform: %s\n", error.AsCString()); + return socket_error; + } + if (protocol == Socket::ProtocolTcp && platform_port == 0) + platform_port = + static_cast<TCPSocket *>(platform_sock.get())->GetLocalPortNumber(); + if (socket_file) { - error = - save_socket_id_to_file(acceptor_up->GetLocalSocketId(), socket_file); + error = save_socket_id_to_file( + protocol == Socket::ProtocolTcp + ? (platform_port ? llvm::to_string(platform_port) : "") + : address, + 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 +541,58 @@ int main_platform(int argc, char *argv[]) { } } - GDBRemoteCommunicationServerPlatform platform( - acceptor_up->GetSocketProtocol(), acceptor_up->GetSocketScheme()); - if (port_offset > 0) - platform.SetPortOffset(port_offset); + std::unique_ptr<TCPSocket> gdb_sock; + // Update gdbserver_port if it is still 0 and protocol is tcp. + error = ListenGdbConnectionsIfNeeded(protocol, gdb_sock, gdb_address, + gdbserver_port); + if (error.Fail()) { + printf("Failed to listen gdb: %s\n", error.AsCString()); + return socket_error; + } - 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); + std::promise<void> child_exited; + MainLoop main_loop; + { + llvm::Expected<std::vector<MainLoopBase::ReadHandleUP>> platform_handles = + platform_sock->Accept( + main_loop, [progname, gdbserver_port, &inferior_arguments, log_file, + log_channels, &main_loop, &child_exited, + &platform_handles](std::unique_ptr<Socket> sock_up) { + printf("Connection established.\n"); + Status error = spawn_process( + progname, sock_up.get(), gdbserver_port, inferior_arguments, + log_file, log_channels, main_loop, child_exited); + 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) { + main_loop.RequestTermination(); + child_exited.set_value(); + } + } + if (!g_server) + platform_handles->clear(); + }); + 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 = + AcceptGdbConnectionsIfNeeded(protocol, gdb_sock, main_loop, + gdbserver_port, inferior_arguments); + 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)); - client_handle(platform, inferior_arguments); - } while (g_server); + main_loop.Run(); + } + child_exited.get_future().get(); 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)); -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
