If the ObjC Step Through Trampoline plan causes a target crash, properly propagate the error back to

the controlling plans so that they don't lose control.

Also change "ThreadPlanStepThrough" to take the return StackID for its backstop breakpoint as an argument
to the constructor rather than having it try to figure it out itself, since it might get it wrong whereas
the caller always knows where it is coming from.

rdar://problem/11402287

llvm-svn: 156529

6 files changed, 83 insertions, 48 deletions

diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 9429bfdb623..03512139286 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -976,9 +976,9 @@ Thread::QueueThreadPlanForStepOut
 }
 
 ThreadPlan *
-Thread::QueueThreadPlanForStepThrough (bool abort_other_plans, bool stop_other_threads)
+Thread::QueueThreadPlanForStepThrough (StackID &return_stack_id, bool abort_other_plans, bool stop_other_threads)
 {
-    ThreadPlanSP thread_plan_sp(new ThreadPlanStepThrough (*this, stop_other_threads));
+    ThreadPlanSP thread_plan_sp(new ThreadPlanStepThrough (*this, return_stack_id, stop_other_threads));
     if (!thread_plan_sp || !thread_plan_sp->ValidatePlan (NULL))
         return NULL;
 
diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp
index f8c675bc664..8dcbe85c3a2 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -69,6 +69,7 @@ bool
 ThreadPlan::MischiefManaged ()
 {
     Mutex::Locker locker(m_plan_complete_mutex);
+    // Mark the plan is complete, but don't override the success flag.
     m_plan_complete = true;
     return true;
 }
diff --git a/lldb/source/Target/ThreadPlanCallFunction.cpp b/lldb/source/Target/ThreadPlanCallFunction.cpp
index fa89b412b3c..7ebe879f904 100644
--- a/lldb/source/Target/ThreadPlanCallFunction.cpp
+++ b/lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -228,7 +228,7 @@ ThreadPlanCallFunction::ThreadPlanCallFunction (Thread &thread,
 
 ThreadPlanCallFunction::~ThreadPlanCallFunction ()
 {
-    DoTakedown();
+    DoTakedown(true);
 }
 
 void
@@ -260,7 +260,7 @@ ThreadPlanCallFunction::ReportRegisterState (const char *message)
 }
 
 void
-ThreadPlanCallFunction::DoTakedown ()
+ThreadPlanCallFunction::DoTakedown (bool success)
 {
     LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_STEP));
     
@@ -274,21 +274,23 @@ ThreadPlanCallFunction::DoTakedown ()
     
     if (!m_takedown_done)
     {
-        ProcessSP process_sp (m_thread.GetProcess());
-        const ABI *abi = process_sp ? process_sp->GetABI().get() : NULL;
-        if (abi && m_return_type.IsValid())
+        if (success)
         {
-            const bool persistent = false;
-            m_return_valobj_sp = abi->GetReturnValueObject (m_thread, m_return_type, persistent);
+            ProcessSP process_sp (m_thread.GetProcess());
+            const ABI *abi = process_sp ? process_sp->GetABI().get() : NULL;
+            if (abi && m_return_type.IsValid())
+            {
+                const bool persistent = false;
+                m_return_valobj_sp = abi->GetReturnValueObject (m_thread, m_return_type, persistent);
+            }
         }
-
         if (log)
             log->Printf ("ThreadPlanCallFunction(%p): DoTakedown called for thread 0x%4.4llx, m_valid: %d complete: %d.\n", this, m_thread.GetID(), m_valid, IsPlanComplete());
         m_takedown_done = true;
         m_stop_address = m_thread.GetStackFrameAtIndex(0)->GetRegisterContext()->GetPC();
         m_real_stop_info_sp = GetPrivateStopReason();
         m_thread.RestoreThreadStateFromCheckpoint(m_stored_thread_state);
-        SetPlanComplete();
+        SetPlanComplete(success);
         ClearBreakpoints();
         if (log && log->GetVerbose())
             ReportRegisterState ("Restoring thread state after function call.  Restored register state:");
@@ -304,7 +306,7 @@ ThreadPlanCallFunction::DoTakedown ()
 void
 ThreadPlanCallFunction::WillPop ()
 {
-    DoTakedown();
+    DoTakedown(true);
 }
 
 void
@@ -342,7 +344,13 @@ ThreadPlanCallFunction::PlanExplainsStop ()
     
     // Check if the breakpoint is one of ours.
     
-    if (BreakpointsExplainStop())
+    StopReason stop_reason;
+    if (!m_real_stop_info_sp)
+        stop_reason = eStopReasonNone;
+    else
+        stop_reason = m_real_stop_info_sp->GetStopReason();
+
+    if (stop_reason == eStopReasonBreakpoint && BreakpointsExplainStop())
         return true;
     
     // If we don't want to discard this plan, than any stop we don't understand should be propagated up the stack.
@@ -352,7 +360,8 @@ ThreadPlanCallFunction::PlanExplainsStop ()
     // Otherwise, check the case where we stopped for an internal breakpoint, in that case, continue on.
     // If it is not an internal breakpoint, consult OkayToDiscard.
     
-    if (m_real_stop_info_sp && m_real_stop_info_sp->GetStopReason() == eStopReasonBreakpoint)
+    
+    if (stop_reason == eStopReasonBreakpoint)
     {
         ProcessSP process_sp (m_thread.CalculateProcess());
         uint64_t break_site_id = m_real_stop_info_sp->GetValue();
@@ -385,7 +394,18 @@ ThreadPlanCallFunction::PlanExplainsStop ()
         // If we want to discard the plan, then we say we explain the stop
         // but if we are going to be discarded, let whoever is above us
         // explain the stop.
-        return ((m_subplan_sp.get() != NULL) && !OkayToDiscard());
+        if (m_subplan_sp != NULL)
+        {
+            if (OkayToDiscard())
+            {
+                DoTakedown(false);
+                return true;
+            }
+            else
+                return false;
+        }
+        else
+            return false;
     }
 }
 
