Pull in r374444 from upstream lldb trunk (by me):

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

PR:		241137
MFC after:	1 month
X-MFC-With:	r353358
This commit is contained in:
Dimitry Andric 2019-10-10 20:33:55 +00:00
parent 6cc0992fb5
commit 390ceeb437
2 changed files with 32 additions and 26 deletions

View File

@ -708,7 +708,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(
@ -735,20 +735,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);
@ -773,14 +775,16 @@ ProcessMonitor::ProcessMonitor(ProcessFreeBSD *process, lldb::pid_t pid,
}
// 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(); }
@ -789,13 +793,15 @@ ProcessMonitor::~ProcessMonitor() { StopMonitor(); }
void ProcessMonitor::StartLaunchOpThread(LaunchArgs *args, Status &error) {
static const char *g_thread_name = "freebsd.op";
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) {
@ -957,14 +963,15 @@ void ProcessMonitor::StartAttachOpThread(AttachArgs *args,
lldb_private::Status &error) {
static const char *g_thread_name = "freebsd.op";
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) {
@ -1384,7 +1391,7 @@ bool ProcessMonitor::DupDescriptor(const FileSpec &file_spec, int fd,
}
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();
@ -1422,10 +1429,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();
}
}

View File

@ -183,8 +183,8 @@ class ProcessMonitor {
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;