[Lldb-commits] [lldb] Refactor `ThreadList::WillResume()` to prepare to support reverse execution (PR #120817)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Dec 20 18:45:03 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Robert O'Callahan (rocallahan)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/120817.diff
3 Files Affected:
- (modified) lldb/include/lldb/Target/Thread.h (+8-5)
- (modified) lldb/source/Target/Thread.cpp (+4-2)
- (modified) lldb/source/Target/ThreadList.cpp (+68-71)
``````````diff
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..2e62e46f886f4c 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,16 +545,78 @@ 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...
+ bool wants_solo_run = run_me_only_list.GetSize(false) > 0;
+ for (pos = m_threads.begin(); pos != end; ++pos) {
+ ThreadSP thread_sp(*pos);
+ // 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.
+ if (thread_sp->GetResumeState() != eStateSuspended &&
+ (!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
+ 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);
+ run_me_only_list.AddThread(thread_sp);
+
+ if (!(thread_to_run && thread_to_run->ShouldRunBeforePublicStop())) {
+ if (thread_sp == GetSelectedThread())
+ thread_to_run = thread_sp;
+
+ if (thread_sp->ShouldRunBeforePublicStop()) {
+ // This takes precedence, so if we find one of these, service it:
+ thread_to_run = thread_sp;
+ break;
+ }
+ }
+ }
+ }
+ }
+
+ if (run_me_only_list.GetSize(false) == 0) {
+ 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) {
@@ -622,19 +632,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) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/120817
More information about the lldb-commits
mailing list