https://github.com/rocallahan updated
https://github.com/llvm/llvm-project/pull/120817
>From b021a56268c74010eae017d2d1169ab8b79978b3 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Tue, 17 Dec 2024 23:40:34 +0000
Subject: [PATCH 1/7] [lldb] Move thread-notification setup to happen later
This lets us check `run_me_only_list.GetSize()` instead of `wants_solo_run`.
---
lldb/source/Target/ThreadList.cpp | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index 1a2d7dd61c778f..b44e53e0673b47 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -540,20 +540,6 @@ bool ThreadList::WillResume() {
}
}
- 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
@@ -607,6 +593,20 @@ bool ThreadList::WillResume() {
}
}
+ 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) {
>From 5164b418ea8e606729b24d0705f70e040aed8757 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Tue, 17 Dec 2024 23:46:01 +0000
Subject: [PATCH 2/7] [lldb] Make `Thread::WillResume()` report whether it
pushed a `ThreadPlanStepOverBreakpoint`
We'll need this later to determine whether we need to account for
the newly-pushed `ThreadPlanStepOverBreakpoint`.
---
lldb/include/lldb/Target/Thread.h | 13 ++++++++-----
lldb/source/Target/Thread.cpp | 6 ++++--
2 files changed, 12 insertions(+), 7 deletions(-)
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) {
>From 0fe7e983f0c194dddb22059c4dd054dcb55d98a6 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Tue, 17 Dec 2024 23:47:54 +0000
Subject: [PATCH 3/7] [lldb] Use `thread_sp` in more places in
`ThreadList::WillResume()`
This makes the code prettier and more consistent.
---
lldb/source/Target/ThreadList.cpp | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index b44e53e0673b47..676596978cf8cf 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -528,12 +528,13 @@ bool ThreadList::WillResume() {
bool wants_solo_run = false;
for (pos = m_threads.begin(); pos != end; ++pos) {
+ ThreadSP thread_sp(*pos);
lldbassert((*pos)->GetCurrentPlan() &&
"thread should not have null thread plan");
- if ((*pos)->GetResumeState() != eStateSuspended &&
- (*pos)->GetCurrentPlan()->StopOthers()) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
+ if (thread_sp->GetResumeState() != eStateSuspended &&
+ thread_sp->GetCurrentPlan()->StopOthers()) {
+ if (thread_sp->IsOperatingSystemPluginThread() &&
+ !thread_sp->GetBackingThread())
continue;
wants_solo_run = true;
break;
@@ -546,12 +547,13 @@ bool ThreadList::WillResume() {
// others, only call setup on the threads that request StopOthers...
for (pos = m_threads.begin(); pos != end; ++pos) {
- if ((*pos)->GetResumeState() != eStateSuspended &&
+ ThreadSP thread_sp(*pos);
+ if (thread_sp->GetResumeState() != eStateSuspended &&
(!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
- if ((*pos)->IsOperatingSystemPluginThread() &&
- !(*pos)->GetBackingThread())
+ if (thread_sp->IsOperatingSystemPluginThread() &&
+ !thread_sp->GetBackingThread())
continue;
- (*pos)->SetupForResume();
+ thread_sp->SetupForResume();
}
}
@@ -574,8 +576,8 @@ bool ThreadList::WillResume() {
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.
>From 070a2561e5ee3e7b083926ac1b1ff7fb3c2483ca Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Tue, 17 Dec 2024 23:55:19 +0000
Subject: [PATCH 4/7] [lldb] Move `SetupForResume()` calls to happen after the
first pass populating `run_me_only_list`
Eventually we will need to determine the execution direction after populating
`run_me_only_list` and before calling `SetupForResume()`.
---
lldb/source/Target/ThreadList.cpp | 46 ++++++++++++++++++++-----------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index 676596978cf8cf..8edbd13c64a8b8 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -541,22 +541,6 @@ bool ThreadList::WillResume() {
}
}
- // 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) {
- ThreadSP thread_sp(*pos);
- if (thread_sp->GetResumeState() != eStateSuspended &&
- (!wants_solo_run || (*pos)->GetCurrentPlan()->StopOthers())) {
- if (thread_sp->IsOperatingSystemPluginThread() &&
- !thread_sp->GetBackingThread())
- continue;
- thread_sp->SetupForResume();
- }
- }
-
// Now go through the threads and see if any thread wants to run just itself.
// if so then pick one and run it.
@@ -595,6 +579,36 @@ bool ThreadList::WillResume() {
}
}
+ // 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) {
+ ThreadSP thread_sp(*pos);
+ 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 (!(stop_others_thread_sp &&
stop_others_thread_sp->ShouldRunBeforePublicStop())) {
+ if (thread_sp == GetSelectedThread())
+ stop_others_thread_sp = thread_sp;
+
+ if (thread_sp->ShouldRunBeforePublicStop()) {
+ // This takes precedence, so if we find one of these, service it:
+ stop_others_thread_sp = thread_sp;
+ break;
+ }
+ }
+ }
+ }
+ }
+
if (run_me_only_list.GetSize(false) == 0) {
Log *log = GetLog(LLDBLog::Step);
if (log && log->GetVerbose())
>From 8387ed904f9dc2c10b4db82a16390334a91e9a9d Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Wed, 18 Dec 2024 00:00:27 +0000
Subject: [PATCH 5/7] [lldb] Set `wants_solo_run` based on whether
`run_me_only_list` is empty
We no longer need a dedicated loop to set `wants_solo_run`.
---
lldb/source/Target/ThreadList.cpp | 32 ++++++++-----------------------
1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index 8edbd13c64a8b8..70a496c4e2603b 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -518,30 +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) {
- ThreadSP thread_sp(*pos);
- lldbassert((*pos)->GetCurrentPlan() &&
- "thread should not have null thread plan");
- if (thread_sp->GetResumeState() != eStateSuspended &&
- thread_sp->GetCurrentPlan()->StopOthers()) {
- if (thread_sp->IsOperatingSystemPluginThread() &&
- !thread_sp->GetBackingThread())
- continue;
- wants_solo_run = true;
- break;
- }
- }
-
- // 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);
@@ -583,8 +560,15 @@ bool ThreadList::WillResume() {
// 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() &&
>From b97eb65d41c6e20d046e241d161dfc46ec359154 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Wed, 18 Dec 2024 01:16:50 +0000
Subject: [PATCH 6/7] [lldb] Move `thread_to_run` selection to happen before
`SetupForResume()` is called
Later, we'll need to know which thread will run before we determine the
execution direction, which has to happen before we call `SetupForResume()`.
We fix up `thread_to_run` after calling `SetupForResume()`, if required.
---
lldb/source/Target/ThreadList.cpp | 32 +++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index 70a496c4e2603b..2c9e2a7a1d8b49 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -556,6 +556,20 @@ bool ThreadList::WillResume() {
}
}
+ ThreadSP thread_to_run;
+ if (run_me_only_list.GetSize(false) > 0) {
+ 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);
+ }
+ }
+
// 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
@@ -580,12 +594,15 @@ bool ThreadList::WillResume() {
run_me_only_list.AddThread(thread_sp);
if (!(stop_others_thread_sp &&
stop_others_thread_sp->ShouldRunBeforePublicStop())) {
- if (thread_sp == GetSelectedThread())
+ 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;
}
}
@@ -622,19 +639,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) {
>From e01ec74a8845367a402d6fc37a5a2b60ad1e0059 Mon Sep 17 00:00:00 2001
From: Robert O'Callahan <rocalla...@google.com>
Date: Wed, 18 Dec 2024 01:24:23 +0000
Subject: [PATCH 7/7] [lldb] Replace `stop_others_thread_sp` with
`thread_to_run`
We don't need `stop_others_thread_sp` to be a separate variable anymore,
we can just use `thread_to_run` from the start.
---
lldb/source/Target/ThreadList.cpp | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/lldb/source/Target/ThreadList.cpp
b/lldb/source/Target/ThreadList.cpp
index 2c9e2a7a1d8b49..b800d6e4aee6b9 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -531,8 +531,7 @@ 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 &&
@@ -546,21 +545,18 @@ 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;
}
}
}
- ThreadSP thread_to_run;
- if (run_me_only_list.GetSize(false) > 0) {
- if (stop_others_thread_sp) {
- thread_to_run = stop_others_thread_sp;
- } else if (run_me_only_list.GetSize(false) == 1) {
+ 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 =
@@ -593,15 +589,12 @@ bool ThreadList::WillResume() {
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
run_me_only_list.AddThread(thread_sp);
- if (!(stop_others_thread_sp &&
stop_others_thread_sp->ShouldRunBeforePublicStop())) {
- if (thread_sp == GetSelectedThread()) {
- stop_others_thread_sp = 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:
- stop_others_thread_sp = thread_sp;
thread_to_run = thread_sp;
break;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
