Author: gclayton
Date: Fri Aug 12 11:46:18 2016
New Revision: 278524

URL: http://llvm.org/viewvc/llvm-project?rev=278524&view=rev
Log:
Switch over to using socketpair for local debugserver connections as they are 
twice as fast as TCP sockets (on macOS at least).

This change opens a socket pair and passes the second socket pair file 
descriptor down to the debugserver binary using a new option: "--fd=N" where N 
is the file descriptor. This file descriptor gets passed via posix_spawn() so 
that there is no need to do any bind/listen or bind/accept calls and eliminates 
the hanshake unix socket that is used to pass the result of the actual port 
that ends up being used so it can save time on launch as well as being faster.

This is currently only enabled on __APPLE__ builds. Other OSs should try 
modifying the #define from ProcessGDBRemote.cpp but the first person will need 
to port the --fd option over to lldb-server. Any OSs that enable 
USE_SOCKETPAIR_FOR_LOCAL_CONNECTION in their native builds can use the socket 
pair stuff. The #define is Apple only right now, but looks like:

#if defined (__APPLE__)
#define USE_SOCKETPAIR_FOR_LOCAL_CONNECTION 1
#endif

<rdar://problem/27814880> 


Modified:
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
    
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/trunk/tools/debugserver/source/debugserver.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=278524&r1=278523&r2=278524&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp 
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Fri 
Aug 12 11:46:18 2016
@@ -1091,7 +1091,8 @@ GDBRemoteCommunication::StartDebugserver
                                                  Platform *platform,
                                                  ProcessLaunchInfo 
&launch_info,
                                                  uint16_t *port,
-                                                 const Args& inferior_args)
+                                                 const Args* inferior_args,
+                                                 int pass_comm_fd)
 {
     Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet 
(GDBR_LOG_PROCESS));
     if (log)
@@ -1171,6 +1172,16 @@ GDBRemoteCommunication::StartDebugserver
         if (url)
             debugserver_args.AppendArgument(url);
 
+        if (pass_comm_fd >= 0)
+        {
+            StreamString fd_arg;
+            fd_arg.Printf("--fd=%i", pass_comm_fd);
+            debugserver_args.AppendArgument(fd_arg.GetData());
+            // 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);
+        }
+
         // use native registers, not the GDB registers
         debugserver_args.AppendArgument("--native-regs");
 
@@ -1189,7 +1200,7 @@ GDBRemoteCommunication::StartDebugserver
         // 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 ((port != nullptr && *port == 0) || port == nullptr)
+        if (pass_comm_fd == -1 && ((port != nullptr && *port == 0) || port == 
nullptr))
         {
             if (url)
             {
@@ -1304,10 +1315,10 @@ GDBRemoteCommunication::StartDebugserver
             }
         } while (has_env_var);
 
-        if (inferior_args.GetArgumentCount() > 0)
+        if (inferior_args && inferior_args->GetArgumentCount() > 0)
         {
             debugserver_args.AppendArgument ("--");
-            debugserver_args.AppendArguments (inferior_args);
+            debugserver_args.AppendArguments (*inferior_args);
         }
 
         // Copy the current environment to the gdbserver/debugserver instance
@@ -1337,8 +1348,7 @@ GDBRemoteCommunication::StartDebugserver
         }
         error = Host::LaunchProcess(launch_info);
         
-        if (error.Success() &&
-            launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID)
+        if (error.Success() && (launch_info.GetProcessID() != 
LLDB_INVALID_PROCESS_ID) && pass_comm_fd == -1)
         {
             if (named_pipe_path.size() > 0)
             {

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=278524&r1=278523&r2=278524&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h 
(original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Fri 
Aug 12 11:46:18 2016
@@ -162,7 +162,8 @@ public:
                             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 = Args());
+                            const Args *inferior_args,
+                            int pass_comm_fd); // Communication file 
descriptor to pass during fork/exec to avoid having to connect/accept
 
     void
     DumpHistory(Stream &strm);

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=278524&r1=278523&r2=278524&view=diff
==============================================================================
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 Fri Aug 12 11:46:18 2016
@@ -149,7 +149,8 @@ GDBRemoteCommunicationServerPlatform::La
                                            nullptr,
                                            debugserver_launch_info,
                                            port_ptr,
-                                           args);
+                                           &args,
+                                           -1);
 
     pid = debugserver_launch_info.GetProcessID();
     if (pid != LLDB_INVALID_PROCESS_ID)

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=278524&r1=278523&r2=278524&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Fri Aug 
12 11:46:18 2016
@@ -16,6 +16,7 @@
 #include <netinet/in.h>
 #include <sys/mman.h>       // for mmap
 #endif
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <time.h>
@@ -64,6 +65,7 @@
 #include "lldb/Target/TargetList.h"
 #include "lldb/Target/ThreadPlanCallFunction.h"
 #include "lldb/Target/SystemRuntime.h"
