Fix execution of platform shell commands on android

[tberghammer]

Summary

Fix execution of platform shell commands on android

• Add missing functionality to the process launcher
• Fixup PATH environment variable to workaround an OS bug
• Add default shell path to the host info structure

[labath]

Looks good overall, but I have a couple of comments...

include/lldb/Host/android/HostInfoAndroid.h

26

How is the constructor removal related to the rest of the patch?

source/Host/android/ProcessLauncherAndroid.cpp

42

Storage specifiers (static) are usually placed before the type (including the "const" keyword).

[tberghammer]

include/lldb/Host/android/HostInfoAndroid.h

26

It is absolutely unrelated just I noticed that construction is already disabled in the bottom most base class (HostInfoBase) and non of the derived have explicit code for doing it, so made it consistent.
Can revert this part if you want.

[labath]

include/lldb/Host/android/HostInfoAndroid.h

26

Sounds like I good thing to do. Keep them removed.

source/Host/android/ProcessLauncherAndroid.cpp

42

This should be read "please put static before const". :)

[labath]

lgtm

include/lldb/Host/android/HostInfoAndroid.h

 
 namespace lldb_private
 {
 
 class HostInfoAndroid : public HostInfoLinux
 {
     friend class HostInfoBase;
 
-  private:
-    // Static class, unconstructable.
-    HostInfoAndroid();
-    ~HostInfoAndroid();
+  public:
+    static FileSpec GetDefaultShell();
 
   protected:
     static void ComputeHostArchitectureSupport(ArchSpec &arch_32, ArchSpec &arch_64);
     static bool ComputeSupportExeDirectory(FileSpec &file_spec);
 };
 
 } // end of namespace lldb_private
 
 #endif // #ifndef lldb_Host_android_HostInfoAndroid_h_

source/Host/android/HostInfoAndroid.cpp

 }
 
 bool
 HostInfoAndroid::ComputeSupportExeDirectory(FileSpec &file_spec)
 {
     file_spec.GetDirectory() = HostInfoLinux::GetProgramFileSpec().GetDirectory();
     return (bool)file_spec.GetDirectory();
 }
+
+FileSpec
+HostInfoAndroid::GetDefaultShell()
+{
+    return FileSpec("/system/bin/sh", false);
+}

source/Host/android/ProcessLauncherAndroid.cpp

 //===-- 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/Host.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/android/ProcessLauncherAndroid.h"
 
 #include "lldb/Target/ProcessLaunchInfo.h"
+#include "lldb/Core/StreamString.h"
 
 #include <limits.h>
 
 using namespace lldb;
 using namespace lldb_private;
 
+static bool
+DupDescriptor(const char *path, int fd, int flags)
+{
+    int target_fd = open(path, 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)
+{
+    const static char* path = "PATH=";
+    const static 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));
 
-    const size_t err_len = 1024;
-    char err_str[err_len];
-
-    lldb::pid_t pid = ::fork ();
-    if (pid < 0)
+    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)
     {
+        // Dup file descriptors if needed.
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD (STDIN_FILENO))
+            if (::strlen(file_action->GetPath()) &&
+                    !DupDescriptor(file_action->GetPath(), STDIN_FILENO, O_RDONLY))
+                exit(-1);
+
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD (STDOUT_FILENO))
+            if (::strlen(file_action->GetPath()) &&
+                    !DupDescriptor(file_action->GetPath(), STDOUT_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
+                exit(-1);
+
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD (STDERR_FILENO))
+            if (::strlen(file_action->GetPath()) &&
+                    !DupDescriptor(file_action->GetPath(), STDERR_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
+                exit(-1);
+
         // Child process
         const char **argv = launch_info.GetArguments().GetConstArgumentVector();
-        const char **envp = launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-        const char *working_dir = launch_info.GetWorkingDirectory();
-        
+        Args env = launch_info.GetEnvironmentEntries();
+        FixupEnvironment(env);
+        const char **envp = env.GetConstArgumentVector();
+
+        const char *working_dir = launch_info.GetWorkingDirectory();
         if (working_dir != nullptr && working_dir[0])
         {
             if (::chdir(working_dir) != 0)
                 exit(-1);
         }
 
         execve(argv[0],
                const_cast<char *const *>(argv),
                const_cast<char *const *>(envp));
         exit(-1);
     }
    
     return HostProcess(pid);
 }