@@ -396,7 +416,7 @@ ThreadPlanCallFunction::ShouldStop (Event *event_ptr)
     {
         ReportRegisterState ("Function completed.  Register state was:");
         
-        DoTakedown();
+        DoTakedown(true);
         
         return true;
     }
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 0db23a114b8..4fd8254971b 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -104,7 +104,7 @@ ThreadPlanStepInRange::ShouldStop (Event *event_ptr)
         // A caveat to this is if we think the frame is older but we're actually in a trampoline.
         // I'm going to make the assumption that you wouldn't RETURN to a trampoline.  So if we are
         // in a trampoline we think the frame is older because the trampoline confused the backtracer.
-        new_plan = m_thread.QueueThreadPlanForStepThrough (false, stop_others);
+        new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
         if (new_plan == NULL)
             return true;
         else if (log)
@@ -140,7 +140,7 @@ ThreadPlanStepInRange::ShouldStop (Event *event_ptr)
     // We may have set the plan up above in the FrameIsOlder section:
     
     if (new_plan == NULL)
-        new_plan = m_thread.QueueThreadPlanForStepThrough (false, stop_others);
+        new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
     
     if (log)
     {
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 1bbbfb3cb5a..44cd14e8b00 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -95,7 +95,7 @@ ThreadPlanStepOverRange::ShouldStop (Event *event_ptr)
         // in a trampoline we think the frame is older because the trampoline confused the backtracer.
         // As below, we step through first, and then try to figure out how to get back out again.
         
-        new_plan = m_thread.QueueThreadPlanForStepThrough (false, stop_others);
+        new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
 
         if (new_plan != NULL && log)
             log->Printf("Thought I stepped out, but in fact arrived at a trampoline.");
@@ -122,7 +122,7 @@ ThreadPlanStepOverRange::ShouldStop (Event *event_ptr)
             }
             else 
             {
-                new_plan = m_thread.QueueThreadPlanForStepThrough (false, stop_others);
+                new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
                 
             }
         }
@@ -143,7 +143,7 @@ ThreadPlanStepOverRange::ShouldStop (Event *event_ptr)
             // in which case we need to get out of there.  But if we are in a stub then it's 
             // likely going to be hard to get out from here.  It is probably easiest to step into the
             // stub, and then it will be straight-forward to step out.        
-            new_plan = m_thread.QueueThreadPlanForStepThrough (false, stop_others);
+            new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
         }
     }
 
diff --git a/lldb/source/Target/ThreadPlanStepThrough.cpp b/lldb/source/Target/ThreadPlanStepThrough.cpp
index 3c4824153cb..003d199e379 100644
--- a/lldb/source/Target/ThreadPlanStepThrough.cpp
+++ b/lldb/source/Target/ThreadPlanStepThrough.cpp
@@ -32,11 +32,12 @@ using namespace lldb_private;
 // FIXME: At present only handles DYLD trampolines.
 //----------------------------------------------------------------------
 
-ThreadPlanStepThrough::ThreadPlanStepThrough (Thread &thread, bool stop_others) :
+ThreadPlanStepThrough::ThreadPlanStepThrough (Thread &thread, StackID &m_stack_id, bool stop_others) :
     ThreadPlan (ThreadPlan::eKindStepThrough, "Step through trampolines and prologues", thread, eVoteNoOpinion, eVoteNoOpinion),
     m_start_address (0),
     m_backstop_bkpt_id (LLDB_INVALID_BREAK_ID),
     m_backstop_addr(LLDB_INVALID_ADDRESS),
