kastiglione created this revision.
kastiglione added reviewers: jingham, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Change `ThreadPlanStack::PopPlan` and `::DiscardPlan` to not do the following:
1. Move the last plan, leaving a moved `ThreadPlanSP` in the plans vector
2. Operate on the last plan
3. Pop the last plan off the plans vector
This leaves a period of time where the last element in the plans vector has
been moved. I am not sure what, if any, guarantees there are when doing this,
but it seems like it would/could leave a null `ThreadPlanSP` in the container.
There are asserts in place to prevent empty/null `ThreadPlanSP` instances from
being pushed on to the stack, and so this could break that invariant during
multithreaded access to the thread plan stack.
An open question is whether this use of `std::move` was the result of a measure
performance problem.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D106171
Files:
lldb/source/Target/ThreadPlanStack.cpp
Index: lldb/source/Target/ThreadPlanStack.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -142,7 +142,7 @@
lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
assert(m_plans.size() > 1 && "Can't pop the base thread plan");
- lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+ lldb::ThreadPlanSP plan_sp = m_plans.back();
m_completed_plans.push_back(plan_sp);
plan_sp->WillPop();
m_plans.pop_back();
@@ -152,7 +152,7 @@
lldb::ThreadPlanSP ThreadPlanStack::DiscardPlan() {
assert(m_plans.size() > 1 && "Can't discard the base thread plan");
- lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+ lldb::ThreadPlanSP plan_sp = m_plans.back();
m_discarded_plans.push_back(plan_sp);
plan_sp->WillPop();
m_plans.pop_back();
Index: lldb/source/Target/ThreadPlanStack.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -142,7 +142,7 @@
lldb::ThreadPlanSP ThreadPlanStack::PopPlan() {
assert(m_plans.size() > 1 && "Can't pop the base thread plan");
- lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+ lldb::ThreadPlanSP plan_sp = m_plans.back();
m_completed_plans.push_back(plan_sp);
plan_sp->WillPop();
m_plans.pop_back();
@@ -152,7 +152,7 @@
lldb::ThreadPlanSP ThreadPlanStack::DiscardPlan() {
assert(m_plans.size() > 1 && "Can't discard the base thread plan");
- lldb::ThreadPlanSP plan_sp = std::move(m_plans.back());
+ lldb::ThreadPlanSP plan_sp = m_plans.back();
m_discarded_plans.push_back(plan_sp);
plan_sp->WillPop();
m_plans.pop_back();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
