[Lldb-commits] [lldb] r276288 - Unify process launching code on linux

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 21 07:54:04 PDT 2016


Author: labath
Date: Thu Jul 21 09:54:03 2016
New Revision: 276288

URL: http://llvm.org/viewvc/llvm-project?rev=276288&view=rev
Log:
Unify process launching code on linux

Summary:
We've had two copies of code for launching processes:
- one in NativeProcessLinux, used for launching debugged processes
- one in ProcessLauncherAndroid, used on android for launching all other kinds of processes

These have over time acquired support for various launch options, but neither supported all of
them. I now replace them with a single implementation ProcessLauncherLinux, which supports all
the options the individual versions supported and set it to be used to launch all processes on
linux.

This also works around the ETXTBSY issue on android when the process is started from the platform
instance, as that used to go through the version which did not contain the workaround.

Reviewers: tberghammer

Subscribers: tberghammer, danalbert, srhines, lldb-commits

Differential Revision: https://reviews.llvm.org/D22457

Added:
    lldb/trunk/include/lldb/Host/linux/ProcessLauncherLinux.h
      - copied, changed from r276254, lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h
    lldb/trunk/source/Host/linux/ProcessLauncherLinux.cpp
Removed:
    lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h
    lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp
Modified:
    lldb/trunk/source/Host/CMakeLists.txt
    lldb/trunk/source/Host/common/Host.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h?rev=276287&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h (original)
+++ lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h (removed)
@@ -1,26 +0,0 @@
-//===-- ProcessLauncherAndroid.h --------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef lldb_Host_android_ProcessLauncherAndroid_h_
-#define lldb_Host_android_ProcessLauncherAndroid_h_
-
-#include "lldb/Host/ProcessLauncher.h"
-
-namespace lldb_private
-{
-
-class ProcessLauncherAndroid : public ProcessLauncher
-{
-  public:
-    virtual HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error);
-};
-
-} // end of namespace lldb_private
-
-#endif

Copied: lldb/trunk/include/lldb/Host/linux/ProcessLauncherLinux.h (from r276254, lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h)
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/linux/ProcessLauncherLinux.h?p2=lldb/trunk/include/lldb/Host/linux/ProcessLauncherLinux.h&p1=lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h&r1=276254&r2=276288&rev=276288&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/android/ProcessLauncherAndroid.h (original)
+++ lldb/trunk/include/lldb/Host/linux/ProcessLauncherLinux.h Thu Jul 21 09:54:03 2016
@@ -15,10 +15,11 @@
 namespace lldb_private
 {
 
-class ProcessLauncherAndroid : public ProcessLauncher
+class ProcessLauncherLinux : public ProcessLauncher
 {
-  public:
-    virtual HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error);
+public:
+    virtual HostProcess
+    LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error);
 };
 
 } // end of namespace lldb_private

Modified: lldb/trunk/source/Host/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/CMakeLists.txt?rev=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Host/CMakeLists.txt (original)
+++ lldb/trunk/source/Host/CMakeLists.txt Thu Jul 21 09:54:03 2016
@@ -120,24 +120,17 @@ else()
       add_host_subdirectory(android
         android/HostInfoAndroid.cpp
         android/LibcGlue.cpp
-        android/ProcessLauncherAndroid.cpp
-        linux/AbstractSocket.cpp
-        linux/Host.cpp
-        linux/HostInfoLinux.cpp
-        linux/HostThreadLinux.cpp
-        linux/LibcGlue.cpp
-        linux/ThisThread.cpp
-        )
-    else()
-      add_host_subdirectory(linux
-        linux/AbstractSocket.cpp
-        linux/Host.cpp
-        linux/HostInfoLinux.cpp
-        linux/HostThreadLinux.cpp
-        linux/LibcGlue.cpp
-        linux/ThisThread.cpp
         )
     endif()
