[Lldb-commits] [lldb] r219578 - llgs: fixes to PTY/gdb-remote inferior stdout/stderr handling, logging addtions.

Todd Fiala todd.fiala at gmail.com
Sat Oct 11 14:42:10 PDT 2014


Author: tfiala
Date: Sat Oct 11 16:42:09 2014
New Revision: 219578

URL: http://llvm.org/viewvc/llvm-project?rev=219578&view=rev
Log:
llgs: fixes to PTY/gdb-remote inferior stdout/stderr handling, logging addtions.

With this change, both local-process llgs and remote-target llgs stdout/stderr
handling from inferior work correctly.

Several log lines have been added around PTY and stdout/stderr redirection
logic on the lldb client side.

Regarding remote llgs execution, see the following:

With these changes, remote llgs with $O now works properly:

$ lldb
(lldb) platform select remote-linux
(lldb) target create ~/some/inferior/exe
(lldb) gdb-remote {some-target}:{port}
(lldb) run

The sequence above will correctly redirect stdout/stderr over gdb-remote $O,
as is needed for remote debugging.  That sequence assumes there is a lldb-gdbserver
exe running on the target with {some-host}:{port}.

You can replace the gdb-remote command with a '(lldb) platform connect
connect://{target-ip}:{target-port}'.  If you do this and have a
lldb-platform running on the remote end, it will go ahead and launch
llgs for lldb for each target instance that is run/attached.

For local debugging with llgs, the following sequence also works, and
uses local PTYs instead to avoid $O and extra gdb-remote messages:

$ lldb
(lldb) settings set platform.plugin.linux.use-llgs true
(lldb) target create ~/some/inferior/exe
(lldb) run

The above will run the inferior using llgs on the local host, and
will use PTYs rather than $O redirection.

This change also removes the logging that happened after the fork but
before the exec when llgs is launching a new inferior process.  Some
aspect of the file handling during that portion of code would not do
the right thing with log handling.  We might want to go back later
and have that communicate over a pipe from the child to parent to pass
along any messages that previously were logged in that section of code.

Modified:
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
    lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/trunk/source/Target/ProcessLaunchInfo.cpp
    lldb/trunk/source/Target/Target.cpp

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Sat Oct 11 16:42:09 2014
@@ -143,29 +143,6 @@ namespace
         return signals;
     }
 
-    const char *
-    GetFilePath(const lldb_private::FileAction *file_action, const char *default_path)
-    {
-        const char *pts_name = "/dev/pts/";
-        const char *path = NULL;
-
-        if (file_action)
-        {
-            if (file_action->GetAction() == FileAction::eFileActionOpen)
-            {
-                path = file_action->GetPath ();
-                // By default the stdio paths passed in will be pseudo-terminal
-                // (/dev/pts). If so, convert to using a different default path
-                // instead to redirect I/O to the debugger console. This should
-                //  also handle user overrides to /dev/null or a different file.
-                if (!path || ::strncmp (path, pts_name, ::strlen (pts_name)) == 0)
-                    path = default_path;
-            }
-        }
-
-        return path;
-    }
-
     Error
     ResolveProcessArchitecture (lldb::pid_t pid, Platform &platform, ArchSpec &arch)
     {
@@ -1159,9 +1136,9 @@ NativeProcessLinux::LaunchArgs::LaunchAr
                                        lldb_private::Module *module,
                                        char const **argv,
                                        char const **envp,
-                                       const char *stdin_path,
-                                       const char *stdout_path,
-                                       const char *stderr_path,
+                                       const std::string &stdin_path,
+                                       const std::string &stdout_path,
+                                       const std::string &stderr_path,
                                        const char *working_dir,
                                        const lldb_private::ProcessLaunchInfo &launch_info)
     : OperationArgs(monitor),
