Skip to content

[lldb] Use ThreadPlanSP instead of raw ThreadPlan* (NFC) #149598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lldb/include/lldb/Target/Thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
///
/// \return
/// A pointer to the next executed plan.
ThreadPlan *GetCurrentPlan() const;
lldb::ThreadPlanSP GetCurrentPlan() const;

/// Unwinds the thread stack for the innermost expression plan currently
/// on the thread plan stack.
Expand Down Expand Up @@ -1311,7 +1311,7 @@ class Thread : public std::enable_shared_from_this<Thread>,

void DiscardPlan();

ThreadPlan *GetPreviousPlan(ThreadPlan *plan) const;
lldb::ThreadPlanSP GetPreviousPlan(ThreadPlan *plan) const;

virtual Unwind &GetUnwinder();

Expand Down
4 changes: 3 additions & 1 deletion lldb/include/lldb/Target/ThreadPlan.h
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,9 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
// This is mostly a formal requirement, it allows us to make the Thread's
// GetPreviousPlan protected, but only friend ThreadPlan to thread.

ThreadPlan *GetPreviousPlan() { return GetThread().GetPreviousPlan(this); }
lldb::ThreadPlanSP GetPreviousPlan() {
return GetThread().GetPreviousPlan(this);
}

// This forwards the private Thread::GetPrivateStopInfo which is generally
// what ThreadPlan's need to know.
Expand Down
2 changes: 1 addition & 1 deletion lldb/include/lldb/Target/ThreadPlanStack.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class ThreadPlanStack {

bool WasPlanDiscarded(ThreadPlan *plan) const;

ThreadPlan *GetPreviousPlan(ThreadPlan *current_plan) const;
lldb::ThreadPlanSP GetPreviousPlan(ThreadPlan *current_plan) const;

ThreadPlan *GetInnermostExpression() const;

Expand Down
73 changes: 34 additions & 39 deletions lldb/source/Target/Thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,15 +609,10 @@ std::string Thread::GetStopDescriptionRaw() {
}

void Thread::WillStop() {
ThreadPlan *current_plan = GetCurrentPlan();

// FIXME: I may decide to disallow threads with no plans. In which
// case this should go to an assert.

if (!current_plan)
return;

current_plan->WillStop();
if (ThreadPlanSP current_plan = GetCurrentPlan())
current_plan->WillStop();
}

bool Thread::SetupToStepOverBreakpointIfNeeded(RunDirection direction) {
Expand Down Expand Up @@ -652,12 +647,12 @@ bool Thread::SetupToStepOverBreakpointIfNeeded(RunDirection direction) {
// Note, don't assume there's a ThreadPlanStepOverBreakpoint, the
// target may not require anything special to step over a breakpoint.

ThreadPlan *cur_plan = GetCurrentPlan();
ThreadPlanSP cur_plan_sp = GetCurrentPlan();

bool push_step_over_bp_plan = false;
if (cur_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) {
if (cur_plan_sp->GetKind() == ThreadPlan::eKindStepOverBreakpoint) {
ThreadPlanStepOverBreakpoint *bp_plan =
(ThreadPlanStepOverBreakpoint *)cur_plan;
(ThreadPlanStepOverBreakpoint *)cur_plan_sp.get();
if (bp_plan->GetBreakpointLoadAddress() != thread_pc)
push_step_over_bp_plan = true;
} else
Expand Down Expand Up @@ -720,12 +715,12 @@ bool Thread::ShouldResume(StateType resume_state) {
// runs.

bool need_to_resume = false;
ThreadPlan *plan_ptr = GetCurrentPlan();
if (plan_ptr) {
need_to_resume = plan_ptr->WillResume(resume_state, true);
ThreadPlanSP plan_sp = GetCurrentPlan();
if (plan_sp) {
need_to_resume = plan_sp->WillResume(resume_state, true);

while ((plan_ptr = GetPreviousPlan(plan_ptr)) != nullptr) {
plan_ptr->WillResume(resume_state, false);
while ((plan_sp = GetPreviousPlan(plan_sp.get()))) {
plan_sp->WillResume(resume_state, false);
}

// If the WillResume for the plan says we are faking a resume, then it will
Expand Down Expand Up @@ -755,7 +750,7 @@ void Thread::DidResume() {
void Thread::DidStop() { SetState(eStateStopped); }

bool Thread::ShouldStop(Event *event_ptr) {
ThreadPlan *current_plan = GetCurrentPlan();
ThreadPlanSP current_plan = GetCurrentPlan();

bool should_stop = true;

Expand Down Expand Up @@ -854,34 +849,34 @@ bool Thread::ShouldStop(Event *event_ptr) {

// If the current plan doesn't explain the stop, then find one that does
// and let it handle the situation.
ThreadPlan *plan_ptr = current_plan;
while ((plan_ptr = GetPreviousPlan(plan_ptr)) != nullptr) {
if (plan_ptr->PlanExplainsStop(event_ptr)) {
LLDB_LOGF(log, "Plan %s explains stop.", plan_ptr->GetName());
ThreadPlanSP plan_sp = current_plan;
while ((plan_sp = GetPreviousPlan(plan_sp.get())) != nullptr) {
if (plan_sp->PlanExplainsStop(event_ptr)) {
LLDB_LOGF(log, "Plan %s explains stop.", plan_sp->GetName());

should_stop = plan_ptr->ShouldStop(event_ptr);
should_stop = plan_sp->ShouldStop(event_ptr);

// plan_ptr explains the stop, next check whether plan_ptr is done,
// if so, then we should take it and all the plans below it off the
// stack.

if (plan_ptr->MischiefManaged()) {
if (plan_sp->MischiefManaged()) {
// We're going to pop the plans up to and including the plan that
// explains the stop.
ThreadPlan *prev_plan_ptr = GetPreviousPlan(plan_ptr);
ThreadPlanSP prev_plan_sp = GetPreviousPlan(plan_sp.get());

do {
if (should_stop)
current_plan->WillStop();
PopPlan();
} while ((current_plan = GetCurrentPlan()) != prev_plan_ptr);
} while ((current_plan = GetCurrentPlan()) != prev_plan_sp);
// Now, if the responsible plan was not "Okay to discard" then
// we're done, otherwise we forward this to the next plan in the
// stack below.
done_processing_current_plan =
(plan_ptr->IsControllingPlan() && !plan_ptr->OkayToDiscard());
(plan_sp->IsControllingPlan() && !plan_sp->OkayToDiscard());
} else {
bool should_force_run = plan_ptr->ShouldRunBeforePublicStop();
bool should_force_run = plan_sp->ShouldRunBeforePublicStop();
if (should_force_run) {
SetShouldRunBeforePublicStop(true);
should_stop = false;
Expand Down Expand Up @@ -952,14 +947,14 @@ bool Thread::ShouldStop(Event *event_ptr) {
// original plan on the stack, This code clears stale plans off the stack.

if (should_stop) {
ThreadPlan *plan_ptr = GetCurrentPlan();
ThreadPlanSP plan_sp = GetCurrentPlan();

// Discard the stale plans and all plans below them in the stack, plus move
// the completed plans to the completed plan stack
while (!plan_ptr->IsBasePlan()) {
bool stale = plan_ptr->IsPlanStale();
ThreadPlan *examined_plan = plan_ptr;
plan_ptr = GetPreviousPlan(examined_plan);
while (!plan_sp->IsBasePlan()) {
bool stale = plan_sp->IsPlanStale();
ThreadPlanSP examined_plan = plan_sp;
plan_sp = GetPreviousPlan(examined_plan.get());

if (stale) {
LLDB_LOGF(
Expand Down Expand Up @@ -1034,16 +1029,16 @@ Vote Thread::ShouldReportStop(Event *event_ptr) {
return GetPlans().GetCompletedPlan(false)->ShouldReportStop(event_ptr);
} else {
Vote thread_vote = eVoteNoOpinion;
ThreadPlan *plan_ptr = GetCurrentPlan();
ThreadPlanSP plan_sp = GetCurrentPlan();
while (true) {
if (plan_ptr->PlanExplainsStop(event_ptr)) {
thread_vote = plan_ptr->ShouldReportStop(event_ptr);
if (plan_sp->PlanExplainsStop(event_ptr)) {
thread_vote = plan_sp->ShouldReportStop(event_ptr);
break;
}
if (plan_ptr->IsBasePlan())
if (plan_sp->IsBasePlan())
break;
else
plan_ptr = GetPreviousPlan(plan_ptr);
plan_sp = GetPreviousPlan(plan_sp.get());
}
LLDB_LOGF(log,
"Thread::ShouldReportStop() tid = 0x%4.4" PRIx64
Expand Down Expand Up @@ -1154,8 +1149,8 @@ void Thread::AutoCompleteThreadPlans(CompletionRequest &request) const {
}
}

ThreadPlan *Thread::GetCurrentPlan() const {
return GetPlans().GetCurrentPlan().get();
ThreadPlanSP Thread::GetCurrentPlan() const {
return GetPlans().GetCurrentPlan();
}

ThreadPlanSP Thread::GetCompletedPlan() const {
Expand All @@ -1182,7 +1177,7 @@ bool Thread::CompletedPlanOverridesBreakpoint() const {
return GetPlans().AnyCompletedPlans();
}

ThreadPlan *Thread::GetPreviousPlan(ThreadPlan *current_plan) const{
lldb::ThreadPlanSP Thread::GetPreviousPlan(ThreadPlan *current_plan) const {
return GetPlans().GetPreviousPlan(current_plan);
}

Expand Down
12 changes: 5 additions & 7 deletions lldb/source/Target/ThreadPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ Vote ThreadPlan::ShouldReportStop(Event *event_ptr) {
Log *log = GetLog(LLDBLog::Step);

if (m_report_stop_vote == eVoteNoOpinion) {
ThreadPlan *prev_plan = GetPreviousPlan();
if (prev_plan) {
if (ThreadPlanSP prev_plan = GetPreviousPlan()) {
Vote prev_vote = prev_plan->ShouldReportStop(event_ptr);
LLDB_LOG(log, "returning previous thread plan vote: {0}", prev_vote);
return prev_vote;
Expand All @@ -93,8 +92,7 @@ Vote ThreadPlan::ShouldReportStop(Event *event_ptr) {

Vote ThreadPlan::ShouldReportRun(Event *event_ptr) {
if (m_report_run_vote == eVoteNoOpinion) {
ThreadPlan *prev_plan = GetPreviousPlan();
if (prev_plan)
if (ThreadPlanSP prev_plan = GetPreviousPlan())
return prev_plan->ShouldReportRun(event_ptr);
}
return m_report_run_vote;
Expand All @@ -103,9 +101,9 @@ Vote ThreadPlan::ShouldReportRun(Event *event_ptr) {
void ThreadPlan::ClearThreadCache() { m_thread = nullptr; }

bool ThreadPlan::StopOthers() {
ThreadPlan *prev_plan;
prev_plan = GetPreviousPlan();
return (prev_plan == nullptr) ? false : prev_plan->StopOthers();
if (ThreadPlanSP prev_plan = GetPreviousPlan())
return prev_plan->StopOthers();
return false;
}

void ThreadPlan::SetStopOthers(bool new_value) {
Expand Down
8 changes: 4 additions & 4 deletions lldb/source/Target/ThreadPlanStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ bool ThreadPlanStack::WasPlanDiscarded(ThreadPlan *in_plan) const {
return false;
}

ThreadPlan *ThreadPlanStack::GetPreviousPlan(ThreadPlan *current_plan) const {
ThreadPlanSP ThreadPlanStack::GetPreviousPlan(ThreadPlan *current_plan) const {
llvm::sys::ScopedReader guard(m_stack_mutex);
if (current_plan == nullptr)
return nullptr;
Expand All @@ -365,20 +365,20 @@ ThreadPlan *ThreadPlanStack::GetPreviousPlan(ThreadPlan *current_plan) const {
int stack_size = m_completed_plans.size();
for (int i = stack_size - 1; i > 0; i--) {
if (current_plan == m_completed_plans[i].get())
return m_completed_plans[i - 1].get();
return m_completed_plans[i - 1];
}

// If this is the first completed plan, the previous one is the
// bottom of the regular plan stack.
if (stack_size > 0 && m_completed_plans[0].get() == current_plan) {
return GetCurrentPlanNoLock().get();
return GetCurrentPlanNoLock();
}

// Otherwise look for it in the regular plans.
stack_size = m_plans.size();
for (int i = stack_size - 1; i > 0; i--) {
if (current_plan == m_plans[i].get())
return m_plans[i - 1].get();
return m_plans[i - 1];
}
return nullptr;
}
Expand Down
Loading