Added new options to lldb-platform:

    --gdbserver-port PORT
    --min-gdbserver-port PORT
    --max-gdbserver-port PORT
    
The --gdbserver-port option can be specified multiple times to tell lldb-platform which ports it can use when launching child GDB server processes.
The --min-gdbserver-port and --max-gdbserver-port options allow a range of ports to be specified for use when launching child GDB server processes.

Fixed the code to manage these ports correctly in GDBRemoteCommunicationServer.

Also changed GDBRemoteCommunicationClient to not set a port when sending the "qLaunchGDBServer" packet so that the remote lldb-platform can decide which ports to use. If the lldb-platform was launched with no --gdbserver-port or --min-gdbserver-port/--max-gdbserver-port options, then port 0 is always used and a unix socket is used between the lldb-platform and child GDB server process to coordinate the use of valid port.

llvm-svn: 195300

4 files changed, 239 insertions, 122 deletions

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 7cd77e58f52..8c23bcd73b0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -2286,7 +2286,7 @@ GDBRemoteCommunicationClient::LaunchGDBserverAndGetPort (lldb::pid_t &pid)
     pid = LLDB_INVALID_PROCESS_ID;
     StringExtractorGDBRemote response;
     StreamString stream;
-    stream.PutCString("qLaunchGDBServer:port:0;");
+    stream.PutCString("qLaunchGDBServer;");
     std::string hostname;
     if (Host::GetHostname (hostname))
     {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
index 77cc6408a92..6d1891a579e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -47,27 +47,8 @@ GDBRemoteCommunicationServer::GDBRemoteCommunicationServer(bool is_platform) :
     m_spawned_pids_mutex (Mutex::eMutexTypeRecursive),
     m_proc_infos (),
     m_proc_infos_index (0),
-    m_lo_port_num (0),
-    m_hi_port_num (0),
-    m_next_port (0),
-    m_use_port_range (false)
+    m_port_map ()
 {
-    // We seldom need to override the port number that the debugserver process
-    // starts with.  We just pass in 0 to let the system choose a random port.
-    // In rare situation where the need arises, use two environment variables
-    // to override.
-    uint16_t lo_port_num = 0;
-    uint16_t hi_port_num = 0;
-    const char *lo_port_c_str = getenv("LLDB_PLATFORM_START_DEBUG_SERVER_LO_PORT");
-    if (lo_port_c_str)
-        lo_port_num = ::atoi(lo_port_c_str);
-    const char *hi_port_c_str = getenv("LLDB_PLATFORM_START_DEBUG_SERVER_HI_PORT");
-    if (hi_port_c_str)
-        hi_port_num = ::atoi(hi_port_c_str);
-    if (lo_port_num && hi_port_num && lo_port_num < hi_port_num)
-    {
-        SetPortRange(lo_port_num, hi_port_num);
-    }
 }
 
 //----------------------------------------------------------------------
@@ -743,6 +724,7 @@ bool
 GDBRemoteCommunicationServer::DebugserverProcessReaped (lldb::pid_t pid)
 {
     Mutex::Locker locker (m_spawned_pids_mutex);
+    FreePortForProcess(pid);
     return m_spawned_pids.erase(pid) > 0;
 }
 bool
@@ -776,106 +758,119 @@ GDBRemoteCommunicationServer::Handle_qLaunchGDBServer (StringExtractorGDBRemote
         std::string hostname;
         // TODO: /tmp/ should not be hardcoded. User might want to override /tmp
         // with the TMPDIR environnement variable
-        char unix_socket_name[PATH_MAX] = "/tmp/XXXXXX";
-        if (::mkstemp (unix_socket_name) == -1)
+        packet.SetFilePos(::strlen ("qLaunchGDBServer;"));
+        std::string name;
+        std::string value;
+        uint16_t port = UINT16_MAX;
+        while (packet.GetNameColonValue(name, value))
         {
-            error.SetErrorStringWithFormat("failed to make temporary path for a unix socket: %s", strerror(errno));
+            if (name.compare ("host") == 0)
+                hostname.swap(value);
+            else if (name.compare ("port") == 0)
+                port = Args::StringToUInt32(value.c_str(), 0, 0);
         }
-        else
-        {
-            packet.SetFilePos(::strlen ("qLaunchGDBServer:"));
-            std::string name;
-            std::string value;
-            uint16_t port = UINT16_MAX;
-            while (packet.GetNameColonValue(name, value))
-            {
-                if (name.compare ("host") == 0)
-                    hostname.swap(value);
-                else if (name.compare ("port") == 0)
-                    port = Args::StringToUInt32(value.c_str(), 0, 0);
-            }
-            if (port == UINT16_MAX)
-                port = GetAndUpdateNextPort();
+        if (port == UINT16_MAX)
+            port = GetNextAvailablePort();
 
-            ::snprintf (connect_url, sizeof(connect_url), "unix-accept://%s", unix_socket_name);
-            // Spawn a new thread to accept the port that gets bound after
-            // binding to port 0 (zero).
-            lldb::thread_t accept_thread = LLDB_INVALID_HOST_THREAD;
+        // Spawn a new thread to accept the port that gets bound after
+        // binding to port 0 (zero).
+        lldb::thread_t accept_thread = LLDB_INVALID_HOST_THREAD;
+        const char *unix_socket_name = NULL;
+        char unix_socket_name_buf[PATH_MAX] = "/tmp/XXXXXXXXX";
 
-            if (port == 0)
+        if (port == 0)
+        {
+            if (::mkstemp (unix_socket_name_buf) == 0)
             {
+                unix_socket_name = unix_socket_name_buf;
+                ::snprintf (connect_url, sizeof(connect_url), "unix-accept://%s", unix_socket_name);
                 accept_thread = Host::ThreadCreate (unix_socket_name,
                                                     AcceptPortFromInferior,
                                                     connect_url,
                                                     &error);
             }
-
-            if (IS_VALID_LLDB_HOST_THREAD(accept_thread))
+            else
             {
-                // Spawn a debugserver and try to get the port it listens to.
-                ProcessLaunchInfo debugserver_launch_info;
-                StreamString host_and_port;
-                if (hostname.empty())
-                    hostname = "localhost";
-                host_and_port.Printf("%s:%u", hostname.c_str(), port);
-                const char *host_and_port_cstr = host_and_port.GetString().c_str();
-                Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
-                if (log)
-                    log->Printf("Launching debugserver with: %s...\n", host_and_port_cstr);
-
-                debugserver_launch_info.SetMonitorProcessCallback(ReapDebugserverProcess, this, false);
-                
-                error = StartDebugserverProcess (host_and_port_cstr,
-                                                 unix_socket_name,
-                                                 debugserver_launch_info);
-
-                lldb::pid_t debugserver_pid = debugserver_launch_info.GetProcessID();
+                error.SetErrorStringWithFormat("failed to make temporary path for a unix socket: %s", strerror(errno));
+            }
+        }
 
+        if (error.Success())
+        {
+            // Spawn a debugserver and try to get the port it listens to.
+            ProcessLaunchInfo debugserver_launch_info;
+            StreamString host_and_port;
+            if (hostname.empty())
+                hostname = "localhost";
+            host_and_port.Printf("%s:%u", hostname.c_str(), port);
+            const char *host_and_port_cstr = host_and_port.GetString().c_str();
+            Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PLATFORM));
+            if (log)
+                log->Printf("Launching debugserver with: %s...\n", host_and_port_cstr);
+
+            debugserver_launch_info.SetMonitorProcessCallback(ReapDebugserverProcess, this, false);
+            
+            error = StartDebugserverProcess (host_and_port_cstr,
+                                             unix_socket_name,
+                                             debugserver_launch_info);
+
+            lldb::pid_t debugserver_pid = debugserver_launch_info.GetProcessID();
+
+
+            if (debugserver_pid != LLDB_INVALID_PROCESS_ID)
+            {
+                Mutex::Locker locker (m_spawned_pids_mutex);
+                m_spawned_pids.insert(debugserver_pid);
+                if (port > 0)
+                    AssociatePortWithProcess(port, debugserver_pid);
+            }
+            else
+            {
+                if (port > 0)
+                    FreePort (port);
+            }
 
-                if (debugserver_pid != LLDB_INVALID_PROCESS_ID)
-                {
-                    Mutex::Locker locker (m_spawned_pids_mutex);
-                    m_spawned_pids.insert(debugserver_pid);
-                }
+            if (error.Success())
+            {
+                bool success = false;
 
-                if (error.Success())
+                if (IS_VALID_LLDB_HOST_THREAD(accept_thread))
                 {
-                    bool success = false;
-
-                    if (accept_thread)
+                    thread_result_t accept_thread_result = NULL;
+                    if (Host::ThreadJoin (accept_thread, &accept_thread_result, &error))
                     {
-                        thread_result_t accept_thread_result = NULL;
-                        if (Host::ThreadJoin (accept_thread, &accept_thread_result, &error))
+                        if (accept_thread_result)
                         {
-                            if (accept_thread_result)
-                            {
-                                port = (intptr_t)accept_thread_result;
-                                char response[256];
-                                const int response_len = ::snprintf (response, sizeof(response), "pid:%" PRIu64 ";port:%u;", debugserver_pid, port);
-                                assert (response_len < sizeof(response));
-                                //m_port_to_pid_map[port] = debugserver_launch_info.GetProcessID();
-                                success = SendPacketNoLock (response, response_len) > 0;
-                            }
+                            port = (intptr_t)accept_thread_result;
+                            char response[256];
+                            const int response_len = ::snprintf (response, sizeof(response), "pid:%" PRIu64 ";port:%u;", debugserver_pid, port);
+                            assert (response_len < sizeof(response));
+                            //m_port_to_pid_map[port] = debugserver_launch_info.GetProcessID();
+                            success = SendPacketNoLock (response, response_len) > 0;
                         }
                     }
-                    else
-                    {
-                        char response[256];
-                        const int response_len = ::snprintf (response, sizeof(response), "pid:%" PRIu64 ";port:%u;", debugserver_pid, port);
-                        assert (response_len < sizeof(response));
-                        //m_port_to_pid_map[port] = debugserver_launch_info.GetProcessID();
-                        success = SendPacketNoLock (response, response_len) > 0;
+                }
+                else
+                {
+                    char response[256];
+                    const int response_len = ::snprintf (response, sizeof(response), "pid:%" PRIu64 ";port:%u;", debugserver_pid, port);
+                    assert (response_len < sizeof(response));
+                    //m_port_to_pid_map[port] = debugserver_launch_info.GetProcessID();
+                    success = SendPacketNoLock (response, response_len) > 0;
 
-                    }
-                    ::unlink (unix_socket_name);
+                }
+                Host::Unlink (unix_socket_name);
 
-                    if (!success)
-                    {
-                        if (debugserver_pid != LLDB_INVALID_PROCESS_ID)
-                            ::kill (debugserver_pid, SIGINT);
-                    }
-                    return success;
+                if (!success)
+                {
+                    if (debugserver_pid != LLDB_INVALID_PROCESS_ID)
+                        ::kill (debugserver_pid, SIGINT);
                 }
+                return success;
+            }
+            else if (accept_thread)
+            {
+                Host::Unlink (unix_socket_name);
             }
         }
     }
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
index 57c34d5c153..3138f9c24d7 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
@@ -26,6 +26,8 @@ class StringExtractorGDBRemote;
 class GDBRemoteCommunicationServer : public GDBRemoteCommunication
 {
 public:
+    typedef std::map<uint16_t, lldb::pid_t> PortMap;
+
     enum
     {
         eBroadcastBitRunPacketSent = kLoUserBroadcastBit
@@ -58,30 +60,79 @@ public:
     // Set both ports to zero to let the platform automatically bind to 
     // a port chosen by the OS.
     void
-    SetPortRange (uint16_t lo_port_num, uint16_t hi_port_num)
+    SetPortMap (PortMap &&port_map)
     {
-        m_lo_port_num = lo_port_num;
-        m_hi_port_num = hi_port_num;
-        m_next_port = m_lo_port_num;
-        m_use_port_range = true;
+        m_port_map = port_map;
     }
 
-    // If we are using a port range, get and update the next port to be used variable.
-    // Otherwise, just return 0.
+    //----------------------------------------------------------------------
+    // 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 UINT16_MAX
+    //
+    // 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.
+    //----------------------------------------------------------------------
     uint16_t
-    GetAndUpdateNextPort ()
+    GetNextAvailablePort ()
     {
-        if (!m_use_port_range)
-            return 0;
-        uint16_t val = m_next_port;
-        if (++m_next_port > m_hi_port_num)
-            m_next_port = m_lo_port_num;
-        return val;
+        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 UINT16_MAX;
     }
 
-protected:
-    //typedef std::map<uint16_t, lldb::pid_t> PortToPIDMap;
+    bool
+    AssociatePortWithProcess (uint16_t port, lldb::pid_t pid)
+    {
+        PortMap::iterator pos = m_port_map.find(port);
+        if (pos != m_port_map.end())
+        {
+            pos->second = pid;
+            return true;
+        }
+        return false;
+    }
+
+    bool
+    FreePort (uint16_t port)
+    {
+        PortMap::iterator pos = m_port_map.find(port);
+        if (pos != m_port_map.end())
+        {
+            pos->second = LLDB_INVALID_PROCESS_ID;
+            return true;
+        }
+        return false;
+    }
 
+    bool
+    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;
+    }
+
+protected:
     lldb::thread_t m_async_thread;
     lldb_private::ProcessLaunchInfo m_process_launch_info;
     lldb_private::Error m_process_launch_error;
@@ -89,11 +140,7 @@ protected:
     lldb_private::Mutex m_spawned_pids_mutex;
     lldb_private::ProcessInstanceInfoList m_proc_infos;
     uint32_t m_proc_infos_index;
-    uint16_t m_lo_port_num;
-    uint16_t m_hi_port_num;
-    //PortToPIDMap m_port_to_pid_map;
-    uint16_t m_next_port;
-    bool m_use_port_range;
+    PortMap m_port_map;
     
 
     size_t
diff --git a/lldb/tools/lldb-platform/lldb-platform.cpp b/lldb/tools/lldb-platform/lldb-platform.cpp
index 21063f9a0f7..1583b4ad3e4 100644
--- a/lldb/tools/lldb-platform/lldb-platform.cpp
+++ b/lldb/tools/lldb-platform/lldb-platform.cpp
@@ -48,9 +48,21 @@ static struct option g_long_options[] =
     { "log-file",           required_argument,  NULL,               'l' },
     { "log-flags",          required_argument,  NULL,               'f' },
     { "listen",             required_argument,  NULL,               'L' },
+    { "gdbserver-port",     required_argument,  NULL,               'p' },
+    { "min-gdbserver-port", required_argument,  NULL,               'm' },
+    { "max-gdbserver-port", required_argument,  NULL,               'M' },
     { NULL,                 0,                  NULL,               0   }
 };
 