+    add_host_subdirectory(linux
+      linux/AbstractSocket.cpp
+      linux/Host.cpp
+      linux/HostInfoLinux.cpp
+      linux/HostThreadLinux.cpp
+      linux/LibcGlue.cpp
+      linux/ProcessLauncherLinux.cpp
+      linux/ThisThread.cpp
+      )
 
   elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
     add_host_subdirectory(freebsd

Removed: lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp?rev=276287&view=auto
==============================================================================
--- lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp (original)
+++ lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp (removed)
@@ -1,108 +0,0 @@
-//===-- ProcessLauncherAndroid.cpp ------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/FileSpec.h"
-#include "lldb/Host/Host.h"
-#include "lldb/Host/HostProcess.h"
-#include "lldb/Host/android/ProcessLauncherAndroid.h"
-
-#include "lldb/Target/ProcessLaunchInfo.h"
-
-#include <limits.h>
-
-using namespace lldb;
-using namespace lldb_private;
-
-static bool
-DupDescriptor(const FileSpec &file_spec, int fd, int flags)
-{
-    int target_fd = ::open(file_spec.GetCString(), flags, 0666);
-
-    if (target_fd == -1)
-        return false;
-
-    if (::dup2(target_fd, fd) == -1)
-        return false;
-
-    return (::close(target_fd) == -1) ? false : true;
-}
-
-// If there is no PATH variable specified inside the environment then set the path to /system/bin.
-// It is required because the default path used by execve() is wrong on android.
-static void
-FixupEnvironment(Args& env)
-{
-    static const char* path = "PATH=";
-    static const int path_len = ::strlen(path);
-    for (const char** args = env.GetConstArgumentVector(); *args; ++args)
-        if (::strncmp(path, *args, path_len) == 0)
-            return;
-    env.AppendArgument("PATH=/system/bin");
-}
-
-HostProcess
-ProcessLauncherAndroid::LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error)
-{
-    // TODO: Handle other launch parameters specified in launc_info
-
-    char exe_path[PATH_MAX];
-    launch_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path));
-
-    lldb::pid_t pid = ::fork();
-    if (pid == static_cast<lldb::pid_t>(-1))
-    {
-        // Fork failed
-        error.SetErrorStringWithFormat("Fork failed with error message: %s", strerror(errno));
-        return HostProcess(LLDB_INVALID_PROCESS_ID);
-    }
-    else if (pid == 0)
-    {
-        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDIN_FILENO)) {
-            FileSpec file_spec = file_action->GetFileSpec();
-            if (file_spec)
-                if (!DupDescriptor(file_spec, STDIN_FILENO, O_RDONLY))
-                    exit(-1);
-        }
-
-        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDOUT_FILENO)) {
-            FileSpec file_spec = file_action->GetFileSpec();
-            if (file_spec)
-                if (!DupDescriptor(file_spec, STDOUT_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
-                    exit(-1);
-        }
-
-        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDERR_FILENO)) {
-            FileSpec file_spec = file_action->GetFileSpec();
-            if (file_spec)
-                if (!DupDescriptor(file_spec, STDERR_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
-                    exit(-1);
-        }
-
-        // Child process
-        const char **argv = launch_info.GetArguments().GetConstArgumentVector();
-
-        Args env = launch_info.GetEnvironmentEntries();
-        FixupEnvironment(env);
-        const char **envp = env.GetConstArgumentVector();
-
-        FileSpec working_dir = launch_info.GetWorkingDirectory();
-        if (working_dir)
-        {
-            if (::chdir(working_dir.GetCString()) != 0)
-                exit(-1);
-        }
-
-        execve(argv[0],
-               const_cast<char *const *>(argv),
-               const_cast<char *const *>(envp));
-        exit(-1);
-    }
-   
-    return HostProcess(pid);
-}

Modified: lldb/trunk/source/Host/common/Host.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/Host.cpp (original)
+++ lldb/trunk/source/Host/common/Host.cpp Thu Jul 21 09:54:03 2016
@@ -66,8 +66,8 @@
 
 #if defined(_WIN32)
 #include "lldb/Host/windows/ProcessLauncherWindows.h"
-#elif defined(__ANDROID__) || defined(__ANDROID_NDK__)
-#include "lldb/Host/android/ProcessLauncherAndroid.h"
+#elif defined(__linux__)
+#include "lldb/Host/linux/ProcessLauncherLinux.h"
 #else
 #include "lldb/Host/posix/ProcessLauncherPosix.h"
 #endif