+    m_return_stack_id (m_stack_id),
     m_stop_others (stop_others)
 {
 
@@ -46,12 +47,11 @@ ThreadPlanStepThrough::ThreadPlanStepThrough (Thread &thread, bool stop_others)
     if (m_sub_plan_sp)
     {
         m_start_address = GetThread().GetRegisterContext()->GetPC(0);
-        m_stack_id = m_thread.GetStackFrameAtIndex(0)->GetStackID();
                 
         // We are going to return back to the concrete frame 1, we might pass by some inlined code that we're in 
         // the middle of by doing this, but it's easier than trying to figure out where the inlined code might return to.
             
-        StackFrameSP return_frame_sp (m_thread.GetFrameWithConcreteFrameIndex(1));
+        StackFrameSP return_frame_sp = m_thread.GetFrameWithStackID (m_stack_id);
         
         if (return_frame_sp)
         {
@@ -73,11 +73,7 @@ ThreadPlanStepThrough::ThreadPlanStepThrough (Thread &thread, bool stop_others)
 
 ThreadPlanStepThrough::~ThreadPlanStepThrough ()
 {
-    if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID)
-    {
-        m_thread.GetProcess()->GetTarget().RemoveBreakpointByID (m_backstop_bkpt_id);
-        m_backstop_bkpt_id = LLDB_INVALID_BREAK_ID;
-    }
+    ClearBackstopBreakpoint ();
 }
 
 void
@@ -161,6 +157,13 @@ ThreadPlanStepThrough::ShouldStop (Event *event_ptr)
     if (IsPlanComplete())
         return true;
         
+    // First, did we hit the backstop breakpoint?
+    if (HitOurBackstopBreakpoint())
+    {
+        SetPlanComplete(false);
+        return true;
+    }
+
     // If we don't have a sub-plan, then we're also done (can't see how we would ever get here
     // without a plan, but just in case.
     
@@ -170,20 +173,27 @@ ThreadPlanStepThrough::ShouldStop (Event *event_ptr)
         return true;
     }
     
-    // First, did we hit the backstop breakpoint?
-    if (HitOurBackstopBreakpoint())
-    {
-        SetPlanComplete();
-        return true;
-    }
-
-
     // If the current sub plan is not done, we don't want to stop.  Actually, we probably won't
     // ever get here in this state, since we generally won't get asked any questions if out
     // current sub-plan is not done...
     if (!m_sub_plan_sp->IsPlanComplete())
+        return false;
+    
+    // If our current sub plan failed, then let's just run to our backstop.  If we can't do that then just stop.
+    if (!m_sub_plan_sp->PlanSucceeded())
+    {
+        if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID)
+        {
+            m_sub_plan_sp.reset();
             return false;
-            
+        }
+        else
+        {
+            SetPlanComplete(false);
+            return true;
+        }
+    }
+        
     // Next see if there is a specific step through plan at our current pc (these might 
     // chain, for instance stepping through a dylib trampoline to the objc dispatch function...)
     LookForPlanToStepThroughFromCurrentPC();
@@ -208,7 +218,7 @@ ThreadPlanStepThrough::StopOthers ()
 StateType
 ThreadPlanStepThrough::GetPlanRunState ()
 {
-    return eStateStepping;
+    return eStateRunning;
 }
 
 bool
@@ -224,14 +234,21 @@ ThreadPlanStepThrough::WillStop ()
     return true;
 }
 
+void
+ThreadPlanStepThrough::ClearBackstopBreakpoint ()
+{
+    if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID)
+    {
+        m_thread.GetProcess()->GetTarget().RemoveBreakpointByID (m_backstop_bkpt_id);
+        m_backstop_bkpt_id = LLDB_INVALID_BREAK_ID;
+    }
+}
+
 bool
 ThreadPlanStepThrough::MischiefManaged ()
 {
     LogSP log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP));
 
-    // ShouldStop will call HappyToStopHere, which will set the plan to complete if
-    // we're done.  So we can just check that here.
-
     if (!IsPlanComplete())
     {
         return false;
@@ -240,12 +257,9 @@ ThreadPlanStepThrough::MischiefManaged ()
     {
         if (log)
             log->Printf("Completed step through step plan.");
+            
+        ClearBackstopBreakpoint ();
         ThreadPlan::MischiefManaged ();
-        if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID)
-        {
-            m_thread.GetProcess()->GetTarget().RemoveBreakpointByID (m_backstop_bkpt_id);
-            m_backstop_bkpt_id = LLDB_INVALID_BREAK_ID;
-        }
         return true;
     }
 }
@@ -262,7 +276,7 @@ ThreadPlanStepThrough::HitOurBackstopBreakpoint()
         {
             StackID cur_frame_zero_id = m_thread.GetStackFrameAtIndex(0)->GetStackID();
             
-            if (cur_frame_zero_id == m_stack_id)
+            if (cur_frame_zero_id == m_return_stack_id)
             {
                 LogSP log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP));
                 if (log)