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

diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h
index 54ee5ac..fb6ab91 100644 (file)
--- 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 e6489b2..c5dbb19 100644 (file)
--- 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 cef99b7..857440b 100644 (file)
--- 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 fa273d6..988eb50 100644 (file)
--- 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 acbcdbb..971506b 100644 (file)
--- 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 4b0e82e..6c1b8c3 100644 (file)
--- 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 07941ea..0993cd3 100644 (file)
--- 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 5cb9edb..c66e55d 100644 (file)
--- 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 f57eb9e..430fbae 100644 (file)
--- 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 c575198..21e4680 100644 (file)
--- 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 031b000..72fc86f 100644 (file)
--- 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 ebeba8c..08a1ee7 100644 (file)
--- 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 1097ef3..37af5b8 100644 (file)
--- 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 a7b1b8e..7c24436 100644 (file)
--- 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 a34cb0f..3dc37f1 100644 (file)
--- 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);
+    }
+}
+
+