Skip to content

Commit 45ee7b8

Browse files
rocallahanfelipepiovezan
authored andcommitted
Refactor ThreadList::WillResume() to prepare to support reverse execution (llvm#120817)
These changes are designed to not change any behavior, but to make it easy to add code to choose the direction of execution after we've identified which thread(s) to run but before we add any `ThreadPlanStepOverBreakpoint`s. And honestly I think they make the existing code a bit clearer. (cherry picked from commit d594d4c)
1 parent 8bdbd74 commit 45ee7b8

File tree

3 files changed

+77
-80
lines changed

3 files changed

+77
-80
lines changed

lldb/include/lldb/Target/Thread.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,14 @@ class Thread : public std::enable_shared_from_this<Thread>,
198198
/// The User resume state for this thread.
199199
lldb::StateType GetResumeState() const { return m_resume_state; }
200200

201-
// This function is called on all the threads before "ShouldResume" and
202-
// "WillResume" in case a thread needs to change its state before the
203-
// ThreadList polls all the threads to figure out which ones actually will
204-
// get to run and how.
205-
void SetupForResume();
201+
/// This function is called on all the threads before "ShouldResume" and
202+
/// "WillResume" in case a thread needs to change its state before the
203+
/// ThreadList polls all the threads to figure out which ones actually will
204+
/// get to run and how.
205+
///
206+
/// \return
207+
/// True if we pushed a ThreadPlanStepOverBreakpoint
208+
bool SetupForResume();
206209

207210
// Do not override this function, it is for thread plan logic only
208211
bool ShouldResume(lldb::StateType resume_state);

lldb/source/Target/Thread.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,15 +641,15 @@ void Thread::WillStop() {
641641
current_plan->WillStop();
642642
}
643643

644-
void Thread::SetupForResume() {
644+
bool Thread::SetupForResume() {
645645
if (GetResumeState() != eStateSuspended) {
646646
// First check whether this thread is going to "actually" resume at all.
647647
// For instance, if we're stepping from one level to the next of an
648648
// virtual inlined call stack, we just change the inlined call stack index
649649
// without actually running this thread. In that case, for this thread we
650650
// shouldn't push a step over breakpoint plan or do that work.
651651
if (GetCurrentPlan()->IsVirtualStep())
652-
return;
652+
return false;
653653

654654
// If we're at a breakpoint push the step-over breakpoint plan. Do this
655655
// before telling the current plan it will resume, since we might change
@@ -687,11 +687,13 @@ void Thread::SetupForResume() {
687687
step_bp_plan->SetAutoContinue(true);
688688
}
689689
QueueThreadPlan(step_bp_plan_sp, false);
690+
return true;
690691
}
691692
}
692693
}
693694
}
694695
}
696+
return false;
695697
}
696698

