From: Zachary Turner Date: Mon, 8 Dec 2014 21:36:42 +0000 (+0000) Subject: Fix some posix assumptions related to running shell commands. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=270e99ab0ae98413b2194b220a8253ec7754b0db;p=platform%2Fupstream%2Fllvm.git 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 --- 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 --- a/lldb/source/Host/common/Host.cpp +++ b/lldb/source/Host/common/Host.cpp @@ -44,6 +44,7 @@ // C++ includes #include +#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 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 --- 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 @@ -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 --- 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 --- 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