Fix some posix assumptions related to running shell commands.

This is a resubmit of r223548, which was reverted due to breaking
tests on Linux and Mac.

This resubmit fixes the reason for the revert by adding back some
accidentally removed code which appends -c to the command line
when running /bin/sh.

This resubmit also differs from the original patch in that it sets
the architecture on the ProcessLaunchInfo.  A follow-up patch will
refactor this to separate the logic for different platforms.

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

Reviewed By: Greg Clayton

llvm-svn: 223695

diff --git a/lldb/source/Host/common/FileSpec.cpp b/lldb/source/Host/common/FileSpec.cpp
index 7cd7729..0af0556 100644 (file)
--- a/lldb/source/Host/common/FileSpec.cpp
+++ b/lldb/source/Host/common/FileSpec.cpp
@@ -1340,18 +1340,31 @@ FileSpec::IsSourceImplementationFile () const
 bool
 FileSpec::IsRelativeToCurrentWorkingDirectory () const
 {
-    const char *directory = m_directory.GetCString();
-    if (directory && directory[0])
+    const char *dir = m_directory.GetCString();
+    llvm::StringRef directory(dir ? dir : "");
+
+    if (directory.size() > 0)
     {
-        // If the path doesn't start with '/' or '~', return true
-        switch (directory[0])
+        if (m_syntax == ePathSyntaxWindows)
         {
-        case '/':
-        case '~':
-            return false;
-        default:
+            if (directory.size() >= 2 && directory[1] == ':')
+                return false;
+            if (directory[0] == '/')
+                return false;
             return true;
         }
+        else
+        {
+            // If the path doesn't start with '/' or '~', return true
+            switch (directory[0])
+            {
+                case '/':
+                case '~':
+                    return false;
+                default:
+                    return true;
+            }
+        }
     }
     else if (m_filename)
     {

diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index a3a22e2..c8daa17 100644 (file)
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -44,6 +44,7 @@
 // C++ includes
 #include <limits>
 
+#include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Core/ArchSpec.h"
@@ -532,6 +533,7 @@ Host::RunShellCommand (const char *command,
 {
     Error error;
     ProcessLaunchInfo launch_info;
+    launch_info.SetArchitecture(HostInfo::GetArchitecture());
     if (run_in_default_shell)
     {
         // Run the command in a shell
@@ -556,9 +558,8 @@ Host::RunShellCommand (const char *command,
     
     if (working_dir)
         launch_info.SetWorkingDirectory(working_dir);
-    char output_file_path_buffer[PATH_MAX];
-    const char *output_file_path = NULL;
-    
+    llvm::SmallString<PATH_MAX> output_file_path;
+
     if (command_output_ptr)
     {
         // Create a temporary file to get the stdout/stderr and redirect the
@@ -567,21 +568,19 @@ Host::RunShellCommand (const char *command,
         FileSpec tmpdir_file_spec;
         if (HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec))
         {
-            tmpdir_file_spec.AppendPathComponent("lldb-shell-output.XXXXXX");
-            strncpy(output_file_path_buffer, tmpdir_file_spec.GetPath().c_str(), sizeof(output_file_path_buffer));
+            tmpdir_file_spec.AppendPathComponent("lldb-shell-output.%%%%%%");
+            llvm::sys::fs::createUniqueFile(tmpdir_file_spec.GetPath().c_str(), output_file_path);
         }
         else
         {
-            strncpy(output_file_path_buffer, "/tmp/lldb-shell-output.XXXXXX", sizeof(output_file_path_buffer));
+            llvm::sys::fs::createTemporaryFile("lldb-shell-output.%%%%%%", "", output_file_path);
         }
-        
-        output_file_path = ::mktemp(output_file_path_buffer);
     }
     
     launch_info.AppendSuppressFileAction (STDIN_FILENO, true, false);
-    if (output_file_path)
+    if (!output_file_path.empty())
     {
-        launch_info.AppendOpenFileAction(STDOUT_FILENO, output_file_path, false, true);
+        launch_info.AppendOpenFileAction(STDOUT_FILENO, output_file_path.c_str(), false, true);
         launch_info.AppendDuplicateFileAction(STDOUT_FILENO, STDERR_FILENO);
     }
     else
@@ -640,7 +639,7 @@ Host::RunShellCommand (const char *command,
             if (command_output_ptr)
             {
                 command_output_ptr->clear();
-                FileSpec file_spec(output_file_path, File::eOpenOptionRead);
+                FileSpec file_spec(output_file_path.c_str(), File::eOpenOptionRead);
                 uint64_t file_size = file_spec.GetByteSize();
                 if (file_size > 0)
                 {
@@ -659,8 +658,9 @@ Host::RunShellCommand (const char *command,
         shell_info->can_delete.SetValue(true, eBroadcastAlways);
     }
 
-    if (output_file_path)
-        ::unlink (output_file_path);
+    FileSpec output_file_spec(output_file_path.c_str(), false);
+    if (FileSystem::GetFileExists(output_file_spec))
+        FileSystem::Unlink(output_file_path.c_str());
     // Handshake with the monitor thread, or just let it know in advance that
     // it can delete "shell_info" in case we timed out and were not able to kill
     // the process...

diff --git a/lldb/source/Host/common/HostInfoBase.cpp b/lldb/source/Host/common/HostInfoBase.cpp
index 9eab299..d65b796 100644 (file)
--- a/lldb/source/Host/common/HostInfoBase.cpp
+++ b/lldb/source/Host/common/HostInfoBase.cpp
@@ -18,7 +18,9 @@
 #include "lldb/Host/HostInfoBase.h"
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include <thread>
 
@@ -264,19 +266,23 @@ HostInfoBase::ComputeTempFileDirectory(FileSpec &file_spec)
     if (!tmpdir_cstr)
         return false;
 
-    StreamString pid_tmpdir;
-    pid_tmpdir.Printf("%s/lldb", tmpdir_cstr);
-    if (!FileSystem::MakeDirectory(pid_tmpdir.GetString().c_str(), eFilePermissionsDirectoryDefault).Success())
+    FileSpec temp_file_spec(tmpdir_cstr, false);
+    temp_file_spec.AppendPathComponent("lldb");
+    if (!FileSystem::MakeDirectory(temp_file_spec.GetPath().c_str(), eFilePermissionsDirectoryDefault).Success())
         return false;
 
-    pid_tmpdir.Printf("/%" PRIu64, Host::GetCurrentProcessID());
-    if (!FileSystem::MakeDirectory(pid_tmpdir.GetString().c_str(), eFilePermissionsDirectoryDefault).Success())
+    std::string pid_str;
+    llvm::raw_string_ostream pid_stream(pid_str);
+    pid_stream << Host::GetCurrentProcessID();
+    temp_file_spec.AppendPathComponent(pid_stream.str().c_str());
+    std::string final_path = temp_file_spec.GetPath();
+    if (!FileSystem::MakeDirectory(final_path.c_str(), eFilePermissionsDirectoryDefault).Success())
         return false;
 
     // Make an atexit handler to clean up the process specify LLDB temp dir
     // and all of its contents.
     ::atexit(CleanupProcessSpecificLLDBTempDir);
-    file_spec.GetDirectory().SetCStringWithLength(pid_tmpdir.GetString().c_str(), pid_tmpdir.GetString().size());
+    file_spec.GetDirectory().SetCStringWithLength(final_path.c_str(), final_path.size());
     return true;
 }
 

diff --git a/lldb/source/Host/windows/FileSystem.cpp b/lldb/source/Host/windows/FileSystem.cpp
index a735ff0..f641ff5 100644 (file)
--- a/lldb/source/Host/windows/FileSystem.cpp
+++ b/lldb/source/Host/windows/FileSystem.cpp
@@ -27,7 +27,7 @@ FileSystem::MakeDirectory(const char *path, uint32_t file_permissions)
     // On Win32, the mode parameter is ignored, as Windows files and directories support a
     // different permission model than POSIX.
     Error error;
-    if (!::CreateDirectory(path, NULL))
+    if (!::CreateDirectory(path, NULL) && GetLastError() != ERROR_ALREADY_EXISTS)
         error.SetError(::GetLastError(), lldb::eErrorTypeWin32);
     return error;
 }

diff --git a/lldb/source/Target/ProcessLaunchInfo.cpp b/lldb/source/Target/ProcessLaunchInfo.cpp
index d5f3d51..451c42d 100644 (file)
--- a/lldb/source/Target/ProcessLaunchInfo.cpp
+++ b/lldb/source/Target/ProcessLaunchInfo.cpp
@@ -396,14 +396,20 @@ ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell (Error &error,
             Args shell_arguments;
             std::string safe_arg;
             shell_arguments.AppendArgument (shell_executable.c_str());
-            shell_arguments.AppendArgument ("-c");
+            const llvm::Triple &triple = GetArchitecture().GetTriple();
+            if (triple.getOS() == llvm::Triple::Win32 && !triple.isWindowsCygwinEnvironment())
+                shell_arguments.AppendArgument("/C");
+            else
+                shell_arguments.AppendArgument("-c");
+
             StreamString shell_command;
             if (will_debug)
             {
                 // Add a modified PATH environment variable in case argv[0]
-                // is a relative path
+                // is a relative path.
                 const char *argv0 = argv[0];
-                if (argv0 && (argv0[0] != '/' && argv0[0] != '~'))
+                FileSpec arg_spec(argv0, false);
+                if (arg_spec.IsRelativeToCurrentWorkingDirectory())
                 {
                     // We have a relative path to our executable which may not work if
                     // we just try to run "a.out" (without it being converted to "./a.out")
@@ -434,7 +440,8 @@ ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell (Error &error,
                     shell_command.PutCString(new_path.c_str());
                 }
 
-                shell_command.PutCString ("exec");
+                if (triple.getOS() != llvm::Triple::Win32 || triple.isWindowsCygwinEnvironment())
+                    shell_command.PutCString("exec");
 
                 // Only Apple supports /usr/bin/arch being able to specify the architecture
                 if (GetArchitecture().IsValid() &&                                          // Valid architecture