labath updated this revision to Diff 145163.
labath marked an inline comment as done.
labath added a comment.

Update the diff.


https://reviews.llvm.org/D46395

Files:
  include/lldb/Host/Host.h
  include/lldb/Host/MonitoringProcessLauncher.h
  include/lldb/Target/ProcessLaunchInfo.h
  source/Host/common/MonitoringProcessLauncher.cpp
  source/Host/common/NativeProcessProtocol.cpp
  source/Host/macosx/Host.mm
  source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  source/Target/ProcessLaunchInfo.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp

Index: unittests/tools/lldb-server/tests/TestClient.cpp
===================================================================
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -97,6 +97,11 @@
   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())
Index: source/Target/ProcessLaunchInfo.cpp
===================================================================
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -189,6 +189,13 @@
   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(),
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -866,11 +866,12 @@
 
   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)
Index: source/Host/macosx/Host.mm
===================================================================
--- source/Host/macosx/Host.mm
+++ source/Host/macosx/Host.mm
@@ -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 @@
     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())
Index: source/Host/common/NativeProcessProtocol.cpp
===================================================================
--- source/Host/common/NativeProcessProtocol.cpp
+++ source/Host/common/NativeProcessProtocol.cpp
@@ -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"
Index: source/Host/common/MonitoringProcessLauncher.cpp
===================================================================
--- source/Host/common/MonitoringProcessLauncher.cpp
+++ source/Host/common/MonitoringProcessLauncher.cpp
@@ -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 @@
   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 {
Index: include/lldb/Target/ProcessLaunchInfo.h
===================================================================
--- include/lldb/Target/ProcessLaunchInfo.h
+++ include/lldb/Target/ProcessLaunchInfo.h
@@ -107,6 +107,12 @@
     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
Index: include/lldb/Host/MonitoringProcessLauncher.h
===================================================================
--- include/lldb/Host/MonitoringProcessLauncher.h
+++ include/lldb/Host/MonitoringProcessLauncher.h
@@ -24,6 +24,9 @@
   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;
 
Index: include/lldb/Host/Host.h
===================================================================
--- include/lldb/Host/Host.h
+++ include/lldb/Host/Host.h
@@ -198,6 +198,9 @@
 
   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);
 
   //------------------------------------------------------------------
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to