[Lldb-commits] [lldb] r235940 - Add an unnamed pipe fail-safe to launching lldb-gdbserver.

Chaoren Lin chaorenl at google.com
Mon Apr 27 16:20:30 PDT 2015


Author: chaoren
Date: Mon Apr 27 18:20:30 2015
New Revision: 235940

URL: http://llvm.org/viewvc/llvm-project?rev=235940&view=rev
Log:
Add an unnamed pipe fail-safe to launching lldb-gdbserver.

Summary:
Currently, launching lldb-gdbserver from platform on Android requires root for
mkfifo() and an explicit TMPDIR variable. This should remove both requirements.

Test Plan: Successfully launched lldb-gdbserver on a non-rooted Android device.

Reviewers: tberghammer, vharron, clayborg

Reviewed By: clayborg

Subscribers: tberghammer, lldb-commits

Differential Revision: http://reviews.llvm.org/D9307

Modified:
    lldb/trunk/include/lldb/Host/posix/PipePosix.h
    lldb/trunk/source/Host/posix/PipePosix.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
    lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp

Modified: lldb/trunk/include/lldb/Host/posix/PipePosix.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/posix/PipePosix.h?rev=235940&r1=235939&r2=235940&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/posix/PipePosix.h (original)
+++ lldb/trunk/include/lldb/Host/posix/PipePosix.h Mon Apr 27 18:20:30 2015
@@ -16,7 +16,7 @@
 namespace lldb_private {
 
 //----------------------------------------------------------------------
-/// @class PipePosix PipePosix	.h "lldb/Host/posix/PipePosix.h"
+/// @class PipePosix PipePosix.h "lldb/Host/posix/PipePosix.h"
 /// @brief A posix-based implementation of Pipe, a class that abtracts
 ///        unix style pipes.
 ///
@@ -36,6 +36,8 @@ public:
     Error
     CreateNew(llvm::StringRef name, bool child_process_inherit) override;
     Error
+    CreateWithFD(int read_fd, int write_fd);
+    Error
     CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name) override;
     Error
     OpenAsReader(llvm::StringRef name, bool child_process_inherit) override;
@@ -68,12 +70,12 @@ public:
     Error
     ReadWithTimeout(void *buf, size_t size, const std::chrono::microseconds &timeout, size_t &bytes_read) override;
 
-private:
     void
     CloseReadFileDescriptor();
     void
     CloseWriteFileDescriptor();
 
+private:
     int m_fds[2];
 };
 

Modified: lldb/trunk/source/Host/posix/PipePosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/PipePosix.cpp?rev=235940&r1=235939&r2=235940&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/PipePosix.cpp (original)
+++ lldb/trunk/source/Host/posix/PipePosix.cpp Mon Apr 27 18:20:30 2015
@@ -187,6 +187,15 @@ PipePosix::CreateNew(llvm::StringRef nam
 }
 
 Error
+PipePosix::CreateWithFD(int read_fd, int write_fd) {
+    if (CanRead() || CanWrite())
+        return Error("Pipe is already opened");
+    m_fds[READ] = read_fd;
+    m_fds[WRITE] = write_fd;
+    return Error();
+}
+
+Error
 PipePosix::CreateWithUniqueName(llvm::StringRef prefix, bool child_process_inherit, llvm::SmallVectorImpl<char>& name)
 {
     llvm::SmallString<PATH_MAX> named_pipe_path;

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=235940&r1=235939&r2=235940&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Mon Apr 27 18:20:30 2015
@@ -770,7 +770,7 @@ GDBRemoteCommunication::StartDebugserver
         }
 
         llvm::SmallString<PATH_MAX> named_pipe_path;
-        Pipe port_named_pipe;
+        Pipe port_pipe;
 
         bool listen = false;
         if (host_and_port[0])
