[Lldb-commits] [lldb] r332250 - Remove Process references from the Host module
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon May 14 08:13:14 PDT 2018
Author: labath
Date: Mon May 14 08:13:13 2018
New Revision: 332250
URL: http://llvm.org/viewvc/llvm-project?rev=332250&view=rev
Log:
Remove Process references from the Host module
The Process class was only being referenced because of the last-ditch
effort in the process launchers to set a process death callback in case
one isn't set already.
Although launching a process for debugging is the most important kind of
"launch" we are doing, it is by far not the only one, so assuming this
particular callback is the one to be used is not a good idea (besides
breaking layering). Instead of assuming a particular exit callback, I
change the launcher code to require the callback to be set by the user (and fix
up the two call sites which did not set the callback already).
Reviewers: jingham, davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D46395
Modified:
lldb/trunk/include/lldb/Host/Host.h
lldb/trunk/include/lldb/Host/MonitoringProcessLauncher.h
lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp
lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
lldb/trunk/source/Host/macosx/Host.mm
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Target/ProcessLaunchInfo.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
Modified: lldb/trunk/include/lldb/Host/Host.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Host.h?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/Host.h (original)
+++ lldb/trunk/include/lldb/Host/Host.h Mon May 14 08:13:13 2018
@@ -199,6 +199,9 @@ public:
static const lldb::UnixSignalsSP &GetUnixSignals();
+ /// Launch the process specified in launch_info. The monitoring callback in
+ /// launch_info must be set, and it will be called when the process
+ /// terminates.
static Status LaunchProcess(ProcessLaunchInfo &launch_info);
//------------------------------------------------------------------
Modified: lldb/trunk/include/lldb/Host/MonitoringProcessLauncher.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MonitoringProcessLauncher.h?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/MonitoringProcessLauncher.h (original)
+++ lldb/trunk/include/lldb/Host/MonitoringProcessLauncher.h Mon May 14 08:13:13 2018
@@ -24,6 +24,9 @@ public:
explicit MonitoringProcessLauncher(
std::unique_ptr<ProcessLauncher> delegate_launcher);
+ /// Launch the process specified in launch_info. The monitoring callback in
+ /// launch_info must be set, and it will be called when the process
+ /// terminates.
HostProcess LaunchProcess(const ProcessLaunchInfo &launch_info,
Status &error) override;
Modified: lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h (original)
+++ lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h Mon May 14 08:13:13 2018
@@ -107,6 +107,12 @@ public:
return m_monitor_callback;
}
+ /// A Monitor callback which does not take any action on process events. Use
+ /// this if you don't need to take any particular action when the process
+ /// terminates, but you still need to reap it.
+ static bool NoOpMonitorCallback(lldb::pid_t pid, bool exited, int signal,
+ int status);
+
bool GetMonitorSignals() const { return m_monitor_signals; }
// If the LaunchInfo has a monitor callback, then arrange to monitor the
Modified: lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp (original)
+++ lldb/trunk/source/Host/common/MonitoringProcessLauncher.cpp Mon May 14 08:13:13 2018
@@ -9,7 +9,6 @@
#include "lldb/Host/MonitoringProcessLauncher.h"
#include "lldb/Host/HostProcess.h"
-#include "lldb/Target/Process.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Status.h"
@@ -58,18 +57,9 @@ MonitoringProcessLauncher::LaunchProcess
if (process.GetProcessId() != LLDB_INVALID_PROCESS_ID) {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
- Host::MonitorChildProcessCallback callback =
- launch_info.GetMonitorProcessCallback();
-
- bool monitor_signals = false;
- if (callback) {
- // If the ProcessLaunchInfo specified a callback, use that.
- monitor_signals = launch_info.GetMonitorSignals();
- } else {
- callback = Process::SetProcessExitStatus;
- }
-
- process.StartMonitoring(callback, monitor_signals);
+ assert(launch_info.GetMonitorProcessCallback());
+ process.StartMonitoring(launch_info.GetMonitorProcessCallback(),
+ launch_info.GetMonitorSignals());
if (log)
log->PutCString("started monitoring child process.");
} else {
Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Mon May 14 08:13:13 2018
@@ -13,7 +13,6 @@
#include "lldb/Host/common/NativeRegisterContext.h"
#include "lldb/Host/common/NativeThreadProtocol.h"
#include "lldb/Host/common/SoftwareBreakpoint.h"
-#include "lldb/Target/Process.h"
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/Log.h"
#include "lldb/lldb-enumerations.h"
Modified: lldb/trunk/source/Host/macosx/Host.mm
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/Host.mm?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/source/Host/macosx/Host.mm (original)
+++ lldb/trunk/source/Host/macosx/Host.mm Mon May 14 08:13:13 2018
@@ -57,7 +57,6 @@
#include "lldb/Host/ConnectionFileDescriptor.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Target/Process.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/CleanUp.h"
#include "lldb/Utility/DataBufferHeap.h"
@@ -68,6 +67,7 @@
#include "lldb/Utility/NameMatches.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-defines.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Errno.h"
@@ -1497,15 +1497,9 @@ Status Host::LaunchProcess(ProcessLaunch
launch_info.SetProcessID(pid);
// Make sure we reap any processes we spawn or we will have zombies.
- if (!launch_info.MonitorProcess()) {
- const bool monitor_signals = false;
- Host::MonitorChildProcessCallback callback = nullptr;
-
- if (!launch_info.GetFlags().Test(lldb::eLaunchFlagDontSetExitStatus))
- callback = Process::SetProcessExitStatus;
-
- StartMonitoringChildProcess(callback, pid, monitor_signals);
- }
+ bool monitoring = launch_info.MonitorProcess());
+ UNUSED_IF_ASSERT_DISABLED(monitoring);
+ assert(monitoring);
} else {
// Invalid process ID, something didn't go well
if (error.Success())
Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Mon May 14 08:13:13 2018
@@ -868,11 +868,12 @@ PlatformPOSIX::DebugProcess(ProcessLaunc
if (IsHost()) {
// We are going to hand this process off to debugserver which will be in
- // charge of setting the exit status. We still need to reap it from lldb
- // but if we let the monitor thread also set the exit status, we set up a
- // race between debugserver & us for who will find out about the debugged
- // process's death.
- launch_info.GetFlags().Set(eLaunchFlagDontSetExitStatus);
+ // charge of setting the exit status. However, we still need to reap it
+ // from lldb. So, make sure we use a exit callback which does not set exit
+ // status.
+ const bool monitor_signals = false;
+ launch_info.SetMonitorProcessCallback(
+ &ProcessLaunchInfo::NoOpMonitorCallback, monitor_signals);
process_sp = Platform::DebugProcess(launch_info, debugger, target, error);
} else {
if (m_remote_platform_sp)
Modified: lldb/trunk/source/Target/ProcessLaunchInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ProcessLaunchInfo.cpp?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/source/Target/ProcessLaunchInfo.cpp (original)
+++ lldb/trunk/source/Target/ProcessLaunchInfo.cpp Mon May 14 08:13:13 2018
@@ -189,6 +189,13 @@ void ProcessLaunchInfo::SetMonitorProces
m_monitor_signals = monitor_signals;
}
+bool ProcessLaunchInfo::NoOpMonitorCallback(lldb::pid_t pid, bool exited, int signal, int status) {
+ Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS);
+ LLDB_LOG(log, "pid = {0}, exited = {1}, signal = {2}, status = {3}", pid,
+ exited, signal, status);
+ return true;
+}
+
bool ProcessLaunchInfo::MonitorProcess() const {
if (m_monitor_callback && ProcessIDIsValid()) {
Host::StartMonitoringChildProcess(m_monitor_callback, GetProcessID(),
Modified: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp?rev=332250&r1=332249&r2=332250&view=diff
==============================================================================
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp Mon May 14 08:13:13 2018
@@ -97,6 +97,11 @@ Expected<std::unique_ptr<TestClient>> Te
Info.SetArchitecture(arch_spec);
Info.SetArguments(args, true);
Info.GetEnvironment() = Host::GetEnvironment();
+ // TODO: Use this callback to detect botched launches. If lldb-server does not
+ // start, we can print a nice error message here instead of hanging in
+ // Accept().
+ Info.SetMonitorProcessCallback(&ProcessLaunchInfo::NoOpMonitorCallback,
+ false);
status = Host::LaunchProcess(Info);
if (status.Fail())
More information about the lldb-commits
mailing list