@@ -1009,8 +1009,8 @@ Host::LaunchProcess (ProcessLaunchInfo &
     std::unique_ptr<ProcessLauncher> delegate_launcher;
 #if defined(_WIN32)
     delegate_launcher.reset(new ProcessLauncherWindows());
-#elif defined(__ANDROID__) || defined(__ANDROID_NDK__)
-    delegate_launcher.reset(new ProcessLauncherAndroid());
+#elif defined(__linux__)
+    delegate_launcher.reset(new ProcessLauncherLinux());
 #else
     delegate_launcher.reset(new ProcessLauncherPosix());
 #endif

Added: lldb/trunk/source/Host/linux/ProcessLauncherLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/linux/ProcessLauncherLinux.cpp?rev=276288&view=auto
==============================================================================
--- lldb/trunk/source/Host/linux/ProcessLauncherLinux.cpp (added)
+++ lldb/trunk/source/Host/linux/ProcessLauncherLinux.cpp Thu Jul 21 09:54:03 2016
@@ -0,0 +1,213 @@
+//===-- ProcessLauncherLinux.cpp --------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Host/linux/ProcessLauncherLinux.h"
+#include "lldb/Core/Log.h"
+#include "lldb/Host/FileSpec.h"
+#include "lldb/Host/Host.h"
+#include "lldb/Host/HostProcess.h"
+#include "lldb/Host/Pipe.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+#include <limits.h>
+#include <sys/personality.h>
+#include <sys/ptrace.h>
+#include <sys/wait.h>
+
+#include <sstream>
+
+using namespace lldb;
+using namespace lldb_private;
+
+static void
+FixupEnvironment(Args &env)
+{
+#ifdef __ANDROID_NDK__
+    // If there is no PATH variable specified inside the environment then set the path to /system/bin.
+    // It is required because the default path used by execve() is wrong on android.
+    static const char *path = "PATH=";
+    static const int path_len = ::strlen(path);
+    for (const char **args = env.GetConstArgumentVector(); *args; ++args)
+        if (::strncmp(path, *args, path_len) == 0)
+            return;
+    env.AppendArgument("PATH=/system/bin");
+#endif
+}
+
+static void LLVM_ATTRIBUTE_NORETURN
+ExitWithError(int error_fd, const char *operation)
+{
+    std::ostringstream os;
+    os << operation << " failed: " << strerror(errno);
+    write(error_fd, os.str().data(), os.str().size());
+    close(error_fd);
+    _exit(1);
+}
+
+static void
+DupDescriptor(int error_fd, const FileSpec &file_spec, int fd, int flags)
+{
+    int target_fd = ::open(file_spec.GetCString(), flags, 0666);
+
+    if (target_fd == -1)
+        ExitWithError(error_fd, "DupDescriptor-open");
+
+    if (target_fd == fd)
+        return;
+
+    if (::dup2(target_fd, fd) == -1)
+        ExitWithError(error_fd, "DupDescriptor-dup2");
+
+    ::close(target_fd);
+    return;
+}
+
+static void LLVM_ATTRIBUTE_NORETURN
+ChildFunc(int error_fd, const ProcessLaunchInfo &info)
+{
+    // First, make sure we disable all logging. If we are logging to stdout, our logs can be
+    // mistaken for inferior output.
+    Log::DisableAllLogChannels(nullptr);
+
+    // Do not inherit setgid powers.
+    if (setgid(getgid()) != 0)
+        ExitWithError(error_fd, "setgid");
+
+    if (info.GetFlags().Test(eLaunchFlagLaunchInSeparateProcessGroup))
+    {
+        if (setpgid(0, 0) != 0)
+            ExitWithError(error_fd, "setpgid");
+    }
+
+    for (size_t i = 0; i < info.GetNumFileActions(); ++i)
+    {
+        const FileAction &action = *info.GetFileActionAtIndex(i);
+        switch (action.GetAction())
+        {
+            case FileAction::eFileActionClose:
+                if (close(action.GetFD()) != 0)
+                    ExitWithError(error_fd, "close");
+                break;
+            case FileAction::eFileActionDuplicate:
+                if (dup2(action.GetFD(), action.GetActionArgument()) == -1)
+                    ExitWithError(error_fd, "dup2");
+                break;
+            case FileAction::eFileActionOpen:
+                DupDescriptor(error_fd, action.GetFileSpec(), action.GetFD(), action.GetActionArgument());
+                break;
+            case FileAction::eFileActionNone:
+                break;
+        }
+    }
+
+    const char **argv = info.GetArguments().GetConstArgumentVector();
+
+    // Change working directory
+    if (info.GetWorkingDirectory() && 0 != ::chdir(info.GetWorkingDirectory().GetCString()))
+        ExitWithError(error_fd, "chdir");
+
+    // Disable ASLR if requested.
+    if (info.GetFlags().Test(lldb::eLaunchFlagDisableASLR))
+    {
+        const unsigned long personality_get_current = 0xffffffff;
+        int value = personality(personality_get_current);
+        if (value == -1)
+            ExitWithError(error_fd, "personality get");
+
+        value = personality(ADDR_NO_RANDOMIZE | value);
+        if (value == -1)
+            ExitWithError(error_fd, "personality set");
+    }
+
+    Args env = info.GetEnvironmentEntries();
+    FixupEnvironment(env);
+    const char **envp = env.GetConstArgumentVector();
+
+    // Clear the signal mask to prevent the child from being affected by
+    // any masking done by the parent.
+    sigset_t set;
+    if (sigemptyset(&set) != 0 || pthread_sigmask(SIG_SETMASK, &set, nullptr) != 0)
+        ExitWithError(error_fd, "pthread_sigmask");
+
+    if (info.GetFlags().Test(eLaunchFlagDebug))
+    {
+        // HACK:
+        // Close everything besides stdin, stdout, and stderr that has no file
+        // action to avoid leaking. Only do this when debugging, as elsewhere we actually rely on
+        // passing open descriptors to child processes.
+        for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
+            if (!info.GetFileActionForFD(fd) && fd != error_fd)
+                close(fd);
+
+        // Start tracing this child that is about to exec.
+        if (ptrace(PTRACE_TRACEME, 0, nullptr, nullptr) == -1)
+            ExitWithError(error_fd, "ptrace");
+    }
+
+    // Execute.  We should never return...
+    execve(argv[0], const_cast<char *const *>(argv), const_cast<char *const *>(envp));
+
+    if (errno == ETXTBSY)
+    {
+        // On android M and earlier we can get this error because the adb deamon can hold a write
+        // handle on the executable even after it has finished uploading it. This state lasts
+        // only a short time and happens only when there are many concurrent adb commands being
+        // issued, such as when running the test suite. (The file remains open when someone does
+        // an "adb shell" command in the fork() child before it has had a chance to exec.) Since
+        // this state should clear up quickly, wait a while and then give it one more go.
+        usleep(50000);
+        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.
+    ExitWithError(error_fd, "execve");
+}
+
+HostProcess
+ProcessLauncherLinux::LaunchProcess(const ProcessLaunchInfo &launch_info, Error &error)
+{
+    char exe_path[PATH_MAX];
+    launch_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path));
+
+    // A pipe used by the child process to report errors.
+    PipePosix pipe;
+    const bool child_processes_inherit = false;
+    error = pipe.CreateNew(child_processes_inherit);
+    if (error.Fail())
+        return HostProcess();
+
+    ::pid_t pid = ::fork();
+    if (pid == -1)
+    {
+        // Fork failed
+        error.SetErrorStringWithFormat("Fork failed with error message: %s", strerror(errno));
+        return HostProcess(LLDB_INVALID_PROCESS_ID);
+    }
+    if (pid == 0)
+    {
+        // child process
+        pipe.CloseReadFileDescriptor();
+        ChildFunc(pipe.ReleaseWriteFileDescriptor(), launch_info);
+    }
+
+    // parent process
+
+    pipe.CloseWriteFileDescriptor();
+    char buf[1000];
+    int r = read(pipe.GetReadFileDescriptor(), buf, sizeof buf);
+
+    if (r == 0)
+        return HostProcess(pid); // No error. We're done.
+
+    error.SetErrorString(buf);
+
+    waitpid(pid, nullptr, 0);
+
+    return HostProcess();
+}

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=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Thu Jul 21 09:54:03 2016
@@ -29,9 +29,11 @@
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostProcess.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Host/common/NativeBreakpoint.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
+#include "lldb/Host/linux/ProcessLauncherLinux.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
@@ -60,8 +62,6 @@
 #include "lldb/Host/linux/Uio.h"
 #include "lldb/Host/android/Android.h"
 
