Skip to content

[lldb] Small cleanup of ProcessEventData::ShouldStop #98154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 10, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 11 additions & 21 deletions lldb/source/Target/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4152,7 +4152,6 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,

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

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

/*
Filter out all suspended threads, they could not be the reason
of stop and no need to perform any actions on them.
*/
if (thread_sp->GetResumeState() != eStateSuspended) {
not_suspended_thread_list.AddThread(thread_sp);
thread_index_array[not_suspended_idx] = thread_sp->GetIndexID();
not_suspended_idx++;
}
if (thread_sp->GetResumeState() != eStateSuspended)
not_suspended_threads.emplace_back(thread_sp, thread_sp->GetIndexID());
}

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

for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) {
curr_thread_list = process_sp->GetThreadList();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curr_thread_list is a reference to the process's thread list, which made this a self-assignment.

for (auto [thread_sp, thread_index] : not_suspended_threads) {
if (curr_thread_list.GetSize() != num_threads) {
Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
LLDB_LOGF(
Expand All @@ -4205,14 +4198,11 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
break;
}

lldb::ThreadSP thread_sp = not_suspended_thread_list.GetThreadAtIndex(idx);

if (thread_sp->GetIndexID() != thread_index_array[idx]) {
if (thread_sp->GetIndexID() != thread_index) {
Log *log(GetLog(LLDBLog::Step | LLDBLog::Process));
LLDB_LOGF(log,
"The thread at position %u changed from %u to %u while "
"processing event.",
idx, thread_index_array[idx], thread_sp->GetIndexID());
LLDB_LOG(log,
"The thread {0} changed from {1} to {2} while processing event.",
thread_sp.get(), thread_index, thread_sp->GetIndexID());
break;
}

Expand Down
Loading