Skip to content

Commit 12239d2

Browse files
authored
[lldb] Small cleanup of ProcessEventData::ShouldStop (#98154)
While looking at a TSAN report (patch coming soon) in the ThreadList class, I noticed that this code would be simpler if it did not use the ThreadList class.
1 parent 2080af5 commit 12239d2

File tree

1 file changed

+11
-21
lines changed

1 file changed

+11
-21
lines changed

lldb/source/Target/Process.cpp

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4152,7 +4152,6 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
41524152

41534153
ThreadList &curr_thread_list = process_sp->GetThreadList();
41544154
uint32_t num_threads = curr_thread_list.GetSize();
4155-
uint32_t idx;
41564155

41574156
// The actions might change one of the thread's stop_info's opinions about
41584157
// whether we should stop the process, so we need to query that as we go.
@@ -4162,23 +4161,18 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
41624161
// get that wrong (which is possible) then the thread list might have
41634162
// changed, and that would cause our iteration here to crash. We could
41644163
// make a copy of the thread list, but we'd really like to also know if it
4165-
// has changed at all, so we make up a vector of the thread ID's and check
4166-
// what we get back against this list & bag out if anything differs.
4167-
ThreadList not_suspended_thread_list(process_sp.get());
4168-
std::vector<uint32_t> thread_index_array(num_threads);
4169-
uint32_t not_suspended_idx = 0;
4170-
for (idx = 0; idx < num_threads; ++idx) {
4164+
// has changed at all, so we store the original thread ID's of all threads and
4165+
// check what we get back against this list & bag out if anything differs.
4166+
std::vector<std::pair<ThreadSP, size_t>> not_suspended_threads;
4167+
for (uint32_t idx = 0; idx < num_threads; ++idx) {
41714168
lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
41724169

41734170
/*
41744171
Filter out all suspended threads, they could not be the reason
41754172
of stop and no need to perform any actions on them.
41764173
*/
4177-
if (thread_sp->GetResumeState() != eStateSuspended) {
4178-
not_suspended_thread_list.AddThread(thread_sp);
4179-
thread_index_array[not_suspended_idx] = thread_sp->GetIndexID();
4180-
not_suspended_idx++;
4181-
}
4174+
if (thread_sp->GetResumeState() != eStateSuspended)
4175+
not_suspended_threads.emplace_back(thread_sp, thread_sp->GetIndexID());
41824176
}
41834177

41844178
// Use this to track whether we should continue from here. We will only
@@ -4194,8 +4188,7 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
41944188
// is, and it's better to let the user decide than continue behind their
41954189
// backs.
41964190

4197-
for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) {
4198-
curr_thread_list = process_sp->GetThreadList();
4191+
for (auto [thread_sp, thread_index] : not_suspended_threads) {
41994192
if (curr_thread_list.GetSize() != num_threads) {
42004193
Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
42014194
LLDB_LOGF(
@@ -4205,14 +4198,11 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
42054198
break;
42064199
}
42074200

4208-
lldb::ThreadSP thread_sp = not_suspended_thread_list.GetThreadAtIndex(idx);
4209-
4210-
if (thread_sp->GetIndexID() != thread_index_array[idx]) {
4201+
if (thread_sp->GetIndexID() != thread_index) {
42114202
Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
4212-
LLDB_LOGF(log,
4213-
"The thread at position %u changed from %u to %u while "
4214-
"processing event.",
4215-
idx, thread_index_array[idx], thread_sp->GetIndexID());
4203+
LLDB_LOG(log,
4204+
"The thread {0} changed from {1} to {2} while processing event.",
4205+
thread_sp.get(), thread_index, thread_sp->GetIndexID());
42164206
break;
42174207
}
42184208

0 commit comments

Comments
 (0)