[Lldb-commits] [lldb] r181091 - After recent OperatingsSystem plug-in changes, the lldb_private::Process and lldb_private::Thread subclasses were changed and the API was not respected properly.
Greg Clayton
gclayton at apple.com
Tue May 7 09:06:32 PDT 2013
On May 6, 2013, at 5:47 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:
> Can you explain this patch in a little more detail?
This is to support OperatingSystem plug-ins. These plug-ins can find threads that are context switched out in memory and aren't actually on core. This is for kernel debugging where you have N cores, but many many more threads. When debugging a kernel you will want to be able to see and backtrace these threads.
> Having two different lists of threads that may or may not be the same list seems like a bad idea. Why couldn't you just have an attribute that makes a thread as not being user visible?
Because the thread IDs can overlap. You want the lldb_private::Process plug-ins to always just update the thread list they know about: the threads that are available through the debugging API. When there is no OS plug-in in use, this becomes the real thread list. When there is a OS plug-in that is creating threads from memory contexts, it may or may not want to show the actual real threads. In our case it doesn't.
If you have only one list, then each lldb_private::Process plug-in needs to understand that the thread list it is given has two types of threads and it makes the UpdateThreadLIst code do more than it should need to.
>
> Also, on first reading it looks like there are different mutexes controlling access to these lists. Is that true?
No. One mutex in the process class.
>
> What do you mean by a "memory" thread, and what is happening in the scenario where a "real" thread is placed inside of a "memory" thread?
Yes. Think of a kernel. N cores with M threads context switched out into memory. You hook up to a kernel and want to see all threads. The OS plug-ins help you do this. Each memory thread can be on a core, or it can simply be in memory. The OS plug-ins are responsible for placing a "real" thread into the correct "memory" thread (and of course all "real" threads are purged from the memory threads on each stop.
>
> Thanks,
> Andy
>
> -----Original Message-----
> From: lldb-commits-bounces at cs.uiuc.edu [mailto:lldb-commits-bounces at cs.uiuc.edu] On Behalf Of Greg Clayton
> Sent: Friday, May 03, 2013 6:39 PM
> To: lldb-commits at cs.uiuc.edu
> Subject: [Lldb-commits] [lldb] r181091 - After recent OperatingsSystem plug-in changes, the lldb_private::Process and lldb_private::Thread subclasses were changed and the API was not respected properly.
>
> Author: gclayton
> Date: Fri May 3 20:38:48 2013
> New Revision: 181091
>
> URL: http://llvm.org/viewvc/llvm-project?rev=181091&view=rev
> Log:
> After recent OperatingsSystem plug-in changes, the lldb_private::Process and lldb_private::Thread subclasses were changed and the API was not respected properly.
>
> This checkin aims to fix this. The process now has two thread lists: a real thread list for threads that are created by the lldb_private::Process subclass, and the user visible threads. The user visible threads are the same as the real threas when no OS plug-in in used. But when an OS plug-in is used, the user thread can be a combination of real and "memory" threads. Real threads can be placed inside of memory threads so that a thread appears to be different, but is still controlled by the actual real thread. When the thread list needs updating, the lldb_private::Process class will call the: lldb_private::Process::UpdateThreadList() function with the old real thread list, and the function is expected to fill in the new real thread list with the current state of the process. After this function, the process will check if there is an OS plug-in being used, and if so, it will give the old user thread list, the new real thread list and the OS plug-in will create the new user t!
hr!
> ead list
> from both of these lists. If there is no OS plug-in, the real thread list is the user thread list.
>
> These changes keep the lldb_private::Process subclasses clean and no changes are required.
>
>
>
> 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/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/OperatingSystem.h?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/OperatingSystem.h (original)
> +++ lldb/trunk/include/lldb/Target/OperatingSystem.h Fri May 3 20:38:48
> +++ 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/Process.h?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/Process.h (original)
> +++ lldb/trunk/include/lldb/Target/Process.h Fri May 3 20:38:48 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/ThreadList.h?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/include/lldb/Target/ThreadList.h (original)
> +++ lldb/trunk/include/lldb/Target/ThreadList.h Fri May 3 20:38:48 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/OperatingSystem/Python/OperatingSystemPython.cpp?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp (original)
> +++ lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPyth
> +++ on.cpp Fri May 3 20:38:48 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/OperatingSystem/Python/OperatingSystemPython.h?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h (original)
> +++ lldb/trunk/source/Plugins/OperatingSystem/Python/OperatingSystemPyth
> +++ on.h Fri May 3 20:38:48 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=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (original)
> +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp Fri
> +++ May 3 20:38:48 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");
> @@ -374,7 +377,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();
> @@ -446,15 +450,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;
> }
>
> @@ -471,7 +477,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));
> + if (!thread_sp)
> + thread_sp = GetKernelThread ();
> + new_thread_list.AddThread(thread_sp);
>
> return new_thread_list.GetSize(false) > 0; } @@ -795,7 +804,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=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (original)
> +++ lldb/trunk/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h Fri May
> +++ 3 20:38:48 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/gdb-remote/ProcessGDBRemote.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original)
> +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
> +++ Fri May 3 20:38:48 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=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp (original)
> +++ lldb/trunk/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp Fri
> +++ May 3 20:38:48 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.cpp?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/Process.cpp (original)
> +++ lldb/trunk/source/Target/Process.cpp Fri May 3 20:38:48 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.cpp?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/Thread.cpp (original)
> +++ lldb/trunk/source/Target/Thread.cpp Fri May 3 20:38:48 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/ThreadList.cpp?rev=181091&r1=181090&r2=181091&view=diff
> ==============================================================================
> --- lldb/trunk/source/Target/ThreadList.cpp (original)
> +++ lldb/trunk/source/Target/ThreadList.cpp Fri May 3 20:38:48 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