+#if defined (__APPLE__)
+#define LOW_PORT    (IPPORT_RESERVED)
+#define HIGH_PORT   (IPPORT_HIFIRSTAUTO)
+#else
+#define LOW_PORT    (1024u)
+#define HIGH_PORT   (49151u)
+#endif
+
+
 //----------------------------------------------------------------------
 // Watch for signals
 //----------------------------------------------------------------------
@@ -97,11 +109,17 @@ main (int argc, char *argv[])
     int ch;
     Debugger::Initialize();
     
+    GDBRemoteCommunicationServer::PortMap gdbserver_portmap;
+    int min_gdbserver_port = 0;
+    int max_gdbserver_port = 0;
+    
+    bool show_usage = false;
+    int option_error = 0;
 //    StreamSP stream_sp (new StreamFile(stdout, false));
 //    const char *log_channels[] = { "host", "process", NULL };
 //    EnableLog (stream_sp, 0, log_channels, NULL);
     
-    while ((ch = getopt_long_only(argc, argv, "l:f:L:", g_long_options, &long_option_index)) != -1)
+    while ((ch = getopt_long_only(argc, argv, "l:f:L:p:m:M:", g_long_options, &long_option_index)) != -1)
     {
 //        DNBLogDebug("option: ch == %c (0x%2.2x) --%s%c%s\n",
 //                    ch, (uint8_t)ch,
@@ -152,15 +170,66 @@ main (int argc, char *argv[])
             listen_host_port.append (optarg);
             break;
 
+        case 'p':
+        case 'm':
+        case 'M':
+            {
+                char *end = NULL;
+                long portnum = strtoul(optarg, &end, 0);
+                if (end && *end == '\0')
+                {
+                    if (LOW_PORT <= portnum && portnum <= HIGH_PORT)
+                    {
+                        if (ch  == 'p')
+                            gdbserver_portmap[(uint16_t)portnum] = LLDB_INVALID_PROCESS_ID;
+                        else if (ch == 'm')
+                            min_gdbserver_port = portnum;
+                        else
+                            max_gdbserver_port = portnum;
+                    }
+                    else
+                    {
+                        fprintf (stderr, "error: port number %li is not in the valid user port range of %u - %u\n", portnum, LOW_PORT, HIGH_PORT);
+                        option_error = 1;
+                    }
+                }
+                else
+                {
+                    fprintf (stderr, "error: invalid port number string %s\n", optarg);
+                    option_error = 2;
+                }
+            }
+            break;
+            
         case 'h':   /* fall-through is intentional */
         case '?':
-            display_usage(progname);
+            show_usage = true;
             break;
         }
     }
+    
+    // Make a port map for a port range that was specified.
+    if (min_gdbserver_port < max_gdbserver_port)
+    {
+        for (uint16_t port = min_gdbserver_port; port < max_gdbserver_port; ++port)
+            gdbserver_portmap[port] = LLDB_INVALID_PROCESS_ID;
+    }
+    else if (min_gdbserver_port != max_gdbserver_port)
+    {
+        fprintf (stderr, "error: --min-gdbserver-port (%u) is greater than --max-gdbserver-port (%u)\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())
+        show_usage = true;
+    
+    if (show_usage || option_error)
+    {
         display_usage(progname);
+        exit(option_error);
+    }
     
     if (log_stream_sp)
     {
@@ -176,6 +245,12 @@ main (int argc, char *argv[])
 
     do {
         GDBRemoteCommunicationServer gdb_server (true);
+        
+        if (!gdbserver_portmap.empty())
+        {
+            gdb_server.SetPortMap(std::move(gdbserver_portmap));
+        }
+
         if (!listen_host_port.empty())
         {
             std::unique_ptr<ConnectionFileDescriptor> conn_ap(new ConnectionFileDescriptor());