+#include "lldb/Utility/CleanUp.h"
 #include "lldb/Utility/PseudoTerminal.h"
 
 // Project includes
@@ -3602,6 +3604,23 @@ ProcessGDBRemote::EstablishConnectionIfN
     }
     return error;
 }
+#if defined (__APPLE__)
+#define USE_SOCKETPAIR_FOR_LOCAL_CONNECTION 1
+#endif
+
+#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
+static bool SetCloexecFlag(int fd)
+{
+#if defined(FD_CLOEXEC) 
+    int flags = ::fcntl(fd, F_GETFD);
+    if (flags == -1)
+        return false;
+    return (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == 0);
+#else
+    return false;
+#endif
+}
+#endif
 
 Error
 ProcessGDBRemote::LaunchAndConnectToDebugserver (const ProcessInfo 
&process_info)
@@ -3624,30 +3643,34 @@ ProcessGDBRemote::LaunchAndConnectToDebu
                                                           false);
         debugserver_launch_info.SetUserID(process_info.GetUserID());
 
-#if defined (__APPLE__) && (defined (__arm__) || defined (__arm64__) || 
defined (__aarch64__))
-        // On iOS, still do a local connection using a random port
-        const char *hostname = "127.0.0.1";
-        uint16_t port = get_random_port ();
-#else
-        // Set hostname being NULL to do the reverse connect where debugserver
-        // will bind to port zero and it will communicate back to us the port
-        // that we will connect to
-        const char *hostname = nullptr;
-        uint16_t port = 0;
-#endif
 
-        StreamString url_str;
-        const char* url = nullptr;
-        if (hostname != nullptr)
-        {
-            url_str.Printf("%s:%u", hostname, port);
-            url = url_str.GetData();
+        int communication_fd = -1;
+#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
+        // Auto close the sockets we might open up unless everything goes OK. 
This
+        // helps us not leak file descriptors when things go wrong.
+        lldb_utility::CleanUp <int, int> our_socket(-1, -1, close);
+        lldb_utility::CleanUp <int, int> gdb_socket(-1, -1, close);
+
+        // Use a socketpair on Apple for now until other platforms can verify 
it
+        // works and is fast enough
+        {
+            int sockets[2]; /* the pair of socket descriptors */
+            if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1)
+            {
+                error.SetErrorToErrno();
+                return error;
+            }
+
+            our_socket.set(sockets[0]);
+            gdb_socket.set(sockets[1]);
         }
 
-        error = m_gdb_comm.StartDebugserverProcess (url,
-                                                    
GetTarget().GetPlatform().get(),
-                                                    debugserver_launch_info,
-                                                    &port);
+        // Don't let any child processes inherit our communication socket
+        SetCloexecFlag(our_socket.get());
+        communication_fd = gdb_socket.get();
+#endif
+
+        error = m_gdb_comm.StartDebugserverProcess(nullptr, 
GetTarget().GetPlatform().get(), debugserver_launch_info, nullptr, nullptr, 
communication_fd);
 
         if (error.Success ())
             m_debugserver_pid = debugserver_launch_info.GetProcessID();
@@ -3655,7 +3678,14 @@ ProcessGDBRemote::LaunchAndConnectToDebu
             m_debugserver_pid = LLDB_INVALID_PROCESS_ID;
 
         if (m_debugserver_pid != LLDB_INVALID_PROCESS_ID)
+        {
+#ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION
+            // Our process spawned correctly, we can now set our connection to 
use our
+            // end of the socket pair
+            m_gdb_comm.SetConnection(new 
ConnectionFileDescriptor(our_socket.release(), true));
+#endif
             StartAsyncThread ();
+        }
 
         if (error.Fail())
         {
@@ -3669,13 +3699,11 @@ ProcessGDBRemote::LaunchAndConnectToDebu
         if (m_gdb_comm.IsConnected())
         {
             // Finish the connection process by doing the handshake without 
connecting (send NULL URL)
-            ConnectToDebugserver (NULL);
+            ConnectToDebugserver(NULL);
         }
         else
         {
-            StreamString connect_url;
-            connect_url.Printf("connect://%s:%u", hostname, port);
-            error = ConnectToDebugserver (connect_url.GetString().c_str());
+            error.SetErrorString("connection failed");
         }
 
     }

Modified: lldb/trunk/tools/debugserver/source/debugserver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/debugserver.cpp?rev=278524&r1=278523&r2=278524&view=diff
==============================================================================
--- lldb/trunk/tools/debugserver/source/debugserver.cpp (original)
+++ lldb/trunk/tools/debugserver/source/debugserver.cpp Fri Aug 12 11:46:18 2016
@@ -872,6 +872,7 @@ static struct option g_long_options[] =
     { "working-dir",        required_argument,  NULL,               'W' },  // 
