From a7f8d96b16a394a87230d592c727906f67a4ba07 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 24 Nov 2020 14:01:29 +0000 Subject: [PATCH] [lldb] Use llvm::Optional for port in LaunchGDBServer Previously we used UINT16_MAX to mean no port/no specifc port. This leads to confusion because 65535 is a valid port number. Instead use an optional. If you want a specific port call LaunchGDBServer as normal, otherwise pass an empty optional and it will be set to the port that gets chosen. (or left empty in the case where we fail to find a port) Reviewed By: labath Differential Revision: https://reviews.llvm.org/D92035 --- .../GDBRemoteCommunicationServerPlatform.cpp | 30 ++++++++++++---------- .../GDBRemoteCommunicationServerPlatform.h | 5 +++- lldb/tools/lldb-server/lldb-platform.cpp | 4 +-- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp index ae12602..4aa1534 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp @@ -157,8 +157,8 @@ GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() {} Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( const lldb_private::Args &args, std::string hostname, lldb::pid_t &pid, - uint16_t &port, std::string &socket_name) { - if (port == UINT16_MAX) { + llvm::Optional &port, std::string &socket_name) { + if (!port) { llvm::Expected available_port = m_port_map.GetNextAvailablePort(); if (available_port) port = *available_port; @@ -179,7 +179,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM)); LLDB_LOGF(log, "Launching debugserver with: %s:%u...", hostname.c_str(), - port); + *port); // Do not run in a new session so that it can not linger after the platform // closes. @@ -194,7 +194,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( #if !defined(__APPLE__) url << m_socket_scheme << "://"; #endif - uint16_t *port_ptr = &port; + uint16_t *port_ptr = port.getPointer(); if (m_socket_protocol == Socket::ProtocolTcp) { llvm::StringRef platform_scheme; llvm::StringRef platform_ip; @@ -205,7 +205,7 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( platform_port, platform_path); UNUSED_IF_ASSERT_DISABLED(ok); assert(ok); - url << platform_ip.str() << ":" << port; + url << platform_ip.str() << ":" << *port; } else { socket_name = GetDomainSocketPath("gdbserver").GetPath(); url << socket_name; @@ -219,11 +219,11 @@ Status GDBRemoteCommunicationServerPlatform::LaunchGDBServer( if (pid != LLDB_INVALID_PROCESS_ID) { std::lock_guard guard(m_spawned_pids_mutex); m_spawned_pids.insert(pid); - if (port > 0) - m_port_map.AssociatePortWithProcess(port, pid); + if (*port > 0) + m_port_map.AssociatePortWithProcess(*port, pid); } else { - if (port > 0) - m_port_map.FreePort(port); + if (*port > 0) + m_port_map.FreePort(*port); } return error; } @@ -243,12 +243,15 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer( packet.SetFilePos(::strlen("qLaunchGDBServer;")); llvm::StringRef name; llvm::StringRef value; - uint16_t port = UINT16_MAX; + llvm::Optional port; while (packet.GetNameColonValue(name, value)) { if (name.equals("host")) hostname = std::string(value); - else if (name.equals("port")) - value.getAsInteger(0, port); + else if (name.equals("port")) { + // Make the Optional valid so we can use its value + port = 0; + value.getAsInteger(0, port.getValue()); + } } lldb::pid_t debugserver_pid = LLDB_INVALID_PROCESS_ID; @@ -269,8 +272,9 @@ GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer( __FUNCTION__, debugserver_pid); StreamGDBRemote response; + assert(port); response.Printf("pid:%" PRIu64 ";port:%u;", debugserver_pid, - port + m_port_offset); + *port + m_port_offset); if (!socket_name.empty()) { response.PutCString("socket_name:"); response.PutStringAsRawHex8(socket_name); diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h index 69261dd..6b964da 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h @@ -16,6 +16,7 @@ #include "GDBRemoteCommunicationServerCommon.h" #include "lldb/Host/Socket.h" +#include "llvm/ADT/Optional.h" #include "llvm/Support/Error.h" namespace lldb_private { @@ -86,8 +87,10 @@ public: 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, uint16_t &port, + lldb::pid_t &pid, llvm::Optional &port, std::string &socket_name); void SetPendingGdbServer(lldb::pid_t pid, uint16_t port, diff --git a/lldb/tools/lldb-server/lldb-platform.cpp b/lldb/tools/lldb-server/lldb-platform.cpp index 05bfe71..a5d4ecf 100644 --- a/lldb/tools/lldb-server/lldb-platform.cpp +++ b/lldb/tools/lldb-server/lldb-platform.cpp @@ -349,13 +349,13 @@ int main_platform(int argc, char *argv[]) { if (platform.IsConnected()) { if (inferior_arguments.GetArgumentCount() > 0) { lldb::pid_t pid = LLDB_INVALID_PROCESS_ID; - uint16_t port = 0; + llvm::Optional port = 0; std::string socket_name; Status error = platform.LaunchGDBServer(inferior_arguments, "", // hostname pid, port, socket_name); if (error.Success()) - platform.SetPendingGdbServer(pid, port, socket_name); + platform.SetPendingGdbServer(pid, *port, socket_name); else fprintf(stderr, "failed to start gdbserver: %s\n", error.AsCString()); } -- 2.7.4