Fixed the StackFrame to correctly resolve the StackID's SymbolContextScope.

Added extra logging for stepping.

Fixed an issue where cached stack frame data could be lost between runs when
the thread plans read a stack frame.

llvm-svn: 112973

9 files changed, 273 insertions, 42 deletions

diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 7297f1e0891..9609303e5cb 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -285,7 +285,7 @@ BreakpointLocation::GetDescription (Stream *s, lldb::DescriptionLevel level)
         if (level == lldb::eDescriptionLevelFull)
         {
             s->PutCString("where = ");
-            sc.DumpStopContext (s, m_owner.GetTarget().GetProcessSP().get(), m_address, true, true, false);
+            sc.DumpStopContext (s, m_owner.GetTarget().GetProcessSP().get(), m_address, false, true, false);
         }
         else
         {
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e8d596a413a..de941924633 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -951,6 +951,7 @@ ProcessGDBRemote::UpdateThreadListIfNeeded ()
     if (log && log->GetMask().IsSet(GDBR_LOG_VERBOSE))
         log->Printf ("ProcessGDBRemote::%s (pid = %i)", __FUNCTION__, GetID());
 
+    Mutex::Locker locker (m_thread_list.GetMutex ());
     const uint32_t stop_id = GetStopID();
     if (m_thread_list.GetSize(false) == 0 || stop_id != m_thread_list.GetStopID())
     {
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index b310f25e995..5237bbe63be 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -163,7 +163,7 @@ StackFrame::GetStackID()
 
     if (m_flags.IsClear (RESOLVED_FRAME_ID_SYMBOL_SCOPE))
     {
-        if (m_id.GetSymbolContextScope () == NULL)
+        if (m_id.GetSymbolContextScope ())
         {
             m_flags.Set (RESOLVED_FRAME_ID_SYMBOL_SCOPE);
         }
@@ -624,7 +624,8 @@ StackFrame::UpdateCurrentFrameFromPreviousFrame (StackFrame &prev_frame)
 void
 StackFrame::UpdatePreviousFrameFromCurrentFrame (StackFrame &curr_frame)
 {
-    assert (GetStackID() == curr_frame.GetStackID());    // TODO: remove this after some testing
+    assert (GetStackID() == curr_frame.GetStackID());        // TODO: remove this after some testing
+    m_id.SetPC (curr_frame.m_id.GetPC());       // Update the Stack ID PC value
     assert (&m_thread == &curr_frame.m_thread);
     m_frame_index = curr_frame.m_frame_index;
     m_unwind_frame_index = curr_frame.m_unwind_frame_index;
@@ -642,3 +643,14 @@ StackFrame::UpdatePreviousFrameFromCurrentFrame (StackFrame &curr_frame)
 }
     
 
+bool
+StackFrame::HasCachedData () const
+{
+    if (m_variable_list_sp.get())
+        return true;
+    if (m_variable_list_value_objects.GetSize() > 0)
+        return true;
+    if (!m_disassembly.GetString().empty())
+        return true;
+    return false;
+}
\ No newline at end of file
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 0b8b8bf3775..d2c9c594b04 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -30,9 +30,14 @@ using namespace lldb_private;
 //----------------------------------------------------------------------
 // StackFrameList constructor
 //----------------------------------------------------------------------
-StackFrameList::StackFrameList(Thread &thread, StackFrameList *prev_frames, bool show_inline_frames) :
+StackFrameList::StackFrameList
+(
+    Thread &thread, 
+    const lldb::StackFrameListSP &prev_frames_sp, 
+    bool show_inline_frames
+) :
     m_thread (thread),
-    m_prev_frames_ap (prev_frames),
+    m_prev_frames_sp (prev_frames_sp),
     m_show_inlined_frames (show_inline_frames),
     m_mutex (Mutex::eMutexTypeRecursive),
     m_frames (),
@@ -49,11 +54,11 @@ StackFrameList::~StackFrameList()
 
 
 uint32_t
-StackFrameList::GetNumFrames()
+StackFrameList::GetNumFrames (bool can_create)
 {
     Mutex::Locker locker (m_mutex);
 
-    if (m_frames.size() <= 1)
+    if (can_create && m_frames.size() <= 1)
     {
         if (m_show_inlined_frames)
         {
@@ -165,9 +170,10 @@ StackFrameList::GetNumFrames()
                     }
                 }
             }
-            StackFrameList *prev_frames = m_prev_frames_ap.get();
-            if (prev_frames)
+
+            if (m_prev_frames_sp)
             {
+                StackFrameList *prev_frames = m_prev_frames_sp.get();
                 StackFrameList *curr_frames = this;
 
 #if defined (DEBUG_STACK_FRAMES)
@@ -189,17 +195,16 @@ StackFrameList::GetNumFrames()
                     StackFrameSP prev_frame_sp (prev_frames->m_frames[prev_frame_idx]);
 
 #if defined (DEBUG_STACK_FRAMES)
-                    s.Printf("\nCurrent frame #%u ", curr_frame_idx);
+                    s.Printf("\n\nCurr frame #%u ", curr_frame_idx);
                     if (curr_frame_sp)
-                        curr_frame_sp->Dump (&s, true);
+                        curr_frame_sp->Dump (&s, true, false);
                     else
                         s.PutCString("NULL");
-                    s.Printf("\nPrevious frame #%u ", prev_frame_idx);
+                    s.Printf("\nPrev frame #%u ", prev_frame_idx);
                     if (prev_frame_sp)
-                        prev_frame_sp->Dump (&s, true);
+                        prev_frame_sp->Dump (&s, true, false);
                     else
                         s.PutCString("NULL");
-                    s.EOL();
 #endif
 
                     StackFrame *curr_frame = curr_frame_sp.get();
@@ -223,8 +228,7 @@ StackFrameList::GetNumFrames()
 #endif
                 }
                 // We are done with the old stack frame list, we can release it now
-                m_prev_frames_ap.release();
-                prev_frames = NULL;
+                m_prev_frames_sp.reset();
             }
             
 #if defined (DEBUG_STACK_FRAMES)
@@ -332,7 +336,6 @@ StackFrameList::GetFrameAtIndex (uint32_t idx)
     return frame_sp;
 }
 
-
 bool
 StackFrameList::SetFrameAtIndex (uint32_t idx, StackFrameSP &frame_sp)
 {
@@ -409,3 +412,104 @@ StackFrameList::InvalidateFrames (uint32_t start_idx)
         }
     }
 }
