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

15 files changed, 146 insertions, 22 deletions

diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h
index 54ee5ac2662..fb6ab91433f 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/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
diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h
index e6489b25e55..c5dbb199ad7 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/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:
 
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index cef99b759dd..857440b7af7 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/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();
diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp
index fa273d6b7d9..988eb5026a8 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/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
             {
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index acbcdbb69ef..971506bd9e0 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/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);
diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp
index 4b0e82e975e..6c1b8c37480 100644
--- a/lldb/source/DataFormatters/FormattersHelpers.cpp
+++ b/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;
 }
 
diff --git a/lldb/source/Interpreter/CommandObject.cpp b/lldb/source/Interpreter/CommandObject.cpp
index 07941ea1cc2..0993cd33a5a 100644
--- a/lldb/source/Interpreter/CommandObject.cpp
+++ b/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)
 {
diff --git a/lldb/source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp b/lldb/source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp
index 5cb9edb7e5f..c66e55dd436 100644
--- a/lldb/source/Plugins/InstrumentationRuntime/AddressSanitizer/AddressSanitizerRuntime.cpp
+++ b/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)
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index f57eb9e92eb..430fbaed54d 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/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();
diff --git a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp b/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
index c5751987162..21e4680e468 100644
--- a/lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
+++ b/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)
         {
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index 031b0005a91..72fc86ffbe2 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/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");
 
diff --git a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp b/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
index ebeba8c46a7..08a1ee74f2d 100644
--- a/lldb/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
+++ b/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;
 
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
index 1097ef36960..37af5b83019 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ b/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;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index a7b1b8eeeb2..7c244363ffd 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/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();
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index a34cb0fa143..3dc37f1fa0e 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/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);
+    }
+}
+
+