[Lldb-commits] [lldb] r223695 - Fix some posix assumptions related to running shell commands.
Zachary Turner
zturner at google.com
Mon Dec 8 13:36:42 PST 2014
Author: zturner
Date: Mon Dec 8 15:36:42 2014
New Revision: 223695
URL: http://llvm.org/viewvc/llvm-project?rev=223695&view=rev
Log:
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
Modified:
lldb/trunk/source/Host/common/FileSpec.cpp
lldb/trunk/source/Host/common/Host.cpp
lldb/trunk/source/Host/common/HostInfoBase.cpp
lldb/trunk/source/Host/windows/FileSystem.cpp
lldb/trunk/source/Target/ProcessLaunchInfo.cpp
Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=223695&r1=223694&r2=223695&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Mon Dec 8 15:36:42 2014
@@ -1340,18 +1340,31 @@ FileSpec::IsSourceImplementationFile ()
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)
{
Modified: lldb/trunk/source/Host/common/Host.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Host.cpp?rev=223695&r1=223694&r2=223695&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/Host.cpp (original)
+++ lldb/trunk/source/Host/common/Host.cpp Mon Dec 8 15:36:42 2014
@@ -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 *comma
{
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 *comma
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 *comma
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 *comma
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 *comma
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...
Modified: lldb/trunk/source/Host/common/HostInfoBase.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/HostInfoBase.cpp?rev=223695&r1=223694&r2=223695&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/HostInfoBase.cpp (original)
+++ lldb/trunk/source/Host/common/HostInfoBase.cpp Mon Dec 8 15:36:42 2014
@@ -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(F
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;
}
Modified: lldb/trunk/source/Host/windows/FileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/windows/FileSystem.cpp?rev=223695&r1=223694&r2=223695&view=diff
==============================================================================
--- lldb/trunk/source/Host/windows/FileSystem.cpp (original)
+++ lldb/trunk/source/Host/windows/FileSystem.cpp Mon Dec 8 15:36:42 2014
@@ -27,7 +27,7 @@ FileSystem::MakeDirectory(const char *pa
// 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;
}
Modified: lldb/trunk/source/Target/ProcessLaunchInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ProcessLaunchInfo.cpp?rev=223695&r1=223694&r2=223695&view=diff
==============================================================================
--- lldb/trunk/source/Target/ProcessLaunchInfo.cpp (original)
+++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp Mon Dec 8 15:36:42 2014
@@ -396,14 +396,20 @@ ProcessLaunchInfo::ConvertArgumentsForLa
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::ConvertArgumentsForLa
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
More information about the lldb-commits
mailing list