+
+void
+StackFrameList::Merge (std::auto_ptr<StackFrameList>& curr_ap, lldb::StackFrameListSP& prev_sp)
+{
+    Mutex::Locker curr_locker (curr_ap.get() ? curr_ap->m_mutex.GetMutex() : NULL);
+    Mutex::Locker prev_locker (prev_sp.get() ? prev_sp->m_mutex.GetMutex() : NULL);
+
+#if defined (DEBUG_STACK_FRAMES)
+    StreamFile s(stdout);
+    s.PutCString("\n\nStackFrameList::Merge():\nPrev:\n");
+    if (prev_sp.get())
+        prev_sp->Dump (&s);
+    else
+        s.PutCString ("NULL");
+    s.PutCString("\nCurr:\n");
+    if (curr_ap.get())
+        curr_ap->Dump (&s);
+    else
+        s.PutCString ("NULL");
+    s.EOL();
+#endif
+
+    if (curr_ap.get() == NULL || curr_ap->GetNumFrames (false) == 0)
+    {
+#if defined (DEBUG_STACK_FRAMES)
+        s.PutCString("No current frames, leave previous frames alone...\n");
+#endif
+        curr_ap.release();
+        return;
+    }
+
+    if (prev_sp.get() == NULL || prev_sp->GetNumFrames (false) == 0)
+    {
+#if defined (DEBUG_STACK_FRAMES)
+        s.PutCString("No previous frames, so use current frames...\n");
+#endif
+        // We either don't have any previous frames, or since we have more than
+        // one current frames it means we have all the frames and can safely
+        // replace our previous frames.
+        prev_sp.reset (curr_ap.release());
+        return;
+    }
+
+    const uint32_t num_curr_frames = curr_ap->GetNumFrames (false);
+    
+    if (num_curr_frames > 1)
+    {
+#if defined (DEBUG_STACK_FRAMES)
+        s.PutCString("We have more than one current frame, so use current frames...\n");
+#endif
+        // We have more than one current frames it means we have all the frames 
+        // and can safely replace our previous frames.
+        prev_sp.reset (curr_ap.release());
+
+#if defined (DEBUG_STACK_FRAMES)
+        s.PutCString("\nMerged:\n");
+        prev_sp->Dump (&s);
+#endif
+        return;
+    }
+
+    StackFrameSP prev_frame_zero_sp(prev_sp->GetFrameAtIndex (0));
+    StackFrameSP curr_frame_zero_sp(curr_ap->GetFrameAtIndex (0));
+    StackID curr_stack_id (curr_frame_zero_sp->GetStackID());
+    StackID prev_stack_id (prev_frame_zero_sp->GetStackID());
+
+    //const uint32_t num_prev_frames = prev_sp->GetNumFrames (false);
+
+#if defined (DEBUG_STACK_FRAMES)
+    s.Printf("\n%u previous frames with one current frame\n", num_prev_frames);
+#endif
+
+    // We have only a single current frame
+    // Our previous stack frames only had a single frame as well...
+    if (curr_stack_id == prev_stack_id)
+    {
+#if defined (DEBUG_STACK_FRAMES)
+        s.Printf("\nPrevious frame #0 is same as current frame #0, merge the cached data\n");
+#endif
+
+        curr_frame_zero_sp->UpdateCurrentFrameFromPreviousFrame (*prev_frame_zero_sp);
+//        prev_frame_zero_sp->UpdatePreviousFrameFromCurrentFrame (*curr_frame_zero_sp);
+//        prev_sp->SetFrameAtIndex (0, prev_frame_zero_sp);
+    }
+    else if (curr_stack_id < prev_stack_id)
+    {
+#if defined (DEBUG_STACK_FRAMES)
+        s.Printf("\nCurrent frame #0 has a stack ID that is less than the previous frame #0, insert current frame zero in front of previous\n");
+#endif
+        prev_sp->m_frames.insert (prev_sp->m_frames.begin(), curr_frame_zero_sp);
+    }
+    
+    curr_ap.release();
+
+#if defined (DEBUG_STACK_FRAMES)
+    s.PutCString("\nMerged:\n");
+    prev_sp->Dump (&s);
+#endif
+
+
+}
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 1c8265add06..ce9216a7466 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -293,17 +293,28 @@ Vote
 Thread::ShouldReportStop (Event* event_ptr)
 {
     StateType thread_state = GetResumeState ();
-    if (thread_state == eStateSuspended
-            || thread_state == eStateInvalid)
+    Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
+
+    if (thread_state == eStateSuspended || thread_state == eStateInvalid)
+    {
+        if (log)
+            log->Printf ("Thread::ShouldReportStop() tid = 0x%4.4x: returning vote %i (state was suspended or invalid)\n", GetID(), eVoteNoOpinion);
         return eVoteNoOpinion;
+    }
 
     if (m_completed_plan_stack.size() > 0)
     {
         // Don't use GetCompletedPlan here, since that suppresses private plans.
+        if (log)
+            log->Printf ("Thread::ShouldReportStop() tid = 0x%4.4x: returning vote  for complete stack's back plan\n", GetID());
         return m_completed_plan_stack.back()->ShouldReportStop (event_ptr);
     }
     else
+    {
+        if (log)
+            log->Printf ("Thread::ShouldReportStop() tid = 0x%4.4x: returning vote  for current plan\n", GetID());
         return GetCurrentPlan()->ShouldReportStop (event_ptr);
+    }
 }
 
 Vote