@@ -1216,18 +1193,39 @@ NativeProcessLinux::LaunchProcess (
     const lldb_private::FileAction *file_action;
 
     // Default of NULL will mean to use existing open file descriptors.
-    const char *stdin_path = NULL;
-    const char *stdout_path = NULL;
-    const char *stderr_path = NULL;
+    std::string stdin_path;
+    std::string stdout_path;
+    std::string stderr_path;
 
     file_action = launch_info.GetFileActionForFD (STDIN_FILENO);
-    stdin_path = GetFilePath (file_action, stdin_path);
+    if (file_action)
+        stdin_path = file_action->GetPath ();
 
     file_action = launch_info.GetFileActionForFD (STDOUT_FILENO);
-    stdout_path = GetFilePath (file_action, stdout_path);
+    if (file_action)
+        stdout_path = file_action->GetPath ();
 
     file_action = launch_info.GetFileActionForFD (STDERR_FILENO);
-    stderr_path = GetFilePath (file_action, stderr_path);
+    if (file_action)
+        stderr_path = file_action->GetPath ();
+
+    if (log)
+    {
+        if (!stdin_path.empty ())
+            log->Printf ("NativeProcessLinux::%s setting STDIN to '%s'", __FUNCTION__, stdin_path.c_str ());
+        else
+            log->Printf ("NativeProcessLinux::%s leaving STDIN as is", __FUNCTION__);
+
+        if (!stdout_path.empty ())
+            log->Printf ("NativeProcessLinux::%s setting STDOUT to '%s'", __FUNCTION__, stdout_path.c_str ());
+        else
+            log->Printf ("NativeProcessLinux::%s leaving STDOUT as is", __FUNCTION__);
+
+        if (!stderr_path.empty ())
+            log->Printf ("NativeProcessLinux::%s setting STDERR to '%s'", __FUNCTION__, stderr_path.c_str ());
+        else
+            log->Printf ("NativeProcessLinux::%s leaving STDERR as is", __FUNCTION__);
+    }
 
     // Create the NativeProcessLinux in launch mode.
     native_process_sp.reset (new NativeProcessLinux ());
@@ -1354,9 +1352,9 @@ NativeProcessLinux::LaunchInferior (
     Module *module,
     const char *argv[],
     const char *envp[],
-    const char *stdin_path,
-    const char *stdout_path,
-    const char *stderr_path,
+    const std::string &stdin_path,
+    const std::string &stdout_path,
+    const std::string &stderr_path,
     const char *working_dir,
     const lldb_private::ProcessLaunchInfo &launch_info,
     lldb_private::Error &error)
@@ -1536,9 +1534,6 @@ NativeProcessLinux::Launch(LaunchArgs *a
 
     const char **argv = args->m_argv;
     const char **envp = args->m_envp;
-    const char *stdin_path = args->m_stdin_path;
-    const char *stdout_path = args->m_stdout_path;
-    const char *stderr_path = args->m_stderr_path;
     const char *working_dir = args->m_working_dir;
 
     lldb_utility::PseudoTerminal terminal;
@@ -1548,7 +1543,6 @@ NativeProcessLinux::Launch(LaunchArgs *a
     NativeThreadProtocolSP thread_sp;
 
     lldb::ThreadSP inferior;
-    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
     // Propagate the environment if one is not supplied.
     if (envp == NULL || envp[0] == NULL)
@@ -1558,7 +1552,7 @@ NativeProcessLinux::Launch(LaunchArgs *a
     {
         args->m_error.SetErrorToGenericError();
         args->m_error.SetErrorString("Process fork failed.");
-        goto FINISH;
+        return false;
     }
 
     // Recognized child exit status codes.
@@ -1575,64 +1569,35 @@ NativeProcessLinux::Launch(LaunchArgs *a
     // Child process.
     if (pid == 0)
     {
-        if (log)
-            log->Printf ("NativeProcessLinux::%s inferior process preparing to fork", __FUNCTION__);
-
-        // Trace this process.
-        if (log)
-            log->Printf ("NativeProcessLinux::%s inferior process issuing PTRACE_TRACEME", __FUNCTION__);
+        // FIXME consider opening a pipe between parent/child and have this forked child
+        // send log info to parent re: launch status, in place of the log lines removed here.
 
+        // Start tracing this child that is about to exec.
         if (PTRACE(PTRACE_TRACEME, 0, NULL, NULL, 0) < 0)
-        {
-            if (log)
-                log->Printf ("NativeProcessLinux::%s inferior process PTRACE_TRACEME failed", __FUNCTION__);
             exit(ePtraceFailed);
-        }
 
         // Do not inherit setgid powers.
-        if (log)
-            log->Printf ("NativeProcessLinux::%s inferior process resetting gid", __FUNCTION__);
-
         if (setgid(getgid()) != 0)
-        {
-            if (log)
-                log->Printf ("NativeProcessLinux::%s inferior process setgid() failed", __FUNCTION__);
             exit(eSetGidFailed);
-        }
 
         // Attempt to have our own process group.
-        // TODO verify if we really want this.
-        if (log)
-            log->Printf ("NativeProcessLinux::%s inferior process resetting process group", __FUNCTION__);
-
         if (setpgid(0, 0) != 0)
         {
-            if (log)
-            {
-                const int error_code = errno;
-                log->Printf ("NativeProcessLinux::%s inferior setpgid() failed, errno=%d (%s), continuing with existing process group %" PRIu64,
-                        __FUNCTION__,
-                        error_code,
-                        strerror (error_code),
-                        static_cast<lldb::pid_t> (getpgid (0)));
-            }
+            // FIXME log that this failed. This is common.
             // Don't allow this to prevent an inferior exec.
         }
 
         // Dup file descriptors if needed.
-        //
-        // FIXME: If two or more of the paths are the same we needlessly open
-        // the same file multiple times.
-        if (stdin_path != NULL && stdin_path[0])
-            if (!DupDescriptor(stdin_path, STDIN_FILENO, O_RDONLY))
+        if (!args->m_stdin_path.empty ())
+            if (!DupDescriptor(args->m_stdin_path.c_str (), STDIN_FILENO, O_RDONLY))
                 exit(eDupStdinFailed);
 
-        if (stdout_path != NULL && stdout_path[0])
-            if (!DupDescriptor(stdout_path, STDOUT_FILENO, O_WRONLY | O_CREAT))
+        if (!args->m_stdout_path.empty ())
+            if (!DupDescriptor(args->m_stdout_path.c_str (), STDOUT_FILENO, O_WRONLY | O_CREAT))
                 exit(eDupStdoutFailed);
 
-        if (stderr_path != NULL && stderr_path[0])
-            if (!DupDescriptor(stderr_path, STDERR_FILENO, O_WRONLY | O_CREAT))
+        if (!args->m_stderr_path.empty ())
+            if (!DupDescriptor(args->m_stderr_path.c_str (), STDERR_FILENO, O_WRONLY | O_CREAT))
                 exit(eDupStderrFailed);
 
         // Change working directory
@@ -1646,34 +1611,36 @@ NativeProcessLinux::Launch(LaunchArgs *a
             const int old_personality = personality (LLDB_PERSONALITY_GET_CURRENT_SETTINGS);
             if (old_personality == -1)
             {
-                if (log)
-                    log->Printf ("NativeProcessLinux::%s retrieval of Linux personality () failed: %s. Cannot disable ASLR.", __FUNCTION__, strerror (errno));
+                // Can't retrieve Linux personality.  Cannot disable ASLR.
             }
             else
             {
                 const int new_personality = personality (ADDR_NO_RANDOMIZE | old_personality);
                 if (new_personality == -1)
                 {
-                    if (log)
-                        log->Printf ("NativeProcessLinux::%s setting of Linux personality () to disable ASLR failed, ignoring: %s", __FUNCTION__, strerror (errno));
-
+                    // Disabling ASLR failed.
                 }
                 else
                 {
-                    if (log)
-                        log->Printf ("NativeProcessLinux::%s disabling ASLR: SUCCESS", __FUNCTION__);
-
+                    // Disabling ASLR succeeded.
                 }
             }
         }
 
-        // Execute.  We should never return.
+        // Execute.  We should never return...
         execve(argv[0],
                const_cast<char *const *>(argv),
                const_cast<char *const *>(envp));
+
+        // ...unless exec fails.  In which case we definitely need to end the child here.
         exit(eExecFailed);
     }
 
+    //
+    // This is the parent code here.
+    //
+    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
+
     // Wait for the child process to trap on its call to execve.
     ::pid_t wpid;
     int status;
@@ -1688,7 +1655,7 @@ NativeProcessLinux::Launch(LaunchArgs *a
         // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
         monitor->SetState (StateType::eStateInvalid);
 
-        goto FINISH;
+        return false;
     }
     else if (WIFEXITED(status))
     {
@@ -1733,7 +1700,7 @@ NativeProcessLinux::Launch(LaunchArgs *a
         // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
         monitor->SetState (StateType::eStateInvalid);
 
-        goto FINISH;
+        return false;
     }
     assert(WIFSTOPPED(status) && (wpid == static_cast< ::pid_t> (pid)) &&
            "Could not sync with inferior process.");
@@ -1753,7 +1720,7 @@ NativeProcessLinux::Launch(LaunchArgs *a
         // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
         monitor->SetState (StateType::eStateInvalid);
 
-        goto FINISH;
+        return false;
     }
 
     // Release the master terminal descriptor and pass it off to the
@@ -1775,7 +1742,7 @@ NativeProcessLinux::Launch(LaunchArgs *a
         // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
         monitor->SetState (StateType::eStateInvalid);
 
-        goto FINISH;
+        return false;
     }
 
     if (log)
@@ -1789,7 +1756,6 @@ NativeProcessLinux::Launch(LaunchArgs *a
     // Let our process instance know the thread has stopped.
     monitor->SetState (StateType::eStateStopped);
 
-FINISH:
     if (log)
     {
         if (args->m_error.Success ())
@@ -2239,8 +2205,6 @@ NativeProcessLinux::MonitorSIGTRAP(const
     case (SIGTRAP | (PTRACE_EVENT_EXIT << 8)):
     {
         // The inferior process or one of its threads is about to exit.
-        // Maintain the process or thread in a state of "limbo" until we are
-        // explicitly commanded to detach, destroy, resume, etc.
         unsigned long data = 0;
         if (!GetEventMessage(pid, &data))
             data = -1;
@@ -2266,14 +2230,11 @@ NativeProcessLinux::MonitorSIGTRAP(const
         if (is_main_thread)
         {
             SetExitStatus (convert_pid_status_to_exit_type (data), convert_pid_status_to_return_code (data), nullptr, true);
-            // Resume the thread so it completely exits.
-            Resume (pid, LLDB_INVALID_SIGNAL_NUMBER);
-        }
-        else
-        {
-            // FIXME figure out the path where we plan to reap the metadata for the thread.
         }
 
+        // Resume the thread so it completely exits.
+        Resume (pid, LLDB_INVALID_SIGNAL_NUMBER);
+
         break;
     }
 
@@ -3804,7 +3765,7 @@ NativeProcessLinux::GetOrCreateThread (l
 Error
 NativeProcessLinux::FixupBreakpointPCAsNeeded (NativeThreadProtocolSP &thread_sp)
 {
-    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
+    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_BREAKPOINTS));
 
     Error error;
 

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Sat Oct 11 16:42:09 2014
@@ -220,9 +220,9 @@ namespace lldb_private
                     lldb_private::Module *module,
                     char const **argv,
                     char const **envp,
-                    const char *stdin_path,
-                    const char *stdout_path,
-                    const char *stderr_path,
+                    const std::string &stdin_path,
+                    const std::string &stdout_path,
+                    const std::string &stderr_path,
                     const char *working_dir,
                     const lldb_private::ProcessLaunchInfo &launch_info);
 
@@ -231,9 +231,9 @@ namespace lldb_private
             lldb_private::Module *m_module; // The executable image to launch.
             char const **m_argv;            // Process arguments.
             char const **m_envp;            // Process environment.
-            const char *m_stdin_path;       // Redirect stdin or NULL.
-            const char *m_stdout_path;      // Redirect stdout or NULL.
-            const char *m_stderr_path;      // Redirect stderr or NULL.
+            const std::string &m_stdin_path;  // Redirect stdin if not empty.
+            const std::string &m_stdout_path; // Redirect stdout if not empty.
+            const std::string &m_stderr_path; // Redirect stderr if not empty.
             const char *m_working_dir;      // Working directory or NULL.
             const lldb_private::ProcessLaunchInfo &m_launch_info;
         };
@@ -260,9 +260,9 @@ namespace lldb_private
             Module *module,
             char const *argv[],
             char const *envp[],
-            const char *stdin_path,
-            const char *stdout_path,
-            const char *stderr_path,
+            const std::string &stdin_path,
+            const std::string &stdout_path,
+            const std::string &stderr_path,
             const char *working_dir,
             const lldb_private::ProcessLaunchInfo &launch_info,
             Error &error);

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp Sat Oct 11 16:42:09 2014
@@ -483,6 +483,27 @@ GDBRemoteCommunicationServer::LaunchProc
         return LaunchPlatformProcess ();
 }
 
+bool
+GDBRemoteCommunicationServer::ShouldRedirectInferiorOutputOverGdbRemote (const lldb_private::ProcessLaunchInfo &launch_info) const
+{
+    // Retrieve the file actions specified for stdout and stderr.
+    auto stdout_file_action = launch_info.GetFileActionForFD (STDOUT_FILENO);
+    auto stderr_file_action = launch_info.GetFileActionForFD (STDERR_FILENO);
+
+    // If neither stdout and stderr file actions are specified, we're not doing anything special, so
+    // assume we want to redirect stdout/stderr over gdb-remote $O messages.
+    if ((stdout_file_action == nullptr) && (stderr_file_action == nullptr))
+    {
+        // Send stdout/stderr over the gdb-remote protocol.
+        return true;
+    }
+
+    // Any other setting for either stdout or stderr implies we are either suppressing
+    // it (with /dev/null) or we've got it set to a PTY.  Either way, we don't want the
+    // output over gdb-remote.
+    return false;
+}
+
 lldb_private::Error
 GDBRemoteCommunicationServer::LaunchProcessForDebugging ()
 {
@@ -507,20 +528,34 @@ GDBRemoteCommunicationServer::LaunchProc
         return error;
     }
 
-    // Setup stdout/stderr mapping from inferior.
-    auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor ();
-    if (terminal_fd >= 0)
+    // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol as needed.
+    // llgs local-process debugging may specify PTYs, which will eliminate the need to reflect inferior
+    // stdout/stderr over the gdb-remote protocol.
+    if (ShouldRedirectInferiorOutputOverGdbRemote (m_process_launch_info))
     {
         if (log)
-            log->Printf ("ProcessGDBRemoteCommunicationServer::%s setting inferior STDIO fd to %d", __FUNCTION__, terminal_fd);
-        error = SetSTDIOFileDescriptor (terminal_fd);
-        if (error.Fail ())
-            return error;
+            log->Printf ("GDBRemoteCommunicationServer::%s pid %" PRIu64 " setting up stdout/stderr redirection via $O gdb-remote commands", __FUNCTION__, m_debugged_process_sp->GetID ());
+
+        // Setup stdout/stderr mapping from inferior to $O
+        auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor ();
+        if (terminal_fd >= 0)
+        {
+            if (log)
+                log->Printf ("ProcessGDBRemoteCommunicationServer::%s setting inferior STDIO fd to %d", __FUNCTION__, terminal_fd);
+            error = SetSTDIOFileDescriptor (terminal_fd);
+            if (error.Fail ())
+                return error;
+        }
+        else
+        {
+            if (log)
+                log->Printf ("ProcessGDBRemoteCommunicationServer::%s ignoring inferior STDIO since terminal fd reported as %d", __FUNCTION__, terminal_fd);
+        }
     }
     else
     {
         if (log)
-            log->Printf ("ProcessGDBRemoteCommunicationServer::%s ignoring inferior STDIO since terminal fd reported as %d", __FUNCTION__, terminal_fd);
+            log->Printf ("GDBRemoteCommunicationServer::%s pid %" PRIu64 " skipping stdout/stderr redirection via $O: inferior will communicate over client-provided file descriptors", __FUNCTION__, m_debugged_process_sp->GetID ());
     }
 
     printf ("Launched '%s' as process %" PRIu64 "...\n", m_process_launch_info.GetArguments ().GetArgumentAtIndex (0), m_process_launch_info.GetProcessID ());
@@ -3757,7 +3792,7 @@ GDBRemoteCommunicationServer::Handle_z (
     }
 
     // Parse out software or hardware breakpoint requested.
-    packet.SetFilePos (strlen("Z"));
+    packet.SetFilePos (strlen("z"));
     if (packet.GetBytesLeft() < 1)
         return SendIllFormedResponse(packet, "Too short z packet, missing software/hardware specifier");
 
@@ -3797,7 +3832,7 @@ GDBRemoteCommunicationServer::Handle_z (
 
     if (want_breakpoint)
     {
-        // Try to set the breakpoint.
+        // Try to clear the breakpoint.
         const Error error = m_debugged_process_sp->RemoveBreakpoint (breakpoint_addr);
         if (error.Success ())
             return SendOKResponse ();

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h Sat Oct 11 16:42:09 2014
@@ -152,8 +152,6 @@ public:
     //------------------------------------------------------------------
     /// Specify the program to launch and its arguments.
     ///
-    /// The LaunchProcess () command can be executed to do the lauching.
-    ///
     /// @param[in] args
     ///     The command line to launch.
     ///
@@ -170,8 +168,6 @@ public:
     //------------------------------------------------------------------
     /// Specify the launch flags for the process.
     ///
-    /// The LaunchProcess () command can be executed to do the lauching.
-    ///
     /// @param[in] launch_flags
     ///     The launch flags to use when launching this process.
     ///
@@ -543,6 +539,9 @@ private:
     void
     ClearProcessSpecificData ();
 
+    bool
+    ShouldRedirectInferiorOutputOverGdbRemote (const lldb_private::ProcessLaunchInfo &launch_info) const;
+
     //------------------------------------------------------------------
     // For GDBRemoteCommunicationServer only
     //------------------------------------------------------------------

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=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Sat Oct 11 16:42:09 2014
@@ -748,8 +748,12 @@ ProcessGDBRemote::WillLaunchOrAttach ()
 Error
 ProcessGDBRemote::DoLaunch (Module *exe_module, ProcessLaunchInfo &launch_info)
 {
+    Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
     Error error;
 
+    if (log)
+        log->Printf ("ProcessGDBRemote::%s() entered", __FUNCTION__);
+
     uint32_t launch_flags = launch_info.GetFlags().Get();
     const char *stdin_path = NULL;
     const char *stdout_path = NULL;
@@ -776,10 +780,21 @@ ProcessGDBRemote::DoLaunch (Module *exe_
             stderr_path = file_action->GetPath();
     }
 
+    if (log)
+    {
+        if (stdin_path || stdout_path || stderr_path)
+            log->Printf ("ProcessGDBRemote::%s provided with STDIO paths via launch_info: stdin=%s, stdout=%s, stdout=%s",
+                         __FUNCTION__,
+                         stdin_path ? stdin_path : "<null>",
+                         stdout_path ? stdout_path : "<null>",
+                         stderr_path ? stderr_path : "<null>");
+        else
+            log->Printf ("ProcessGDBRemote::%s no STDIO paths given via launch_info", __FUNCTION__);
+    }
+
     //  ::LogSetBitMask (GDBR_LOG_DEFAULT);
     //  ::LogSetOptions (LLDB_LOG_OPTION_THREADSAFE | LLDB_LOG_OPTION_PREPEND_TIMESTAMP | LLDB_LOG_OPTION_PREPEND_PROC_AND_THREAD);
     //  ::LogSetLogFile ("/dev/stdout");
-    Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
 
     ObjectFile * object_file = exe_module->GetObjectFile();
     if (object_file)
@@ -816,6 +831,13 @@ ProcessGDBRemote::DoLaunch (Module *exe_
 
                 if (stderr_path == NULL)
                     stderr_path = slave_name;
+
+                if (log)
+                    log->Printf ("ProcessGDBRemote::%s adjusted STDIO paths for local platform (IsHost() is true) using slave: stdin=%s, stdout=%s, stdout=%s",
+                                 __FUNCTION__,
+                                 stdin_path ? stdin_path : "<null>",
+                                 stdout_path ? stdout_path : "<null>",
+                                 stderr_path ? stderr_path : "<null>");
             }
 
             // Set STDIN to /dev/null if we want STDIO disabled or if either
@@ -833,7 +855,14 @@ ProcessGDBRemote::DoLaunch (Module *exe_
             if (disable_stdio || (stderr_path == NULL && (stdin_path || stdout_path)))
                 stderr_path = "/dev/null";
 
-            if (stdin_path) 
+            if (log)
+                log->Printf ("ProcessGDBRemote::%s final STDIO paths after all adjustments: stdin=%s, stdout=%s, stdout=%s",
+                             __FUNCTION__,
+                             stdin_path ? stdin_path : "<null>",
+                             stdout_path ? stdout_path : "<null>",
+                             stderr_path ? stderr_path : "<null>");
+
+            if (stdin_path)
                 m_gdb_comm.SetSTDIN (stdin_path);
             if (stdout_path)
                 m_gdb_comm.SetSTDOUT (stdout_path);
@@ -1025,18 +1054,38 @@ ProcessGDBRemote::DidLaunchOrAttach (Arc
         // prefer that over the Host information as it will be more specific
         // to our process.
 
-        if (m_gdb_comm.GetProcessArchitecture().IsValid())
-            process_arch = m_gdb_comm.GetProcessArchitecture();
+        const ArchSpec &remote_process_arch = m_gdb_comm.GetProcessArchitecture();
+        if (remote_process_arch.IsValid())
+        {
+            process_arch = remote_process_arch;
+            if (log)
+                log->Printf ("ProcessGDBRemote::%s gdb-remote had process architecture, using %s %s",
+                             __FUNCTION__,
+                             process_arch.GetArchitectureName () ? process_arch.GetArchitectureName () : "<null>",
+                             process_arch.GetTriple().getTriple ().c_str() ? process_arch.GetTriple().getTriple ().c_str() : "<null>");
+        }
         else
+        {
             process_arch = m_gdb_comm.GetHostArchitecture();
+            if (log)
+                log->Printf ("ProcessGDBRemote::%s gdb-remote did not have process architecture, using gdb-remote host architecture %s %s",
+                             __FUNCTION__,
+                             process_arch.GetArchitectureName () ? process_arch.GetArchitectureName () : "<null>",
+                             process_arch.GetTriple().getTriple ().c_str() ? process_arch.GetTriple().getTriple ().c_str() : "<null>");
+        }
 
         if (process_arch.IsValid())
         {
             ArchSpec &target_arch = GetTarget().GetArchitecture();
-
             if (target_arch.IsValid())
             {
-                // If the remote host is ARM and we have apple as the vendor, then 
+                if (log)
+                    log->Printf ("ProcessGDBRemote::%s analyzing target arch, currently %s %s",
+                                 __FUNCTION__,
+                                 target_arch.GetArchitectureName () ? target_arch.GetArchitectureName () : "<null>",
+                                 target_arch.GetTriple().getTriple ().c_str() ? target_arch.GetTriple().getTriple ().c_str() : "<null>");
+
+                // If the remote host is ARM and we have apple as the vendor, then
                 // ARM executables and shared libraries can have mixed ARM architectures.
                 // You can have an armv6 executable, and if the host is armv7, then the
                 // system will load the best possible architecture for all shared libraries
@@ -1047,6 +1096,11 @@ ProcessGDBRemote::DidLaunchOrAttach (Arc
                     process_arch.GetTriple().getVendor() == llvm::Triple::Apple)
                 {
                     GetTarget().SetArchitecture (process_arch);
+                    if (log)
+                        log->Printf ("ProcessGDBRemote::%s remote process is ARM/Apple, setting target arch to %s %s",
+                                     __FUNCTION__,
+                                     process_arch.GetArchitectureName () ? process_arch.GetArchitectureName () : "<null>",
+                                     process_arch.GetTriple().getTriple ().c_str() ? process_arch.GetTriple().getTriple ().c_str() : "<null>");
                 }
                 else
                 {
@@ -1065,7 +1119,14 @@ ProcessGDBRemote::DidLaunchOrAttach (Arc
                                 target_triple.setEnvironment (remote_triple.getEnvironment());
                         }
                     }
+
                 }
+
+                if (log)
+                    log->Printf ("ProcessGDBRemote::%s final target arch after adjustments for remote architecture: %s %s",
+                                 __FUNCTION__,
+                                 target_arch.GetArchitectureName () ? target_arch.GetArchitectureName () : "<null>",
+                                 target_arch.GetTriple().getTriple ().c_str() ? target_arch.GetTriple().getTriple ().c_str() : "<null>");
             }
             else
             {
@@ -1094,7 +1155,12 @@ ProcessGDBRemote::DoAttachToProcessWithI
 Error
 ProcessGDBRemote::DoAttachToProcessWithID (lldb::pid_t attach_pid, const ProcessAttachInfo &attach_info)
 {
+    Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PROCESS));
     Error error;
+
+    if (log)
+        log->Printf ("ProcessGDBRemote::%s()", __FUNCTION__);
+
     // Clear out and clean up from any current state
     Clear();
     if (attach_pid != LLDB_INVALID_PROCESS_ID)
@@ -1117,13 +1183,14 @@ ProcessGDBRemote::DoAttachToProcessWithI
         if (error.Success())
         {
             m_gdb_comm.SetDetachOnError(attach_info.GetDetachOnError());
-            
+
             char packet[64];
             const int packet_len = ::snprintf (packet, sizeof(packet), "vAttach;%" PRIx64, attach_pid);
             SetID (attach_pid);            
             m_async_broadcaster.BroadcastEvent (eBroadcastBitAsyncContinue, new EventDataBytes (packet, packet_len));
         }
     }
+
     return error;
 }
 

Modified: lldb/trunk/source/Target/ProcessLaunchInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ProcessLaunchInfo.cpp?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Target/ProcessLaunchInfo.cpp (original)
+++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp Sat Oct 11 16:42:09 2014
@@ -9,6 +9,7 @@
 
 #include "lldb/Host/Config.h"
 
+#include "lldb/Core/Log.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Target/FileAction.h"
 #include "lldb/Target/Target.h"
@@ -262,13 +263,22 @@ ProcessLaunchInfo::SetDetachOnError (boo
 void
 ProcessLaunchInfo::FinalizeFileActions (Target *target, bool default_to_use_pty)
 {
+    Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
+
     // If nothing for stdin or stdout or stderr was specified, then check the process for any default
     // settings that were set with "settings set"
     if (GetFileActionForFD(STDIN_FILENO) == NULL || GetFileActionForFD(STDOUT_FILENO) == NULL ||
         GetFileActionForFD(STDERR_FILENO) == NULL)
     {
+        if (log)
+            log->Printf ("ProcessLaunchInfo::%s at least one of stdin/stdout/stderr was not set, evaluating default handling",
+                         __FUNCTION__);
+
         if (m_flags.Test(eLaunchFlagDisableSTDIO))
         {
+            if (log)
+                log->Printf ("ProcessLaunchInfo::%s eLaunchFlagDisableSTDIO set, adding suppression action for stdin, stdout and stderr",
+                             __FUNCTION__);
             AppendSuppressFileAction (STDIN_FILENO , true, false);
             AppendSuppressFileAction (STDOUT_FILENO, false, true);
             AppendSuppressFileAction (STDERR_FILENO, false, true);
@@ -289,29 +299,63 @@ ProcessLaunchInfo::FinalizeFileActions (
                 err_path = target->GetStandardErrorPath();
             }
 
+            if (log)
+                log->Printf ("ProcessLaunchInfo::%s target stdin='%s', target stdout='%s', stderr='%s'",
+                             __FUNCTION__,
+                              in_path ?  in_path.GetPath().c_str () : "<null>",
+                             out_path ? out_path.GetPath().c_str () : "<null>",
+                             err_path ? err_path.GetPath().c_str () : "<null>");
+
             char path[PATH_MAX];
             if (in_path && in_path.GetPath(path, sizeof(path)))
+            {
                 AppendOpenFileAction(STDIN_FILENO, path, true, false);
+                if (log)
+                    log->Printf ("ProcessLaunchInfo::%s appended stdin open file action for %s",
+                                 __FUNCTION__,
+                                 in_path.GetPath().c_str ());
+            }
 
             if (out_path && out_path.GetPath(path, sizeof(path)))
+            {
                 AppendOpenFileAction(STDOUT_FILENO, path, false, true);
+                if (log)
+                    log->Printf ("ProcessLaunchInfo::%s appended stdout open file action for %s",
+                                 __FUNCTION__,
+                                 out_path.GetPath().c_str ());
+            }
 
             if (err_path && err_path.GetPath(path, sizeof(path)))
+            {
+                if (log)
+                    log->Printf ("ProcessLaunchInfo::%s appended stderr open file action for %s",
+                                 __FUNCTION__,
+                                 err_path.GetPath().c_str ());
                 AppendOpenFileAction(STDERR_FILENO, path, false, true);
+            }
+
+            if (default_to_use_pty && (!in_path || !out_path || !err_path))
+            {
+                if (log)
+                    log->Printf ("ProcessLaunchInfo::%s default_to_use_pty is set, and at least one stdin/stderr/stdout is unset, so generating a pty to use for it",
+                                 __FUNCTION__);
 
-            if (default_to_use_pty && (!in_path || !out_path || !err_path)) {
-                if (m_pty->OpenFirstAvailableMaster(O_RDWR| O_NOCTTY, NULL, 0)) {
+                if (m_pty->OpenFirstAvailableMaster(O_RDWR| O_NOCTTY, NULL, 0))
+                {
                     const char *slave_path = m_pty->GetSlaveName(NULL, 0);
 
-                    if (!in_path) {
+                    if (!in_path)
+                    {
                         AppendOpenFileAction(STDIN_FILENO, slave_path, true, false);
                     }
 
-                    if (!out_path) {
+                    if (!out_path)
+                    {
                         AppendOpenFileAction(STDOUT_FILENO, slave_path, false, true);
                     }
 
-                    if (!err_path) {
+                    if (!err_path)
+                    {
                         AppendOpenFileAction(STDERR_FILENO, slave_path, false, true);
                     }
                 }

Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=219578&r1=219577&r2=219578&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Sat Oct 11 16:42:09 2014
@@ -2375,6 +2375,13 @@ Target::Launch (Listener &listener, Proc
     // Finalize the file actions, and if none were given, default to opening
     // up a pseudo terminal
     const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false;
+    if (log)
+        log->Printf ("Target::%s have platform=%s, platform_sp->IsHost()=%s, default_to_use_pty=%s",
+                     __FUNCTION__,
+                     platform_sp ? "true" : "false",
+                     platform_sp ? (platform_sp->IsHost () ? "true" : "false") : "n/a",
+                     default_to_use_pty ? "true" : "false");
+
     launch_info.FinalizeFileActions (this, default_to_use_pty);
     
     if (state == eStateConnected)





More information about the lldb-commits mailing list