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 ↗

(On Diff #21013)

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

source/Host/android/ProcessLauncherAndroid.cpp

42 ↗

(On Diff #21013)

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

[tberghammer]

include/lldb/Host/android/HostInfoAndroid.h

26 ↗

(On Diff #21013)

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 ↗

(On Diff #21013)

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

source/Host/android/ProcessLauncherAndroid.cpp

42 ↗

(On Diff #21013)

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

[labath]

lgtm

lldb/trunk/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_

lldb/trunk/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);
+}

lldb/trunk/source/Host/android/ProcessLauncherAndroid.cpp

 
 #include "lldb/Target/ProcessLaunchInfo.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)
+{
+    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));
 
-    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)
     {
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDIN_FILENO)) {
+            const char* path = file_action->GetPath();
+            if (path && ::strlen(path))
+                if (!DupDescriptor(path, STDIN_FILENO, O_RDONLY))
+                    exit(-1);
+        }
+
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDOUT_FILENO)) {
+            const char* path = file_action->GetPath();
+            if (path && ::strlen(path))
+                if (!DupDescriptor(path, STDOUT_FILENO, O_WRONLY | O_CREAT | O_TRUNC))
+                    exit(-1);
+        }
+
+        if (const lldb_private::FileAction *file_action = launch_info.GetFileActionForFD(STDERR_FILENO)) {
+            const char* path = file_action->GetPath();
+            if (path && ::strlen(path))
+                if (!DupDescriptor(path, 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);
 }