dim created this revision.
dim added reviewers: devnexen, emaste, MaskRay, mgorny.
Herald added a subscriber: krytarowski.
dim added a comment.
After rLLDB365761 <https://reviews.llvm.org/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, revert the wrapping, 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.
https://reviews.llvm.org/D68723
Files:
source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
source/Plugins/Process/FreeBSD/ProcessMonitor.h
Index: source/Plugins/Process/FreeBSD/ProcessMonitor.h
===================================================================
--- source/Plugins/Process/FreeBSD/ProcessMonitor.h
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.h
@@ -183,8 +183,8 @@
private:
ProcessFreeBSD *m_process;
- llvm::Expected<lldb_private::HostThread> m_operation_thread;
- llvm::Expected<lldb_private::HostThread> m_monitor_thread;
+ lldb_private::HostThread m_operation_thread;
+ lldb_private::HostThread m_monitor_thread;
lldb::pid_t m_pid;
int m_terminal_fd;
Index: source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
===================================================================
--- source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
+++ source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
@@ -703,7 +703,7 @@
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,21 @@
}
// Finally, start monitoring the child process for change in state.
- m_monitor_thread = Host::StartMonitoringChildProcess(
+ auto 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 +769,15 @@
}
// Finally, start monitoring the child process for change in state.
- m_monitor_thread = Host::StartMonitoringChildProcess(
+ auto 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 +786,15 @@
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.IsJoinable())
return;
- m_operation_thread =
+ auto operation_thread =
ThreadLauncher::LaunchThread(g_thread_name, LaunchOpThread, args);
- if (!m_operation_thread)
- error = m_operation_thread.takeError();
+ if (operation_thread)
+ m_operation_thread = *operation_thread;
+ else
+ error = operation_thread.takeError();
}
void *ProcessMonitor::LaunchOpThread(void *arg) {
@@ -952,14 +956,15 @@
lldb_private::Status &error) {
static const char *g_thread_name = "lldb.process.freebsd.operation";
- if (m_operation_thread->IsJoinable())
+ if (m_operation_thread.IsJoinable())
return;
- m_operation_thread =
+ auto operation_thread =
ThreadLauncher::LaunchThread(g_thread_name, AttachOpThread, args);
-
- if (!m_operation_thread)
- error = m_operation_thread.takeError();
+ if (operation_thread)
+ m_operation_thread = *operation_thread;
+ else
+ error = operation_thread.takeError();
}
void *ProcessMonitor::AttachOpThread(void *arg) {
@@ -1374,10 +1379,10 @@
}
void ProcessMonitor::StopMonitoringChildProcess() {
- if (m_monitor_thread->IsJoinable()) {
- m_monitor_thread->Cancel();
- m_monitor_thread->Join(nullptr);
- m_monitor_thread->Reset();
+ if (m_monitor_thread.IsJoinable()) {
+ m_monitor_thread.Cancel();
+ m_monitor_thread.Join(nullptr);
+ m_monitor_thread.Reset();
}
}
@@ -1412,10 +1417,10 @@
bool ProcessMonitor::WaitForInitialTIDStop(lldb::tid_t tid) { return true; }
void ProcessMonitor::StopOpThread() {
- if (!m_operation_thread->IsJoinable())
+ if (!m_operation_thread.IsJoinable())
return;
- m_operation_thread->Cancel();
- m_operation_thread->Join(nullptr);
- m_operation_thread->Reset();
+ m_operation_thread.Cancel();
+ m_operation_thread.Join(nullptr);
+ m_operation_thread.Reset();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
