https://github.com/labath updated https://github.com/llvm/llvm-project/pull/98154
>From 27d419b047d0c2e95978b8c88dd84641aae423f2 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Tue, 9 Jul 2024 15:00:05 +0200 Subject: [PATCH] [lldb] Small cleanup of ProcessEventData::ShouldStop 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. --- lldb/source/Target/Process.cpp | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 6fac0df1d7a66..dc7f6c9e86a47 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -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. @@ -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 @@ -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(); + 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( @@ -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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits