Bug 30863 - Step doesn't stop with conditional breakpoint on the next line
Differential Revisions:
https://reviews.llvm.org/D26497 (committed r290168, temporary reverted r290197)
https://reviews.llvm.org/D28945 (fix for Ubuntu tests fail)
https://reviews.llvm.org/D29909 (fix for TestCallThatThrows test fail)
llvm-svn: 295168
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index c1a518c..f68a67c 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5227,15 +5227,9 @@
do_resume = false;
handle_running_event = true;
} else {
- StopInfoSP stop_info_sp(thread_sp->GetStopInfo());
- StopReason stop_reason = eStopReasonInvalid;
- if (stop_info_sp)
- stop_reason = stop_info_sp->GetStopReason();
+ ThreadPlanSP plan = thread->GetCompletedPlan();
+ if (plan == thread_plan_sp && plan->PlanSucceeded()) {
- // FIXME: We only check if the stop reason is plan complete,
- // should we make sure that
- // it is OUR plan that is complete?
- if (stop_reason == eStopReasonPlanComplete) {
if (log)
log->PutCString("Process::RunThreadPlan(): execution "
"completed successfully.");
@@ -5246,9 +5240,11 @@
return_value = eExpressionCompleted;
} else {
+ StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
// Something restarted the target, so just wait for it to
// stop for real.
- if (stop_reason == eStopReasonBreakpoint) {
+ if (stop_info_sp &&
+ stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
if (log)
log->Printf("Process::RunThreadPlan() stopped for "
"breakpoint: %s.",
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 89081f3..185ff4d 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -269,6 +269,7 @@
if (!m_should_perform_action)
return;
m_should_perform_action = false;
+ bool internal_breakpoint = true;
ThreadSP thread_sp(m_thread_wp.lock());
@@ -495,6 +496,9 @@
if (callback_says_stop)
m_should_stop = true;
+ if (m_should_stop && !bp_loc_sp->GetBreakpoint().IsInternal())
+ internal_breakpoint = false;
+
// If we are going to stop for this breakpoint, then remove the
// breakpoint.
if (callback_says_stop && bp_loc_sp &&
@@ -526,6 +530,20 @@
"Process::%s could not find breakpoint site id: %" PRId64 "...",
__FUNCTION__, m_value);
}
+
+ if ((m_should_stop == false || internal_breakpoint)
+ && thread_sp->CompletedPlanOverridesBreakpoint()) {
+
+ // Override should_stop decision when we have
+ // completed step plan additionally to the breakpoint
+ m_should_stop = true;
+
+ // Here we clean the preset stop info so the next
+ // GetStopInfo call will find the appropriate stop info,
+ // which should be the stop info related to the completed plan
+ thread_sp->ResetStopInfo();
+ }
+
if (log)
log->Printf("Process::%s returning from action with m_should_stop: %d.",
__FUNCTION__, m_should_stop);
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 7b2772e..fb702fd 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -380,24 +380,32 @@
if (m_destroy_called)
return m_stop_info_sp;
- ThreadPlanSP plan_sp(GetCompletedPlan());
+ ThreadPlanSP completed_plan_sp(GetCompletedPlan());
ProcessSP process_sp(GetProcess());
const uint32_t stop_id = process_sp ? process_sp->GetStopID() : UINT32_MAX;
- if (plan_sp && plan_sp->PlanSucceeded()) {
- return StopInfo::CreateStopReasonWithPlan(plan_sp, GetReturnValueObject(),
- GetExpressionVariable());
+
+ // Here we select the stop info according to priorirty:
+ // - m_stop_info_sp (if not trace) - preset value
+ // - completed plan stop info - new value with plan from completed plan stack
+ // - m_stop_info_sp (trace stop reason is OK now)
+ // - ask GetPrivateStopInfo to set stop info
+
+ bool have_valid_stop_info = m_stop_info_sp &&
+ m_stop_info_sp ->IsValid() &&
+ m_stop_info_stop_id == stop_id;
+ bool have_valid_completed_plan = completed_plan_sp && completed_plan_sp->PlanSucceeded();
+ bool plan_overrides_trace =
+ have_valid_stop_info && have_valid_completed_plan
+ && (m_stop_info_sp->GetStopReason() == eStopReasonTrace);
+
+ if (have_valid_stop_info && !plan_overrides_trace) {
+ return m_stop_info_sp;
+ } else if (have_valid_completed_plan) {
+ return StopInfo::CreateStopReasonWithPlan(
+ completed_plan_sp, GetReturnValueObject(), GetExpressionVariable());
} else {
- if ((m_stop_info_stop_id == stop_id) || // Stop info is valid, just return
- // what we have (even if empty)
- (m_stop_info_sp &&
- m_stop_info_sp
- ->IsValid())) // Stop info is valid, just return what we have
- {
- return m_stop_info_sp;
- } else {
- GetPrivateStopInfo();
- return m_stop_info_sp;
- }
+ GetPrivateStopInfo();
+ return m_stop_info_sp;
}
}
@@ -459,6 +467,12 @@
// date...
}
+void Thread::ResetStopInfo() {
+ if (m_stop_info_sp) {
+ m_stop_info_sp.reset();
+ }
+}
+
void Thread::SetStopInfo(const lldb::StopInfoSP &stop_info_sp) {
m_stop_info_sp = stop_info_sp;
if (m_stop_info_sp) {
@@ -526,7 +540,8 @@
if (process_sp)
saved_state.orig_stop_id = process_sp->GetStopID();
saved_state.current_inlined_depth = GetCurrentInlinedDepth();
-
+ saved_state.m_completed_plan_stack = m_completed_plan_stack;
+
return true;
}
@@ -559,6 +574,7 @@
SetStopInfo(saved_state.stop_info_sp);
GetStackFrameList()->SetCurrentInlinedDepth(
saved_state.current_inlined_depth);
+ m_completed_plan_stack = saved_state.m_completed_plan_stack;
return true;
}
@@ -895,6 +911,9 @@
if (should_stop) {
ThreadPlan *plan_ptr = GetCurrentPlan();
+
+ // Discard the stale plans and all plans below them in the stack,
+ // plus move the completed plans to the completed plan stack
while (!PlanIsBasePlan(plan_ptr)) {
bool stale = plan_ptr->IsPlanStale();
ThreadPlan *examined_plan = plan_ptr;
@@ -905,7 +924,15 @@
log->Printf(
"Plan %s being discarded in cleanup, it says it is already done.",
examined_plan->GetName());
- DiscardThreadPlansUpToPlan(examined_plan);
+ while (GetCurrentPlan() != examined_plan) {
+ DiscardPlan();
+ }
+ if (examined_plan->IsPlanComplete()) {
+ // plan is complete but does not explain the stop (example: step to a line
+ // with breakpoint), let us move the plan to completed_plan_stack anyway
+ PopPlan();
+ } else
+ DiscardPlan();
}
}
}
@@ -1133,6 +1160,10 @@
return false;
}
+bool Thread::CompletedPlanOverridesBreakpoint() {
+ return (!m_completed_plan_stack.empty()) ;
+}
+
ThreadPlan *Thread::GetPreviousPlan(ThreadPlan *current_plan) {
if (current_plan == nullptr)
return nullptr;
diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp
index ff11ec0..c3ee0b2 100644
--- a/lldb/source/Target/ThreadPlanStepInstruction.cpp
+++ b/lldb/source/Target/ThreadPlanStepInstruction.cpp
@@ -94,6 +94,15 @@
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
StackID cur_frame_id = m_thread.GetStackFrameAtIndex(0)->GetStackID();
if (cur_frame_id == m_stack_id) {
+ // Set plan Complete when we reach next instruction
+ uint64_t pc = m_thread.GetRegisterContext()->GetPC(0);
+ uint32_t max_opcode_size = m_thread.CalculateTarget()
+ ->GetArchitecture().GetMaximumOpcodeByteSize();
+ bool next_instruction_reached = (pc > m_instruction_addr) &&
+ (pc <= m_instruction_addr + max_opcode_size);
+ if (next_instruction_reached) {
+ SetPlanComplete();
+ }
return (m_thread.GetRegisterContext()->GetPC(0) != m_instruction_addr);
} else if (cur_frame_id < m_stack_id) {
// If the current frame is younger than the start frame and we are stepping
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index 1647d85..5398b26 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -461,6 +461,16 @@
// One tricky bit here is that some stubs don't push a frame, so we should.
// check that we are in the same symbol.
if (!InRange()) {
+ // Set plan Complete when we reach next instruction just after the range
+ lldb::addr_t addr = m_thread.GetRegisterContext()->GetPC() - 1;
+ size_t num_ranges = m_address_ranges.size();
+ for (size_t i = 0; i < num_ranges; i++) {
+ bool in_range = m_address_ranges[i].ContainsLoadAddress(
+ addr, m_thread.CalculateTarget().get());
+ if (in_range) {
+ SetPlanComplete();
+ }
+ }
return true;
}
}