-#define LLDB_PERSONALITY_GET_CURRENT_SETTINGS  0xffffffff
-
 // Support hardware breakpoints in case it has not been defined
 #ifndef TRAP_HWBKPT
   #define TRAP_HWBKPT 4
@@ -130,37 +130,6 @@ ResolveProcessArchitecture(lldb::pid_t p
         return Error("failed to retrieve a valid architecture from the exe module");
 }
 
-// Used to notify the parent about which part of the launch sequence failed.
-enum LaunchCallSpecifier
-{
-    ePtraceFailed,
-    eDupStdinFailed,
-    eDupStdoutFailed,
-    eDupStderrFailed,
-    eChdirFailed,
-    eExecFailed,
-    eSetGidFailed,
-    eSetSigMaskFailed,
-    eLaunchCallMax = eSetSigMaskFailed
-};
-
-static uint8_t LLVM_ATTRIBUTE_NORETURN
-ExitChildAbnormally(LaunchCallSpecifier spec)
-{
-    static_assert(eLaunchCallMax < 0x8, "Have more launch calls than we are able to represent");
-    // This may truncate the topmost bits of the errno because the exit code is only 8 bits wide.
-    // However, it should still give us a pretty good indication of what went wrong. (And the
-    // most common errors have small numbers anyway).
-    _exit(unsigned(spec) | (errno << 3));
-}
-
-// The second member is the errno (or its 5 lowermost bits anyway).
-inline std::pair<LaunchCallSpecifier, uint8_t>
-DecodeChildExitCode(int exit_code)
-{
-    return std::make_pair(LaunchCallSpecifier(exit_code & 0x7), exit_code >> 3);
-}
-
 void
 MaybeLogLaunchInfo(const ProcessLaunchInfo &info)
 {
@@ -411,101 +380,6 @@ NativeProcessLinux::AttachToInferior (Ma
     Attach(pid, error);
 }
 
-void
-NativeProcessLinux::ChildFunc(const ProcessLaunchInfo &info)
-{
-    // Start tracing this child that is about to exec.
-    if (ptrace(PTRACE_TRACEME, 0, nullptr, nullptr) == -1)
-        ExitChildAbnormally(ePtraceFailed);
-
-    // Do not inherit setgid powers.
-    if (setgid(getgid()) != 0)
-        ExitChildAbnormally(eSetGidFailed);
-
-    // Attempt to have our own process group.
-    if (setpgid(0, 0) != 0)
-    {
-        // FIXME log that this failed. This is common.
-        // Don't allow this to prevent an inferior exec.
-    }
-
-    // Dup file descriptors if needed.
-    if (const FileAction *action = info.GetFileActionForFD(STDIN_FILENO))
-        if (!DupDescriptor(action->GetFileSpec(), STDIN_FILENO, O_RDONLY))
-            ExitChildAbnormally(eDupStdinFailed);
-
-    if (const FileAction *action = info.GetFileActionForFD(STDOUT_FILENO))
-        if (!DupDescriptor(action->GetFileSpec(), STDOUT_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
-            ExitChildAbnormally(eDupStdoutFailed);
-
-    if (const FileAction *action = info.GetFileActionForFD(STDERR_FILENO))
-        if (!DupDescriptor(action->GetFileSpec(), STDERR_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
-            ExitChildAbnormally(eDupStderrFailed);
-
-    // Close everything besides stdin, stdout, and stderr that has no file
-    // action to avoid leaking
-    for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
-        if (!info.GetFileActionForFD(fd))
-            close(fd);
-
-    // Change working directory
-    if (info.GetWorkingDirectory() && 0 != ::chdir(info.GetWorkingDirectory().GetCString()))
-        ExitChildAbnormally(eChdirFailed);
-
-    // Disable ASLR if requested.
-    if (info.GetFlags().Test(lldb::eLaunchFlagDisableASLR))
-    {
-        const int old_personality = personality(LLDB_PERSONALITY_GET_CURRENT_SETTINGS);
-        if (old_personality == -1)
-        {
-            // Can't retrieve Linux personality.  Cannot disable ASLR.
-        }
-        else
-        {
-            const int new_personality = personality(ADDR_NO_RANDOMIZE | old_personality);
-            if (new_personality == -1)
-            {
-                // Disabling ASLR failed.
-            }
-            else
-            {
-                // Disabling ASLR succeeded.
-            }
-        }
-    }
-
-    // Clear the signal mask to prevent the child from being affected by
-    // any masking done by the parent.
-    sigset_t set;
-    if (sigemptyset(&set) != 0 || pthread_sigmask(SIG_SETMASK, &set, nullptr) != 0)
-        ExitChildAbnormally(eSetSigMaskFailed);
-
-    const char **argv = info.GetArguments().GetConstArgumentVector();
-
-    // Propagate the environment if one is not supplied.
-    const char **envp = info.GetEnvironmentEntries().GetConstArgumentVector();
-    if (envp == NULL || envp[0] == NULL)
-        envp = const_cast<const char **>(environ);
-
-    // Execute.  We should never return...
-    execve(argv[0], const_cast<char *const *>(argv), const_cast<char *const *>(envp));
-
-    if (errno == ETXTBSY)
-    {
-        // On android M and earlier we can get this error because the adb deamon can hold a write
-        // handle on the executable even after it has finished uploading it. This state lasts
-        // only a short time and happens only when there are many concurrent adb commands being
-        // issued, such as when running the test suite. (The file remains open when someone does
-        // an "adb shell" command in the fork() child before it has had a chance to exec.) Since
-        // this state should clear up quickly, wait a while and then give it one more go.
-        usleep(50000);
-        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.
-    ExitChildAbnormally(eExecFailed);
-}
-
 Error
 NativeProcessLinux::LaunchInferior(MainLoop &mainloop, ProcessLaunchInfo &launch_info)
 {
@@ -516,34 +390,11 @@ NativeProcessLinux::LaunchInferior(MainL
 
     SetState(eStateLaunching);
 
-    lldb_utility::PseudoTerminal terminal;
-    const size_t err_len = 1024;
-    char err_str[err_len];
-    lldb::pid_t pid;
-
     MaybeLogLaunchInfo(launch_info);
 
-    if ((pid = terminal.Fork(err_str, err_len)) == static_cast<lldb::pid_t> (-1))
-    {
-        error.SetErrorToGenericError();
-        error.SetErrorStringWithFormat("Process fork failed: %s", err_str);
+    ::pid_t pid = ProcessLauncherLinux().LaunchProcess(launch_info, error).GetProcessId();
+    if (error.Fail())
         return error;
-    }
-
-    // Child process.
-    if (pid == 0)
-    {
-        // First, make sure we disable all logging. If we are logging to stdout, our logs can be
-        // mistaken for inferior output.
-        Log::DisableAllLogChannels(nullptr);
-
-        // terminal has already dupped the tty descriptors to stdin/out/err.
-        // This closes original fd from which they were copied (and avoids
-        // leaking descriptors to the debugged process.
-        terminal.CloseSlaveFileDescriptor();
-
-        ChildFunc(launch_info);
-    }
 
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
@@ -563,53 +414,6 @@ NativeProcessLinux::LaunchInferior(MainL
 
         return error;
     }
-    else if (WIFEXITED(status))
-    {
-        auto p = DecodeChildExitCode(WEXITSTATUS(status));
-        Error child_error(p.second, eErrorTypePOSIX);
-        const char *failure_reason;
-        switch (p.first)
-        {
-            case ePtraceFailed:
-                failure_reason = "Child ptrace failed";
-                break;
-            case eDupStdinFailed:
-                failure_reason = "Child open stdin failed";
-                break;
-            case eDupStdoutFailed:
-                failure_reason = "Child open stdout failed";
-                break;
-            case eDupStderrFailed:
-                failure_reason = "Child open stderr failed";
-                break;
-            case eChdirFailed:
-                failure_reason = "Child failed to set working directory";
-                break;
-            case eExecFailed:
-                failure_reason = "Child exec failed";
-                break;
-            case eSetGidFailed:
-                failure_reason = "Child setgid failed";
-                break;
-            case eSetSigMaskFailed:
-                failure_reason = "Child failed to set signal mask";
-                break;
-        }
-        error.SetErrorStringWithFormat("%s: %d - %s (error code truncated)", failure_reason, child_error.GetError(), child_error.AsCString());
-
-        if (log)
-        {
-            log->Printf ("NativeProcessLinux::%s inferior exited with status %d before issuing a STOP",
-                    __FUNCTION__,
-                    WEXITSTATUS(status));
-        }
-
-        // Mark the inferior as invalid.
-        // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
-        SetState (StateType::eStateInvalid);
-
-        return error;
-    }
     assert(WIFSTOPPED(status) && (wpid == static_cast< ::pid_t> (pid)) &&
            "Could not sync with inferior process.");
 
@@ -632,29 +436,30 @@ NativeProcessLinux::LaunchInferior(MainL
 
     // Release the master terminal descriptor and pass it off to the
     // NativeProcessLinux instance.  Similarly stash the inferior pid.
-    m_terminal_fd = terminal.ReleaseMasterFileDescriptor();
+    m_terminal_fd = launch_info.GetPTY().ReleaseMasterFileDescriptor();
     m_pid = pid;
     launch_info.SetProcessID(pid);
 
-    // Set the terminal fd to be in non blocking mode (it simplifies the
-    // implementation of ProcessLinux::GetSTDOUT to have a non-blocking
-    // descriptor to read from).
-    error = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
-    if (error.Fail())
+    if (m_terminal_fd != -1)
     {
-        if (log)
-            log->Printf ("NativeProcessLinux::%s inferior EnsureFDFlags failed for ensuring terminal O_NONBLOCK setting: %s",
-                    __FUNCTION__, error.AsCString ());
-
-        // Mark the inferior as invalid.
-        // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
-        SetState (StateType::eStateInvalid);
+        error = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
+        if (error.Fail())
+        {
+            if (log)
+                log->Printf(
+                    "NativeProcessLinux::%s inferior EnsureFDFlags failed for ensuring terminal O_NONBLOCK setting: %s",
+                    __FUNCTION__, error.AsCString());
+
+            // Mark the inferior as invalid.
+            // FIXME this could really use a new state - eStateLaunchFailure.  For now, using eStateInvalid.
+            SetState(StateType::eStateInvalid);
 
-        return error;
+            return error;
+        }
     }
 
     if (log)
-        log->Printf ("NativeProcessLinux::%s() adding pid = %" PRIu64, __FUNCTION__, pid);
+        log->Printf("NativeProcessLinux::%s() adding pid = %" PRIu64, __FUNCTION__, uint64_t(pid));
 
     ResolveProcessArchitecture(m_pid, m_arch);
     NativeThreadLinuxSP thread_sp = AddThread(pid);
@@ -2516,20 +2321,6 @@ NativeProcessLinux::Detach(lldb::tid_t t
 }
 
 bool
-NativeProcessLinux::DupDescriptor(const FileSpec &file_spec, int fd, int flags)
-{
-    int target_fd = open(file_spec.GetCString(), flags, 0666);
-
-    if (target_fd == -1)
-        return false;
-
-    if (dup2(target_fd, fd) == -1)
-        return false;
-
-    return (close(target_fd) == -1) ? false : true;
-}
-
-bool
 NativeProcessLinux::HasThreadNoLock (lldb::tid_t thread_id)
 {
     for (auto thread_sp : m_threads)

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=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Thu Jul 21 09:54:03 2016
@@ -166,15 +166,9 @@ namespace process_linux {
         ::pid_t
         Attach(lldb::pid_t pid, Error &error);
 
-        static void
-        ChildFunc(const ProcessLaunchInfo &launch_info) LLVM_ATTRIBUTE_NORETURN;
-
         static Error
         SetDefaultPtraceOpts(const lldb::pid_t);
 
-        static bool
-        DupDescriptor(const FileSpec &file_spec, int fd, int flags);
-
         static void *
         MonitorThread(void *baton);
 

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp?rev=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp Thu Jul 21 09:54:03 2016
@@ -884,8 +884,8 @@ GDBRemoteCommunicationServerCommon::Hand
     FileAction file_action;
     std::string path;
     packet.GetHexByteString(path);
-    const bool read = false;
-    const bool write = true;
+    const bool read = true;
+    const bool write = false;
     if (file_action.Open(STDIN_FILENO, FileSpec{path, false}, read, write))
     {
         m_process_launch_info.AppendFileAction(file_action);
@@ -901,8 +901,8 @@ GDBRemoteCommunicationServerCommon::Hand
     FileAction file_action;
     std::string path;
     packet.GetHexByteString(path);
-    const bool read = true;
-    const bool write = false;
+    const bool read = false;
+    const bool write = true;
     if (file_action.Open(STDOUT_FILENO, FileSpec{path, false}, read, write))
     {
         m_process_launch_info.AppendFileAction(file_action);
@@ -918,8 +918,8 @@ GDBRemoteCommunicationServerCommon::Hand
     FileAction file_action;
     std::string path;
     packet.GetHexByteString(path);
-    const bool read = true;
-    const bool write = false;
+    const bool read = false;
+    const bool write = true;
     if (file_action.Open(STDERR_FILENO, FileSpec{path, false}, read, write))
     {
         m_process_launch_info.AppendFileAction(file_action);

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=276288&r1=276287&r2=276288&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Thu Jul 21 09:54:03 2016
@@ -203,6 +203,15 @@ GDBRemoteCommunicationServerLLGS::Launch
     if (!m_process_launch_info.GetArguments ().GetArgumentCount ())
         return Error ("%s: no process command line specified to launch", __FUNCTION__);
 
+    const bool should_forward_stdio = m_process_launch_info.GetFileActionForFD(STDIN_FILENO) == nullptr ||
+                                      m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) == nullptr ||
+                                      m_process_launch_info.GetFileActionForFD(STDERR_FILENO) == nullptr;
+    m_process_launch_info.SetLaunchInSeparateProcessGroup(true);
+    m_process_launch_info.GetFlags().Set(eLaunchFlagDebug);
+
+    const bool default_to_use_pty = true;
+    m_process_launch_info.FinalizeFileActions(nullptr, default_to_use_pty);
+
     Error error;
     {
         std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
@@ -226,11 +235,7 @@ GDBRemoteCommunicationServerLLGS::Launch
     // file actions non-null
     // process launch -i/e/o will also make these file actions non-null
     // nullptr means that the traffic is expected to flow over gdb-remote protocol
-    if (
-        m_process_launch_info.GetFileActionForFD(STDIN_FILENO) == nullptr  ||
-        m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) == nullptr  ||
-        m_process_launch_info.GetFileActionForFD(STDERR_FILENO) == nullptr
-        )
+    if (should_forward_stdio)
     {
         // nullptr means it's not redirected to file or pty (in case of LLGS local)
         // at least one of stdio will be transferred pty<->gdb-remote
@@ -998,14 +1003,6 @@ GDBRemoteCommunicationServerLLGS::StartS
     if (! m_stdio_communication.IsConnected())
         return;
 
-    // llgs local-process debugging may specify PTY paths, which will make these
-    // file actions non-null
-    // process launch -e/o will also make these file actions non-null
-    // nullptr means that the traffic is expected to flow over gdb-remote protocol
-    if ( m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) &&
-         m_process_launch_info.GetFileActionForFD(STDERR_FILENO))
-        return;
-
     Error error;
     lldbassert(! m_stdio_handle_up);
     m_stdio_handle_up = m_mainloop.RegisterReadObject(




More information about the lldb-commits mailing list