Skip to content

Commit c8c7df7

Browse files
[lldb][NFC] Remove downstream changes related to ThreadPlan migration
The initial implementation of swift async stepping involved migrating ThreadPlans from thread to thread. With the OS plugins introduced in [1], this is no longer needed and the associated code is dead. To find the relevant diffs, files were compared with their upstream counterparts, and code mentioning the following concepts was deleted: * "synchronizing plans", * "detached plans", * "setting TIDs on a thread plan", * "activating a thread plan stack". The storage for thread plan stacks had been substantially changed downstream to allow for the concepts above. This patch restores the original storage data structures. These were the files compared: * ThreadPlanStack{.cpp, .h} * ThreadPlan{.cpp, .h} * Process{.cpp, .h} [1]: #9839
1 parent 47665e0 commit c8c7df7

File tree

6 files changed

+14
-180
lines changed

6 files changed

+14
-180
lines changed

lldb/include/lldb/Target/Process.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2380,18 +2380,6 @@ bool PruneThreadPlansForTID(lldb::tid_t tid);
23802380
/// Prune ThreadPlanStacks for all unreported threads.
23812381
void PruneThreadPlans();
23822382

2383-
void SynchronizeThreadPlans();
2384-
2385-
/// From the detached thread plan stacks, find the first stack that explains
2386-
/// the stop represented by the thread and the event.
2387-
lldb::ThreadPlanSP FindDetachedPlanExplainingStop(Thread &thread, Event *event_ptr);
2388-
2389-
/// Helper function for FindDetachedPlanExplainingStop. Exists only to be
2390-
/// marked as a C++ friend of `ThreadPlan`.
2391-
lldb::ThreadPlanSP DoesStackExplainStopNoLock(ThreadPlanStack &stack,
2392-
Thread &thread,
2393-
Event *event_ptr);
2394-
23952383
/// Find the thread plan stack associated with thread with \a tid.
23962384
///
23972385
/// \param[in] tid

lldb/include/lldb/Target/ThreadPlan.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -486,26 +486,6 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
486486
return m_takes_iteration_count;
487487
}
488488