697699
bool Thread::ShouldResume(StateType resume_state) {

lldb/source/Target/ThreadList.cpp

Lines changed: 65 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -504,58 +504,7 @@ bool ThreadList::WillResume() {
504504

505505
collection::iterator pos, end = m_threads.end();
506506

507-
// See if any thread wants to run stopping others. If it does, then we won't
508-
// setup the other threads for resume, since they aren't going to get a
509-
// chance to run. This is necessary because the SetupForResume might add
510-
// "StopOthers" plans which would then get to be part of the who-gets-to-run
511-
// negotiation, but they're coming in after the fact, and the threads that
512-
// are already set up should take priority.
513-
514-
bool wants_solo_run = false;
515-
516-
for (pos = m_threads.begin(); pos != end; ++pos) {
517-
lldbassert((*pos)->GetCurrentPlan() &&
518-
"thread should not have null thread plan");
519-
if ((*pos)->GetResumeState() != eStateSuspended &&
520-
(*pos)->GetCurrentPlan()->StopOthers()) {
521-
if ((*pos)->IsOperatingSystemPluginThread() &&
522-
!(*pos)->GetBackingThread())
523-
continue;
524-
wants_solo_run = true;
525-
break;
526-
}
527-
}
528-
529-
if (wants_solo_run) {
530-
Log *log = GetLog(LLDBLog::Step);
531-
if (log && log->GetVerbose())
532-
LLDB_LOGF(log, "Turning on notification of new threads while single "
533-
"stepping a thread.");
534-
m_process.StartNoticingNewThreads();
535-
} else {
536-
Log *log = GetLog(LLDBLog::Step);
537-
if (log && log->GetVerbose())
538-
LLDB_LOGF(log, "Turning off notification of new threads while single "
539-
"stepping a thread.");
540-
m_process.StopNoticingNewThreads();
541-
}
542-
543-
// Give all the threads that are likely to run a last chance to set up their
544-
// state before we negotiate who is actually going to get a chance to run...
545-
// Don't set to resume suspended threads, and if any thread wanted to stop
546-
// others, only call setup on the threads that request StopOthers...
547-
548-
for (pos = m_threads.begin(); pos != end; ++pos) {
549-
if ((*pos)->GetResumeState() != eStateSuspended &&
550-
(!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
551-
if ((*pos)->IsOperatingSystemPluginThread() &&
552-
!(*pos)->GetBackingThread())
553-
continue;
554-
(*pos)->SetupForResume();
555-
}
556-
}
557-
558-
// Now go through the threads and see if any thread wants to run just itself.
507+
// Go through the threads and see if any thread wants to run just itself.
559508
// if so then pick one and run it.
560509

561510
ThreadList run_me_only_list(m_process);
@@ -568,34 +517,90 @@ bool ThreadList::WillResume() {
568517
// There are two special kinds of thread that have priority for "StopOthers":
569518
// a "ShouldRunBeforePublicStop thread, or the currently selected thread. If
570519
// we find one satisfying that critereon, put it here.
571-
ThreadSP stop_others_thread_sp;
572-
520+
ThreadSP thread_to_run;
573521
for (pos = m_threads.begin(); pos != end; ++pos) {
574522
ThreadSP thread_sp(*pos);
575523
if (thread_sp->GetResumeState() != eStateSuspended &&
576524
thread_sp->GetCurrentPlan()->StopOthers()) {
577-
if ((*pos)->IsOperatingSystemPluginThread() &&
578-
!(*pos)->GetBackingThread())
525+
if (thread_sp->IsOperatingSystemPluginThread() &&
526+
!thread_sp->GetBackingThread())
579527
continue;
580528

581529
// You can't say "stop others" and also want yourself to be suspended.
582530
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
583531
run_me_only_list.AddThread(thread_sp);
584532

585533
if (thread_sp == GetSelectedThread())
586-
stop_others_thread_sp = thread_sp;
587-
534+
thread_to_run = thread_sp;
535+
588536
if (thread_sp->ShouldRunBeforePublicStop()) {
589537
// This takes precedence, so if we find one of these, service it:
590-
stop_others_thread_sp = thread_sp;
538+
thread_to_run = thread_sp;
591539
break;
592540
}
593541
}
594542
}
595543

544+
if (run_me_only_list.GetSize(false) > 0 && !thread_to_run) {
545+
if (run_me_only_list.GetSize(false) == 1) {
546+
thread_to_run = run_me_only_list.GetThreadAtIndex(0);
547+
} else {
548+
int random_thread =
549+
(int)((run_me_only_list.GetSize(false) * (double)rand()) /
550+
(RAND_MAX + 1.0));
551+
thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
552+
}
553+
}
554+
555+
// Give all the threads that are likely to run a last chance to set up their
556+
// state before we negotiate who is actually going to get a chance to run...
557+
// Don't set to resume suspended threads, and if any thread wanted to stop
558+
// others, only call setup on the threads that request StopOthers...
559+
if (thread_to_run != nullptr) {
560+
// See if any thread wants to run stopping others. If it does, then we
561+
// won't setup the other threads for resume, since they aren't going to get
562+
// a chance to run. This is necessary because the SetupForResume might add
563+
// "StopOthers" plans which would then get to be part of the who-gets-to-run
564+
// negotiation, but they're coming in after the fact, and the threads that
565+
// are already set up should take priority.
566+
thread_to_run->SetupForResume();
567+
} else {
568+
for (pos = m_threads.begin(); pos != end; ++pos) {
569+
ThreadSP thread_sp(*pos);
570+
if (thread_sp->GetResumeState() != eStateSuspended) {
571+
if (thread_sp->IsOperatingSystemPluginThread() &&
572+
!thread_sp->GetBackingThread())
573+
continue;
574+
if (thread_sp->SetupForResume()) {
575+
// You can't say "stop others" and also want yourself to be suspended.
576+
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
577+
thread_to_run = thread_sp;
578+
if (thread_sp->ShouldRunBeforePublicStop()) {
579+
// This takes precedence, so if we find one of these, service it:
580+
break;
581+
}
582+
}
583+
}
584+
}
585+
}
586+
587+
if (thread_to_run != nullptr) {
588+
Log *log = GetLog(LLDBLog::Step);
589+
if (log && log->GetVerbose())
590+
LLDB_LOGF(log, "Turning on notification of new threads while single "
591+
"stepping a thread.");
592+
m_process.StartNoticingNewThreads();
593+
} else {
594+
Log *log = GetLog(LLDBLog::Step);
595+
if (log && log->GetVerbose())
596+
LLDB_LOGF(log, "Turning off notification of new threads while single "
597+
"stepping a thread.");
598+
m_process.StopNoticingNewThreads();
599+
}
600+
596601
bool need_to_resume = true;
597602

598-
if (run_me_only_list.GetSize(false) == 0) {
603+
if (thread_to_run == nullptr) {
599604
// Everybody runs as they wish:
600605
for (pos = m_threads.begin(); pos != end; ++pos) {
601606
ThreadSP thread_sp(*pos);
@@ -608,19 +613,6 @@ bool ThreadList::WillResume() {
608613
need_to_resume = false;
609614
}
610615
} else {
611-
ThreadSP thread_to_run;
612-
613-
if (stop_others_thread_sp) {
614-
thread_to_run = stop_others_thread_sp;
615-
} else if (run_me_only_list.GetSize(false) == 1) {
616-
thread_to_run = run_me_only_list.GetThreadAtIndex(0);
617-
} else {
618-
int random_thread =
619-
(int)((run_me_only_list.GetSize(false) * (double)rand()) /
620-
(RAND_MAX + 1.0));
621-
thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
622-
}
623-
624616
for (pos = m_threads.begin(); pos != end; ++pos) {
625617
ThreadSP thread_sp(*pos);
626618
if (thread_sp == thread_to_run) {

0 commit comments

Comments
 (0)