@@ -797,7 +808,7 @@ StackFrameList &
 Thread::GetStackFrameList ()
 {
     if (m_curr_frames_ap.get() == NULL)
-        m_curr_frames_ap.reset (new StackFrameList (*this, m_prev_frames_ap.release(), true));
+        m_curr_frames_ap.reset (new StackFrameList (*this, m_prev_frames_sp, true));
     return *m_curr_frames_ap;
 }
 
@@ -813,8 +824,13 @@ Thread::GetStackFrameCount()
 void
 Thread::ClearStackFrames ()
 {
-    if (m_curr_frames_ap.get())
-        m_prev_frames_ap = m_curr_frames_ap;
+    if (m_curr_frames_ap.get() && m_curr_frames_ap->GetNumFrames (false) > 1)
+        m_prev_frames_sp.reset (m_curr_frames_ap.release());
+    else
+        m_curr_frames_ap.release();
+
+//    StackFrameList::Merge (m_curr_frames_ap, m_prev_frames_sp);
+//    assert (m_curr_frames_ap.get() == NULL);
 }
 
 lldb::StackFrameSP
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index 553641daefe..1c1cf3e393a 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -10,6 +10,8 @@
 
 #include <algorithm>
 
+#include "lldb/Core/Log.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/ThreadList.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
@@ -177,24 +179,51 @@ ThreadList::ShouldStop (Event *event_ptr)
 
     // Running events should never stop, obviously...
 