@@ -783,11 +783,33 @@ GDBRemoteCommunication::StartDebugserver
             {
                 // Binding to port zero, we need to figure out what port it ends up
                 // using using a named pipe...
-                error = port_named_pipe.CreateWithUniqueName("debugserver-named-pipe", false, named_pipe_path);
-                if (error.Fail())
-                    return error;
-                debugserver_args.AppendArgument("--named-pipe");
-                debugserver_args.AppendArgument(named_pipe_path.c_str());
+                error = port_pipe.CreateWithUniqueName("debugserver-named-pipe", false, named_pipe_path);
+                if (error.Success())
+                {
+                    debugserver_args.AppendArgument("--named-pipe");
+                    debugserver_args.AppendArgument(named_pipe_path.c_str());
+                }
+                else
+                {
+                    if (log)
+                        log->Printf("GDBRemoteCommunication::%s() "
+                                "named pipe creation failed: %s",
+                                __FUNCTION__, error.AsCString());
+                    // let's try an unnamed pipe
+                    error = port_pipe.CreateNew(true);
+                    if (error.Fail())
+                    {
+                        if (log)
+                            log->Printf("GDBRemoteCommunication::%s() "
+                                    "unnamed pipe creation failed: %s",
+                                    __FUNCTION__, error.AsCString());
+                        return error;
+                    }
+                    int write_fd = port_pipe.GetWriteFileDescriptor();
+                    debugserver_args.AppendArgument("--pipe");
+                    debugserver_args.AppendArgument(std::to_string(write_fd).c_str());
+                    launch_info.AppendCloseFileAction(port_pipe.GetReadFileDescriptor());
+                }
             }
             else
             {
@@ -869,50 +891,57 @@ GDBRemoteCommunication::StartDebugserver
         {
             if (named_pipe_path.size() > 0)
             {
-                error = port_named_pipe.OpenAsReader(named_pipe_path, false);
+                error = port_pipe.OpenAsReader(named_pipe_path, false);
+                if (error.Fail())
+                    if (log)
+                        log->Printf("GDBRemoteCommunication::%s() "
+                                "failed to open named pipe %s for reading: %s",
+                                __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
+            }
+
+            if (port_pipe.CanWrite())
+                port_pipe.CloseWriteFileDescriptor();
+            if (port_pipe.CanRead())
+            {
+                char port_cstr[256];
+                port_cstr[0] = '\0';
+                size_t num_bytes = sizeof(port_cstr);
+                // Read port from pipe with 10 second timeout.
+                error = port_pipe.ReadWithTimeout(port_cstr, num_bytes,
+                        std::chrono::seconds{10}, num_bytes);
                 if (error.Success())
                 {
-                    char port_cstr[256];
-                    port_cstr[0] = '\0';
-                    size_t num_bytes = sizeof(port_cstr);
-                    // Read port from pipe with 10 second timeout.
-                    error = port_named_pipe.ReadWithTimeout(port_cstr, num_bytes, std::chrono::microseconds(10 * 1000000), num_bytes);
-                    if (error.Success())
-                    {
-                        assert (num_bytes > 0 && port_cstr[num_bytes-1] == '\0');
-                        out_port = StringConvert::ToUInt32(port_cstr, 0);
-                        if (log)
-                            log->Printf("GDBRemoteCommunication::%s() debugserver listens %u port", __FUNCTION__, out_port);
-                    }
-                    else
-                    {
-                        if (log)
-                            log->Printf("GDBRemoteCommunication::%s() failed to read a port value from named pipe %s: %s", __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
-
-                    }
-                    port_named_pipe.Close();
+                    assert(num_bytes > 0 && port_cstr[num_bytes-1] == '\0');
+                    out_port = StringConvert::ToUInt32(port_cstr, 0);
+                    if (log)
+                        log->Printf("GDBRemoteCommunication::%s() "
+                                "debugserver listens %u port",
+                                __FUNCTION__, out_port);
                 }
                 else
                 {
                     if (log)
-                        log->Printf("GDBRemoteCommunication::%s() failed to open named pipe %s for reading: %s", __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
+                        log->Printf("GDBRemoteCommunication::%s() "
+                                "failed to read a port value from pipe %s: %s",
+                                __FUNCTION__, named_pipe_path.c_str(), error.AsCString());
+
                 }
-                const auto err = port_named_pipe.Delete(named_pipe_path);
+                port_pipe.Close();
+            }
+
+            if (named_pipe_path.size() > 0)
+            {
+                const auto err = port_pipe.Delete(named_pipe_path);
                 if (err.Fail())
                 {
                     if (log)
-                        log->Printf ("GDBRemoteCommunication::%s failed to delete pipe %s: %s", __FUNCTION__, named_pipe_path.c_str(), err.AsCString());
+                        log->Printf ("GDBRemoteCommunication::%s failed to delete pipe %s: %s",
+                                __FUNCTION__, named_pipe_path.c_str(), err.AsCString());
                 }
             }
-            else if (listen)
-            {
-                
-            }
-            else
-            {
-                // Make sure we actually connect with the debugserver...
-                JoinListenThread();
-            }
+
+            // Make sure we actually connect with the debugserver...
+            JoinListenThread();
         }
     }
     else

Modified: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp?rev=235940&r1=235939&r2=235940&view=diff
==============================================================================
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp (original)
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp Mon Apr 27 18:20:30 2015
@@ -77,7 +77,8 @@ static struct option g_long_options[] =
     { "log-file",           required_argument,  NULL,               'l' },
     { "log-flags",          required_argument,  NULL,               'f' },
     { "attach",             required_argument,  NULL,               'a' },
-    { "named-pipe",         required_argument,  NULL,               'P' },
+    { "named-pipe",         required_argument,  NULL,               'N' },
+    { "pipe",               required_argument,  NULL,               'U' },
     { "native-regs",        no_argument,        NULL,               'r' },  // Specify to use the native registers instead of the gdb defaults for the architecture.  NOTE: this is a do-nothing arg as it's behavior is default now.  FIXME remove call from lldb-platform.
     { "reverse-connect",    no_argument,        NULL,               'R' },  // Specifies that llgs attaches to the client address:port rather than llgs listening for a connection from address on port.
     { "setsid",             no_argument,        NULL,               'S' },  // Call setsid() to make llgs run in its own session.
@@ -122,7 +123,16 @@ signal_handler(int signo)
 static void
 display_usage (const char *progname, const char* subcommand)
 {
-    fprintf(stderr, "Usage:\n  %s %s [--log-file log-file-path] [--log-flags flags] [--lldb-command command]* [--platform platform_name] [--setsid] [--named-pipe named-pipe-path] [--native-regs] [--attach pid] [[HOST]:PORT] "
+    fprintf(stderr, "Usage:\n  %s %s "
+            "[--log-file log-file-path] "
+            "[--log-flags flags] "
+            "[--lldb-command command]* "
+            "[--platform platform_name] "
+            "[--setsid] "
+            "[--named-pipe named-pipe-path] "
+            "[--native-regs] "
+            "[--attach pid] "
+            "[[HOST]:PORT] "
             "[-- PROGRAM ARG1 ARG2 ...]\n", progname, subcommand);
     exit(0);
 }
@@ -309,24 +319,44 @@ JoinListenThread ()
 }
 
 Error
-writePortToPipe (const char *const named_pipe_path, const uint16_t port)
+WritePortToPipe(Pipe port_pipe, const uint16_t port)
 {
-    Pipe port_name_pipe;
-    // Wait for 10 seconds for pipe to be opened.
-    auto error = port_name_pipe.OpenAsWriterWithTimeout (named_pipe_path, false, std::chrono::microseconds (10 * 1000000));
-    if (error.Fail ())
-        return error;
-
     char port_str[64];
-    const auto port_str_len = ::snprintf (port_str, sizeof (port_str), "%u", port);
+    const auto port_str_len = ::snprintf(port_str, sizeof(port_str), "%u", port);
 
     size_t bytes_written = 0;
     // Write the port number as a C string with the NULL terminator.
-    return port_name_pipe.Write (port_str, port_str_len + 1, bytes_written);
+    return port_pipe.Write(port_str, port_str_len + 1, bytes_written);
+}
+
+Error
+writePortToPipe(const char *const named_pipe_path, const uint16_t port)
+{
+    Pipe port_name_pipe;
+    // Wait for 10 seconds for pipe to be opened.
+    auto error = port_name_pipe.OpenAsWriterWithTimeout(named_pipe_path, false,
+            std::chrono::seconds{10});
+    if (error.Fail())
+        return error;
+    return WritePortToPipe(port_name_pipe, port);
+}
+
+Error
+writePortToPipe(int unnamed_pipe_fd, const uint16_t port)
+{
+    Pipe port_pipe;
+    // Wait for 10 seconds for pipe to be opened.
+    auto error = port_pipe.CreateWithFD(Pipe::kInvalidDescriptor, unnamed_pipe_fd);
+    if (error.Fail())
+        return error;
+    return WritePortToPipe(port_pipe, port);
 }
 
 void
-ConnectToRemote (GDBRemoteCommunicationServerLLGS &gdb_server, bool reverse_connect, const char *const host_and_port, const char *const progname, const char *const subcommand, const char *const named_pipe_path)
+ConnectToRemote(GDBRemoteCommunicationServerLLGS &gdb_server,
+        bool reverse_connect, const char *const host_and_port,
+        const char *const progname, const char *const subcommand,
+        const char *const named_pipe_path, int unnamed_pipe_fd)
 {
     Error error;
 
@@ -417,6 +447,25 @@ ConnectToRemote (GDBRemoteCommunicationS
                 }
             }
 
+            // If we have an unnamed pipe to write the port number back to, do that now.
+            if (unnamed_pipe_fd >= 0 && connection_portno == 0)
+            {
+                const uint16_t bound_port = s_listen_connection_up->GetListeningPort(10);
+                if (bound_port > 0)
+                {
+                    error = writePortToPipe(unnamed_pipe_fd, bound_port);
+                    if (error.Fail())
+                    {
+                        fprintf(stderr, "failed to write to the unnamed pipe: %s",
+                                error.AsCString());
+                    }
+                }
+                else
+                {
+                    fprintf(stderr, "unable to get the bound port for the listening connection\n");
+                }
+            }
+
             // Join the listener thread.
             if (!JoinListenThread ())
             {
@@ -512,6 +561,7 @@ main_gdbserver (int argc, char *argv[])
     std::string platform_name;
     std::string attach_target;
     std::string named_pipe_path;
+    int unnamed_pipe_fd = -1;
     bool reverse_connect = false;
 
     lldb::DebuggerSP debugger_sp = Debugger::CreateInstance ();
@@ -587,11 +637,15 @@ main_gdbserver (int argc, char *argv[])
                 platform_name = optarg;
             break;
 
-        case 'P': // named pipe
+        case 'N': // named pipe
             if (optarg && optarg[0])
                 named_pipe_path = optarg;
             break;
 
+        case 'U': // unnamed pipe
+            if (optarg && optarg[0])
+                unnamed_pipe_fd = StringConvert::ToUInt32(optarg, -1);
+
         case 'r':
             // Do nothing, native regs is the default these days
             break;
@@ -692,7 +746,9 @@ main_gdbserver (int argc, char *argv[])
     // Print version info.
     printf("%s-%s", LLGS_PROGRAM_NAME, LLGS_VERSION_STR);
 
-    ConnectToRemote (gdb_server, reverse_connect, host_and_port, progname, subcommand, named_pipe_path.c_str ());
+    ConnectToRemote(gdb_server, reverse_connect,
+                    host_and_port, progname, subcommand,
+                    named_pipe_path.c_str(), unnamed_pipe_fd);
 
     fprintf(stderr, "lldb-server exiting...\n");
 





More information about the lldb-commits mailing list