489-
bool IsTID(lldb::tid_t tid) { return tid == m_tid; }
490-
bool HasTID() { return m_tid != LLDB_INVALID_THREAD_ID; }
491-
lldb::tid_t GetTID() { return m_tid; }
492-
493-
void SetTID(lldb::tid_t tid) {
494-
if (m_tid != tid) {
495-
m_tid = tid;
496-
ClearThreadCache();
497-
}
498-
}
499-
500-
void ClearTID() {
501-
m_tid = LLDB_INVALID_THREAD_ID;
502-
ClearThreadCache();
503-
}
504-
505-
friend lldb::ThreadPlanSP
506-
Process::DoesStackExplainStopNoLock(ThreadPlanStack &stack, Thread &thread,
507-
Event *event_ptr);
508-
509489
protected:
510490
// Constructors and Destructors
511491
ThreadPlan(ThreadPlanKind kind, const char *name, Thread &thread,

lldb/include/lldb/Target/ThreadPlanStack.h

Lines changed: 6 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,6 @@ class ThreadPlanStack {
9797
/// generated.
9898
void ClearThreadCache();
9999

100-
bool IsTID(lldb::tid_t tid) {
101-
return GetTID() == tid;
102-
}
103-
lldb::tid_t GetTID();
104-
void SetTID(lldb::tid_t tid);
105-
106100
private:
107101
lldb::ThreadPlanSP DiscardPlanNoLock();
108102
lldb::ThreadPlanSP GetCurrentPlanNoLock() const;
@@ -122,9 +116,6 @@ class ThreadPlanStack {
122116
// completed plan checkpoints.
123117
std::unordered_map<size_t, PlanStack> m_completed_plan_store;
124118
mutable llvm::sys::RWMutex m_stack_mutex;
125-
126-
// ThreadPlanStacks shouldn't be copied.
127-
ThreadPlanStack(ThreadPlanStack &rhs) = delete;
128119
};
129120

130121
class ThreadPlanStackMap {
@@ -139,35 +130,16 @@ class ThreadPlanStackMap {
139130
void AddThread(Thread &thread) {
140131
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
141132
lldb::tid_t tid = thread.GetID();
142-
// If we already have a ThreadPlanStack for this thread, use it.
143-
if (m_plans_list.find(tid) != m_plans_list.end())
144-
return;
145-
146-
m_plans_up_container.emplace_back(
147-
std::make_unique<ThreadPlanStack>(thread));
148-
m_plans_list.emplace(tid, m_plans_up_container.back().get());
133+
m_plans_list.emplace(tid, thread);
149134
}
150135

151136
bool RemoveTID(lldb::tid_t tid) {
152137
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
153138
auto result = m_plans_list.find(tid);
154139
if (result == m_plans_list.end())
155140
return false;
156-
ThreadPlanStack *removed_stack = result->second;
141+
result->second.ThreadDestroyed(nullptr);
157142
m_plans_list.erase(result);
158-
// Now find it in the stack storage:
159-
auto end = m_plans_up_container.end();
160-
auto iter = std::find_if(m_plans_up_container.begin(), end,
161-
[&] (std::unique_ptr<ThreadPlanStack> &stack) {
162-
return stack->IsTID(tid);
163-
});
164-
if (iter == end)
165-
return false;
166-
167-
// Then tell the stack its thread has been destroyed:
168-
removed_stack->ThreadDestroyed(nullptr);
169-
// And then remove it from the container so it goes away.
170-
m_plans_up_container.erase(iter);
171143
return true;
172144
}
173145

@@ -177,69 +149,22 @@ class ThreadPlanStackMap {
177149
if (result == m_plans_list.end())
178150
return nullptr;
179151
else
180-
return result->second;
152+
return &result->second;
181153
}
182154

183155
/// Clear the Thread* cache that each ThreadPlan contains.
184156
///
185157
/// This is useful in situations like when a new Thread list is being
186158
/// generated.
187159
void ClearThreadCache() {
188-
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
189160
for (auto &plan_list : m_plans_list)
190-
plan_list.second->ClearThreadCache();
191-
}
192-
193-
// rename to Reactivate?
194-
void Activate(ThreadPlanStack &stack) {
195-
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
196-
// Remove this from the detached plan list:
197-
auto end = m_detached_plans.end();
198-
auto iter = std::find_if(m_detached_plans.begin(), end,
199-
[&] (ThreadPlanStack *elem) {
200-
return elem == &stack; });
201-
if (iter != end)
202-
m_detached_plans.erase(iter);
203-
204-
if (m_plans_list.find(stack.GetTID()) == m_plans_list.end())
205-
m_plans_list.emplace(stack.GetTID(), &stack);
206-
else
207-
m_plans_list.at(stack.GetTID()) = &stack;
208-
}
209-
210-
void ScanForDetachedPlanStacks() {
211-
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
212-
llvm::SmallVector<lldb::tid_t, 2> invalidated_tids;
213-
for (auto &pair : m_plans_list)
214-
if (pair.second->GetTID() != pair.first)
215-
invalidated_tids.push_back(pair.first);
216-
217-
for (auto tid : invalidated_tids) {
218-
auto it = m_plans_list.find(tid);
219-
ThreadPlanStack *stack = it->second;
220-
m_plans_list.erase(it);
221-
m_detached_plans.push_back(stack);
222-
}
223-
}
224-
225-
// This gets the vector of pointers to thread plans that aren't
226-
// currently running on a thread. This is generally for thread
227-
// plans that represent asynchronous operations waiting to be
228-
// scheduled.
229-
// The vector will never have null ThreadPlanStacks in it.
230-
lldb::ThreadPlanSP FindThreadPlanInStack(
231-
llvm::function_ref<lldb::ThreadPlanSP(ThreadPlanStack &)> fn) {
232-
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
233-
for (auto *stack : m_detached_plans)
234-
if (auto plan = fn(*stack))
235-
return plan;
236-
return {};
161+
plan_list.second.ClearThreadCache();
237162
}
238163

239164
void Clear() {
240165
std::lock_guard<std::recursive_mutex> guard(m_stack_map_mutex);
241166
for (auto &plan : m_plans_list)
242-
plan.second->ThreadDestroyed(nullptr);
167+
plan.second.ThreadDestroyed(nullptr);
243168
m_plans_list.clear();
244169
}
245170

@@ -256,23 +181,8 @@ class ThreadPlanStackMap {
256181

257182
private:
258183
Process &m_process;
259-
// We don't want to make copies of these ThreadPlanStacks, there needs to be
260-
// just one of these tracking each piece of work. But we need to move the
261-
// work from "attached to a TID" state to "detached" state, which is most
262-
// conveniently done by having organizing containers for each of the two
263-
// states.
264-
// To make it easy to move these non-copyable entities in and out of the
265-
// organizing containers, we make the ThreadPlanStacks into unique_ptr's in a
266-
// storage container - m_plans_up_container. Storing unique_ptrs means we
267-
// can then use the pointer to the ThreadPlanStack in the "organizing"
268-
// containers, the TID->Stack map m_plans_list, and the detached plans
269-
// vector m_detached_plans.
270-
271-
using PlansStore = std::vector<std::unique_ptr<ThreadPlanStack>>;
272-
PlansStore m_plans_up_container;
273-
std::vector<ThreadPlanStack *> m_detached_plans;
274184
mutable std::recursive_mutex m_stack_map_mutex;
275-
using PlansList = std::unordered_map<lldb::tid_t, ThreadPlanStack *>;
185+
using PlansList = std::unordered_map<lldb::tid_t, ThreadPlanStack>;
276186
PlansList m_plans_list;
277187

278188
};

lldb/source/Target/Process.cpp

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,34 +1373,6 @@ void Process::UpdateThreadListIfNeeded() {
13731373
}
13741374
}
13751375

1376-
void Process::SynchronizeThreadPlans() {
1377-
m_thread_plans.ScanForDetachedPlanStacks();
1378-
}
1379-
1380-
ThreadPlanSP Process::FindDetachedPlanExplainingStop(Thread &thread,
1381-
Event *event_ptr) {
1382-
return m_thread_plans.FindThreadPlanInStack(
1383-
[&](ThreadPlanStack &stack) -> ThreadPlanSP {
1384-
return DoesStackExplainStopNoLock(stack, thread, event_ptr);
1385-
});
1386-
}
1387-
1388-
// This extracted function only exists so that it can be marked a friend of
1389-
// `ThreadPlan`, which is needed to call `DoPlanExplainsStop`.
1390-
ThreadPlanSP Process::DoesStackExplainStopNoLock(ThreadPlanStack &stack,
1391-
Thread &thread,
1392-
Event *event_ptr) {
1393-
ThreadPlanSP plan_sp = stack.GetCurrentPlan();
1394-
plan_sp->SetTID(thread.GetID());
1395-
if (plan_sp->DoPlanExplainsStop(event_ptr)) {
1396-
stack.SetTID(thread.GetID());
1397-
m_thread_plans.Activate(stack);
1398-
return plan_sp;
1399-
}
1400-
plan_sp->ClearTID();
1401-
return {};
1402-
}
1403-
14041376
ThreadPlanStack *Process::FindThreadPlans(lldb::tid_t tid) {
14051377
return m_thread_plans.Find(tid);
14061378
}
@@ -3850,10 +3822,8 @@ bool Process::ShouldBroadcastEvent(Event *event_ptr) {
38503822
// restarted... Asking the thread list is also not likely to go well,
38513823
// since we are running again. So in that case just report the event.
38523824

3853-
if (!was_restarted) {
3825+
if (!was_restarted)
38543826
should_resume = !m_thread_list.ShouldStop(event_ptr);
3855-
SynchronizeThreadPlans();
3856-
}
38573827

38583828
if (was_restarted || should_resume || m_resume_requested) {
38593829
Vote report_stop_vote = m_thread_list.ShouldReportStop(event_ptr);

lldb/source/Target/Thread.cpp

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -765,6 +765,7 @@ void Thread::DidResume() {
765765
void Thread::DidStop() { SetState(eStateStopped); }
766766

767767
bool Thread::ShouldStop(Event *event_ptr) {
768+
ThreadPlan *current_plan = GetCurrentPlan();
768769
bool should_stop = true;
769770

770771
Log *log = GetLog(LLDBLog::Step);
@@ -818,6 +819,9 @@ bool Thread::ShouldStop(Event *event_ptr) {
818819
LLDB_LOGF(log, "Plan stack initial state:\n%s", s.GetData());
819820
}
820821

822+
// The top most plan always gets to do the trace log...
823+
current_plan->DoTraceLog();
824+
821825
// First query the stop info's ShouldStopSynchronous. This handles
822826
// "synchronous" stop reasons, for example the breakpoint command on internal
823827
// breakpoints. If a synchronous stop reason says we should not stop, then
@@ -830,16 +834,6 @@ bool Thread::ShouldStop(Event *event_ptr) {
830834
return false;
831835
}
832836

833-
// Call this after ShouldStopSynchronous.
834-
ThreadPlan *current_plan;
835-
if (auto plan = GetProcess()->FindDetachedPlanExplainingStop(*this, event_ptr))
836-
current_plan = plan.get();
837-
else
838-
current_plan = GetCurrentPlan();
839-
840-
// The top most plan always gets to do the trace log…
841-
current_plan->DoTraceLog();
842-
843837
// If we've already been restarted, don't query the plans since the state
844838
// they would examine is not current.
845839
if (Process::ProcessEventData::GetRestartedFromEvent(event_ptr))
@@ -856,7 +850,6 @@ bool Thread::ShouldStop(Event *event_ptr) {
856850
// decide whether they still need to do more work.
857851

858852
bool done_processing_current_plan = false;
859-
860853
if (!current_plan->PlanExplainsStop(event_ptr)) {
861854
if (current_plan->TracerExplainsStop()) {
862855
done_processing_current_plan = true;

lldb/source/Target/ThreadPlanStack.cpp

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -409,13 +409,6 @@ void ThreadPlanStack::WillResume() {
409409
m_discarded_plans.clear();
410410
}
411411

412-
lldb::tid_t ThreadPlanStack::GetTID() { return GetCurrentPlan()->GetTID(); }
413-
414-
void ThreadPlanStack::SetTID(lldb::tid_t tid) {
415-
for (auto plan_sp : m_plans)
416-
plan_sp->SetTID(tid);
417-
}
418-
419412
void ThreadPlanStackMap::Update(ThreadList &current_threads,
420413
bool delete_missing,
421414
bool check_for_new) {
@@ -469,8 +462,8 @@ void ThreadPlanStackMap::DumpPlans(Stream &strm,
469462
index_id = thread_sp->GetIndexID();
470463

471464
if (condense_if_trivial) {
472-
if (!elem.second->AnyPlans() && !elem.second->AnyCompletedPlans() &&
473-
!elem.second->AnyDiscardedPlans()) {
465+
if (!elem.second.AnyPlans() && !elem.second.AnyCompletedPlans() &&
466+
!elem.second.AnyDiscardedPlans()) {
474467
strm.Printf("thread #%u: tid = 0x%4.4" PRIx64 "\n", index_id, tid);
475468
strm.IndentMore();
476469
strm.Indent();
@@ -483,7 +476,7 @@ void ThreadPlanStackMap::DumpPlans(Stream &strm,
483476
strm.Indent();
484477
strm.Printf("thread #%u: tid = 0x%4.4" PRIx64 ":\n", index_id, tid);
485478

486-
elem.second->DumpThreadPlans(strm, desc_level, internal);
479+
elem.second.DumpThreadPlans(strm, desc_level, internal);
487480
}
488481
}
489482

0 commit comments

Comments
 (0)