[Lldb-commits] [lldb] r374444 - Fix process launch failure on FreeBSD after r365761

Dimitry Andric via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 10 13:26:59 PDT 2019


Author: dim
Date: Thu Oct 10 13:26:59 2019
New Revision: 374444

URL: http://llvm.org/viewvc/llvm-project?rev=374444&view=rev
Log:
Fix process launch failure on FreeBSD after r365761

Summary:
After rLLDB365761, and with `LLVM_ENABLE_ABI_BREAKING_CHECKS` enabled,
launching any process on FreeBSD crashes lldb with:

```
Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).
```

This is because `m_operation_thread` and `m_monitor_thread` were wrapped
in `llvm::Expected<>`, but this requires the objects to be correctly
initialized before accessing them.

To fix the crashes, use `llvm::Optional<>` for the members (as indicated
by labath), and use local variables to store the return values of
`LaunchThread` and `StartMonitoringChildProcess`.  Then, only assign to
the member variables after checking if the return values indicated
success.

Reviewers: devnexen, emaste, MaskRay, mgorny

Reviewed By: devnexen

Subscribers: jfb, labath, krytarowski, lldb-commits

Differential Revision: https://reviews.llvm.org/D68723

Modified:
    lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
    lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h

Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp?rev=374444&r1=374443&r2=374444&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp Thu Oct 10 13:26:59 2019
@@ -703,7 +703,7 @@ ProcessMonitor::ProcessMonitor(
     const lldb_private::ProcessLaunchInfo & /* launch_info */,
     lldb_private::Status &error)
     : m_process(static_cast<ProcessFreeBSD *>(process)),
-      m_operation_thread(nullptr), m_monitor_thread(nullptr), m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
+      m_operation_thread(), m_monitor_thread(), m_pid(LLDB_INVALID_PROCESS_ID), m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   std::unique_ptr<LaunchArgs> args(
@@ -730,20 +730,22 @@ ProcessMonitor::ProcessMonitor(
   }
 
   // Finally, start monitoring the child process for change in state.
-  m_monitor_thread = Host::StartMonitoringChildProcess(
+  llvm::Expected<lldb_private::HostThread> monitor_thread =
+    Host::StartMonitoringChildProcess(
       std::bind(&ProcessMonitor::MonitorCallback, this, _1, _2, _3, _4),
       GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (!monitor_thread || !monitor_thread->IsJoinable()) {
     error.SetErrorToGenericError();
     error.SetErrorString("Process launch failed.");
     return;
   }
+  m_monitor_thread = *monitor_thread;
 }
 
 ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
                                lldb_private::Status &error)
     : m_process(static_cast<ProcessFreeBSD *>(process)),
-      m_operation_thread(nullptr), m_monitor_thread(nullptr), m_pid(pid), m_terminal_fd(-1), m_operation(0) {
+      m_operation_thread(), m_monitor_thread(), m_pid(pid), m_terminal_fd(-1), m_operation(0) {
   using namespace std::placeholders;
 
   sem_init(&m_operation_pending, 0, 0);
@@ -768,14 +770,16 @@ ProcessMonitor::ProcessMonitor(ProcessFr
   }
 
   // Finally, start monitoring the child process for change in state.
-  m_monitor_thread = Host::StartMonitoringChildProcess(
+  llvm::Expected<lldb_private::HostThread> monitor_thread =
+    Host::StartMonitoringChildProcess(
       std::bind(&ProcessMonitor::MonitorCallback, this, _1, _2, _3, _4),
       GetPID(), true);
-  if (!m_monitor_thread->IsJoinable()) {
+  if (!monitor_thread || !monitor_thread->IsJoinable()) {
     error.SetErrorToGenericError();
     error.SetErrorString("Process attach failed.");
     return;
   }
+  m_monitor_thread = *monitor_thread;
 }
 
 ProcessMonitor::~ProcessMonitor() { StopMonitor(); }
@@ -784,13 +788,15 @@ ProcessMonitor::~ProcessMonitor() { Stop
 void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status &error) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
     return;
 
-  m_operation_thread =
-      ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
-  if (!m_operation_thread)
-     error = m_operation_thread.takeError();
+  llvm::Expected<lldb_private::HostThread> operation_thread =
+    ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
+  if (operation_thread)
+    m_operation_thread = *operation_thread;
+  else
+    error = operation_thread.takeError();
 }
 
 void *ProcessMonitor::LaunchOpThread(void *arg) {
@@ -952,14 +958,15 @@ void ProcessMonitor::StartAttachOpThread
                                          lldb_private::Status &error) {
   static const char *g_thread_name = "lldb.process.freebsd.operation";
 
-  if (m_operation_thread->IsJoinable())
+  if (m_operation_thread && m_operation_thread->IsJoinable())
     return;
 
-  m_operation_thread =
-      ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args);
-
-  if (!m_operation_thread)
-	error = m_operation_thread.takeError();
+  llvm::Expected<lldb_private::HostThread> operation_thread =
+    ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args);
+  if (operation_thread)
+    m_operation_thread = *operation_thread;
+  else
+    error = operation_thread.takeError();
 }
 
 void *ProcessMonitor::AttachOpThread(void *arg) {
@@ -1374,7 +1381,7 @@ bool ProcessMonitor::DupDescriptor(const
 }
 
 void ProcessMonitor::StopMonitoringChildProcess() {
-  if (m_monitor_thread->IsJoinable()) {
+  if (m_monitor_thread && m_monitor_thread->IsJoinable()) {
     m_monitor_thread->Cancel();
     m_monitor_thread->Join(nullptr);
     m_monitor_thread->Reset();
@@ -1412,10 +1419,9 @@ void ProcessMonitor::StopMonitor() {
 bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
 
 void ProcessMonitor::StopOpThread() {
-  if (!m_operation_thread->IsJoinable())
-    return;
-
-  m_operation_thread->Cancel();
-  m_operation_thread->Join(nullptr);
-  m_operation_thread->Reset();
+  if (m_operation_thread && m_operation_thread->IsJoinable()) {
+    m_operation_thread->Cancel();
+    m_operation_thread->Join(nullptr);
+    m_operation_thread->Reset();
+  }
 }

Modified: lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h?rev=374444&r1=374443&r2=374444&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h (original)
+++ lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.h Thu Oct 10 13:26:59 2019
@@ -183,8 +183,8 @@ public:
 private:
   ProcessFreeBSD *m_process;
 
-  llvm::Expected<lldb_private::HostThread> m_operation_thread;
-  llvm::Expected<lldb_private::HostThread> m_monitor_thread;
+  llvm::Optional<lldb_private::HostThread> m_operation_thread;
+  llvm::Optional<lldb_private::HostThread> m_monitor_thread;
   lldb::pid_t m_pid;
 
   int m_terminal_fd;




More information about the lldb-commits mailing list