[Lldb-commits] [lldb] d594d4c - Refactor `ThreadList::WillResume()` to prepare to support reverse execution (#120817)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jan 15 13:18:29 PST 2025
Author: Robert O'Callahan
Date: 2025-01-15T13:18:25-08:00
New Revision: d594d4cef7d0ba15370435aac362fe44224c1bab
URL: https://github.com/llvm/llvm-project/commit/d594d4cef7d0ba15370435aac362fe44224c1bab
DIFF: https://github.com/llvm/llvm-project/commit/d594d4cef7d0ba15370435aac362fe44224c1bab.diff
LOG: Refactor `ThreadList::WillResume()` to prepare to support reverse execution (#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.
Added:
Modified:
lldb/include/lldb/Target/Thread.h
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadList.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 38b65b2bc58490..ef66fa11574db9 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -200,11 +200,14 @@ class Thread : public std::enable_shared_from_this<Thread>,
/// The User resume state for this thread.
lldb::StateType GetResumeState() const { return m_resume_state; }
- // This function is called on all the threads before "ShouldResume" and
- // "WillResume" in case a thread needs to change its state before the
- // ThreadList polls all the threads to figure out which ones actually will
- // get to run and how.
- void SetupForResume();
+ /// This function is called on all the threads before "ShouldResume" and
+ /// "WillResume" in case a thread needs to change its state before the
+ /// ThreadList polls all the threads to figure out which ones actually will
+ /// get to run and how.
+ ///
+ /// \return
+ /// True if we pushed a ThreadPlanStepOverBreakpoint
+ bool SetupForResume();
// Do not override this function, it is for thread plan logic only
bool ShouldResume(lldb::StateType resume_state);
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index a6130f6b925bbf..b5261310970611 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -617,7 +617,7 @@ void Thread::WillStop() {
current_plan->WillStop();
}
-void Thread::SetupForResume() {
+bool Thread::SetupForResume() {
if (GetResumeState() != eStateSuspended) {
// First check whether this thread is going to "actually" resume at all.
// For instance, if we're stepping from one level to the next of an
@@ -625,7 +625,7 @@ void Thread::SetupForResume() {
// without actually running this thread. In that case, for this thread we
// shouldn't push a step over breakpoint plan or do that work.
if (GetCurrentPlan()->IsVirtualStep())
- return;
+ return false;
// If we're at a breakpoint push the step-over breakpoint plan. Do this
// before telling the current plan it will resume, since we might change
@@ -663,11 +663,13 @@ void Thread::SetupForResume() {
step_bp_plan->SetAutoContinue(true);
}
QueueThreadPlan(step_bp_plan_sp, false);
+ return true;
}
}
}
}
}
+ return false;
}
bool Thread::ShouldResume(StateType resume_state) {
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index 1a2d7dd61c778f..6cbef330bf4888 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -518,58 +518,7 @@ bool ThreadList::WillResume() {
collection::iterator pos, end = m_threads.end();
- // See if any thread wants to run stopping others. If it does, then we won't
- // setup the other threads for resume, since they aren't going to get a
- // chance to run. This is necessary because the SetupForResume might add
- // "StopOthers" plans which would then get to be part of the who-gets-to-run
- // negotiation, but they're coming in after the fact, and the threads that
- // are already set up should take priority.
-
- bool wants_solo_run = false;
-
- for (pos = m_threads.begin(); pos != end; ++pos) {
- lldbassert((*pos)->GetCurrentPlan() &&
- "thread should not have null thread plan");
- if ((*pos)->GetResumeState() != eStateSuspended &&
- (*pos)->GetCurrentPlan()->StopOthers()) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
- continue;
- wants_solo_run = true;
- break;
- }
- }
-
- if (wants_solo_run) {
- Log *log = GetLog(LLDBLog::Step);
- if (log && log->GetVerbose())
- LLDB_LOGF(log, "Turning on notification of new threads while single "
- "stepping a thread.");
- m_process.StartNoticingNewThreads();
- } else {
- Log *log = GetLog(LLDBLog::Step);
- if (log && log->GetVerbose())
- LLDB_LOGF(log, "Turning off notification of new threads while single "
- "stepping a thread.");
- m_process.StopNoticingNewThreads();
- }
-
- // Give all the threads that are likely to run a last chance to set up their
- // state before we negotiate who is actually going to get a chance to run...
- // Don't set to resume suspended threads, and if any thread wanted to stop
- // others, only call setup on the threads that request StopOthers...
-
- for (pos = m_threads.begin(); pos != end; ++pos) {
- if ((*pos)->GetResumeState() != eStateSuspended &&
- (!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
- continue;
- (*pos)->SetupForResume();
- }
- }
-
- // Now go through the threads and see if any thread wants to run just itself.
+ // Go through the threads and see if any thread wants to run just itself.
// if so then pick one and run it.
ThreadList run_me_only_list(m_process);
@@ -582,14 +531,13 @@ bool ThreadList::WillResume() {
// There are two special kinds of thread that have priority for "StopOthers":
// a "ShouldRunBeforePublicStop thread, or the currently selected thread. If
// we find one satisfying that critereon, put it here.
- ThreadSP stop_others_thread_sp;
-
+ ThreadSP thread_to_run;
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp->GetResumeState() != eStateSuspended &&
thread_sp->GetCurrentPlan()->StopOthers()) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
+ if (thread_sp->IsOperatingSystemPluginThread() &&
+ !thread_sp->GetBackingThread())
continue;
// You can't say "stop others" and also want yourself to be suspended.
@@ -597,19 +545,76 @@ bool ThreadList::WillResume() {
run_me_only_list.AddThread(thread_sp);
if (thread_sp == GetSelectedThread())
- stop_others_thread_sp = thread_sp;
-
+ thread_to_run = thread_sp;
+
if (thread_sp->ShouldRunBeforePublicStop()) {
// This takes precedence, so if we find one of these, service it:
- stop_others_thread_sp = thread_sp;
+ thread_to_run = thread_sp;
break;
}
}
}
+ if (run_me_only_list.GetSize(false) > 0 && !thread_to_run) {
+ if (run_me_only_list.GetSize(false) == 1) {
+ thread_to_run = run_me_only_list.GetThreadAtIndex(0);
+ } else {
+ int random_thread =
+ (int)((run_me_only_list.GetSize(false) * (double)rand()) /
+ (RAND_MAX + 1.0));
+ thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
+ }
+ }
+
+ // Give all the threads that are likely to run a last chance to set up their
+ // state before we negotiate who is actually going to get a chance to run...
+ // Don't set to resume suspended threads, and if any thread wanted to stop
+ // others, only call setup on the threads that request StopOthers...
+ if (thread_to_run != nullptr) {
+ // See if any thread wants to run stopping others. If it does, then we
+ // won't setup the other threads for resume, since they aren't going to get
+ // a chance to run. This is necessary because the SetupForResume might add
+ // "StopOthers" plans which would then get to be part of the who-gets-to-run
+ // negotiation, but they're coming in after the fact, and the threads that
+ // are already set up should take priority.
+ thread_to_run->SetupForResume();
+ } else {
+ for (pos = m_threads.begin(); pos != end; ++pos) {
+ ThreadSP thread_sp(*pos);
+ if (thread_sp->GetResumeState() != eStateSuspended) {
+ if (thread_sp->IsOperatingSystemPluginThread() &&
+ !thread_sp->GetBackingThread())
+ continue;
+ if (thread_sp->SetupForResume()) {
+ // You can't say "stop others" and also want yourself to be suspended.
+ assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+ thread_to_run = thread_sp;
+ if (thread_sp->ShouldRunBeforePublicStop()) {
+ // This takes precedence, so if we find one of these, service it:
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ if (thread_to_run != nullptr) {
+ Log *log = GetLog(LLDBLog::Step);
+ if (log && log->GetVerbose())
+ LLDB_LOGF(log, "Turning on notification of new threads while single "
+ "stepping a thread.");
+ m_process.StartNoticingNewThreads();
+ } else {
+ Log *log = GetLog(LLDBLog::Step);
+ if (log && log->GetVerbose())
+ LLDB_LOGF(log, "Turning off notification of new threads while single "
+ "stepping a thread.");
+ m_process.StopNoticingNewThreads();
+ }
+
bool need_to_resume = true;
- if (run_me_only_list.GetSize(false) == 0) {
+ if (thread_to_run == nullptr) {
// Everybody runs as they wish:
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
@@ -622,19 +627,6 @@ bool ThreadList::WillResume() {
need_to_resume = false;
}
} else {
- ThreadSP thread_to_run;
-
- if (stop_others_thread_sp) {
- thread_to_run = stop_others_thread_sp;
- } else if (run_me_only_list.GetSize(false) == 1) {
- thread_to_run = run_me_only_list.GetThreadAtIndex(0);
- } else {
- int random_thread =
- (int)((run_me_only_list.GetSize(false) * (double)rand()) /
- (RAND_MAX + 1.0));
- thread_to_run = run_me_only_list.GetThreadAtIndex(random_thread);
- }
-
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp == thread_to_run) {
More information about the lldb-commits
mailing list