[Lldb-commits] REGRESSIONS: [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
Mon May 6 09:49:19 PDT 2013


On May 6, 2013, at 8:34 AM, "Thirumurthi, Ashok" <ashok.thirumurthi at intel.com> wrote:

> 
> 
> -----Original Message-----
> From: Thirumurthi, Ashok 
> Sent: Monday, May 06, 2013 11:13 AM
> To: 'Greg Clayton'
> Subject: REGRESSIONS: [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.
> 
> Hi Greg,
> 
> This commit introduced 89 test failures on the Linux buildbots.  Here are some snippets from the log that suggest that breakpoints are broken.  Are analogous changes required in ProcessPOSIX to remain in step with the new code around thread lists?

The test suite was passing on MacOSX. We will need to look at these on linux and figure out what is going on. For linux, there really shouldn't be any changes, though I would have to look at how the POSIX plug-in was reporting the stop reasons. I just took a look at the POSIXThread and noticed that it doesn't use:

lldb_private::Thread::m_actual_stop_info_sp, it uses its own POSIXThread::m_stop_info. Can someone try and remove "POSIXThread::m_stop_info" and use the "SetStopInfo" accessor:

void
Thread::SetStopInfo (const lldb::StopInfoSP &stop_info_sp);

Here is a patch that fixes the issue:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: linux.patch
Type: application/octet-stream
Size: 2616 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20130506/0bc6d99f/attachment.obj>
-------------- next part --------------


Let me know if this helps and/or fixes the issue?


I now have a Mac mini running Ubuntu 12.04, but I have still yet to be able to compile llvm/clang. The machine has 2GB of RAM, but linking clang always fails with GCC 4.7. The link stage starts but it gets killed with SIGKILL for some reason. Compiling with clang 3.2 crashes on the first few source files compiles (APFloat.cpp). I might need to just build my own clang and install it? What does everyone else do? The build instructions for linux don't seem to work for me.


> 
>  File "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lang/objc/objc-builtin-types/TestObjCBuiltinTypes.py", line 56, in objc_builtin_types
>    self.assertTrue (len(thread_list) != 0, "No thread stopped at our breakpoint.")
> 
>  File "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lang/cpp/unique-types/TestUniqueTypes.py", line 54, in unique_types
>    'stop reason = breakpoint'])
> 
>  File "/home/llvmbb/llvm-build-dir/lldb-x86_64-clang/llvm/tools/lldb/test/lang/cpp/stl/TestSTL.py", line 114, in sbtype_template_apis
>    self.assertTrue(thread.IsValid(), "There should be a thread stopped due to breakpoint condition")
> 
> - Ashok
> 
> 
> -----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 9: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
> 
> _______________________________________________
> 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