[Lldb-commits] [lldb] [lldb] Have Host::RunShellCommand ret stderr & stdout seperately (PR #184548)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Mar 3 21:22:15 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jason Molenda (jasonmolenda)
<details>
<summary>Changes</summary>
Host::RunShellCommand takes a std::string &command_output argument and a bool hide_stderr=false defaulted argument. If the shell command returns stderr and stdout text, it is intermixed in the same command_output, unless hide_stderr=true.
In SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile we call an external program to find a binary and dSYM by uuid, and the external program returns a plist (xml) output. In some cases, it printed a (harmless) warning message to stderr, and then a complete plist output to stdout. We attempt to parse the combination of these two streams, and the parse fails - we don't get the output.
This patch removes hide_stderr and instead adds a
std::string &error_output argument, and callers can choose to use/print/return error_output, or ignore it. This keeps command_output the clean stdout from the shell command.
rdar://168621579
---
Patch is 35.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/184548.diff
19 Files Affected:
- (modified) lldb/include/lldb/Host/Host.h (+80-37)
- (modified) lldb/include/lldb/Target/Platform.h (+4)
- (modified) lldb/include/lldb/Target/RemoteAwarePlatform.h (+2-1)
- (modified) lldb/source/API/SBPlatform.cpp (+2-1)
- (modified) lldb/source/Commands/CommandObjectPlatform.cpp (+6-3)
- (modified) lldb/source/Host/common/Host.cpp (+58-14)
- (modified) lldb/source/Host/macosx/objcxx/Host.mm (+2-2)
- (modified) lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm (+6-2)
- (modified) lldb/source/Host/windows/Host.cpp (+4-3)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+7-6)
- (modified) lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (+5-5)
- (modified) lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp (+4-2)
- (modified) lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h (+2)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+2)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+2)
- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp (+3-1)
- (modified) lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp (+4-1)
- (modified) lldb/source/Target/Platform.cpp (+7-2)
- (modified) lldb/source/Target/RemoteAwarePlatform.cpp (+8-7)
``````````diff
diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index 4e19d15b06743..a9ab31d180591 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -195,65 +195,108 @@ class Host {
static Status ShellExpandArguments(ProcessLaunchInfo &launch_info);
/// Run a shell command.
- /// \arg command shouldn't be empty
- /// \arg working_dir Pass empty FileSpec to use the current working directory
- /// \arg status_ptr Pass NULL if you don't want the process exit status
- /// \arg signo_ptr Pass NULL if you don't want the signal that caused the
- /// process to exit
- /// \arg command_output Pass NULL if you don't want the command output
- /// \arg hide_stderr if this is false, redirect stderr to stdout
+ /// \param[in] command
+ /// Command to execute, should not be empty.
+ /// \param[in] working_dir
+ /// Pass empty FileSpec to use the current working directory
+ /// \param[out] status_ptr
+ /// Pass nullptr if you don't want the process exit status
+ /// \param[out] signo_ptr
+ /// Pass nullptr if you don't want the signal that caused the
+ /// process to exit
+ /// \param[out] command_output
+ /// Pass nullptr if you don't want the command output
+ /// \param[out] error_output
+ /// Pass nullptr if you don't want the command error output
+ /// \param[in] timeout
+ /// Timeout duration to enforce
+ /// \param[in] run_in_shell
+ /// Run in a subshell, with glob expansion of args
static Status RunShellCommand(llvm::StringRef command,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output,
+ std::string *error_output,
const Timeout<std::micro> &timeout,
- bool run_in_shell = true,
- bool hide_stderr = false);
+ bool run_in_shell = true);
/// Run a shell command.
- /// \arg shell Pass an empty string if you want to use the default shell
- /// interpreter \arg command \arg working_dir Pass empty FileSpec to use the
- /// current working directory \arg status_ptr Pass NULL if you don't want
- /// the process exit status \arg signo_ptr Pass NULL if you don't want the
- /// signal that caused
- /// the process to exit
- /// \arg command_output Pass NULL if you don't want the command output
- /// \arg hide_stderr If this is \b false, redirect stderr to stdout
+ /// \param[in] shell
+ /// Pass an empty string to use the default shell
+ /// \param[in] command
+ /// Command to execute, should not be empty.
+ /// \param[in] working_dir
+ /// Pass empty FileSpec to use the current working directory
+ /// \param[out] status_ptr
+ /// Pass nullptr if you don't want the process exit status
+ /// \param[out] signo_ptr
+ /// Pass nullptr if you don't want the signal that caused the
+ /// process to exit
+ /// \param[out] command_output
+ /// Pass nullptr if you don't want the command output
+ /// \param[out] error_output
+ /// Pass nullptr if you don't want the command error output
+ /// \param[in] timeout
+ /// Timeout duration to enforce
+ /// \param[in] run_in_shell
+ /// Run in a subshell, with glob expansion of args
static Status RunShellCommand(llvm::StringRef shell, llvm::StringRef command,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output,
+ std::string *error_output,
const Timeout<std::micro> &timeout,
- bool run_in_shell = true,
- bool hide_stderr = false);
+ bool run_in_shell = true);
/// Run a shell command.
- /// \arg working_dir Pass empty FileSpec to use the current working directory
- /// \arg status_ptr Pass NULL if you don't want the process exit status
- /// \arg signo_ptr Pass NULL if you don't want the signal that caused the
- /// process to exit
- /// \arg command_output Pass NULL if you don't want the command output
- /// \arg hide_stderr if this is false, redirect stderr to stdout
+ /// \param[in] args
+ /// Command to execute
+ /// \param[in] working_dir
+ /// Pass empty FileSpec to use the current working directory
+ /// \param[out] status_ptr
+ /// Pass nullptr if you don't want the process exit status
+ /// \param[out] signo_ptr
+ /// Pass nullptr if you don't want the signal that caused the
+ /// process to exit
+ /// \param[out] command_output
+ /// Pass nullptr if you don't want the command output
+ /// \param[out] error_output
+ /// Pass nullptr if you don't want the command error output
+ /// \param[in] timeout
+ /// Timeout duration to enforce
+ /// \param[in] run_in_shell
+ /// Run in a subshell, with glob expansion of args
static Status RunShellCommand(const Args &args, const FileSpec &working_dir,
int *status_ptr, int *signo_ptr,
std::string *command_output,
+ std::string *error_output,
const Timeout<std::micro> &timeout,
- bool run_in_shell = true,
- bool hide_stderr = false);
+ bool run_in_shell = true);
/// Run a shell command.
- /// \arg shell Pass an empty string if you want to use the default
- /// shell interpreter \arg command \arg working_dir Pass empty FileSpec to use
- /// the current working directory \arg status_ptr Pass NULL if you don't
- /// want the process exit status \arg signo_ptr Pass NULL if you don't
- /// want the signal that caused the
- /// process to exit
- /// \arg command_output Pass NULL if you don't want the command output
- /// \arg hide_stderr If this is \b false, redirect stderr to stdout
+ /// \param[in] shell
+ /// Pass an empty string to use the default shell
+ /// \param[in] args
+ /// Command to execute
+ /// \param[in] working_dir
+ /// Pass empty FileSpec to use the current working directory
+ /// \param[out] status_ptr
+ /// Pass nullptr if you don't want the process exit status
+ /// \param[out] signo_ptr
+ /// Pass nullptr if you don't want the signal that caused the
+ /// process to exit
+ /// \param[out] command_output
+ /// Pass nullptr if you don't want the command output
+ /// \param[out] error_output
+ /// Pass nullptr if you don't want the command error output
+ /// \param[in] timeout
+ /// Timeout duration to enforce
+ /// \param[in] run_in_shell
+ /// Run in a subshell, with glob expansion of args
static Status RunShellCommand(llvm::StringRef shell, const Args &args,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output,
+ std::string *error_output,
const Timeout<std::micro> &timeout,
- bool run_in_shell = true,
- bool hide_stderr = false);
+ bool run_in_shell = true);
static llvm::Error OpenFileInExternalEditor(llvm::StringRef editor,
const FileSpec &file_spec,
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 637d4c37b90bc..ee79c946ad91d 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -677,6 +677,8 @@ class Platform : public PluginInterface {
// the process to exit
std::string
*command_output, // Pass nullptr if you don't want the command output
+ std::string
+ *error_output, // Pass nullptr if you don't want the command output
const Timeout<std::micro> &timeout);
virtual lldb_private::Status RunShellCommand(
@@ -688,6 +690,8 @@ class Platform : public PluginInterface {
// the process to exit
std::string
*command_output, // Pass nullptr if you don't want the command output
+ std::string
+ *error_output, // Pass nullptr if you don't want the command output
const Timeout<std::micro> &timeout);
virtual void SetLocalCacheDirectory(const char *local);
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index de13b18f30d85..c1d8d9994654d 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -70,12 +70,13 @@ class RemoteAwarePlatform : public Platform {
Status RunShellCommand(llvm::StringRef command, const FileSpec &working_dir,
int *status_ptr, int *signo_ptr,
- std::string *command_output,
+ std::string *command_output, std::string *error_output,
const Timeout<std::micro> &timeout) override;
Status RunShellCommand(llvm::StringRef interpreter, llvm::StringRef command,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output,
+ std::string *error,
const Timeout<std::micro> &timeout) override;
const char *GetHostname() override;
diff --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 9a0b47ce80336..5bab9d9e112fd 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -559,12 +559,13 @@ SBError SBPlatform::Run(SBPlatformShellCommand &shell_command) {
if (!platform_working_dir.empty())
shell_command.SetWorkingDirectory(platform_working_dir.c_str());
}
+ std::string stderr_output;
return platform_sp->RunShellCommand(
shell_command.m_opaque_ptr->m_shell, command,
FileSpec(shell_command.GetWorkingDirectory()),
&shell_command.m_opaque_ptr->m_status,
&shell_command.m_opaque_ptr->m_signo,
- &shell_command.m_opaque_ptr->m_output,
+ &shell_command.m_opaque_ptr->m_output, &stderr_output,
shell_command.m_opaque_ptr->m_timeout);
});
}
diff --git a/lldb/source/Commands/CommandObjectPlatform.cpp b/lldb/source/Commands/CommandObjectPlatform.cpp
index 4865c48c82b7f..d7c87f36f6e2e 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -1706,13 +1706,16 @@ class CommandObjectPlatformShell : public CommandObjectRaw {
if (platform_sp) {
FileSpec working_dir{};
std::string output;
+ std::string error_output;
int status = -1;
int signo = -1;
- error = (platform_sp->RunShellCommand(m_options.m_shell_interpreter, cmd,
- working_dir, &status, &signo,
- &output, m_options.m_timeout));
+ error = (platform_sp->RunShellCommand(
+ m_options.m_shell_interpreter, cmd, working_dir, &status, &signo,
+ &output, &error_output, m_options.m_timeout));
if (!output.empty())
result.GetOutputStream().PutCString(output);
+ if (!error_output.empty())
+ result.GetOutputStream().PutCString(error_output);
if (status > 0) {
if (signo > 0) {
const char *signo_cstr = Host::GetSignalAsCString(signo);
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index 510f9c7696d12..8c0de586eaa7d 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -389,39 +389,43 @@ MonitorShellCommand(std::shared_ptr<ShellInfo> shell_info, lldb::pid_t pid,
Status Host::RunShellCommand(llvm::StringRef command,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output_ptr,
+ std::string *command_error_ptr,
const Timeout<std::micro> &timeout,
- bool run_in_shell, bool hide_stderr) {
+ bool run_in_shell) {
return RunShellCommand(llvm::StringRef(), Args(command), working_dir,
- status_ptr, signo_ptr, command_output_ptr, timeout,
- run_in_shell, hide_stderr);
+ status_ptr, signo_ptr, command_output_ptr,
+ command_error_ptr, timeout, run_in_shell);
}
Status Host::RunShellCommand(llvm::StringRef shell_path,
llvm::StringRef command,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output_ptr,
+ std::string *command_error_ptr,
const Timeout<std::micro> &timeout,
- bool run_in_shell, bool hide_stderr) {
+ bool run_in_shell) {
return RunShellCommand(shell_path, Args(command), working_dir, status_ptr,
- signo_ptr, command_output_ptr, timeout, run_in_shell,
- hide_stderr);
+ signo_ptr, command_output_ptr, command_error_ptr,
+ timeout);
}
Status Host::RunShellCommand(const Args &args, const FileSpec &working_dir,
int *status_ptr, int *signo_ptr,
std::string *command_output_ptr,
+ std::string *command_error_ptr,
const Timeout<std::micro> &timeout,
- bool run_in_shell, bool hide_stderr) {
+ bool run_in_shell) {
return RunShellCommand(llvm::StringRef(), args, working_dir, status_ptr,
- signo_ptr, command_output_ptr, timeout, run_in_shell,
- hide_stderr);
+ signo_ptr, command_output_ptr, command_error_ptr,
+ timeout);
}
Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
const FileSpec &working_dir, int *status_ptr,
int *signo_ptr, std::string *command_output_ptr,
+ std::string *command_error_ptr,
const Timeout<std::micro> &timeout,
- bool run_in_shell, bool hide_stderr) {
+ bool run_in_shell) {
Status error;
ProcessLaunchInfo launch_info;
launch_info.SetArchitecture(HostInfo::GetArchitecture());
@@ -448,9 +452,10 @@ Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
if (working_dir)
launch_info.SetWorkingDirectory(working_dir);
llvm::SmallString<64> output_file_path;
+ llvm::SmallString<64> error_file_path;
if (command_output_ptr) {
- // Create a temporary file to get the stdout/stderr and redirect the output
+ // Create a temporary file to get the stdout and redirect the output
// of the command into this file. We will later read this file if all goes
// well and fill the data into "command_output_ptr"
if (FileSpec tmpdir_file_spec = HostInfo::GetProcessTempDir()) {
@@ -463,7 +468,22 @@ Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
}
}
+ if (command_error_ptr) {
+ // Create a temporary file to get the stderr and redirect the output
+ // of the command into this file. We will later read this file if all goes
+ // well and fill the data into "command_output_ptr"
+ if (FileSpec tmpdir_file_spec = HostInfo::GetProcessTempDir()) {
+ tmpdir_file_spec.AppendPathComponent("lldb-shell-error.%%%%%%");
+ llvm::sys::fs::createUniqueFile(tmpdir_file_spec.GetPath(),
+ error_file_path);
+ } else {
+ llvm::sys::fs::createTemporaryFile("lldb-shell-error.%%%%%%", "",
+ error_file_path);
+ }
+ }
+
FileSpec output_file_spec(output_file_path.str());
+ FileSpec error_file_spec(error_file_path.str());
// Set up file descriptors.
launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
if (output_file_spec)
@@ -472,8 +492,9 @@ Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
else
launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
- if (output_file_spec && !hide_stderr)
- launch_info.AppendDuplicateFileAction(STDOUT_FILENO, STDERR_FILENO);
+ if (error_file_spec)
+ launch_info.AppendOpenFileAction(STDERR_FILENO, error_file_spec, false,
+ true);
else
launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
@@ -524,10 +545,33 @@ Status Host::RunShellCommand(llvm::StringRef shell_path, const Args &args,
}
}
}
+ if (command_error_ptr) {
+ command_error_ptr->clear();
+ uint64_t file_size =
+ FileSystem::Instance().GetByteSize(error_file_spec);
+ if (file_size > 0) {
+ if (file_size > command_error_ptr->max_size()) {
+ error = Status::FromErrorStringWithFormat(
+ "shell command error output is too large to fit into a "
+ "std::string");
+ } else {
+ WritableDataBufferSP Buffer =
+ FileSystem::Instance().CreateWritableDataBuffer(
+ error_file_spec);
+ if (error.Success())
+ command_error_ptr->assign(
+ reinterpret_cast<char *>(Buffer->GetBytes()),
+ Buffer->GetByteSize());
+ }
+ }
+ }
}
}
- llvm::sys::fs::remove(output_file_spec.GetPath());
+ if (output_file_spec)
+ llvm::sys::fs::remove(output_file_spec.GetPath());
+ if (error_file_spec)
+ llvm::sys::fs::remove(error_file_spec.GetPath());
return error;
}
diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm
index f52b78f257ca8..4d6ad08e1ad74 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -1516,6 +1516,7 @@ static bool ShouldLaunchUsingXPC(ProcessLaunchInfo &launch_info) {
int status;
std::string output;
+ std::string error_output;
FileSpec cwd(launch_info.GetWorkingDirectory());
if (!FileSystem::Instance().Exists(cwd)) {
char *wd = getcwd(nullptr, 0);
@@ -1530,10 +1531,9 @@ static bool ShouldLaunchUsingXPC(ProcessLaunchInfo &launch_info) {
}
}
bool run_in_shell = true;
- bool hide_stderr = true;
Status e =
RunShellCommand(expand_command, cwd, &status, nullptr, &output,
- std::chrono::seconds(10), run_in_shell, hide_stderr);
+ &error_output, std::chrono::seconds(10), run_in_shell);
if (e.Fail())
return e;
diff --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 63cef827a91c3..7f5acadffef94 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -452,12 +452,14 @@ static bool ResolveAndVerifyCandidateSupportDir(FileSpec &path) {
int status = 0;
int signo = 0;
std::string output_str;
+ std::string error_str;
// The first time after Xcode was updated or freshly installed,
// xcrun can take surprisingly long to build up its database.
auto timeout = std::chrono::seconds(60);
bool run_in_shell = false;
...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/184548
More information about the lldb-commits
mailing list