[lldb] Use llvm::Optional for port in LaunchGDBServer

[DavidSpickett]

Summary

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)

[DavidSpickett]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

197

I know this is a bit unorthodox but things got messy trying to pass an Optional into StartDebugServerProcess. So I figured it's better to keep StartDebugServerProcess the same for now and limit the scope of any mistakes. (the logic gets quite hard to follow)

[labath]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

197

I think that's fine.

[DavidSpickett]

• Reformat GDBRemoteCommunicationServerPlatform.h to fix include order.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h

 
 #include <map>
 #include <mutex>
 #include <set>
 
 #include "GDBRemoteCommunicationServerCommon.h"
 #include "lldb/Host/Socket.h"
 
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/Error.h"
 
 namespace lldb_private {
 namespace process_gdb_remote {
 
 class GDBRemoteCommunicationServerPlatform
     : public GDBRemoteCommunicationServerCommon {
 public:
public:
   // 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, uint16_t &port,
+                         lldb::pid_t &pid, llvm::Optional<uint16_t> &port,
                          std::string &socket_name);
 
   void SetPendingGdbServer(lldb::pid_t pid, uint16_t port,
                            const std::string &socket_name);
 
 protected:
   const Socket::SocketProtocol m_socket_protocol;
   const std::string m_socket_scheme;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

RegisterPacketHandler(StringExtractorGDBRemote::eServerPacketType_interrupt,
                         });
 }
 
 // Destructor
 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<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).
 
   // ignore the hostname send from the remote end, just use the ip address that
   // we're currently communicating with as the hostname
 
   // 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(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.
   debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
   debugserver_launch_info.SetMonitorProcessCallback(
       std::bind(&GDBRemoteCommunicationServerPlatform::DebugserverProcessReaped,
                 this, std::placeholders::_1),
       false);
 
   std::ostringstream url;
 // debugserver does not accept the URL scheme prefix.
 #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;
     int platform_port;
     llvm::StringRef platform_path;
     std::string platform_uri = GetConnection()->GetURI();
     bool ok = UriParser::Parse(platform_uri, platform_scheme, platform_ip,
                                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;
     port_ptr = nullptr;
   }
 
   Status error = StartDebugserverProcess(
       url.str().c_str(), nullptr, debugserver_launch_info, port_ptr, &args, -1);
 
   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 (*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;
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
     StringExtractorGDBRemote &packet) {
   // Spawn a local debugserver as a platform so we can then attach or launch a
   // process...
 
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
   LLDB_LOGF(log, "GDBRemoteCommunicationServerPlatform::%s() called",
             __FUNCTION__);
 
   ConnectionFileDescriptor file_conn;
   std::string hostname;
   packet.SetFilePos(::strlen("qLaunchGDBServer;"));
   llvm::StringRef name;
   llvm::StringRef value;
-  uint16_t port = UINT16_MAX;
+  llvm::Optional<uint16_t> 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;
   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);
 
   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);
     response.PutChar(';');
   }
 
   PacketResult packet_result = SendPacketNoLock(response.GetString());
   if (packet_result != PacketResult::Success) {

lldb/tools/lldb-server/lldb-platform.cpp

#endif
       // connections while a connection is active.
       acceptor_up.reset();
     }
     platform.SetConnection(std::unique_ptr<Connection>(conn));
 
     if (platform.IsConnected()) {
       if (inferior_arguments.GetArgumentCount() > 0) {
         lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
-        uint16_t port = 0;
+        llvm::Optional<uint16_t> 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());
       }
 
       // After we connected, we need to get an initial ack from...
       if (platform.HandshakeWithClient()) {
         bool interrupt = false;
         bool done = false;