This change introduces a "ExpressionExecutionThread" to the ThreadList.

Turns out that most of the code that runs expressions (e.g. the ObjC runtime grubber) on
behalf of the expression parser was using the currently selected thread.  But sometimes,
e.g. when we are evaluating breakpoint conditions/commands, we don't select the thread
we're running on, we instead set the context for the interpreter, and explicitly pass
that to other callers.  That wasn't getting communicated to these utility expressions, so
they would run on some other thread instead, and that could cause a variety of subtle and
hard to reproduce problems.  

I also went through the commands and cleaned up the use of GetSelectedThread.  All those
uses should have been trying the thread in the m_exe_ctx belonging to the command object
first.  It would actually have been pretty hard to get misbehavior in these cases, but for
correctness sake it is good to make this usage consistent.

<rdar://problem/24978569>

llvm-svn: 263326

lldb/include/lldb/Interpreter/CommandObject.h

@@ -481,6 +481,12 @@ protected:
     // is present you want to prime the dummy target with entities that will be copied over to new targets.
     Target *GetSelectedOrDummyTarget(bool prefer_dummy = false);
     Target *GetDummyTarget();
+    
+    // If a command needs to use the "current" thread, use this call.
+    // Command objects will have an ExecutionContext to use, and that may or may not have a thread in it.  If it
+    // does, you should use that by default, if not, then use the ExecutionContext's target's selected thread, etc...
+    // This call insulates you from the details of this calculation.
+    Thread *GetDefaultThread();
 
     //------------------------------------------------------------------
     /// Check the command to make sure anything required by this

lldb/include/lldb/Target/ThreadList.h

@@ -16,6 +16,7 @@
 #include "lldb/Core/UserID.h"
 #include "lldb/Utility/Iterable.h"
 #include "lldb/Target/ThreadCollection.h"
