This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG41d0b20cc90f: [lldb] Avoid moving ThreadPlanSP from plans vector (authored by kastiglione).
Changed prior to commit: https://reviews.llvm.org/D106171?vs=362880&id=363345#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106171/new/ https://reviews.llvm.org/D106171 Files: lldb/include/lldb/Target/ThreadPlan.h lldb/include/lldb/Target/ThreadPlanCallFunction.h lldb/include/lldb/Target/ThreadPlanCallUserExpression.h lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h lldb/source/Target/ThreadPlan.cpp lldb/source/Target/ThreadPlanCallFunction.cpp lldb/source/Target/ThreadPlanCallUserExpression.cpp lldb/source/Target/ThreadPlanStack.cpp lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
Index: lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp =================================================================== --- lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -124,9 +124,7 @@ return true; } -void ThreadPlanStepOverBreakpoint::WillPop() { - ReenableBreakpointSite(); -} +void ThreadPlanStepOverBreakpoint::DidPop() { ReenableBreakpointSite(); } bool ThreadPlanStepOverBreakpoint::MischiefManaged() { lldb::addr_t pc_addr = GetThread().GetRegisterContext()->GetPC(); Index: lldb/source/Target/ThreadPlanStack.cpp =================================================================== --- lldb/source/Target/ThreadPlanStack.cpp +++ lldb/source/Target/ThreadPlanStack.cpp @@ -150,10 +150,13 @@ std::lock_guard<std::recursive_mutex> guard(m_stack_mutex); assert(m_plans.size() > 1 && "Can't pop the base thread plan"); - lldb::ThreadPlanSP plan_sp = std::move(m_plans.back()); - m_completed_plans.push_back(plan_sp); - plan_sp->WillPop(); + // Note that moving the top element of the vector would leave it in an + // undefined state, and break the guarantee that the stack's thread plans are + // all valid. + lldb::ThreadPlanSP plan_sp = m_plans.back(); m_plans.pop_back(); + m_completed_plans.push_back(plan_sp); + plan_sp->DidPop(); return plan_sp; } @@ -161,10 +164,13 @@ std::lock_guard<std::recursive_mutex> guard(m_stack_mutex); assert(m_plans.size() > 1 && "Can't discard the base thread plan"); - lldb::ThreadPlanSP plan_sp = std::move(m_plans.back()); - m_discarded_plans.push_back(plan_sp); - plan_sp->WillPop(); + // Note that moving the top element of the vector would leave it in an + // undefined state, and break the guarantee that the stack's thread plans are + // all valid. + lldb::ThreadPlanSP plan_sp = m_plans.back(); m_plans.pop_back(); + m_discarded_plans.push_back(plan_sp); + plan_sp->DidPop(); return plan_sp; } Index: lldb/source/Target/ThreadPlanCallUserExpression.cpp =================================================================== --- lldb/source/Target/ThreadPlanCallUserExpression.cpp +++ lldb/source/Target/ThreadPlanCallUserExpression.cpp @@ -59,8 +59,8 @@ m_user_expression_sp->WillStartExecuting(); } -void ThreadPlanCallUserExpression::WillPop() { - ThreadPlanCallFunction::WillPop(); +void ThreadPlanCallUserExpression::DidPop() { + ThreadPlanCallFunction::DidPop(); if (m_user_expression_sp) m_user_expression_sp.reset(); } Index: lldb/source/Target/ThreadPlanCallFunction.cpp =================================================================== --- lldb/source/Target/ThreadPlanCallFunction.cpp +++ lldb/source/Target/ThreadPlanCallFunction.cpp @@ -209,7 +209,7 @@ } } -void ThreadPlanCallFunction::WillPop() { DoTakedown(PlanSucceeded()); } +void ThreadPlanCallFunction::DidPop() { DoTakedown(PlanSucceeded()); } void ThreadPlanCallFunction::GetDescription(Stream *s, DescriptionLevel level) { if (level == eDescriptionLevelBrief) { Index: lldb/source/Target/ThreadPlan.cpp =================================================================== --- lldb/source/Target/ThreadPlan.cpp +++ lldb/source/Target/ThreadPlan.cpp @@ -149,7 +149,7 @@ void ThreadPlan::DidPush() {} -void ThreadPlan::WillPop() {} +void ThreadPlan::DidPop() {} bool ThreadPlan::OkayToDiscard() { return IsMasterPlan() ? m_okay_to_discard : true; Index: lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h =================================================================== --- lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h +++ lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h @@ -26,7 +26,7 @@ bool StopOthers() override; lldb::StateType GetPlanRunState() override; bool WillStop() override; - void WillPop() override; + void DidPop() override; bool MischiefManaged() override; void ThreadDestroyed() override; void SetAutoContinue(bool do_it); Index: lldb/include/lldb/Target/ThreadPlanCallUserExpression.h =================================================================== --- lldb/include/lldb/Target/ThreadPlanCallUserExpression.h +++ lldb/include/lldb/Target/ThreadPlanCallUserExpression.h @@ -32,7 +32,7 @@ void DidPush() override; - void WillPop() override; + void DidPop() override; lldb::StopInfoSP GetRealStopInfo() override; Index: lldb/include/lldb/Target/ThreadPlanCallFunction.h =================================================================== --- lldb/include/lldb/Target/ThreadPlanCallFunction.h +++ lldb/include/lldb/Target/ThreadPlanCallFunction.h @@ -68,10 +68,10 @@ // been cleaned up. lldb::addr_t GetFunctionStackPointer() { return m_function_sp; } - // Classes that derive from FunctionCaller, and implement their own WillPop + // Classes that derive from FunctionCaller, and implement their own DidPop // methods should call this so that the thread state gets restored if the // plan gets discarded. - void WillPop() override; + void DidPop() override; // If the thread plan stops mid-course, this will be the stop reason that // interrupted us. Once DoTakedown is called, this will be the real stop Index: lldb/include/lldb/Target/ThreadPlan.h =================================================================== --- lldb/include/lldb/Target/ThreadPlan.h +++ lldb/include/lldb/Target/ThreadPlan.h @@ -81,7 +81,7 @@ // // Cleaning up after your plans: // -// When the plan is moved from the plan stack its WillPop method is always +// When the plan is moved from the plan stack its DidPop method is always // called, no matter why. Once it is moved off the plan stack it is done, and // won't get a chance to run again. So you should undo anything that affects // target state in this method. But be sure to leave the plan able to @@ -413,7 +413,7 @@ virtual void DidPush(); - virtual void WillPop(); + virtual void DidPop(); ThreadPlanKind GetKind() const { return m_kind; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits