Thread hardening part 3. Now lldb_private::Thread objects have std::weak_ptr
objects for the backlink to the lldb_private::Process. The issues we were
running into before was someone was holding onto a shared pointer to a
lldb_private::Thread for too long, and the lldb_private::Process parent object
would get destroyed and the lldb_private::Thread had a "Process &m_process"
member which would just treat whatever memory that used to be a Process as a
valid Process. This was mostly happening for lldb_private::StackFrame objects
that had a member like "Thread &m_thread". So this completes the internal
strong/weak changes.
Documented the ExecutionContext and ExecutionContextRef classes so that our
LLDB developers can understand when and where to use ExecutionContext and
ExecutionContextRef objects.
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@151009 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index 07b17b4..0648845 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -17,6 +17,7 @@
#include "lldb/Core/RegisterValue.h"
#include "lldb/Core/Scalar.h"
#include "lldb/Core/StreamString.h"
+#include "lldb/Target/ExecutionContext.h"
// Project includes
#include "Utility/StringExtractorGDBRemote.h"
#include "ProcessGDBRemote.h"
@@ -61,18 +62,6 @@
{
}
-ProcessGDBRemote &
-GDBRemoteRegisterContext::GetGDBProcess()
-{
- return static_cast<ProcessGDBRemote &>(m_thread.GetProcess());
-}
-
-ThreadGDBRemote &
-GDBRemoteRegisterContext::GetGDBThread()
-{
- return static_cast<ThreadGDBRemote &>(m_thread);
-}
-
void
GDBRemoteRegisterContext::InvalidateAllRegisters ()
{
@@ -158,7 +147,14 @@
bool
GDBRemoteRegisterContext::ReadRegisterBytes (const RegisterInfo *reg_info, DataExtractor &data)
{
- GDBRemoteCommunicationClient &gdb_comm (GetGDBProcess().GetGDBRemote());
+ ExecutionContext exe_ctx (CalculateThread());
+
+ Process *process = exe_ctx.GetProcessPtr();
+ Thread *thread = exe_ctx.GetThreadPtr();
+ if (process == NULL || thread == NULL)
+ return false;
+
+ GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
InvalidateIfNeeded(false);
@@ -170,7 +166,8 @@
if (gdb_comm.GetSequenceMutex (locker))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
- if (thread_suffix_supported || GetGDBProcess().GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ ProcessSP process_sp (m_thread.GetProcess());
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
{
char packet[64];
StringExtractorGDBRemote response;
@@ -238,7 +235,14 @@
bool
GDBRemoteRegisterContext::WriteRegisterBytes (const lldb_private::RegisterInfo *reg_info, DataExtractor &data, uint32_t data_offset)
{
- GDBRemoteCommunicationClient &gdb_comm (GetGDBProcess().GetGDBRemote());
+ ExecutionContext exe_ctx (CalculateThread());
+
+ Process *process = exe_ctx.GetProcessPtr();
+ Thread *thread = exe_ctx.GetThreadPtr();
+ if (process == NULL || thread == NULL)
+ return false;
+
+ GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
// FIXME: This check isn't right because IsRunning checks the Public state, but this
// is work you need to do - for instance in ShouldStop & friends - before the public
// state has been changed.
@@ -264,7 +268,8 @@
if (gdb_comm.GetSequenceMutex (locker))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
- if (thread_suffix_supported || GetGDBProcess().GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ ProcessSP process_sp (m_thread.GetProcess());
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
{
uint32_t offset, end_offset;
StreamString packet;
@@ -334,7 +339,15 @@
bool
GDBRemoteRegisterContext::ReadAllRegisterValues (lldb::DataBufferSP &data_sp)
{
- GDBRemoteCommunicationClient &gdb_comm (GetGDBProcess().GetGDBRemote());
+ ExecutionContext exe_ctx (CalculateThread());
+
+ Process *process = exe_ctx.GetProcessPtr();
+ Thread *thread = exe_ctx.GetThreadPtr();
+ if (process == NULL || thread == NULL)
+ return false;
+
+ GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
+
StringExtractorGDBRemote response;
Mutex::Locker locker;
@@ -342,7 +355,8 @@
{
char packet[32];
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
- if (thread_suffix_supported || GetGDBProcess().GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ ProcessSP process_sp (m_thread.GetProcess());
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
{
int packet_len = 0;
if (thread_suffix_supported)
@@ -382,13 +396,22 @@
if (!data_sp || data_sp->GetBytes() == NULL || data_sp->GetByteSize() == 0)
return false;
- GDBRemoteCommunicationClient &gdb_comm (GetGDBProcess().GetGDBRemote());
+ ExecutionContext exe_ctx (CalculateThread());
+
+ Process *process = exe_ctx.GetProcessPtr();
+ Thread *thread = exe_ctx.GetThreadPtr();
+ if (process == NULL || thread == NULL)
+ return false;
+
+ GDBRemoteCommunicationClient &gdb_comm (((ProcessGDBRemote *)process)->GetGDBRemote());
+
StringExtractorGDBRemote response;
Mutex::Locker locker;
if (gdb_comm.GetSequenceMutex (locker))
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
- if (thread_suffix_supported || GetGDBProcess().GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ ProcessSP process_sp (m_thread.GetProcess());
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
{
// The data_sp contains the entire G response packet including the
// G, and if the thread suffix is supported, it has the thread suffix
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
index d1955e8..9f96e77 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -240,12 +240,6 @@
void
SetAllRegisterValid (bool b);
- ProcessGDBRemote &
- GetGDBProcess();
-
- ThreadGDBRemote &
- GetGDBThread();
-
GDBRemoteDynamicRegisterInfo &m_reg_info;
std::vector<bool> m_reg_valid;
lldb_private::DataExtractor m_reg_data;
diff --git a/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8424d2a..53b7a24 100644
--- a/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1125,7 +1125,7 @@
tid_t tid = thread_ids[i];
ThreadSP thread_sp (old_thread_list.FindThreadByID (tid, false));
if (!thread_sp)
- thread_sp.reset (new ThreadGDBRemote (*this, tid));
+ thread_sp.reset (new ThreadGDBRemote (shared_from_this(), tid));
new_thread_list.AddThread(thread_sp);
}
}
@@ -1201,7 +1201,7 @@
if (!thread_sp)
{
// Create the thread if we need to
- thread_sp.reset (new ThreadGDBRemote (*this, tid));
+ thread_sp.reset (new ThreadGDBRemote (shared_from_this(), tid));
m_thread_list.AddThread(thread_sp);
}
}
@@ -1292,7 +1292,7 @@
else if (reason.compare("breakpoint") == 0)
{
addr_t pc = gdb_thread->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess().GetBreakpointSiteList().FindByAddress(pc);
+ lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
if (bp_site_sp)
{
// If the breakpoint is for this thread, then we'll report the hit, but if it is for another thread,
@@ -1335,7 +1335,7 @@
// Currently we are going to assume SIGTRAP means we are either
// hitting a breakpoint or hardware single stepping.
addr_t pc = gdb_thread->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess().GetBreakpointSiteList().FindByAddress(pc);
+ lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
if (bp_site_sp)
{
// If the breakpoint is for this thread, then we'll report the hit, but if it is for another thread,
diff --git a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
index 4938591..55cb222 100644
--- a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -32,18 +32,25 @@
// Thread Registers
//----------------------------------------------------------------------
-ThreadGDBRemote::ThreadGDBRemote (ProcessGDBRemote &process, lldb::tid_t tid) :
- Thread(process, tid),
+ThreadGDBRemote::ThreadGDBRemote (const ProcessSP &process_sp, lldb::tid_t tid) :
+ Thread(process_sp, tid),
m_thread_name (),
m_dispatch_queue_name (),
m_thread_dispatch_qaddr (LLDB_INVALID_ADDRESS)
{
- ProcessGDBRemoteLog::LogIf(GDBR_LOG_THREAD, "%p: ThreadGDBRemote::ThreadGDBRemote (pid = %i, tid = 0x%4.4x)", this, m_process.GetID(), GetID());
+ ProcessGDBRemoteLog::LogIf(GDBR_LOG_THREAD, "%p: ThreadGDBRemote::ThreadGDBRemote (pid = %i, tid = 0x%4.4x)",
+ this,
+ process_sp ? process_sp->GetID() : LLDB_INVALID_PROCESS_ID,
+ GetID());
}
ThreadGDBRemote::~ThreadGDBRemote ()
{
- ProcessGDBRemoteLog::LogIf(GDBR_LOG_THREAD, "%p: ThreadGDBRemote::~ThreadGDBRemote (pid = %i, tid = 0x%4.4x)", this, m_process.GetID(), GetID());
+ ProcessSP process_sp(GetProcess());
+ ProcessGDBRemoteLog::LogIf(GDBR_LOG_THREAD, "%p: ThreadGDBRemote::~ThreadGDBRemote (pid = %i, tid = 0x%4.4x)",
+ this,
+ process_sp ? process_sp->GetID() : LLDB_INVALID_PROCESS_ID,
+ GetID());
DestroyThread();
}
@@ -60,8 +67,16 @@
ThreadGDBRemote::GetQueueName ()
{
// Always re-fetch the dispatch queue name since it can change
+
if (m_thread_dispatch_qaddr != 0 || m_thread_dispatch_qaddr != LLDB_INVALID_ADDRESS)
- return GetGDBProcess().GetDispatchQueueNameForThread (m_thread_dispatch_qaddr, m_dispatch_queue_name);
+ {
+ ProcessSP process_sp (GetProcess());
+ if (process_sp)
+ {
+ ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
+ return gdb_process->GetDispatchQueueNameForThread (m_thread_dispatch_qaddr, m_dispatch_queue_name);
+ }
+ }
return NULL;
}
@@ -79,32 +94,37 @@
if (log)
log->Printf ("Resuming thread: %4.4llx with state: %s.", GetID(), StateAsCString(resume_state));
- ProcessGDBRemote &process = GetGDBProcess();
- switch (resume_state)
+ ProcessSP process_sp (GetProcess());
+ if (process_sp)
{
- case eStateSuspended:
- case eStateStopped:
- // Don't append anything for threads that should stay stopped.
- break;
+ ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
+ switch (resume_state)
+ {
+ case eStateSuspended:
+ case eStateStopped:
+ // Don't append anything for threads that should stay stopped.
+ break;
- case eStateRunning:
- if (m_process.GetUnixSignals().SignalIsValid (signo))
- process.m_continue_C_tids.push_back(std::make_pair(GetID(), signo));
- else
- process.m_continue_c_tids.push_back(GetID());
- break;
+ case eStateRunning:
+ if (gdb_process->GetUnixSignals().SignalIsValid (signo))
+ gdb_process->m_continue_C_tids.push_back(std::make_pair(GetID(), signo));
+ else
+ gdb_process->m_continue_c_tids.push_back(GetID());
+ break;
- case eStateStepping:
- if (m_process.GetUnixSignals().SignalIsValid (signo))
- process.m_continue_S_tids.push_back(std::make_pair(GetID(), signo));
- else
- process.m_continue_s_tids.push_back(GetID());
- break;
+ case eStateStepping:
+ if (gdb_process->GetUnixSignals().SignalIsValid (signo))
+ gdb_process->m_continue_S_tids.push_back(std::make_pair(GetID(), signo));
+ else
+ gdb_process->m_continue_s_tids.push_back(GetID());
+ break;
- default:
- break;
+ default:
+ break;
+ }
+ return true;
}
- return true;
+ return false;
}
void
@@ -167,8 +187,16 @@
if (frame)
concrete_frame_idx = frame->GetConcreteFrameIndex ();
+
if (concrete_frame_idx == 0)
- reg_ctx_sp.reset (new GDBRemoteRegisterContext (*this, concrete_frame_idx, GetGDBProcess().m_register_info, read_all_registers_at_once));
+ {
+ ProcessSP process_sp (GetProcess());
+ if (process_sp)
+ {
+ ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
+ reg_ctx_sp.reset (new GDBRemoteRegisterContext (*this, concrete_frame_idx, gdb_process->m_register_info, read_all_registers_at_once));
+ }
+ }
else if (m_unwinder_ap.get())
reg_ctx_sp = m_unwinder_ap->CreateRegisterContextForFrame (frame);
return reg_ctx_sp;
@@ -185,25 +213,29 @@
lldb::StopInfoSP
ThreadGDBRemote::GetPrivateStopReason ()
{
- const uint32_t process_stop_id = GetProcess().GetStopID();
- if (m_thread_stop_reason_stop_id != process_stop_id ||
- (m_actual_stop_info_sp && !m_actual_stop_info_sp->IsValid()))
+ ProcessSP process_sp (GetProcess());
+ if (process_sp)
{
- // If GetGDBProcess().SetThreadStopInfo() doesn't find a stop reason
- // for this thread, then m_actual_stop_info_sp will not ever contain
- // a valid stop reason and the "m_actual_stop_info_sp->IsValid() == false"
- // check will never be able to tell us if we have the correct stop info
- // for this thread and we will continually send qThreadStopInfo packets
- // down to the remote GDB server, so we need to keep our own notion
- // of the stop ID that m_actual_stop_info_sp is valid for (even if it
- // contains nothing). We use m_thread_stop_reason_stop_id for this below.
- m_thread_stop_reason_stop_id = process_stop_id;
- m_actual_stop_info_sp.reset();
+ const uint32_t process_stop_id = process_sp->GetStopID();
+ if (m_thread_stop_reason_stop_id != process_stop_id ||
+ (m_actual_stop_info_sp && !m_actual_stop_info_sp->IsValid()))
+ {
+ // If GetGDBProcess().SetThreadStopInfo() doesn't find a stop reason
+ // for this thread, then m_actual_stop_info_sp will not ever contain
+ // a valid stop reason and the "m_actual_stop_info_sp->IsValid() == false"
+ // check will never be able to tell us if we have the correct stop info
+ // for this thread and we will continually send qThreadStopInfo packets
+ // down to the remote GDB server, so we need to keep our own notion
+ // of the stop ID that m_actual_stop_info_sp is valid for (even if it
+ // contains nothing). We use m_thread_stop_reason_stop_id for this below.
+ m_thread_stop_reason_stop_id = process_stop_id;
+ m_actual_stop_info_sp.reset();
- StringExtractorGDBRemote stop_packet;
- ProcessGDBRemote &gdb_process = GetGDBProcess();
- if (gdb_process.GetGDBRemote().GetThreadStopInfo(GetID(), stop_packet))
- gdb_process.SetThreadStopInfo (stop_packet);
+ StringExtractorGDBRemote stop_packet;
+ ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
+ if (gdb_process->GetGDBRemote().GetThreadStopInfo(GetID(), stop_packet))
+ gdb_process->SetThreadStopInfo (stop_packet);
+ }
}
return m_actual_stop_info_sp;
}
diff --git a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
index 09f8684..3eb6295 100644
--- a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -21,7 +21,7 @@
class ThreadGDBRemote : public lldb_private::Thread
{
public:
- ThreadGDBRemote (ProcessGDBRemote &process, lldb::tid_t tid);
+ ThreadGDBRemote (const lldb::ProcessSP &process_sp, lldb::tid_t tid);
virtual
~ThreadGDBRemote ();
@@ -47,18 +47,6 @@
virtual void
ClearStackFrames ();
- ProcessGDBRemote &
- GetGDBProcess ()
- {
- return (ProcessGDBRemote &)m_process;
- }
-
- const ProcessGDBRemote &
- GetGDBProcess () const
- {
- return (ProcessGDBRemote &)m_process;
- }
-
void
Dump (lldb_private::Log *log, uint32_t index);