+    Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
 
     bool should_stop = false;    
     m_process->UpdateThreadListIfNeeded();
 
     collection::iterator pos, end = m_threads.end();
 
+    if (log)
+        log->Printf ("%s %zu threads\n", __FUNCTION__, m_threads.size());
+
     // Run through the threads and ask whether we should stop.  Don't ask
     // suspended threads, however, it makes more sense for them to preserve their
     // state across the times the process runs but they don't get a chance to.
     for (pos = m_threads.begin(); pos != end; ++pos)
     {
         ThreadSP thread_sp(*pos);
-        if ((thread_sp->GetResumeState () != eStateSuspended) && (thread_sp->ThreadStoppedForAReason()))
+        
+        if (log)
+            log->Printf ("%s thread 0x%4.4x: pc = 0x%16.16llx ", __FUNCTION__, thread_sp->GetID (), thread_sp->GetRegisterContext()->GetPC());
+
+        if (thread_sp->GetResumeState () == eStateSuspended)
+        {
+            if (log)
+                log->Printf("ignore: thread was suspended\n", thread_sp->GetID (), thread_sp->GetRegisterContext()->GetPC());
+            continue;
+        }
+        
+        if (thread_sp->ThreadStoppedForAReason() == false)
         {
-            should_stop |=  thread_sp->ShouldStop(event_ptr);
+            if (log)
+                log->Printf("ignore: no stop reason\n", thread_sp->GetID (), thread_sp->GetRegisterContext()->GetPC());
+            continue;
+
         }
+
+        const bool thread_should_stop = thread_sp->ShouldStop(event_ptr);
+        if (log)
+            log->Printf("should_stop = %i\n", thread_sp->GetID (), thread_sp->GetRegisterContext()->GetPC(), thread_should_stop);
+        if (thread_should_stop)
+            should_stop |= true;
     }
 
