This commit does two things. One, it converts the return value of the QueueThreadPlanXXX
plan providers from a "ThreadPlan *" to a "lldb::ThreadPlanSP". That was needed to fix
a bug where the ThreadPlanStepInRange wasn't checking with its sub-plans to make sure they
succeed before trying to proceed further. If the sub-plan failed and as a result didn't make
any progress, you could end up retrying the same failing algorithm in an infinite loop.
<rdar://problem/14043602>
git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@186618 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/source/Target/StopInfo.cpp b/source/Target/StopInfo.cpp
index 23b0b5d..2479368 100644
--- a/source/Target/StopInfo.cpp
+++ b/source/Target/StopInfo.cpp
@@ -670,12 +670,12 @@
StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo();
assert (stored_stop_info_sp.get() == this);
- ThreadPlan *new_plan = thread_sp->QueueThreadPlanForStepSingleInstruction(false, // step-over
- false, // abort_other_plans
- true); // stop_other_threads
- new_plan->SetIsMasterPlan (true);
- new_plan->SetOkayToDiscard (false);
- new_plan->SetPrivate (true);
+ ThreadPlanSP new_plan_sp(thread_sp->QueueThreadPlanForStepSingleInstruction(false, // step-over
+ false, // abort_other_plans
+ true)); // stop_other_threads
+ new_plan_sp->SetIsMasterPlan (true);
+ new_plan_sp->SetOkayToDiscard (false);
+ new_plan_sp->SetPrivate (true);
process->GetThreadList().SetSelectedThreadByID (thread_sp->GetID());
process->Resume ();
process->WaitForProcessToStop (NULL);
diff --git a/source/Target/Thread.cpp b/source/Target/Thread.cpp
index 12ec40f..401eac2 100644
--- a/source/Target/Thread.cpp
+++ b/source/Target/Thread.cpp
@@ -1337,15 +1337,15 @@
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueFundamentalPlan (bool abort_other_plans)
{
ThreadPlanSP thread_plan_sp (new ThreadPlanBase(*this));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepSingleInstruction
(
bool step_over,
@@ -1355,10 +1355,10 @@
{
ThreadPlanSP thread_plan_sp (new ThreadPlanStepInstruction (*this, step_over, stop_other_threads, eVoteNoOpinion, eVoteNoOpinion));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepOverRange
(
bool abort_other_plans,
@@ -1371,10 +1371,10 @@
thread_plan_sp.reset (new ThreadPlanStepOverRange (*this, range, addr_context, stop_other_threads));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepInRange
(
bool abort_other_plans,
@@ -1396,19 +1396,19 @@
thread_plan_sp.reset (plan);
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepOverBreakpointPlan (bool abort_other_plans)
{
ThreadPlanSP thread_plan_sp (new ThreadPlanStepOverBreakpoint (*this));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepOut
(
bool abort_other_plans,
@@ -1431,26 +1431,26 @@
if (thread_plan_sp->ValidatePlan(NULL))
{
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
else
{
- return NULL;
+ return ThreadPlanSP();
}
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepThrough (StackID &return_stack_id, bool abort_other_plans, bool 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;
+ return ThreadPlanSP();
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForCallFunction (bool abort_other_plans,
Address& function,
lldb::addr_t arg,
@@ -1466,20 +1466,20 @@
unwind_on_error,
ignore_breakpoints));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForRunToAddress (bool abort_other_plans,
Address &target_addr,
bool stop_other_threads)
{
ThreadPlanSP thread_plan_sp (new ThreadPlanRunToAddress (*this, target_addr, stop_other_threads));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
-ThreadPlan *
+ThreadPlanSP
Thread::QueueThreadPlanForStepUntil (bool abort_other_plans,
lldb::addr_t *address_list,
size_t num_addresses,
@@ -1488,7 +1488,7 @@
{
ThreadPlanSP thread_plan_sp (new ThreadPlanStepUntil (*this, address_list, num_addresses, stop_other_threads, frame_idx));
QueueThreadPlan (thread_plan_sp, abort_other_plans);
- return thread_plan_sp.get();
+ return thread_plan_sp;
}
diff --git a/source/Target/ThreadPlanShouldStopHere.cpp b/source/Target/ThreadPlanShouldStopHere.cpp
index 71543ae..8766234 100644
--- a/source/Target/ThreadPlanShouldStopHere.cpp
+++ b/source/Target/ThreadPlanShouldStopHere.cpp
@@ -45,21 +45,21 @@
m_baton = baton;
}
-ThreadPlan *
+ThreadPlanSP
ThreadPlanShouldStopHere::InvokeShouldStopHereCallback ()
{
if (m_callback)
{
- ThreadPlan *return_plan = m_callback (m_owner, m_flags, m_baton);
+ ThreadPlanSP return_plan_sp(m_callback (m_owner, m_flags, m_baton));
Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP));
if (log)
{
lldb::addr_t current_addr = m_owner->GetThread().GetRegisterContext()->GetPC(0);
- if (return_plan)
+ if (return_plan_sp)
{
StreamString s;
- return_plan->GetDescription (&s, lldb::eDescriptionLevelFull);
+ return_plan_sp->GetDescription (&s, lldb::eDescriptionLevelFull);
log->Printf ("ShouldStopHere callback found a step out plan from 0x%" PRIx64 ": %s.", current_addr, s.GetData());
}
else
@@ -67,8 +67,8 @@
log->Printf ("ShouldStopHere callback didn't find a step out plan from: 0x%" PRIx64 ".", current_addr);
}
}
- return return_plan;
+ return return_plan_sp;
}
else
- return NULL;
+ return ThreadPlanSP();
}
diff --git a/source/Target/ThreadPlanStepInRange.cpp b/source/Target/ThreadPlanStepInRange.cpp
index b7ebfea..c1f14bd 100644
--- a/source/Target/ThreadPlanStepInRange.cpp
+++ b/source/Target/ThreadPlanStepInRange.cpp
@@ -82,7 +82,11 @@
{
s->Printf ("Stepping through range (stepping into functions): ");
DumpRanges(s);
- s->Printf ("targeting %s.", m_step_into_target.AsCString());
+ const char *step_into_target = m_step_into_target.AsCString();
+ if (step_into_target && step_into_target[0] != '\0')
+ s->Printf (" targeting %s.", m_step_into_target.AsCString());
+ else
+ s->PutChar('.');
}
}
@@ -90,7 +94,6 @@
ThreadPlanStepInRange::ShouldStop (Event *event_ptr)
{
Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_STEP));
- m_no_more_plans = false;
if (log)
{
@@ -103,13 +106,24 @@
if (IsPlanComplete())
return true;
- ThreadPlan* new_plan = NULL;
-
+ m_no_more_plans = false;
+ if (m_sub_plan_sp && m_sub_plan_sp->IsPlanComplete())
+ {
+ if (!m_sub_plan_sp->PlanSucceeded())
+ {
+ SetPlanComplete();
+ m_no_more_plans = true;
+ return true;
+ }
+ else
+ m_sub_plan_sp.reset();
+ }
+
if (m_virtual_step)
{
// If we've just completed a virtual step, all we need to do is check for a ShouldStopHere plan, and otherwise
// we're done.
- new_plan = InvokeShouldStopHereCallback();
+ m_sub_plan_sp = InvokeShouldStopHereCallback();
}
else
{
@@ -131,8 +145,8 @@
// 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 (m_stack_id, false, stop_others);
- if (new_plan == NULL)
+ m_sub_plan_sp = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
+ if (!m_sub_plan_sp)
return true;
else if (log)
{
@@ -167,26 +181,26 @@
// We may have set the plan up above in the FrameIsOlder section:
- if (new_plan == NULL)
- new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
+ if (!m_sub_plan_sp)
+ m_sub_plan_sp = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
if (log)
{
- if (new_plan != NULL)
- log->Printf ("Found a step through plan: %s", new_plan->GetName());
+ if (m_sub_plan_sp)
+ log->Printf ("Found a step through plan: %s", m_sub_plan_sp->GetName());
else
log->Printf ("No step through plan found.");
}
// If not, give the "should_stop" callback a chance to push a plan to get us out of here.
// But only do that if we actually have stepped in.
- if (!new_plan && frame_order == eFrameCompareYounger)
- new_plan = InvokeShouldStopHereCallback();
+ if (!m_sub_plan_sp && frame_order == eFrameCompareYounger)
+ m_sub_plan_sp = InvokeShouldStopHereCallback();
// If we've stepped in and we are going to stop here, check to see if we were asked to
// run past the prologue, and if so do that.
- if (new_plan == NULL && frame_order == eFrameCompareYounger && m_step_past_prologue)
+ if (!m_sub_plan_sp && frame_order == eFrameCompareYounger && m_step_past_prologue)
{
lldb::StackFrameSP curr_frame = m_thread.GetStackFrameAtIndex(0);
if (curr_frame)
@@ -217,13 +231,13 @@
if (log)
log->Printf ("Pushing past prologue ");
- new_plan = m_thread.QueueThreadPlanForRunToAddress(false, func_start_address,true);
+ m_sub_plan_sp = m_thread.QueueThreadPlanForRunToAddress(false, func_start_address,true);
}
}
}
}
- if (new_plan == NULL)
+ if (!m_sub_plan_sp)
{
m_no_more_plans = true;
SetPlanComplete();
@@ -303,7 +317,7 @@
return false;
}
-ThreadPlan *
+ThreadPlanSP
ThreadPlanStepInRange::DefaultShouldStopHereCallback (ThreadPlan *current_plan, Flags &flags, void *baton)
{
bool should_step_out = false;
@@ -375,7 +389,7 @@
0); // Frame index
}
- return NULL;
+ return ThreadPlanSP();
}
bool
diff --git a/source/Target/ThreadPlanStepOverRange.cpp b/source/Target/ThreadPlanStepOverRange.cpp
index 679ef46..7b8539c 100644
--- a/source/Target/ThreadPlanStepOverRange.cpp
+++ b/source/Target/ThreadPlanStepOverRange.cpp
@@ -87,7 +87,7 @@
else
stop_others = false;
- ThreadPlan* new_plan = NULL;
+ ThreadPlanSP new_plan_sp;
FrameComparison frame_order = CompareCurrentFrameToStartFrame();
@@ -100,9 +100,9 @@
// 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 (m_stack_id, false, stop_others);
+ new_plan_sp = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
- if (new_plan != NULL && log)
+ if (new_plan_sp && log)
log->Printf("Thought I stepped out, but in fact arrived at a trampoline.");
}
else if (frame_order == eFrameCompareYounger)
@@ -142,7 +142,7 @@
if (older_ctx_is_equivalent)
{
- new_plan = m_thread.QueueThreadPlanForStepOut (false,
+ new_plan_sp = m_thread.QueueThreadPlanForStepOut (false,
NULL,
true,
stop_others,
@@ -152,7 +152,7 @@
}
else
{
- new_plan = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
+ new_plan_sp = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
}
}
@@ -173,7 +173,7 @@
// 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 (m_stack_id, false, stop_others);
+ new_plan_sp = m_thread.QueueThreadPlanForStepThrough (m_stack_id, false, stop_others);
}
else
{
@@ -254,7 +254,7 @@
{
const bool abort_other_plans = false;
const bool stop_other_threads = false;
- new_plan = m_thread.QueueThreadPlanForRunToAddress(abort_other_plans,
+ new_plan_sp = m_thread.QueueThreadPlanForRunToAddress(abort_other_plans,
next_line_address,
stop_other_threads);
break;
@@ -273,12 +273,12 @@
// If we get to this point, we're not going to use a previously set "next branch" breakpoint, so delete it:
ClearNextBranchBreakpoint();
- if (new_plan == NULL)
+ if (!new_plan_sp)
m_no_more_plans = true;
else
m_no_more_plans = false;
- if (new_plan == NULL)
+ if (!new_plan_sp)
{
// For efficiencies sake, we know we're done here so we don't have to do this
// calculation again in MischiefManaged.