The working directory that the inferior process should have (only if 
debugserver launches the process)
     { "platform",           required_argument,  NULL,               'p' },  // 
Put this executable into a remote platform mode
     { "unix-socket",        required_argument,  NULL,               'u' },  // 
If we need to handshake with our parent process, an option will be passed down 
that specifies a unix socket name to use
+    { "fd",                 required_argument,  NULL,            'FDSC' },  // 
A file descriptor was passed to this process when spawned that is already open 
and ready for communication
     { "named-pipe",         required_argument,  NULL,               'P' },
     { "reverse-connect",    no_argument,        NULL,               'R' },
     { "env",                required_argument,  NULL,               'e' },  // 
When debugserver launches the process, set a single environment entry as 
specified by the option value ("./debugserver -e FOO=1 -e BAR=2 localhost:1234 
-- /bin/ls")
@@ -949,6 +950,7 @@ main (int argc, char *argv[])
     int ch;
     int long_option_index = 0;
     int debug = 0;
+    int communication_fd = -1;
     std::string compile_options;
     std::string waitfor_pid_name;           // Wait for a process that starts 
with this name
     std::string attach_pid_name;
@@ -1248,6 +1250,12 @@ main (int argc, char *argv[])
                         remote->Context().PushEnvironment(env_entry);
                 }
                 break;
+
+            case 'FDSC':
+                // File descriptor passed to this process during fork/exec and 
is already
+                // open and ready for communication.
+                communication_fd = atoi(optarg);
+                break;
         }
     }
 
@@ -1324,7 +1332,7 @@ main (int argc, char *argv[])
     char str[PATH_MAX];
     str[0] = '\0';
 
-    if (g_lockdown_opt == 0 && g_applist_opt == 0)
+    if (g_lockdown_opt == 0 && g_applist_opt == 0 && communication_fd == -1)
     {
         // Make sure we at least have port
         if (argc < 1)
@@ -1475,6 +1483,15 @@ main (int argc, char *argv[])
                     if (remote->Comm().OpenFile (str))
                         mode = eRNBRunLoopModeExit;
                 }
+                else if (communication_fd >= 0)
+                {
+                    // We were passed a file descriptor to use during 
fork/exec that is already open
+                    // in our process, so lets just use it!
+                    if (remote->Comm().useFD(communication_fd))
+                        mode = eRNBRunLoopModeExit;
+                    else
+                        remote->StartReadRemoteDataThread();
+                }
 
                 if (mode != eRNBRunLoopModeExit)
                 {
@@ -1600,6 +1617,16 @@ main (int argc, char *argv[])
                         if (remote->Comm().OpenFile (str))
                             mode = eRNBRunLoopModeExit;
                     }
+                    else if (communication_fd >= 0)
+                    {
+                        // We were passed a file descriptor to use during 
fork/exec that is already open
+                        // in our process, so lets just use it!
+                        if (remote->Comm().useFD(communication_fd))
+                            mode = eRNBRunLoopModeExit;
+                        else
+                            remote->StartReadRemoteDataThread();
+                    }
+
                     if (mode != eRNBRunLoopModeExit)
                         RNBLogSTDOUT ("Waiting for debugger instructions for 
process %d.\n", attach_pid);
                 }
@@ -1625,6 +1652,15 @@ main (int argc, char *argv[])
                             if (remote->Comm().OpenFile (str))
                                 mode = eRNBRunLoopModeExit;
                         }
+                        else if (communication_fd >= 0)
+                        {
+                            // We were passed a file descriptor to use during 
fork/exec that is already open
+                            // in our process, so lets just use it!
+                            if (remote->Comm().useFD(communication_fd))
+                                mode = eRNBRunLoopModeExit;
+                            else
+                                remote->StartReadRemoteDataThread();
+                        }
 
                         if (mode != eRNBRunLoopModeExit)
                         {
@@ -1657,6 +1693,15 @@ main (int argc, char *argv[])
                     if (remote->Comm().OpenFile (str))
                         mode = eRNBRunLoopModeExit;
                 }
+                else if (communication_fd >= 0)
+                {
+                    // We were passed a file descriptor to use during 
fork/exec that is already open
+                    // in our process, so lets just use it!
+                    if (remote->Comm().useFD(communication_fd))
+                        mode = eRNBRunLoopModeExit;
+                    else
+                        remote->StartReadRemoteDataThread();
+                }
 
                 if (mode != eRNBRunLoopModeExit)
                     mode = RNBRunLoopPlatform (remote);


_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to