<rdar://problem/13700260>
<rdar://problem/13723772>
Modified the lldb_private::Thread to work much better with the OperatingSystem plug-ins. Operating system plug-ins can now return have a "core" key/value pair in each thread dictionary for the OperatingSystemPython plug-ins which allows the core threads to be contained with memory threads. It also allows these memory threads to be stepped, resumed, and controlled just as if they were the actual backing threads themselves.
A few things are introduced:
- lldb_private::Thread now has a GetProtocolID() method which returns the thread protocol ID for a given thread. The protocol ID (Thread::GetProtocolID()) is usually the same as the thread id (Thread::GetID()), but it can differ when a memory thread has its own id, but is backed by an actual API thread.
- Cleaned up the Thread::WillResume() code to do the mandatory parts in Thread::ShouldResume(), and let the thread subclasses override the Thread::WillResume() which is now just a notification.
- Cleaned up ClearStackFrames() implementations so that fewer thread subclasses needed to override them
- Changed the POSIXThread class a bit since it overrode Thread::WillResume(). It is doing the wrong thing by calling "Thread::SetResumeState()" on its own, this shouldn't be done by thread subclasses, but the current code might rely on it so I left it in with a TODO comment with an explanation.
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@180886 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
index bf3dc4d..b17a6af 100644
--- a/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ b/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -155,7 +155,7 @@
int packet_len = 0;
const uint32_t reg = reg_info->kinds[eRegisterKindLLDB];
if (gdb_comm.GetThreadSuffixSupported())
- packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4" PRIx64 ";", reg, m_thread.GetID());
+ packet_len = ::snprintf (packet, sizeof(packet), "p%x;thread:%4.4" PRIx64 ";", reg, m_thread.GetProtocolID());
else
packet_len = ::snprintf (packet, sizeof(packet), "p%x", reg);
assert (packet_len < (sizeof(packet) - 1));
@@ -187,7 +187,7 @@
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
- if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
{
char packet[64];
StringExtractorGDBRemote response;
@@ -196,7 +196,7 @@
{
// Get all registers in one packet
if (thread_suffix_supported)
- packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64 ";", m_thread.GetID());
+ packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
else
packet_len = ::snprintf (packet, sizeof(packet), "g");
assert (packet_len < (sizeof(packet) - 1));
@@ -314,7 +314,7 @@
lldb::endian::InlHostByteOrder());
if (gdb_comm.GetThreadSuffixSupported())
- packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetID());
+ packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
// Invalidate just this register
SetRegisterIsValid(reg, false);
@@ -340,7 +340,7 @@
StreamString packet;
StringExtractorGDBRemote response;
- packet.Printf ("QSyncThreadState:%4.4" PRIx64 ";", m_thread.GetID());
+ packet.Printf ("QSyncThreadState:%4.4" PRIx64 ";", m_thread.GetProtocolID());
if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
packet.GetString().size(),
response,
@@ -386,7 +386,7 @@
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
- if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
{
StreamString packet;
StringExtractorGDBRemote response;
@@ -401,7 +401,7 @@
lldb::endian::InlHostByteOrder());
if (thread_suffix_supported)
- packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetID());
+ packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
// Invalidate all register values
InvalidateIfNeeded (true);
@@ -508,11 +508,11 @@
char packet[32];
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
- if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
{
int packet_len = 0;
if (thread_suffix_supported)
- packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64, m_thread.GetID());
+ packet_len = ::snprintf (packet, sizeof(packet), "g;thread:%4.4" PRIx64, m_thread.GetProtocolID());
else
packet_len = ::snprintf (packet, sizeof(packet), "g");
assert (packet_len < (sizeof(packet) - 1));
@@ -529,7 +529,7 @@
if (thread_suffix_supported)
{
char thread_id_cstr[64];
- ::snprintf (thread_id_cstr, sizeof(thread_id_cstr), ";thread:%4.4" PRIx64 ";", m_thread.GetID());
+ ::snprintf (thread_id_cstr, sizeof(thread_id_cstr), ";thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
response_str.append (thread_id_cstr);
}
data_sp.reset (new DataBufferHeap (response_str.c_str(), response_str.size()));
@@ -579,7 +579,7 @@
{
const bool thread_suffix_supported = gdb_comm.GetThreadSuffixSupported();
ProcessSP process_sp (m_thread.GetProcess());
- if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetID()))
+ if (thread_suffix_supported || static_cast<ProcessGDBRemote *>(process_sp.get())->GetGDBRemote().SetCurrentThread(m_thread.GetProtocolID()))
{
// The data_sp contains the entire G response packet including the
// G, and if the thread suffix is supported, it has the thread suffix
@@ -652,7 +652,7 @@
lldb::endian::InlHostByteOrder());
if (thread_suffix_supported)
- packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetID());
+ packet.Printf (";thread:%4.4" PRIx64 ";", m_thread.GetProtocolID());
SetRegisterIsValid(reg, false);
if (gdb_comm.SendPacketAndWaitForResponse(packet.GetString().c_str(),
diff --git a/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 35e455d..1b4627a 100644
--- a/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1322,7 +1322,13 @@
for (size_t i=0; i<num_thread_ids; ++i)
{
tid_t tid = m_thread_ids[i];
- ThreadSP thread_sp (old_thread_list_copy.RemoveThreadByID (tid, false));
+ ThreadSP thread_sp (old_thread_list_copy.RemoveThreadByProtocolID(tid, false));
+ if (thread_sp)
+ {
+ ThreadSP backing_thread_sp (thread_sp->GetBackingThread());
+ if (backing_thread_sp && backing_thread_sp->GetProtocolID() == tid)
+ thread_sp = backing_thread_sp;
+ }
if (!thread_sp)
thread_sp.reset (new ThreadGDBRemote (*this, tid));
new_thread_list.AddThread(thread_sp);
@@ -1337,7 +1343,7 @@
ThreadSP old_thread_sp(old_thread_list_copy.GetThreadAtIndex (i, false));
if (old_thread_sp)
{
- lldb::tid_t old_thread_id = old_thread_sp->GetID();
+ lldb::tid_t old_thread_id = old_thread_sp->GetProtocolID();
m_thread_id_to_index_id_map.erase(old_thread_id);
}
}
@@ -1379,6 +1385,8 @@
std::vector<addr_t> exc_data;
addr_t thread_dispatch_qaddr = LLDB_INVALID_ADDRESS;
ThreadSP thread_sp;
+ ThreadSP backing_thread_sp;
+ ThreadGDBRemote *gdb_thread = NULL;
while (stop_packet.GetNameColonValue(name, value))
{
@@ -1400,34 +1408,21 @@
// hold onto the mutex between the call to m_thread_list.FindThreadByID(...)
// and the m_thread_list.AddThread(...) so it doesn't change on us
Mutex::Locker locker (m_thread_list.GetMutex ());
- thread_sp = m_thread_list.FindThreadByID(tid, false);
- if (!thread_sp)
+ thread_sp = m_thread_list.FindThreadByProtocolID(tid, false);
+
+ if (thread_sp)
{
- // If there is an operating system plug-in it might hiding the actual API
- // thread inside a ThreadMemory...
- if (GetOperatingSystem())
- {
- bool found_backing_thread = false;
- const uint32_t num_threads = m_thread_list.GetSize();
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx)
- {
- thread_sp = m_thread_list.GetThreadAtIndex(thread_idx)->GetBackingThread();
- if (thread_sp && thread_sp->GetID() == tid)
- {
- found_backing_thread = true;
- break;
- }
- }
-
- if (!found_backing_thread)
- thread_sp.reset();
- }
+ backing_thread_sp = thread_sp->GetBackingThread();
+ if (backing_thread_sp)
+ gdb_thread = static_cast<ThreadGDBRemote *> (backing_thread_sp.get());
+ else
+ gdb_thread = static_cast<ThreadGDBRemote *> (thread_sp.get());
}
-
- if (!thread_sp)
+ else
{
// Create the thread if we need to
thread_sp.reset (new ThreadGDBRemote (*this, tid));
+ gdb_thread = static_cast<ThreadGDBRemote *> (thread_sp.get());
m_thread_list.AddThread(thread_sp);
}
}
@@ -1488,7 +1483,7 @@
// We have a register number that contains an expedited
// register value. Lets supply this register to our thread
// so it won't have to go and read it.
- if (thread_sp)
+ if (gdb_thread)
{
uint32_t reg = Args::StringToUInt32 (name.c_str(), UINT32_MAX, 16);
@@ -1497,7 +1492,7 @@
StringExtractor reg_value_extractor;
// Swap "value" over into "reg_value_extractor"
reg_value_extractor.GetStringRef().swap(value);
- if (!static_cast<ThreadGDBRemote *> (thread_sp.get())->PrivateSetRegisterValue (reg, reg_value_extractor))
+ if (!gdb_thread->PrivateSetRegisterValue (reg, reg_value_extractor))
{
Host::SetCrashDescriptionWithFormat("Setting thread register '%s' (decoded to %u (0x%x)) with value '%s' for stop packet: '%s'",
name.c_str(),
@@ -1513,20 +1508,18 @@
if (thread_sp)
{
- ThreadGDBRemote *gdb_thread = static_cast<ThreadGDBRemote *> (thread_sp.get());
-
gdb_thread->SetThreadDispatchQAddr (thread_dispatch_qaddr);
gdb_thread->SetName (thread_name.empty() ? NULL : thread_name.c_str());
if (exc_type != 0)
{
const size_t exc_data_size = exc_data.size();
- gdb_thread->SetStopInfo (StopInfoMachException::CreateStopReasonWithMachException (*thread_sp,
- exc_type,
- exc_data_size,
- exc_data_size >= 1 ? exc_data[0] : 0,
- exc_data_size >= 2 ? exc_data[1] : 0,
- exc_data_size >= 3 ? exc_data[2] : 0));
+ thread_sp->SetStopInfo (StopInfoMachException::CreateStopReasonWithMachException (*thread_sp,
+ exc_type,
+ exc_data_size,
+ exc_data_size >= 1 ? exc_data[0] : 0,
+ exc_data_size >= 2 ? exc_data[1] : 0,
+ exc_data_size >= 3 ? exc_data[2] : 0));
}
else
{
@@ -1535,27 +1528,27 @@
{
if (reason.compare("trace") == 0)
{
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp));
handled = true;
}
else if (reason.compare("breakpoint") == 0)
{
- addr_t pc = gdb_thread->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+ addr_t pc = thread_sp->GetRegisterContext()->GetPC();
+ lldb::BreakpointSiteSP bp_site_sp = thread_sp->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,
// we can just report no reason. We don't need to worry about stepping over the breakpoint here, that
// will be taken care of when the thread resumes and notices that there's a breakpoint under the pc.
handled = true;
- if (bp_site_sp->ValidForThisThread (gdb_thread))
+ if (bp_site_sp->ValidForThisThread (thread_sp.get()))
{
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID()));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID()));
}
else
{
StopInfoSP invalid_stop_info_sp;
- gdb_thread->SetStopInfo (invalid_stop_info_sp);
+ thread_sp->SetStopInfo (invalid_stop_info_sp);
}
}
@@ -1568,12 +1561,12 @@
{
break_id_t watch_id = LLDB_INVALID_WATCH_ID;
// TODO: locate the watchpoint somehow...
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithWatchpointID (*thread_sp, watch_id));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithWatchpointID (*thread_sp, watch_id));
handled = true;
}
else if (reason.compare("exception") == 0)
{
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithException(*thread_sp, description.c_str()));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithException(*thread_sp, description.c_str()));
handled = true;
}
}
@@ -1585,22 +1578,22 @@
// Currently we are going to assume SIGTRAP means we are either
// hitting a breakpoint or hardware single stepping.
handled = true;
- addr_t pc = gdb_thread->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp = gdb_thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+ addr_t pc = thread_sp->GetRegisterContext()->GetPC();
+ lldb::BreakpointSiteSP bp_site_sp = thread_sp->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,
// we can just report no reason. We don't need to worry about stepping over the breakpoint here, that
// will be taken care of when the thread resumes and notices that there's a breakpoint under the pc.
- if (bp_site_sp->ValidForThisThread (gdb_thread))
+ if (bp_site_sp->ValidForThisThread (thread_sp.get()))
{
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID()));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID()));
}
else
{
StopInfoSP invalid_stop_info_sp;
- gdb_thread->SetStopInfo (invalid_stop_info_sp);
+ thread_sp->SetStopInfo (invalid_stop_info_sp);
}
}
else
@@ -1608,31 +1601,31 @@
// If we were stepping then assume the stop was the result of the trace. If we were
// not stepping then report the SIGTRAP.
// FIXME: We are still missing the case where we single step over a trap instruction.
- if (gdb_thread->GetTemporaryResumeState() == eStateStepping)
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp));
+ if (thread_sp->GetTemporaryResumeState() == eStateStepping)
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp));
else
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithSignal(*thread_sp, signo));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithSignal(*thread_sp, signo));
}
}
if (!handled)
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithSignal (*thread_sp, signo));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithSignal (*thread_sp, signo));
}
else
{
StopInfoSP invalid_stop_info_sp;
- gdb_thread->SetStopInfo (invalid_stop_info_sp);
+ thread_sp->SetStopInfo (invalid_stop_info_sp);
}
if (!description.empty())
{
- lldb::StopInfoSP stop_info_sp (gdb_thread->GetStopInfo ());
+ lldb::StopInfoSP stop_info_sp (thread_sp->GetStopInfo ());
if (stop_info_sp)
{
stop_info_sp->SetDescription (description.c_str());
}
else
{
- gdb_thread->SetStopInfo (StopInfo::CreateStopReasonWithException (*thread_sp, description.c_str()));
+ thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithException (*thread_sp, description.c_str()));
}
}
}
@@ -1797,8 +1790,8 @@
|| reason == eStopReasonException)
{
if (log)
- log->Printf ("ProcessGDBRemote::DoDestroy() - thread: %" PRId64 " stopped with reason: %s.",
- thread_sp->GetID(),
+ log->Printf ("ProcessGDBRemote::DoDestroy() - thread: 0x%4.4" PRIx64 " stopped with reason: %s.",
+ thread_sp->GetProtocolID(),
stop_info_sp->GetDescription());
stop_looks_like_crash = true;
break;
@@ -1832,8 +1825,8 @@
&& reason != eStopReasonException)
{
if (log)
- log->Printf ("ProcessGDBRemote::DoDestroy() - Suspending thread: %" PRId64 " before running.",
- thread_sp->GetID());
+ log->Printf ("ProcessGDBRemote::DoDestroy() - Suspending thread: 0x%4.4" PRIx64 " before running.",
+ thread_sp->GetProtocolID());
thread_sp->SetResumeState(eStateSuspended);
}
}
diff --git a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
index f1084d6..321fe8d 100644
--- a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -80,24 +80,14 @@
return NULL;
}
-bool
+void
ThreadGDBRemote::WillResume (StateType resume_state)
{
- // Call the Thread::WillResume first. If we stop at a signal, the stop info
- // class for signal will set the resume signal that we need below. The signal
- // stuff obeys the Process::UnixSignal defaults.
- // If the thread's WillResume returns false, that means that we aren't going to actually resume,
- // in which case we should not do the rest of our "resume" work.
-
- if (!Thread::WillResume(resume_state))
- return false;
-
- ClearStackFrames();
-
int signo = GetResumeSignal();
+ const lldb::user_id_t tid = GetProtocolID();
Log *log(lldb_private::GetLogIfAnyCategoriesSet (GDBR_LOG_THREAD));
if (log)
- log->Printf ("Resuming thread: %4.4" PRIx64 " with state: %s.", GetID(), StateAsCString(resume_state));
+ log->Printf ("Resuming thread: %4.4" PRIx64 " with state: %s.", tid, StateAsCString(resume_state));
ProcessSP process_sp (GetProcess());
if (process_sp)
@@ -112,24 +102,22 @@
case eStateRunning:
if (gdb_process->GetUnixSignals().SignalIsValid (signo))
- gdb_process->m_continue_C_tids.push_back(std::make_pair(GetID(), signo));
+ gdb_process->m_continue_C_tids.push_back(std::make_pair(tid, signo));
else
- gdb_process->m_continue_c_tids.push_back(GetID());
+ gdb_process->m_continue_c_tids.push_back(tid);
break;
case eStateStepping:
if (gdb_process->GetUnixSignals().SignalIsValid (signo))
- gdb_process->m_continue_S_tids.push_back(std::make_pair(GetID(), signo));
+ gdb_process->m_continue_S_tids.push_back(std::make_pair(tid, signo));
else
- gdb_process->m_continue_s_tids.push_back(GetID());
+ gdb_process->m_continue_s_tids.push_back(tid);
break;
default:
break;
}
- return true;
}
- return false;
}
void
@@ -147,16 +135,6 @@
GetRegisterContext()->InvalidateIfNeeded (force);
}
-void
-ThreadGDBRemote::ClearStackFrames ()
-{
- Unwind *unwinder = GetUnwinder ();
- if (unwinder)
- unwinder->Clear();
- Thread::ClearStackFrames();
-}
-
-
bool
ThreadGDBRemote::ThreadIDIsValid (lldb::tid_t thread)
{
@@ -245,7 +223,7 @@
StringExtractorGDBRemote stop_packet;
ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
- if (gdb_process->GetGDBRemote().GetThreadStopInfo(GetID(), stop_packet))
+ if (gdb_process->GetGDBRemote().GetThreadStopInfo(GetProtocolID(), stop_packet))
gdb_process->SetThreadStopInfo (stop_packet);
}
}
diff --git a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
index 375b7a5..8dfc4bb 100644
--- a/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ b/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -26,7 +26,7 @@
virtual
~ThreadGDBRemote ();
- virtual bool
+ virtual void
WillResume (lldb::StateType resume_state);
virtual void
@@ -44,9 +44,6 @@
virtual lldb::RegisterContextSP
CreateRegisterContextForFrame (lldb_private::StackFrame *frame);
- virtual void
- ClearStackFrames ();
-
void
Dump (lldb_private::Log *log, uint32_t index);