+#include "lldb/Target/Thread.h"
 
 namespace lldb_private {
 
@@ -44,7 +45,43 @@ public:
     // selected at index 0.
     lldb::ThreadSP
     GetSelectedThread ();
+    
+    // Manage the thread to use for running expressions.  This is usually the Selected thread,
+    // but sometimes (e.g. when evaluating breakpoint conditions & stop hooks) it isn't.
+    class ExpressionExecutionThreadPusher
+    {
+        public:
+        ExpressionExecutionThreadPusher(ThreadList &thread_list, lldb::tid_t tid) :
+            m_thread_list(&thread_list),
+            m_tid(tid)
+        {
+            m_thread_list->PushExpressionExecutionThread(m_tid);
+        }
+        
+        ExpressionExecutionThreadPusher(lldb::ThreadSP thread_sp);
+        
+        ~ExpressionExecutionThreadPusher()
+        {
+            if (m_thread_list)
+                m_thread_list->PopExpressionExecutionThread(m_tid);
+        }
+        
+        private:
+            ThreadList *m_thread_list;
+            lldb::tid_t m_tid;
+    };
 
+    lldb::ThreadSP
+    GetExpressionExecutionThread();
+    
+protected:
+    void
+    PushExpressionExecutionThread(lldb::tid_t tid);
+    
+    void
+    PopExpressionExecutionThread(lldb::tid_t tid);
+
+public:
     bool
     SetSelectedThreadByID (lldb::tid_t tid, bool notify = false);
 
@@ -147,6 +184,7 @@ 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.
     lldb::tid_t m_selected_tid;  ///< For targets that need the notion of a current thread.
+    std::vector<lldb::tid_t> m_expression_tid_stack;
 
 private:
 

lldb/source/Commands/CommandObjectProcess.cpp

@@ -708,7 +708,7 @@ protected:
 
             if (m_options.m_ignore > 0)
             {
-                ThreadSP sel_thread_sp(process->GetThreadList().GetSelectedThread());
+                ThreadSP sel_thread_sp(GetDefaultThread()->shared_from_this());
                 if (sel_thread_sp)
                 {
                     StopInfoSP stop_info_sp = sel_thread_sp->GetStopInfo();

lldb/source/Commands/CommandObjectThread.cpp

@@ -509,7 +509,8 @@ protected:
 
         if (command.GetArgumentCount() == 0)
         {
-            thread = process->GetThreadList().GetSelectedThread().get();
+            thread = GetDefaultThread();
+
             if (thread == nullptr)
             {
                 result.AppendError ("no selected thread in process");
@@ -919,7 +920,7 @@ public:
                 // lock before calling process->Resume below.
                 Mutex::Locker locker (process->GetThreadList().GetMutex());
                 const uint32_t num_threads = process->GetThreadList().GetSize();
-                Thread *current_thread = process->GetThreadList().GetSelectedThread().get();
+                Thread *current_thread = GetDefaultThread();
                 if (current_thread == nullptr)
                 {
                     result.AppendError ("the process doesn't have a current thread");
@@ -1170,7 +1171,7 @@ protected:
 
             if (m_options.m_thread_idx == LLDB_INVALID_THREAD_ID)
             {
-                thread = process->GetThreadList().GetSelectedThread().get();
+                thread = GetDefaultThread();
             }
             else
             {

lldb/source/Commands/CommandObjectType.cpp

@@ -3423,8 +3423,16 @@ protected:
     bool
     DoExecute (const char *command, CommandReturnObject &result) override
     {
-        auto target_sp = m_interpreter.GetDebugger().GetSelectedTarget();
-        auto frame_sp = target_sp->GetProcessSP()->GetThreadList().GetSelectedThread()->GetSelectedFrame();
+        TargetSP target_sp = m_interpreter.GetDebugger().GetSelectedTarget();
+        Thread *thread = GetDefaultThread();
+        if (!thread)
+        {
+            result.AppendError("no default thread");
+            result.SetStatus(lldb::eReturnStatusFailed);
+            return false;
+        }
+        
+        StackFrameSP frame_sp = thread->GetSelectedFrame();
         ValueObjectSP result_valobj_sp;
         EvaluateExpressionOptions options;
         lldb::ExpressionResults expr_result = target_sp->EvaluateExpression(command, frame_sp.get(), result_valobj_sp, options);

lldb/source/DataFormatters/FormattersHelpers.cpp

@@ -140,13 +140,18 @@ lldb_private::formatters::GetViableFrame (ExecutionContext exe_ctx)
     if (frame)
         return frame;
     
+    Thread *thread = exe_ctx.GetThreadPtr();
+    if (thread)
+        return thread->GetSelectedFrame().get();
+
     Process* process = exe_ctx.GetProcessPtr();
     if (!process)
         return nullptr;
     
-    ThreadSP thread_sp(process->GetThreadList().GetSelectedThread());
-    if (thread_sp)
-        return thread_sp->GetSelectedFrame().get();
+    thread = process->GetThreadList().GetSelectedThread().get();
+    if (thread)
+        return thread->GetSelectedFrame().get();
+    
     return nullptr;
 }
 

lldb/source/Interpreter/CommandObject.cpp

@@ -1027,6 +1027,31 @@ CommandObject::GetSelectedOrDummyTarget(bool prefer_dummy)
     return m_interpreter.GetDebugger().GetSelectedOrDummyTarget(prefer_dummy);
 }
 
+Thread *
+CommandObject::GetDefaultThread()
+{
+    Thread *thread_to_use = m_exe_ctx.GetThreadPtr();
+    if (thread_to_use)
+        return thread_to_use;
+    
+    Process *process = m_exe_ctx.GetProcessPtr();
+    if (!process)
+    {
+        Target *target = m_exe_ctx.GetTargetPtr();
+        if (!target)
+        {
+            target = m_interpreter.GetDebugger().GetSelectedTarget().get();
+        }
+        if (target)
+            process = target->GetProcessSP().get();
+    }
+
+    if (process)
+        return process->GetThreadList().GetSelectedThread().get();
+    else
+        return nullptr;
+}
+
 bool
 CommandObjectParsed::Execute (const char *args_string, CommandReturnObject &result)
 {

lldb/source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp

@@ -173,7 +173,7 @@ AddressSanitizerRuntime::RetrieveReportData()
     if (!process_sp)
         return StructuredData::ObjectSP();
 
-    ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
+    ThreadSP thread_sp = process_sp->GetThreadList().GetExpressionExecutionThread();
     StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
     
     if (!frame_sp)

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

@@ -1229,7 +1229,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(RemoteNXMapTable &hash_table
     
     ExecutionContext exe_ctx;
     
-    ThreadSP thread_sp = process->GetThreadList().GetSelectedThread();
+    ThreadSP thread_sp = process->GetThreadList().GetExpressionExecutionThread();
     
     if (!thread_sp)
         return false;
@@ -1473,7 +1473,7 @@ AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache()
     
     ExecutionContext exe_ctx;
     
-    ThreadSP thread_sp = process->GetThreadList().GetSelectedThread();
+    ThreadSP thread_sp = process->GetThreadList().GetExpressionExecutionThread();
     
     if (!thread_sp)
         return DescriptorMapUpdateResult::Fail();

lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp

@@ -148,7 +148,7 @@ MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address)
     ProcessSP process_sp = m_process_wp.lock();
     if (process_sp)
     {
-        ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
+        ThreadSP thread_sp = process_sp->GetThreadList().GetExpressionExecutionThread();
 
         if (thread_sp)
         {

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

@@ -867,7 +867,7 @@ PlatformPOSIX::EvaluateLibdlExpression(lldb_private::Process* process,
             return error;
     }
 
-    ThreadSP thread_sp(process->GetThreadList().GetSelectedThread());
+    ThreadSP thread_sp(process->GetThreadList().GetExpressionExecutionThread());
     if (!thread_sp)
         return Error("Selected thread isn't valid");
 

lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp

@@ -43,7 +43,7 @@ lldb_private::InferiorCallMmap (Process *process,
                                 addr_t fd,
                                 addr_t offset)
 {
-    Thread *thread = process->GetThreadList().GetSelectedThread().get();
+    Thread *thread = process->GetThreadList().GetExpressionExecutionThread().get();
     if (thread == NULL)
         return false;
 
@@ -144,7 +144,7 @@ lldb_private::InferiorCallMunmap (Process *process,
                                   addr_t addr,
                                   addr_t length)
 {
-   Thread *thread = process->GetThreadList().GetSelectedThread().get();
+   Thread *thread = process->GetThreadList().GetExpressionExecutionThread().get();
    if (thread == NULL)
        return false;
    
@@ -219,7 +219,7 @@ lldb_private::InferiorCall (Process *process,
                             addr_t &returned_func,
                             bool trap_exceptions)
 {
-    Thread *thread = process->GetThreadList().GetSelectedThread().get();
+    Thread *thread = process->GetThreadList().GetExpressionExecutionThread().get();
     if (thread == NULL || address == NULL)
         return false;
 

lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp

@@ -486,7 +486,7 @@ SystemRuntimeMacOSX::GetExtendedBacktraceThread (ThreadSP real_thread, ConstStri
         }
         else
         {
-            ThreadSP cur_thread_sp (m_process->GetThreadList().GetSelectedThread());
+            ThreadSP cur_thread_sp (m_process->GetThreadList().GetExpressionExecutionThread());
             AppleGetThreadItemInfoHandler::GetThreadItemInfoReturnInfo ret = m_get_thread_item_info_handler.GetThreadItemInfo (*cur_thread_sp.get(), real_thread->GetID(), m_page_to_free, m_page_to_free_size, error);
             m_page_to_free = LLDB_INVALID_ADDRESS;
             m_page_to_free_size = 0;
@@ -524,7 +524,7 @@ SystemRuntimeMacOSX::GetExtendedBacktraceFromItemRef (lldb::addr_t item_ref)
     ThreadSP return_thread_sp;
 
     AppleGetItemInfoHandler::GetItemInfoReturnInfo ret;
-    ThreadSP cur_thread_sp (m_process->GetThreadList().GetSelectedThread());
+    ThreadSP cur_thread_sp (m_process->GetThreadList().GetExpressionExecutionThread());
     Error error;
     ret = m_get_item_info_handler.GetItemInfo (*cur_thread_sp.get(), item_ref, m_page_to_free, m_page_to_free_size, error);
     m_page_to_free = LLDB_INVALID_ADDRESS;
@@ -696,7 +696,7 @@ SystemRuntimeMacOSX::PopulateQueueList (lldb_private::QueueList &queue_list)
     if (BacktraceRecordingHeadersInitialized())
     {
         AppleGetQueuesHandler::GetQueuesReturnInfo queue_info_pointer;
-        ThreadSP cur_thread_sp (m_process->GetThreadList().GetSelectedThread());
+        ThreadSP cur_thread_sp (m_process->GetThreadList().GetExpressionExecutionThread());
         if (cur_thread_sp)
         { 
             Error error;
@@ -760,7 +760,7 @@ SystemRuntimeMacOSX::GetPendingItemRefsForQueue (lldb::addr_t queue)
 {
     PendingItemsForQueue pending_item_refs;
     AppleGetPendingItemsHandler::GetPendingItemsReturnInfo pending_items_pointer;
-    ThreadSP cur_thread_sp (m_process->GetThreadList().GetSelectedThread());
+    ThreadSP cur_thread_sp (m_process->GetThreadList().GetExpressionExecutionThread());
     if (cur_thread_sp)
     { 
         Error error;
@@ -859,7 +859,7 @@ SystemRuntimeMacOSX::CompleteQueueItem (QueueItem *queue_item, addr_t item_ref)
 {
     AppleGetItemInfoHandler::GetItemInfoReturnInfo ret;
 
-    ThreadSP cur_thread_sp (m_process->GetThreadList().GetSelectedThread());
+    ThreadSP cur_thread_sp (m_process->GetThreadList().GetExpressionExecutionThread());
     Error error;
     ret = m_get_item_info_handler.GetItemInfo (*cur_thread_sp.get(), item_ref, m_page_to_free, m_page_to_free_size, error);
     m_page_to_free = LLDB_INVALID_ADDRESS;

lldb/source/Target/StopInfo.cpp

@@ -371,6 +371,10 @@ protected:
                     // running all the callbacks.
 
                     m_should_stop = false;
+                    
+                    // We don't select threads as we go through them testing breakpoint conditions and running commands.
+                    // So we need to set the thread for expression evaluation here:
+                    ThreadList::ExpressionExecutionThreadPusher thread_pusher(thread_sp);
 
                     ExecutionContext exe_ctx (thread_sp->GetStackFrameAtIndex(0));
                     Process *process  = exe_ctx.GetProcessPtr();

lldb/source/Target/ThreadList.cpp

@@ -71,6 +71,30 @@ ThreadList::~ThreadList()
     Clear();
 }
 
+lldb::ThreadSP
+ThreadList::GetExpressionExecutionThread()
+{
+    if (m_expression_tid_stack.empty())
+        return GetSelectedThread();
+    ThreadSP expr_thread_sp = FindThreadByID(m_expression_tid_stack.back());
+    if (expr_thread_sp)
+        return expr_thread_sp;
+    else
+        return GetSelectedThread();
+}
+
+void
+ThreadList::PushExpressionExecutionThread(lldb::tid_t tid)
+{
+    m_expression_tid_stack.push_back(tid);
+}
+    
+void
+ThreadList::PopExpressionExecutionThread(lldb::tid_t tid)
+{
+    assert(m_expression_tid_stack.back() == tid);
+    m_expression_tid_stack.pop_back();
+}
 
 uint32_t
 ThreadList::GetStopID () const
@@ -828,3 +852,16 @@ ThreadList::GetMutex ()
     return m_process->m_thread_mutex;
 }
 
+ThreadList::ExpressionExecutionThreadPusher::ExpressionExecutionThreadPusher (lldb::ThreadSP thread_sp) :
+    m_thread_list(nullptr),
+    m_tid(LLDB_INVALID_THREAD_ID)
+{
+    if (thread_sp)
+    {
+        m_tid = thread_sp->GetID();
+        m_thread_list = &thread_sp->GetProcess()->GetThreadList();
+        m_thread_list->PushExpressionExecutionThread(m_tid);
+    }
+}
+
+