Fix stepping over the inserted breakpoint trap when the NEXT instruction

also contains a breakpoint.

<rdar://problem/18519712>

llvm-svn: 219263

2 files changed, 51 insertions, 22 deletions

diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 30e0ffb909b..f4b15f5c9d2 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -943,30 +943,30 @@ Thread::ShouldStop (Event* event_ptr)
         if (over_ride_stop)
             should_stop = false;
 
-        // One other potential problem is that we set up a master plan, then stop in before it is complete - for instance
-        // by hitting a breakpoint during a step-over - then do some step/finish/etc operations that wind up
-        // past the end point condition of the initial plan.  We don't want to strand the original plan on the stack,
-        // This code clears stale plans off the stack.
+    }
+
+    // One other potential problem is that we set up a master plan, then stop in before it is complete - for instance
+    // by hitting a breakpoint during a step-over - then do some step/finish/etc operations that wind up
+    // past the end point condition of the initial plan.  We don't want to strand the original plan on the stack,
+    // This code clears stale plans off the stack.
 
-        if (should_stop)
+    if (should_stop)
+    {
+        ThreadPlan *plan_ptr = GetCurrentPlan();
+        while (!PlanIsBasePlan(plan_ptr))
         {
-            ThreadPlan *plan_ptr = GetCurrentPlan();
-            while (!PlanIsBasePlan(plan_ptr))
-            {
-                bool stale = plan_ptr->IsPlanStale ();
-                ThreadPlan *examined_plan = plan_ptr;
-                plan_ptr = GetPreviousPlan (examined_plan);
+            bool stale = plan_ptr->IsPlanStale ();
+            ThreadPlan *examined_plan = plan_ptr;
+            plan_ptr = GetPreviousPlan (examined_plan);
 
-                if (stale)
-                {
-                    if (log)
-                        log->Printf("Plan %s being discarded in cleanup, it says it is already done.",
-                                    examined_plan->GetName());
-                    DiscardThreadPlansUpToPlan(examined_plan);
-                }
+            if (stale)
+            {
+                if (log)
+                    log->Printf("Plan %s being discarded in cleanup, it says it is already done.",
+                                examined_plan->GetName());
+                DiscardThreadPlansUpToPlan(examined_plan);
             }
         }
-
     }
 
     if (log)
diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
index dc011e54540..6f285e25fd0 100644
--- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
@@ -64,11 +64,33 @@ ThreadPlanStepOverBreakpoint::DoPlanExplainsStop (Event *event_ptr)
     StopInfoSP stop_info_sp = GetPrivateStopInfo ();
     if (stop_info_sp)
     {
+        // It's a little surprising that we stop here for a breakpoint hit.  However, when you single step ONTO a breakpoint
+        // we still want to call that a breakpoint hit, and trigger the actions, etc.  Otherwise you would see the
+        // PC at the breakpoint without having triggered the actions, then you'd continue, the PC wouldn't change,
+        // and you'd see the breakpoint hit, which would be odd.
+        // So the lower levels fake "step onto breakpoint address" and return that as a breakpoint.  So our trace
+        // step COULD appear as a breakpoint hit if the next instruction also contained a breakpoint.
         StopReason reason = stop_info_sp->GetStopReason();
-        if (reason == eStopReasonTrace || reason == eStopReasonNone)
+
+        switch (reason)
+        {
+        case eStopReasonTrace:
+        case eStopReasonNone:
             return true;
-        else
+        case eStopReasonBreakpoint:
+            // It's a little surprising that we stop here for a breakpoint hit.  However, when you single step ONTO a
+            // breakpoint we still want to call that a breakpoint hit, and trigger the actions, etc.  Otherwise you
+            // would see the PC at the breakpoint without having triggered the actions, then you'd continue, the PC
+            // wouldn't change, and you'd see the breakpoint hit, which would be odd.
+            // So the lower levels fake "step onto breakpoint address" and return that as a breakpoint hit.  So our trace
+            // step COULD appear as a breakpoint hit if the next instruction also contained a breakpoint.  We don't want
+            // to handle that, since we really don't know what to do with breakpoint hits.  But make sure we don't set
+            // ourselves to auto-continue or we'll wrench control away from the plans that can deal with this.
+            SetAutoContinue(false);
+            return false;
+        default:
             return false;
+        }
     }
     return false;
 }
@@ -76,7 +98,7 @@ ThreadPlanStepOverBreakpoint::DoPlanExplainsStop (Event *event_ptr)
 bool
 ThreadPlanStepOverBreakpoint::ShouldStop (Event *event_ptr)
 {
-    return false;
+    return !ShouldAutoContinue(event_ptr);
 }
 
 bool
@@ -163,3 +185,10 @@ ThreadPlanStepOverBreakpoint::ShouldAutoContinue (Event *event_ptr)
 {
     return m_auto_continue;
 }
+
+bool
+ThreadPlanStepOverBreakpoint::IsPlanStale()
+{
+    return m_thread.GetRegisterContext()->GetPC() != m_breakpoint_addr;
+}
+