[Lldb-commits] [lldb] r350617 - ProcessLaunchInfo: Remove Target reference
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 8 03:55:19 PST 2019
Author: labath
Date: Tue Jan 8 03:55:19 2019
New Revision: 350617
URL: http://llvm.org/viewvc/llvm-project?rev=350617&view=rev
Log:
ProcessLaunchInfo: Remove Target reference
Summary:
The target was being used in FinalizeFileActions to provide default
values for stdin/out/err. Also, most of the logic of this function was
very specific to how the lldb's Target class wants to launch processes,
so I, move it to Target::FinalizeFileActions, inverting the dependency.
The only piece of logic that was useful elsewhere (lldb-server) was the
part which sets up a pty and relevant file actions. I've kept this part
as ProcessLaunchInfo::SetUpPtyRedirection.
This makes ProcessLaunchInfo independent of any high-level lldb constructs.
Reviewers: zturner, jingham, teemperor
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D56196
Modified:
lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
lldb/trunk/include/lldb/Target/Target.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Target/ProcessLaunchInfo.cpp
lldb/trunk/source/Target/Target.cpp
Modified: lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h?rev=350617&r1=350616&r2=350617&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h (original)
+++ lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h Tue Jan 8 03:55:19 2019
@@ -52,7 +52,10 @@ public:
bool AppendSuppressFileAction(int fd, bool read, bool write);
- void FinalizeFileActions(Target *target, bool default_to_use_pty);
+ // Redirect stdin/stdout/stderr to a pty, if no action for the respective file
+ // descriptor is specified. (So if stdin and stdout already have file actions,
+ // but stderr doesn't, then only stderr will be redirected to a pty.)
+ llvm::Error SetUpPtyRedirection();
size_t GetNumFileActions() const { return m_file_actions.size(); }
Modified: lldb/trunk/include/lldb/Target/Target.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Target.h?rev=350617&r1=350616&r2=350617&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Target.h (original)
+++ lldb/trunk/include/lldb/Target/Target.h Tue Jan 8 03:55:19 2019
@@ -1354,6 +1354,8 @@ private:
void AddBreakpoint(lldb::BreakpointSP breakpoint_sp, bool internal);
+ void FinalizeFileActions(ProcessLaunchInfo &info);
+
DISALLOW_COPY_AND_ASSIGN(Target);
};
Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=350617&r1=350616&r2=350617&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Tue Jan 8 03:55:19 2019
@@ -218,8 +218,10 @@ Status GDBRemoteCommunicationServerLLGS:
m_process_launch_info.SetLaunchInSeparateProcessGroup(true);
m_process_launch_info.GetFlags().Set(eLaunchFlagDebug);
- const bool default_to_use_pty = true;
- m_process_launch_info.FinalizeFileActions(nullptr, default_to_use_pty);
+ if (should_forward_stdio) {
+ if (llvm::Error Err = m_process_launch_info.SetUpPtyRedirection())
+ return Status(std::move(Err));
+ }
{
std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
Modified: lldb/trunk/source/Target/ProcessLaunchInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ProcessLaunchInfo.cpp?rev=350617&r1=350616&r2=350617&view=diff
==============================================================================
--- lldb/trunk/source/Target/ProcessLaunchInfo.cpp (original)
+++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp Tue Jan 8 03:55:19 2019
@@ -14,7 +14,6 @@
#include "lldb/Host/HostInfo.h"
#include "lldb/Target/FileAction.h"
#include "lldb/Target/ProcessLaunchInfo.h"
-#include "lldb/Target/Target.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/StreamString.h"
@@ -206,123 +205,38 @@ void ProcessLaunchInfo::SetDetachOnError
m_flags.Clear(lldb::eLaunchFlagDetachOnError);
}
-void ProcessLaunchInfo::FinalizeFileActions(Target *target,
- bool default_to_use_pty) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
-
- // If nothing for stdin or stdout or stderr was specified, then check the
- // process for any default settings that were set with "settings set"
- if (GetFileActionForFD(STDIN_FILENO) == nullptr ||
- GetFileActionForFD(STDOUT_FILENO) == nullptr ||
- GetFileActionForFD(STDERR_FILENO) == nullptr) {
- if (log)
- log->Printf("ProcessLaunchInfo::%s at least one of stdin/stdout/stderr "
- "was not set, evaluating default handling",
- __FUNCTION__);
-
- if (m_flags.Test(eLaunchFlagLaunchInTTY)) {
- // Do nothing, if we are launching in a remote terminal no file actions
- // should be done at all.
- return;
- }
-
- if (m_flags.Test(eLaunchFlagDisableSTDIO)) {
- if (log)
- log->Printf("ProcessLaunchInfo::%s eLaunchFlagDisableSTDIO set, adding "
- "suppression action for stdin, stdout and stderr",
- __FUNCTION__);
- AppendSuppressFileAction(STDIN_FILENO, true, false);
- AppendSuppressFileAction(STDOUT_FILENO, false, true);
- AppendSuppressFileAction(STDERR_FILENO, false, true);
- } else {
- // Check for any values that might have gotten set with any of: (lldb)
- // settings set target.input-path (lldb) settings set target.output-path
- // (lldb) settings set target.error-path
- FileSpec in_file_spec;
- FileSpec out_file_spec;
- FileSpec err_file_spec;
- if (target) {
- // Only override with the target settings if we don't already have an
- // action for in, out or error
- if (GetFileActionForFD(STDIN_FILENO) == nullptr)
- in_file_spec = target->GetStandardInputPath();
- if (GetFileActionForFD(STDOUT_FILENO) == nullptr)
- out_file_spec = target->GetStandardOutputPath();
- if (GetFileActionForFD(STDERR_FILENO) == nullptr)
- err_file_spec = target->GetStandardErrorPath();
- }
-
- if (log)
- log->Printf("ProcessLaunchInfo::%s target stdin='%s', target "
- "stdout='%s', stderr='%s'",
- __FUNCTION__,
- in_file_spec ? in_file_spec.GetCString() : "<null>",
- out_file_spec ? out_file_spec.GetCString() : "<null>",
- err_file_spec ? err_file_spec.GetCString() : "<null>");
-
- if (in_file_spec) {
- AppendOpenFileAction(STDIN_FILENO, in_file_spec, true, false);
- if (log)
- log->Printf(
- "ProcessLaunchInfo::%s appended stdin open file action for %s",
- __FUNCTION__, in_file_spec.GetCString());
- }
-
- if (out_file_spec) {
- AppendOpenFileAction(STDOUT_FILENO, out_file_spec, false, true);
- if (log)
- log->Printf(
- "ProcessLaunchInfo::%s appended stdout open file action for %s",
- __FUNCTION__, out_file_spec.GetCString());
- }
-
- if (err_file_spec) {
- AppendOpenFileAction(STDERR_FILENO, err_file_spec, false, true);
- if (log)
- log->Printf(
- "ProcessLaunchInfo::%s appended stderr open file action for %s",
- __FUNCTION__, err_file_spec.GetCString());
- }
-
- if (default_to_use_pty &&
- (!in_file_spec || !out_file_spec || !err_file_spec)) {
- if (log)
- log->Printf("ProcessLaunchInfo::%s default_to_use_pty is set, and at "
- "least one stdin/stderr/stdout is unset, so generating a "
- "pty to use for it",
- __FUNCTION__);
+llvm::Error ProcessLaunchInfo::SetUpPtyRedirection() {
+ Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS);
+ LLDB_LOG(log, "Generating a pty to use for stdin/out/err");
- int open_flags = O_RDWR | O_NOCTTY;
+ int open_flags = O_RDWR | O_NOCTTY;
#if !defined(_WIN32)
- // We really shouldn't be specifying platform specific flags that are
- // intended for a system call in generic code. But this will have to
- // do for now.
- open_flags |= O_CLOEXEC;
+ // We really shouldn't be specifying platform specific flags that are
+ // intended for a system call in generic code. But this will have to
+ // do for now.
+ open_flags |= O_CLOEXEC;
#endif
- if (m_pty->OpenFirstAvailableMaster(open_flags, nullptr, 0)) {
- const FileSpec slave_file_spec(m_pty->GetSlaveName(nullptr, 0));
-
- // Only use the slave tty if we don't have anything specified for
- // input and don't have an action for stdin
- if (!in_file_spec && GetFileActionForFD(STDIN_FILENO) == nullptr) {
- AppendOpenFileAction(STDIN_FILENO, slave_file_spec, true, false);
- }
-
- // Only use the slave tty if we don't have anything specified for
- // output and don't have an action for stdout
- if (!out_file_spec && GetFileActionForFD(STDOUT_FILENO) == nullptr) {
- AppendOpenFileAction(STDOUT_FILENO, slave_file_spec, false, true);
- }
-
- // Only use the slave tty if we don't have anything specified for
- // error and don't have an action for stderr
- if (!err_file_spec && GetFileActionForFD(STDERR_FILENO) == nullptr) {
- AppendOpenFileAction(STDERR_FILENO, slave_file_spec, false, true);
- }
- }
- }
- }
- }
+ if (!m_pty->OpenFirstAvailableMaster(open_flags, nullptr, 0)) {
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "PTY::OpenFirstAvailableMaster failed");
+ }
+ const FileSpec slave_file_spec(m_pty->GetSlaveName(nullptr, 0));
+
+ // Only use the slave tty if we don't have anything specified for
+ // input and don't have an action for stdin
+ if (GetFileActionForFD(STDIN_FILENO) == nullptr)
+ AppendOpenFileAction(STDIN_FILENO, slave_file_spec, true, false);
+
+ // Only use the slave tty if we don't have anything specified for
+ // output and don't have an action for stdout
+ if (GetFileActionForFD(STDOUT_FILENO) == nullptr)
+ AppendOpenFileAction(STDOUT_FILENO, slave_file_spec, false, true);
+
+ // Only use the slave tty if we don't have anything specified for
+ // error and don't have an action for stderr
+ if (GetFileActionForFD(STDERR_FILENO) == nullptr)
+ AppendOpenFileAction(STDERR_FILENO, slave_file_spec, false, true);
+ return llvm::Error::success();
}
bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
Modified: lldb/trunk/source/Target/Target.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Target.cpp?rev=350617&r1=350616&r2=350617&view=diff
==============================================================================
--- lldb/trunk/source/Target/Target.cpp (original)
+++ lldb/trunk/source/Target/Target.cpp Tue Jan 8 03:55:19 2019
@@ -2834,18 +2834,7 @@ Status Target::Launch(ProcessLaunchInfo
PlatformSP platform_sp(GetPlatform());
- // Finalize the file actions, and if none were given, default to opening up a
- // pseudo terminal
- const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false;
- if (log)
- log->Printf("Target::%s have platform=%s, platform_sp->IsHost()=%s, "
- "default_to_use_pty=%s",
- __FUNCTION__, platform_sp ? "true" : "false",
- platform_sp ? (platform_sp->IsHost() ? "true" : "false")
- : "n/a",
- default_to_use_pty ? "true" : "false");
-
- launch_info.FinalizeFileActions(this, default_to_use_pty);
+ FinalizeFileActions(launch_info);
if (state == eStateConnected) {
if (launch_info.GetFlags().Test(eLaunchFlagLaunchInTTY)) {
@@ -3061,6 +3050,86 @@ Status Target::Attach(ProcessAttachInfo
return error;
}
+void Target::FinalizeFileActions(ProcessLaunchInfo &info) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+
+ // Finalize the file actions, and if none were given, default to opening up a
+ // pseudo terminal
+ PlatformSP platform_sp = GetPlatform();
+ const bool default_to_use_pty =
+ m_platform_sp ? m_platform_sp->IsHost() : false;
+ LLDB_LOG(
+ log,
+ "have platform={0}, platform_sp->IsHost()={1}, default_to_use_pty={2}",
+ bool(platform_sp),
+ platform_sp ? (platform_sp->IsHost() ? "true" : "false") : "n/a",
+ default_to_use_pty);
+
+ // If nothing for stdin or stdout or stderr was specified, then check the
+ // process for any default settings that were set with "settings set"
+ if (info.GetFileActionForFD(STDIN_FILENO) == nullptr ||
+ info.GetFileActionForFD(STDOUT_FILENO) == nullptr ||
+ info.GetFileActionForFD(STDERR_FILENO) == nullptr) {
+ LLDB_LOG(log, "at least one of stdin/stdout/stderr was not set, evaluating "
+ "default handling");
+
+ if (info.GetFlags().Test(eLaunchFlagLaunchInTTY)) {
+ // Do nothing, if we are launching in a remote terminal no file actions
+ // should be done at all.
+ return;
+ }
+
+ if (info.GetFlags().Test(eLaunchFlagDisableSTDIO)) {
+ LLDB_LOG(log, "eLaunchFlagDisableSTDIO set, adding suppression action "
+ "for stdin, stdout and stderr");
+ info.AppendSuppressFileAction(STDIN_FILENO, true, false);
+ info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
+ info.AppendSuppressFileAction(STDERR_FILENO, false, true);
+ } else {
+ // Check for any values that might have gotten set with any of: (lldb)
+ // settings set target.input-path (lldb) settings set target.output-path
+ // (lldb) settings set target.error-path
+ FileSpec in_file_spec;
+ FileSpec out_file_spec;
+ FileSpec err_file_spec;
+ // Only override with the target settings if we don't already have an
+ // action for in, out or error
+ if (info.GetFileActionForFD(STDIN_FILENO) == nullptr)
+ in_file_spec = GetStandardInputPath();
+ if (info.GetFileActionForFD(STDOUT_FILENO) == nullptr)
+ out_file_spec = GetStandardOutputPath();
+ if (info.GetFileActionForFD(STDERR_FILENO) == nullptr)
+ err_file_spec = GetStandardErrorPath();
+
+ LLDB_LOG(log, "target stdin='{0}', target stdout='{1}', stderr='{1}'",
+ in_file_spec, out_file_spec, err_file_spec);
+
+ if (in_file_spec) {
+ info.AppendOpenFileAction(STDIN_FILENO, in_file_spec, true, false);
+ LLDB_LOG(log, "appended stdin open file action for {0}", in_file_spec);
+ }
+
+ if (out_file_spec) {
+ info.AppendOpenFileAction(STDOUT_FILENO, out_file_spec, false, true);
+ LLDB_LOG(log, "appended stdout open file action for {0}",
+ out_file_spec);
+ }
+
+ if (err_file_spec) {
+ info.AppendOpenFileAction(STDERR_FILENO, err_file_spec, false, true);
+ LLDB_LOG(log, "appended stderr open file action for {0}",
+ err_file_spec);
+ }
+
+ if (default_to_use_pty &&
+ (!in_file_spec || !out_file_spec || !err_file_spec)) {
+ llvm::Error Err = info.SetUpPtyRedirection();
+ LLDB_LOG_ERROR(log, std::move(Err), "SetUpPtyRedirection failed: {0}");
+ }
+ }
+ }
+}
+
//--------------------------------------------------------------
// Target::StopHook
//--------------------------------------------------------------
More information about the lldb-commits
mailing list