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
This commit is contained in:
Zachary Turner 2014-12-08 21:36:42 +00:00
parent a536df4b28
commit 270e99ab0a
5 changed files with 58 additions and 32 deletions

View File

@ -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)
{

View File

@ -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...

View File

@ -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;
}

View File

@ -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;
}

View File

@ -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