[Lldb-commits] [lldb] r181340 - Reinstating r181091 and r181106 with fix for Linux regressions.
Kaylor, Andrew
andrew.kaylor at intel.com
Tue May 7 12:54:45 PDT 2013
I noticed the m_cached_stop_info_sp handling in Thread KDP.cpp, and I figured it was there to handle something like you describe. I just wanted to get the simplest fix in place that would have us all back up and running.
The Linux multi-threaded situation is currently a mess, but I expect we'll be straightening it out over the next couple of weeks. We've got a prototype that mostly works, but we still need to smooth out a few edges.
If I understand you correctly, if we tolerate the lack of stop info caching on Linux for now and then implement CalculateStopInfo as you suggest below (using m_actual_stop_info_sp?) and also clear stop reasons for threads as needed then things will end up working properly. Is that correct or do we still need to implement some sort of stop info caching?
-Andy
-----Original Message-----
From: Greg Clayton [mailto:gclayton at apple.com]
Sent: Tuesday, May 07, 2013 12:07 PM
To: Kaylor, Andrew
Subject: Re: [Lldb-commits] [lldb] r181340 - Reinstating r181091 and r181106 with fix for Linux regressions.
Thanks for figuring this out!!!
A few concerns I have about my patch. You will want to make sure that your Thread::SetStopInfo() is being called _after_ the lldb_private::Process stop ID has been updated, or your class must be able to re-register the last valid stop info if the stop ID gets bumped. If your thread classes can give out the actual stop ID again, then you can stop reading right now. If they can't the read on.
If your code does something like:
// Calculate stop infos for all threads
CalculateAllStopInfos(); // This would call Thread::SetStopInfo() for each thread
// Now tell the process it has stopped
SetPrivateState (eStateStopped); // This function will bump the process stop ID...
Now later some code will ask about the stop and if you actually called Thread::SetStopInfo() in CalculateAllStopInfos(), all stop infos will be out of date (their stop ID will be one less than the current process stop ID) and the POSIXThread will be asked again why it stopped (Thread::GetPrivateStopReason()).
So there is merit to saving your stop info in your local POSIXThread class like you were doing if this is the case. So that when it is asked for you can just then do a:
Thread::SetStopInfo(m_cached_stop_info_sp);
This Thread::SetStopInfo() function call will adopt the m_cached_stop_info_sp and update all of the stop IDs...
It looks like your fix was to return the m_actual_stop_info_sp for POSIXThread::GetPrivateStopReason()?
lldb::StopInfoSP
POSIXThread::GetPrivateStopReason()
{
- return m_stop_info;
+ return m_actual_stop_info_sp;
}
My new changes, which aren't in yet, simplify the lldb_private::Thread class' job where the base classes don't override Thread::GetPrivateStopReason() anymore, they just implement:
bool
Thread::CalculateStopInfo();
This function must either call Thread::SetStopInfo() with a valid or empty stop reason and return true, or return false and we will update the stop info being empty and valid for the current stop ID. The Thread::GetPrivateStopReason() was renamed to Thread::GetPrivateStopInfo() and it doesn't need to be overridden in base classes, base classes just override the Thread::CalculateStopInfo() which always returns the current stop info for the thread. No extra logic. In your case this would mean:
bool
POSIXThread:: CalculateStopInfo()
{
SetStopInfo (m_actual_stop_info_sp);
return true;
}
One thing you guys will want to do is to make sure you clear the m_actual_stop_info_sp if the thread has no stop reason. When doing multi-threaded debugging, you don't want he old stop reason to persist. Do you guys currently clear this for threads with no stop reason?
Greg
On May 7, 2013, at 11:35 AM, Andrew Kaylor <andrew.kaylor at intel.com> wrote:
> Author: akaylor
> Date: Tue May 7 13:35:34 2013
> New Revision: 181340
>
> URL: http://llvm.org/viewvc/llvm-project?rev=181340&view=rev
> Log:
> Reinstating r181091 and r181106 with fix for Linux regressions.
>
> Modified:
> lldb/trunk/include/lldb/Target/OperatingSystem.h
> lldb/trunk/include/lldb/Target/Process.h
> lldb/trunk/include/lldb/Target/ThreadList.h
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
> lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
> lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
> lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.cpp
> lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.h
> lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
> lldb/trunk/source/Target/Process.cpp
> lldb/trunk/source/Target/Thread.cpp
> lldb/trunk/source/Target/ThreadList.cpp
>
> Modified: lldb/trunk/include/lldb/Target/OperatingSystem.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Ope
> ratingSystem.h?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/include/lldb/Target/OperatingSystem.h (original)
> +++ lldb/trunk/include/lldb/Target/OperatingSystem.h Tue May 7
> +++ 13:35:34 2013
> @@ -65,7 +65,9 @@ public:
> // Plug-in Methods
> //------------------------------------------------------------------
> virtual bool
> - UpdateThreadList (ThreadList &old_thread_list, ThreadList &new_thread_list) = 0;
> + UpdateThreadList (ThreadList &old_thread_list,
> + ThreadList &real_thread_list,
> + ThreadList &new_thread_list) = 0;
>
> virtual void
> ThreadWasSelected (Thread *thread) = 0;
>
> Modified: lldb/trunk/include/lldb/Target/Process.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Pro
> cess.h?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/include/lldb/Target/Process.h (original)
> +++ lldb/trunk/include/lldb/Target/Process.h Tue May 7 13:35:34 2013
> @@ -3631,7 +3631,10 @@ protected:
> std::map<uint64_t, uint32_t> m_thread_id_to_index_id_map;
> int m_exit_status; ///< The exit status of the process, or -1 if not set.
> std::string m_exit_string; ///< A textual description of why a process exited.
> - ThreadList m_thread_list; ///< The threads for this process.
> + Mutex m_thread_mutex;
> + ThreadList m_thread_list_real; ///< The threads for this process as are known to the protocol we are debugging with
> + ThreadList m_thread_list; ///< The threads for this process as the user will see them. This is usually the same as
> + ///<
> + m_thread_list_real, but might be different if there is an OS plug-in
> + creating memory threads
> std::vector<Notifications> m_notifications; ///< The list of notifications that this process can deliver.
> std::vector<lldb::addr_t> m_image_tokens;
> Listener &m_listener;
>
> Modified: lldb/trunk/include/lldb/Target/ThreadList.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Thr
> eadList.h?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/include/lldb/Target/ThreadList.h (original)
> +++ lldb/trunk/include/lldb/Target/ThreadList.h Tue May 7 13:35:34
> +++ 2013
> @@ -13,7 +13,6 @@
> #include <vector>
>
> #include "lldb/lldb-private.h"
> -#include "lldb/Host/Mutex.h"
> #include "lldb/Core/UserID.h"
>
>
> @@ -130,10 +129,7 @@ public:
> SetStopID (uint32_t stop_id);
>
> Mutex &
> - GetMutex ()
> - {
> - return m_threads_mutex;
> - }
> + GetMutex ();
>
> void
> Update (ThreadList &rhs);
> @@ -150,7 +146,6 @@ protected:
> Process *m_process; ///< The process that manages this thread list.
> uint32_t m_stop_id; ///< The process stop ID that this thread list is valid for.
> collection m_threads; ///< The threads for this process.
> - mutable Mutex m_threads_mutex;
> lldb::tid_t m_selected_tid; ///< For targets that need the notion of a current thread.
>
> private:
>
> Modified:
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython
> .cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Operatin
> gSystem/Python/OperatingSystemPython.cpp?rev=181340&r1=181339&r2=18134
> 0&view=diff
> ======================================================================
> ========
> ---
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython
> .cpp (original)
> +++ lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPy
> +++ thon.cpp Tue May 7 13:35:34 2013
> @@ -171,7 +171,9 @@ OperatingSystemPython::GetPluginVersion(
> }
>
> bool
> -OperatingSystemPython::UpdateThreadList (ThreadList &old_thread_list,
> ThreadList &new_thread_list)
> +OperatingSystemPython::UpdateThreadList (ThreadList &old_thread_list,
> + ThreadList &core_thread_list,
> + ThreadList &new_thread_list)
> {
> if (!m_interpreter || !m_python_object_sp)
> return false;
> @@ -194,8 +196,6 @@ OperatingSystemPython::UpdateThreadList
> PythonList threads_list(m_interpreter->OSPlugin_ThreadsInfo(m_python_object_sp));
> if (threads_list)
> {
> - ThreadList core_thread_list(new_thread_list);
> -
> uint32_t i;
> const uint32_t num_threads = threads_list.GetSize();
> for (i=0; i<num_threads; ++i)
>
> Modified:
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython
> .h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Operatin
> gSystem/Python/OperatingSystemPython.h?rev=181340&r1=181339&r2=181340&
> view=diff
> ======================================================================
> ========
> ---
> lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython
> .h (original)
> +++ lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPy
> +++ thon.h Tue May 7 13:35:34 2013
> @@ -65,7 +65,8 @@ public:
> // lldb_private::OperatingSystem Methods
> //------------------------------------------------------------------
> virtual bool
> - UpdateThreadList (lldb_private::ThreadList &old_thread_list,
> + UpdateThreadList (lldb_private::ThreadList &old_thread_list,
> + lldb_private::ThreadList &real_thread_list,
> lldb_private::ThreadList &new_thread_list);
>
> virtual void
>
> Modified:
> lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> MacOSX-Kernel/ProcessKDP.cpp?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
> (original)
> +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Tue
> +++ May 7 13:35:34 2013
> @@ -44,6 +44,8 @@
> using namespace lldb;
> using namespace lldb_private;
>
> +static const lldb::tid_t g_kernel_tid = 1;
> +
> const char *
> ProcessKDP::GetPluginNameStatic()
> {
> @@ -116,7 +118,8 @@ ProcessKDP::ProcessKDP(Target& target, L
> m_async_thread (LLDB_INVALID_HOST_THREAD),
> m_dyld_plugin_name (),
> m_kernel_load_addr (LLDB_INVALID_ADDRESS),
> - m_command_sp()
> + m_command_sp(),
> + m_kernel_thread_wp()
> {
> m_async_broadcaster.SetEventName (eBroadcastBitAsyncThreadShouldExit, "async thread should exit");
> m_async_broadcaster.SetEventName (eBroadcastBitAsyncContinue, "async thread continue");
> @@ -377,7 +380,8 @@ ProcessKDP::DoResume ()
> bool resume = false;
>
> // With KDP there is only one thread we can tell what to do
> - ThreadSP kernel_thread_sp (GetKernelThread(m_thread_list, m_thread_list));
> + ThreadSP kernel_thread_sp
> + (m_thread_list.FindThreadByProtocolID(g_kernel_tid));
> +
> if (kernel_thread_sp)
> {
> const StateType thread_resume_state =
> kernel_thread_sp->GetTemporaryResumeState();
> @@ -449,15 +453,17 @@ ProcessKDP::DoResume () }
>
> lldb::ThreadSP
> -ProcessKDP::GetKernelThread(ThreadList &old_thread_list, ThreadList
> &new_thread_list)
> +ProcessKDP::GetKernelThread()
> {
> // KDP only tells us about one thread/core. Any other threads will usually
> // be the ones that are read from memory by the OS plug-ins.
> - const lldb::tid_t kernel_tid = 1;
> - ThreadSP thread_sp (old_thread_list.FindThreadByID (kernel_tid, false));
> +
> + ThreadSP thread_sp (m_kernel_thread_wp.lock());
> if (!thread_sp)
> - thread_sp.reset(new ThreadKDP (*this, kernel_tid));
> - new_thread_list.AddThread(thread_sp);
> + {
> + thread_sp.reset(new ThreadKDP (*this, g_kernel_tid));
> + m_kernel_thread_wp = thread_sp;
> + }
> return thread_sp;
> }
>
> @@ -474,7 +480,10 @@ ProcessKDP::UpdateThreadList (ThreadList
>
> // Even though there is a CPU mask, it doesn't mean we can see each CPU
> // indivudually, there is really only one. Lets call this thread 1.
> - GetKernelThread (old_thread_list, new_thread_list);
> + ThreadSP thread_sp (old_thread_list.FindThreadByProtocolID(g_kernel_tid, false));
> + if (!thread_sp)
> + thread_sp = GetKernelThread ();
> + new_thread_list.AddThread(thread_sp);
>
> return new_thread_list.GetSize(false) > 0; } @@ -798,7 +807,7 @@
> ProcessKDP::AsyncThread (void *arg)
> is_running = true;
> if (process->m_comm.WaitForPacketWithTimeoutMicroSeconds (exc_reply_packet, 1 * USEC_PER_SEC))
> {
> - ThreadSP thread_sp (process->GetKernelThread(process->GetThreadList(), process->GetThreadList()));
> + ThreadSP thread_sp
> + (process->GetKernelThread());
> if (thread_sp)
> {
> lldb::RegisterContextSP reg_ctx_sp
> (thread_sp->GetRegisterContext());
>
> Modified: lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> MacOSX-Kernel/ProcessKDP.h?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
> (original)
> +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h Tue
> +++ May 7 13:35:34 2013
> @@ -225,12 +225,6 @@ protected:
> bool
> ProcessIDIsValid ( ) const;
>
> - // static void
> - // STDIOReadThreadBytesReceived (void *baton, const void *src, size_t src_len);
> -
> - // void
> - // AppendSTDOUT (const char* s, size_t len);
> -
> void
> Clear ( );
>
> @@ -245,8 +239,7 @@ protected:
> };
>
> lldb::ThreadSP
> - GetKernelThread (lldb_private::ThreadList &old_thread_list,
> - lldb_private::ThreadList &new_thread_list);
> + GetKernelThread ();
>
> //------------------------------------------------------------------
> /// Broadcaster event bits definitions.
> @@ -257,6 +250,7 @@ protected:
> std::string m_dyld_plugin_name;
> lldb::addr_t m_kernel_load_addr;
> lldb::CommandObjectSP m_command_sp;
> + lldb::ThreadWP m_kernel_thread_wp;
>
>
> bool
>
> Modified: lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> POSIX/POSIXThread.cpp?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.cpp (original)
> +++ lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.cpp Tue May 7
> +++ 13:35:34 2013
> @@ -140,7 +140,7 @@ POSIXThread::CreateRegisterContextForFra
> lldb::StopInfoSP
> POSIXThread::GetPrivateStopReason()
> {
> - return m_stop_info;
> + return m_actual_stop_info_sp;
> }
>
> Unwind *
> @@ -262,19 +262,19 @@ POSIXThread::BreakNotify(const ProcessMe
>
>
> m_breakpoint = bp_site;
> - m_stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id);
> + SetStopInfo
> + (StopInfo::CreateStopReasonWithBreakpointSiteID(*this, bp_id));
> }
>
> void
> POSIXThread::TraceNotify(const ProcessMessage &message) {
> - m_stop_info = StopInfo::CreateStopReasonToTrace(*this);
> + SetStopInfo (StopInfo::CreateStopReasonToTrace(*this));
> }
>
> void
> POSIXThread::LimboNotify(const ProcessMessage &message) {
> - m_stop_info = lldb::StopInfoSP(new POSIXLimboStopInfo(*this));
> + SetStopInfo (lldb::StopInfoSP(new POSIXLimboStopInfo(*this)));
> }
>
> void
> @@ -282,7 +282,7 @@ POSIXThread::SignalNotify(const ProcessM {
> int signo = message.GetSignal();
>
> - m_stop_info = StopInfo::CreateStopReasonWithSignal(*this, signo);
> + SetStopInfo (StopInfo::CreateStopReasonWithSignal(*this, signo));
> SetResumeSignal(signo);
> }
>
> @@ -291,7 +291,7 @@ POSIXThread::SignalDeliveredNotify(const
> {
> int signo = message.GetSignal();
>
> - m_stop_info = StopInfo::CreateStopReasonWithSignal(*this, signo);
> + SetStopInfo (StopInfo::CreateStopReasonWithSignal(*this, signo));
> SetResumeSignal(signo);
> }
>
> @@ -306,15 +306,14 @@ POSIXThread::CrashNotify(const ProcessMe
> if (log)
> log->Printf ("POSIXThread::%s () signo = %i, reason = '%s'",
> __FUNCTION__, signo, message.PrintCrashReason());
>
> - m_stop_info = lldb::StopInfoSP(new POSIXCrashStopInfo(
> - *this, signo, message.GetCrashReason()));
> + SetStopInfo (lldb::StopInfoSP(new POSIXCrashStopInfo(*this,
> + signo, message.GetCrashReason())));
> SetResumeSignal(signo);
> }
>
> void
> POSIXThread::ThreadNotify(const ProcessMessage &message) {
> - m_stop_info = lldb::StopInfoSP(new POSIXNewThreadStopInfo(*this));
> + SetStopInfo (lldb::StopInfoSP(new
> + POSIXNewThreadStopInfo(*this)));
> }
>
> unsigned
>
> Modified: lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> POSIX/POSIXThread.h?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.h (original)
> +++ lldb/trunk/source/Plugins/Process/POSIX/POSIXThread.h Tue May 7
> +++ 13:35:34 2013
> @@ -84,7 +84,6 @@ private:
> std::unique_ptr<lldb_private::StackFrame> m_frame_ap;
>
> lldb::BreakpointSiteSP m_breakpoint;
> - lldb::StopInfoSP m_stop_info;
>
> ProcessMonitor &
> GetMonitor();
>
> Modified:
> lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> gdb-remote/ProcessGDBRemote.cpp?rev=181340&r1=181339&r2=181340&view=di
> ff
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> (original)
> +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> +++ Tue May 7 13:35:34 2013
> @@ -1281,14 +1281,14 @@ ProcessGDBRemote::DoResume () void
> ProcessGDBRemote::ClearThreadIDList () {
> - Mutex::Locker locker(m_thread_list.GetMutex());
> + Mutex::Locker locker(m_thread_list_real.GetMutex());
> m_thread_ids.clear();
> }
>
> bool
> ProcessGDBRemote::UpdateThreadIDList () {
> - Mutex::Locker locker(m_thread_list.GetMutex());
> + Mutex::Locker locker(m_thread_list_real.GetMutex());
> bool sequence_mutex_unavailable = false;
> m_gdb_comm.GetCurrentThreadIDs (m_thread_ids, sequence_mutex_unavailable);
> if (sequence_mutex_unavailable)
> @@ -1323,12 +1323,6 @@ ProcessGDBRemote::UpdateThreadList (Thre
> {
> tid_t tid = m_thread_ids[i];
> 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);
> @@ -1385,7 +1379,6 @@ ProcessGDBRemote::SetThreadStopInfo (Str
> 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)) @@
> -1404,31 +1397,24 @@ ProcessGDBRemote::SetThreadStopInfo (Str
> {
> // thread in big endian hex
> lldb::tid_t tid = Args::StringToUInt64 (value.c_str(), LLDB_INVALID_THREAD_ID, 16);
> - // m_thread_list does have its own mutex, but we need to
> - // 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.FindThreadByProtocolID(tid, false);
> + // m_thread_list_real does have its own mutex, but we need to
> + // hold onto the mutex between the call to m_thread_list_real.FindThreadByID(...)
> + // and the m_thread_list_real.AddThread(...) so it doesn't change on us
> + Mutex::Locker locker (m_thread_list_real.GetMutex ());
> + thread_sp =
> + m_thread_list_real.FindThreadByProtocolID(tid, false);
>
> - if (thread_sp)
> - {
> - 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());
> - }
> - else
> + if (!thread_sp)
> {
> // 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);
> + m_thread_list_real.AddThread(thread_sp);
> }
> + gdb_thread = static_cast<ThreadGDBRemote *>
> + (thread_sp.get());
> +
> }
> else if (name.compare("threads") == 0)
> {
> - Mutex::Locker locker(m_thread_list.GetMutex());
> + Mutex::Locker
> + locker(m_thread_list_real.GetMutex());
> m_thread_ids.clear();
> // A comma separated list of all threads in the current
> // process that includes the thread for this stop
> reply @@ -1508,6 +1494,9 @@ ProcessGDBRemote::SetThreadStopInfo (Str
>
> if (thread_sp)
> {
> + // Clear the stop info just in case we don't set it to anything
> + thread_sp->SetStopInfo (StopInfoSP());
> +
> gdb_thread->SetThreadDispatchQAddr (thread_dispatch_qaddr);
> gdb_thread->SetName (thread_name.empty() ? NULL : thread_name.c_str());
> if (exc_type != 0)
> @@ -1610,11 +1599,6 @@ ProcessGDBRemote::SetThreadStopInfo (Str
> if (!handled)
> thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithSignal (*thread_sp, signo));
> }
> - else
> - {
> - StopInfoSP invalid_stop_info_sp;
> - thread_sp->SetStopInfo (invalid_stop_info_sp);
> - }
>
> if (!description.empty())
> {
> @@ -1647,7 +1631,7 @@ ProcessGDBRemote::SetThreadStopInfo (Str void
> ProcessGDBRemote::RefreshStateAfterStop () {
> - Mutex::Locker locker(m_thread_list.GetMutex());
> + Mutex::Locker locker(m_thread_list_real.GetMutex());
> m_thread_ids.clear();
> // Set the thread stop info. It might have a "threads" key whose value is
> // a list of all thread IDs in the current process, so
> m_thread_ids might @@ -1662,7 +1646,7 @@
> ProcessGDBRemote::RefreshStateAfterStop
>
> // Let all threads recover from stopping and do any clean up based
> // on the previous thread state (if any).
> - m_thread_list.RefreshStateAfterStop();
> + m_thread_list_real.RefreshStateAfterStop();
>
> }
>
> @@ -2330,6 +2314,7 @@ void
> ProcessGDBRemote::Clear()
> {
> m_flags = 0;
> + m_thread_list_real.Clear();
> m_thread_list.Clear();
> }
>
>
> Modified:
> lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/
> gdb-remote/ThreadGDBRemote.cpp?rev=181340&r1=181339&r2=181340&view=dif
> f
> ======================================================================
> ========
> --- lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
> (original)
> +++ lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
> +++ Tue May 7 13:35:34 2013
> @@ -204,27 +204,46 @@ ThreadGDBRemote::GetPrivateStopReason ()
> if (process_sp)
> {
> 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 (m_thread_stop_reason_stop_id == process_stop_id)
> + {
> + // Our stop info is up to date even if it is empty...
> + return m_actual_stop_info_sp;
> + }
> +
> + if (m_actual_stop_info_sp && m_actual_stop_info_sp->IsValid())
> + {
> + // The stop info is up to date, reset it so everything updates
> + SetStopInfo (m_actual_stop_info_sp);
> + }
> + else
> {
> if (IsStillAtLastBreakpointHit())
> - return m_actual_stop_info_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();
> + {
> + SetStopInfo(m_actual_stop_info_sp);
> + }
> + else
> + {
> + // 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_actual_stop_info_sp.reset();
>
> - StringExtractorGDBRemote stop_packet;
> - ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
> - if (gdb_process->GetGDBRemote().GetThreadStopInfo(GetProtocolID(), stop_packet))
> - gdb_process->SetThreadStopInfo (stop_packet);
> + StringExtractorGDBRemote stop_packet;
> + ProcessGDBRemote *gdb_process = static_cast<ProcessGDBRemote *>(process_sp.get());
> + if (gdb_process->GetGDBRemote().GetThreadStopInfo(GetProtocolID(), stop_packet))
> + {
> + gdb_process->SetThreadStopInfo (stop_packet);
> + }
> + else
> + {
> + SetStopInfo (StopInfoSP());
> + }
> + }
> }
> }
> return m_actual_stop_info_sp;
>
> Modified: lldb/trunk/source/Target/Process.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.c
> pp?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Target/Process.cpp (original)
> +++ lldb/trunk/source/Target/Process.cpp Tue May 7 13:35:34 2013
> @@ -1016,6 +1016,8 @@ Process::Process(Target &target, Listene
> m_thread_id_to_index_id_map (),
> m_exit_status (-1),
> m_exit_string (),
> + m_thread_mutex (Mutex::eMutexTypeRecursive),
> + m_thread_list_real (this),
> m_thread_list (this),
> m_notifications (),
> m_image_tokens (),
> @@ -1140,6 +1142,7 @@ Process::Finalize()
> m_abi_sp.reset();
> m_os_ap.reset();
> m_dyld_ap.reset();
> + m_thread_list_real.Destroy();
> m_thread_list.Destroy();
> std::vector<Notifications> empty_notifications;
> m_notifications.swap(empty_notifications);
> @@ -1537,10 +1540,12 @@ Process::UpdateThreadListIfNeeded ()
> // m_thread_list does have its own mutex, but we need to
> // hold onto the mutex between the call to UpdateThreadList(...)
> // and the os->UpdateThreadList(...) so it doesn't change
> on us
> + ThreadList &old_thread_list = m_thread_list;
> + ThreadList real_thread_list(this);
> ThreadList new_thread_list(this);
> // Always update the thread list with the protocol specific
> // thread list, but only update if "true" is returned
> - if (UpdateThreadList (m_thread_list, new_thread_list))
> + if (UpdateThreadList (m_thread_list_real,
> + real_thread_list))
> {
> // Don't call into the OperatingSystem to update the thread list if we are shutting down, since
> // that may call back into the SBAPI's, requiring the
> API lock which is already held by whoever is @@ -1552,16 +1557,23 @@ Process::UpdateThreadListIfNeeded ()
> {
> // Clear any old backing threads where memory threads might have been
> // backed by actual threads from the lldb_private::Process subclass
> - size_t num_old_threads = m_thread_list.GetSize(false);
> + size_t num_old_threads =
> + old_thread_list.GetSize(false);
> for (size_t i=0; i<num_old_threads; ++i)
> - m_thread_list.GetThreadAtIndex(i, false)->ClearBackingThread();
> + old_thread_list.GetThreadAtIndex(i,
> + false)->ClearBackingThread();
>
> // Now let the OperatingSystem plug-in update the thread list
> - os->UpdateThreadList (m_thread_list, new_thread_list);
> + os->UpdateThreadList (old_thread_list, // Old list full of threads created by OS plug-in
> + real_thread_list, // The actual thread list full of threads created by each lldb_private::Process subclass
> + new_thread_list); // The new thread list that we will show to the user that gets filled in
> + }
> + else
> + {
> + // No OS plug-in, the new thread list is the same as the real thread list
> + new_thread_list = real_thread_list;
> }
> - m_thread_list.Update (new_thread_list);
> - m_thread_list.SetStopID (stop_id);
> }
> + m_thread_list.Update (new_thread_list);
> + m_thread_list.SetStopID (stop_id);
> }
> }
> }
>
> Modified: lldb/trunk/source/Target/Thread.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Thread.cp
> p?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Target/Thread.cpp (original)
> +++ lldb/trunk/source/Target/Thread.cpp Tue May 7 13:35:34 2013
> @@ -357,18 +357,24 @@ lldb::StopInfoSP Thread::GetStopInfo () {
> ThreadPlanSP plan_sp (GetCompletedPlan());
> + ProcessSP process_sp (GetProcess());
> + const uint32_t stop_id = process_sp ? process_sp->GetStopID() :
> + UINT32_MAX;
> if (plan_sp && plan_sp->PlanSucceeded())
> + {
> return StopInfo::CreateStopReasonWithPlan (plan_sp,
> GetReturnValueObject());
> + }
> else
> {
> - ProcessSP process_sp (GetProcess());
> - if (process_sp
> - && m_actual_stop_info_sp
> - && m_actual_stop_info_sp->IsValid()
> - && m_thread_stop_reason_stop_id == process_sp->GetStopID())
> + if ((m_thread_stop_reason_stop_id == stop_id) || // Stop info is valid, just return what we have (even if empty)
> + (m_actual_stop_info_sp && m_actual_stop_info_sp->IsValid())) // Stop info is valid, just return what we have
> + {
> return m_actual_stop_info_sp;
> + }
> else
> - return GetPrivateStopReason ();
> + {
> + GetPrivateStopReason ();
> + return m_actual_stop_info_sp;
> + }
> }
> }
>
>
> Modified: lldb/trunk/source/Target/ThreadList.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadLis
> t.cpp?rev=181340&r1=181339&r2=181340&view=diff
> ======================================================================
> ========
> --- lldb/trunk/source/Target/ThreadList.cpp (original)
> +++ lldb/trunk/source/Target/ThreadList.cpp Tue May 7 13:35:34 2013
> @@ -24,16 +24,14 @@ ThreadList::ThreadList (Process *process
> m_process (process),
> m_stop_id (0),
> m_threads(),
> - m_threads_mutex (Mutex::eMutexTypeRecursive),
> m_selected_tid (LLDB_INVALID_THREAD_ID) { }
>
> ThreadList::ThreadList (const ThreadList &rhs) :
> - m_process (),
> - m_stop_id (),
> + m_process (rhs.m_process),
> + m_stop_id (rhs.m_stop_id),
> m_threads (),
> - m_threads_mutex (Mutex::eMutexTypeRecursive),
> m_selected_tid ()
> {
> // Use the assignment operator since it uses the mutex @@ -47,8
> +45,7 @@ ThreadList::operator = (const ThreadList
> {
> // Lock both mutexes to make sure neither side changes anyone on us
> // while the assignement occurs
> - Mutex::Locker locker_lhs(m_threads_mutex);
> - Mutex::Locker locker_rhs(rhs.m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> m_process = rhs.m_process;
> m_stop_id = rhs.m_stop_id;
> m_threads = rhs.m_threads;
> @@ -83,14 +80,14 @@ ThreadList::SetStopID (uint32_t stop_id) void
> ThreadList::AddThread (const ThreadSP &thread_sp) {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> m_threads.push_back(thread_sp);
> }
>
> uint32_t
> ThreadList::GetSize (bool can_update)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> return m_threads.size();
> @@ -99,7 +96,7 @@ ThreadList::GetSize (bool can_update) ThreadSP
> ThreadList::GetThreadAtIndex (uint32_t idx, bool can_update) {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
>
> @@ -112,7 +109,7 @@ ThreadList::GetThreadAtIndex (uint32_t i ThreadSP
> ThreadList::FindThreadByID (lldb::tid_t tid, bool can_update) {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> @@ -134,7 +131,7 @@ ThreadList::FindThreadByID (lldb::tid_t
> ThreadSP
> ThreadList::FindThreadByProtocolID (lldb::tid_t tid, bool can_update)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> @@ -157,7 +154,7 @@ ThreadList::FindThreadByProtocolID (lldb
> ThreadSP
> ThreadList::RemoveThreadByID (lldb::tid_t tid, bool can_update)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> @@ -180,7 +177,7 @@ ThreadList::RemoveThreadByID (lldb::tid_
> ThreadSP
> ThreadList::RemoveThreadByProtocolID (lldb::tid_t tid, bool can_update)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> @@ -206,7 +203,7 @@ ThreadList::GetThreadSPForThreadPtr (Thr
> ThreadSP thread_sp;
> if (thread_ptr)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> uint32_t idx = 0;
> const uint32_t num_threads = m_threads.size();
> @@ -227,7 +224,7 @@ ThreadList::GetThreadSPForThreadPtr (Thr
> ThreadSP
> ThreadList::FindThreadByIndexID (uint32_t index_id, bool can_update)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> if (can_update)
> m_process->UpdateThreadListIfNeeded();
> @@ -263,7 +260,7 @@ ThreadList::ShouldStop (Event *event_ptr
> collection threads_copy;
> {
> // Scope for locker
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> m_process->UpdateThreadListIfNeeded();
> threads_copy = m_threads;
> @@ -331,7 +328,7 @@ ThreadList::ShouldStop (Event *event_ptr
> Vote
> ThreadList::ShouldReportStop (Event *event_ptr)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> Vote result = eVoteNoOpinion;
> m_process->UpdateThreadListIfNeeded();
> @@ -383,7 +380,7 @@ Vote
> ThreadList::ShouldReportRun (Event *event_ptr)
> {
>
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> Vote result = eVoteNoOpinion;
> m_process->UpdateThreadListIfNeeded();
> @@ -422,7 +419,7 @@ ThreadList::ShouldReportRun (Event *even
> void
> ThreadList::Clear()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> m_stop_id = 0;
> m_threads.clear();
> m_selected_tid = LLDB_INVALID_THREAD_ID;
> @@ -431,7 +428,7 @@ ThreadList::Clear()
> void
> ThreadList::Destroy()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> const uint32_t num_threads = m_threads.size();
> for (uint32_t idx = 0; idx < num_threads; ++idx)
> {
> @@ -442,7 +439,7 @@ ThreadList::Destroy()
> void
> ThreadList::RefreshStateAfterStop ()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> m_process->UpdateThreadListIfNeeded();
>
> @@ -460,7 +457,7 @@ ThreadList::DiscardThreadPlans ()
> {
> // You don't need to update the thread list here, because only threads
> // that you currently know about have any thread plans.
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
>
> collection::iterator pos, end = m_threads.end();
> for (pos = m_threads.begin(); pos != end; ++pos)
> @@ -475,7 +472,7 @@ ThreadList::WillResume ()
> // But we only do this for threads that are running, user suspended
> // threads stay where they are.
>
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> m_process->UpdateThreadListIfNeeded();
>
> collection::iterator pos, end = m_threads.end();
> @@ -626,7 +623,7 @@ ThreadList::WillResume ()
> void
> ThreadList::DidResume ()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> collection::iterator pos, end = m_threads.end();
> for (pos = m_threads.begin(); pos != end; ++pos)
> {
> @@ -641,7 +638,7 @@ ThreadList::DidResume ()
> ThreadSP
> ThreadList::GetSelectedThread ()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> ThreadSP thread_sp = FindThreadByID(m_selected_tid);
> if (!thread_sp.get())
> {
> @@ -656,7 +653,7 @@ ThreadList::GetSelectedThread ()
> bool
> ThreadList::SetSelectedThreadByID (lldb::tid_t tid, bool notify)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> ThreadSP selected_thread_sp(FindThreadByID(tid));
> if (selected_thread_sp)
> {
> @@ -675,7 +672,7 @@ ThreadList::SetSelectedThreadByID (lldb:
> bool
> ThreadList::SetSelectedThreadByIndexID (uint32_t index_id, bool notify)
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> ThreadSP selected_thread_sp (FindThreadByIndexID(index_id));
> if (selected_thread_sp.get())
> {
> @@ -707,8 +704,7 @@ ThreadList::Update (ThreadList &rhs)
> {
> // Lock both mutexes to make sure neither side changes anyone on us
> // while the assignement occurs
> - Mutex::Locker locker_lhs(m_threads_mutex);
> - Mutex::Locker locker_rhs(rhs.m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> m_process = rhs.m_process;
> m_stop_id = rhs.m_stop_id;
> m_threads.swap(rhs.m_threads);
> @@ -745,9 +741,15 @@ ThreadList::Update (ThreadList &rhs)
> void
> ThreadList::Flush ()
> {
> - Mutex::Locker locker(m_threads_mutex);
> + Mutex::Locker locker(GetMutex());
> collection::iterator pos, end = m_threads.end();
> for (pos = m_threads.begin(); pos != end; ++pos)
> (*pos)->Flush ();
> }
>
> +Mutex &
> +ThreadList::GetMutex ()
> +{
> + return m_process->m_thread_mutex;
> +}
> +
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
More information about the lldb-commits
mailing list