+    if (log)
+        log->Printf ("%s overall should_stop = %i\n", __FUNCTION__, should_stop);
+
     if (should_stop)
     {
         for (pos = m_threads.begin(); pos != end; ++pos)
@@ -210,10 +239,17 @@ ThreadList::ShouldStop (Event *event_ptr)
 Vote
 ThreadList::ShouldReportStop (Event *event_ptr)
 {
+    Mutex::Locker locker(m_threads_mutex);
+
     Vote result = eVoteNoOpinion;
     m_process->UpdateThreadListIfNeeded();
     collection::iterator pos, end = m_threads.end();
 
+    Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
+
+    if (log)
+        log->Printf ("%s %zu threads\n", __FUNCTION__, m_threads.size());
+
     // Run through the threads and ask whether we should report this event.
     // For stopping, a YES vote wins over everything.  A NO vote wins over NO opinion.
     for (pos = m_threads.begin(); pos != end; ++pos)
@@ -221,26 +257,52 @@ ThreadList::ShouldReportStop (Event *event_ptr)
         ThreadSP thread_sp(*pos);
         if (thread_sp->ThreadStoppedForAReason() && (thread_sp->GetResumeState () != eStateSuspended))
         {
-            switch (thread_sp->ShouldReportStop (event_ptr))
+            const lldb::Vote vote = thread_sp->ShouldReportStop (event_ptr);
+            if (log)
+                log->Printf  ("%s thread 0x%4.4x: pc = 0x%16.16llx vote: %s\n", 
+                              __FUNCTION__,
+                              thread_sp->GetID (), 
+                              thread_sp->GetRegisterContext()->GetPC(),
+                              GetVoteAsCString (vote));
+            switch (vote)
             {
-                case eVoteNoOpinion:
-                    continue;
-                case eVoteYes:
-                    result = eVoteYes;
-                    break;
-                case eVoteNo:
-                    if (result == eVoteNoOpinion)
-                        result = eVoteNo;
-                    break;
+            case eVoteNoOpinion:
+                continue;
+
+            case eVoteYes:
+                result = eVoteYes;
+                break;
+
+            case eVoteNo:
+                if (result == eVoteNoOpinion)
+                {
+                    result = eVoteNo;
+                }
+                else
+                {
+                    if (log)
+                        log->Printf ("%s thread 0x%4.4x: pc = 0x%16.16llx voted %s, but lost out because result was %s\n", 
+                                     __FUNCTION__,
+                                     thread_sp->GetID (), 
+                                     thread_sp->GetRegisterContext()->GetPC(),
+                                     GetVoteAsCString (vote),
+                                     GetVoteAsCString (result));
+                }
+                break;
             }
         }
     }
+    if (log)
+        log->Printf ("%s returning %s\n", __FUNCTION__, GetVoteAsCString (result));
     return result;
 }
 
 Vote
 ThreadList::ShouldReportRun (Event *event_ptr)
 {
+
+    Mutex::Locker locker(m_threads_mutex);
+
     Vote result = eVoteNoOpinion;
     m_process->UpdateThreadListIfNeeded();
     collection::iterator pos, end = m_threads.end();
diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp
index 6d3e99caf2b..9675292def5 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -13,9 +13,10 @@
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
-#include "lldb/Target/Thread.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Core/State.h"
+#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/Thread.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -88,12 +89,21 @@ ThreadPlan::MischiefManaged ()
 Vote
 ThreadPlan::ShouldReportStop (Event *event_ptr)
 {
+    Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
+
     if (m_stop_vote == eVoteNoOpinion)
     {
         ThreadPlan *prev_plan = GetPreviousPlan ();
         if (prev_plan)
-            return prev_plan->ShouldReportStop (event_ptr);
+        {
+            Vote prev_vote = prev_plan->ShouldReportStop (event_ptr);
+            if (log)
+                log->Printf ("ThreadPlan::ShouldReportStop() returning previous thread plan vote %s\n", GetVoteAsCString (prev_vote));
+            return prev_vote;
+        }
     }
+    if (log)
+        log->Printf ("ThreadPlan::ShouldReportStop() returning vote %s\n", GetVoteAsCString (m_stop_vote));
     return m_stop_vote;
 }
 
@@ -128,8 +138,21 @@ ThreadPlan::WillResume (StateType resume_state, bool current_plan)
         Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
 
         if (log)
-            log->Printf("Thread #%u: tid = 0x%4.4x about to resume the \"%s\" plan - state: %s - stop others: %d.", 
-                        m_thread.GetIndexID(), m_thread.GetID(),  m_name.c_str(), StateAsCString(resume_state), StopOthers());
+        {
+            RegisterContext *reg_ctx = m_thread.GetRegisterContext();
+            addr_t pc = reg_ctx->GetPC();
+            addr_t sp = reg_ctx->GetSP();
+            addr_t fp = reg_ctx->GetFP();
+            log->Printf("Thread #%u: tid = 0x%4.4x (pc = 0x%8.8llx, sp = 0x%8.8llx, fp = 0x%8.8llx) about to resume the \"%s\" plan - state: %s - stop others: %d.", 
+                        m_thread.GetIndexID(), 
+                        m_thread.GetID(),  
+                        (uint64_t)pc,
+                        (uint64_t)sp,
+                        (uint64_t)fp,
+                        m_name.c_str(), 
+                        StateAsCString(resume_state), 
+                        StopOthers());
+        }
     }
     return true;
 }
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index 0885e954f40..dd215666082 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -84,10 +84,12 @@ ThreadPlanStepRange::PlanExplainsStop ()
 Vote
 ThreadPlanStepRange::ShouldReportStop (Event *event_ptr)
 {
-    if (IsPlanComplete())
-        return eVoteYes;
-    else
-        return eVoteNo;
+    Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP);
+
+    const Vote vote = IsPlanComplete() ? eVoteYes : eVoteNo;
+    if (log)
+        log->Printf ("ThreadPlanStepRange::ShouldReportStop() returning vote %i\n", eVoteYes);
+    return vote;
 }
 
 bool
diff --git a/lldb/source/lldb.cpp b/lldb/source/lldb.cpp
index fc9d0e83f6b..b6d56794eff 100644
--- a/lldb/source/lldb.cpp
+++ b/lldb/source/lldb.cpp
@@ -129,3 +129,14 @@ lldb_private::GetDefaultArchitecture ()
     static ArchSpec g_default_arch;
     return g_default_arch;
 }
+
+
+const char *
+lldb_private::GetVoteAsCString (lldb::Vote vote)
+{
+    static const char * g_vote_cstrings[] = { "no", "no opinion", "yes" };
+    if (vote >= eVoteNo && vote <= eVoteYes)
+        return g_vote_cstrings[vote-1];
+    return "invalid";
+}
+