https://github.com/barsolo2000 updated https://github.com/llvm/llvm-project/pull/180101
>From 774b855c6d08d551063e8674a3aa13a8458df02f Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Thu, 5 Feb 2026 17:53:25 -0800 Subject: [PATCH 1/6] Improving performance of multiple threads stepping over the same breakpoint --- lldb/include/lldb/Target/ThreadList.h | 21 ++++ .../Target/ThreadPlanStepOverBreakpoint.h | 14 +++ lldb/source/Target/ThreadList.cpp | 101 ++++++++++++++++++ .../Target/ThreadPlanStepOverBreakpoint.cpp | 18 +++- 4 files changed, 150 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h index c108962003598..97541ed83b200 100644 --- a/lldb/include/lldb/Target/ThreadList.h +++ b/lldb/include/lldb/Target/ThreadList.h @@ -9,7 +9,9 @@ #ifndef LLDB_TARGET_THREADLIST_H #define LLDB_TARGET_THREADLIST_H +#include <map> #include <mutex> +#include <set> #include <vector> #include "lldb/Target/Thread.h" @@ -141,6 +143,19 @@ class ThreadList : public ThreadCollection { /// Precondition: both thread lists must be belong to the same process. void Update(ThreadList &rhs); + /// Called by ThreadPlanStepOverBreakpoint when a thread finishes stepping + /// over a breakpoint. This tracks which threads are still stepping over + /// each breakpoint address, and only re-enables the breakpoint when ALL + /// threads have finished stepping over it. + void ThreadFinishedSteppingOverBreakpoint(lldb::addr_t breakpoint_addr, + lldb::tid_t tid); + + /// Register a thread that is about to step over a breakpoint. + /// The breakpoint will be re-enabled only after all registered threads + /// have called ThreadFinishedSteppingOverBreakpoint. + void RegisterThreadSteppingOverBreakpoint(lldb::addr_t breakpoint_addr, + lldb::tid_t tid); + protected: void SetShouldReportStop(Vote vote); @@ -154,6 +169,12 @@ class ThreadList : public ThreadCollection { m_selected_tid; ///< For targets that need the notion of a current thread. std::vector<lldb::tid_t> m_expression_tid_stack; + /// Tracks which threads are currently stepping over each breakpoint address. + /// Key: breakpoint address, Value: set of thread IDs stepping over it. + /// When a thread finishes stepping, it's removed from the set. When the set + /// becomes empty, the breakpoint is re-enabled. + std::map<lldb::addr_t, std::set<lldb::tid_t>> m_threads_stepping_over_bp; + private: ThreadList() = delete; }; diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h index 0da8dbf44ffd8..d2cea98318ba7 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h @@ -36,6 +36,19 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan { lldb::addr_t GetBreakpointLoadAddress() const { return m_breakpoint_addr; } + /// When set to true, the breakpoint site will NOT be re-enabled directly + /// by this plan. Instead, the plan will call + /// ThreadList::ThreadFinishedSteppingOverBreakpoint() when it completes, + /// allowing ThreadList to track all threads stepping over the same + /// breakpoint and only re-enable it when ALL threads have finished. + void SetDeferReenableBreakpointSite(bool defer) { + m_defer_reenable_breakpoint_site = defer; + } + + bool GetDeferReenableBreakpointSite() const { + return m_defer_reenable_breakpoint_site; + } + protected: bool DoPlanExplainsStop(Event *event_ptr) override; bool DoWillResume(lldb::StateType resume_state, bool current_plan) override; @@ -47,6 +60,7 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan { lldb::user_id_t m_breakpoint_site_id; bool m_auto_continue; bool m_reenabled_breakpoint_site; + bool m_defer_reenable_breakpoint_site = false; ThreadPlanStepOverBreakpoint(const ThreadPlanStepOverBreakpoint &) = delete; const ThreadPlanStepOverBreakpoint & diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 77a1c40b95f70..b9ebfe871ce34 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -15,6 +15,7 @@ #include "lldb/Target/Thread.h" #include "lldb/Target/ThreadList.h" #include "lldb/Target/ThreadPlan.h" +#include "lldb/Target/ThreadPlanStepOverBreakpoint.h" #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" @@ -576,6 +577,12 @@ bool ThreadList::WillResume(RunDirection &direction) { assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction); } } else { + // Pre-scan to find all threads that need to step over a breakpoint, + // and group them by breakpoint address. This optimization allows us to + // step multiple threads over the same breakpoint with minimal breakpoint + // swaps - only the last thread in each group will re-enable the breakpoint. + std::map<lldb::addr_t, std::vector<ThreadSP>> breakpoint_groups; + for (pos = m_threads.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); if (thread_sp->GetResumeState() != eStateSuspended) { @@ -589,6 +596,14 @@ bool ThreadList::WillResume(RunDirection &direction) { assert(thread_sp->GetCurrentPlan()->GetDirection() == direction); // You can't say "stop others" and also want yourself to be suspended. assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended); + + // Get the breakpoint address from the step-over-breakpoint plan + ThreadPlan *current_plan = thread_sp->GetCurrentPlan(); + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(current_plan); + lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress(); + breakpoint_groups[bp_addr].push_back(thread_sp); + thread_to_run = thread_sp; if (thread_sp->ShouldRunBeforePublicStop()) { // This takes precedence, so if we find one of these, service it: @@ -597,6 +612,30 @@ bool ThreadList::WillResume(RunDirection &direction) { } } } + + // For each group of threads at the same breakpoint, register them with + // ThreadList and set them to use deferred re-enable. The breakpoint will + // only be re-enabled when ALL threads have finished stepping over it. + for (auto &group : breakpoint_groups) { + lldb::addr_t bp_addr = group.first; + std::vector<ThreadSP> &threads = group.second; + + if (threads.size() > 1) { + // Multiple threads stepping over the same breakpoint - use tracking + for (ThreadSP &thread_sp : threads) { + // Register this thread as stepping over the breakpoint + RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID()); + + // Set the plan to defer re-enabling (use callback instead) + ThreadPlan *plan = thread_sp->GetCurrentPlan(); + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + bp_plan->SetDeferReenableBreakpointSite(true); + } + } + // Single thread at breakpoint - keeps default behavior (re-enable + // directly) + } } if (thread_to_run != nullptr) { @@ -801,3 +840,65 @@ ThreadList::ExpressionExecutionThreadPusher::ExpressionExecutionThreadPusher( m_thread_list->PushExpressionExecutionThread(m_tid); } } + +void ThreadList::RegisterThreadSteppingOverBreakpoint(addr_t breakpoint_addr, + tid_t tid) { + std::lock_guard<std::recursive_mutex> guard(GetMutex()); + m_threads_stepping_over_bp[breakpoint_addr].insert(tid); + + Log *log = GetLog(LLDBLog::Step); + LLDB_LOGF(log, + "ThreadList::%s: Registered thread 0x%" PRIx64 + " stepping over breakpoint at 0x%" PRIx64 " (now %zu threads)", + __FUNCTION__, tid, breakpoint_addr, + m_threads_stepping_over_bp[breakpoint_addr].size()); +} + +void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, + tid_t tid) { + std::lock_guard<std::recursive_mutex> guard(GetMutex()); + + Log *log = GetLog(LLDBLog::Step); + + auto it = m_threads_stepping_over_bp.find(breakpoint_addr); + if (it == m_threads_stepping_over_bp.end()) { + // No threads registered for this breakpoint - just re-enable it directly + LLDB_LOGF(log, + "ThreadList::%s: Thread 0x%" PRIx64 + " finished stepping over breakpoint at 0x%" PRIx64 + " but no threads were registered, re-enabling directly", + __FUNCTION__, tid, breakpoint_addr); + BreakpointSiteSP bp_site_sp( + m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr)); + if (bp_site_sp) { + m_process.EnableBreakpointSite(bp_site_sp.get()); + } + return; + } + + // Remove this thread from the set + it->second.erase(tid); + + LLDB_LOGF(log, + "ThreadList::%s: Thread 0x%" PRIx64 + " finished stepping over breakpoint at 0x%" PRIx64 + " (%zu threads remaining)", + __FUNCTION__, tid, breakpoint_addr, it->second.size()); + + // If no more threads are stepping over this breakpoint, re-enable it + if (it->second.empty()) { + LLDB_LOGF(log, + "ThreadList::%s: All threads finished stepping over breakpoint " + "at 0x%" PRIx64 ", re-enabling breakpoint", + __FUNCTION__, breakpoint_addr); + + BreakpointSiteSP bp_site_sp( + m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr)); + if (bp_site_sp) { + m_process.EnableBreakpointSite(bp_site_sp.get()); + } + + // Clean up the entry + m_threads_stepping_over_bp.erase(it); + } +} diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp index 3602527a9231b..70538e21153b6 100644 --- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -10,6 +10,7 @@ #include "lldb/Target/Process.h" #include "lldb/Target/RegisterContext.h" +#include "lldb/Target/ThreadList.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/Stream.h" @@ -155,10 +156,19 @@ bool ThreadPlanStepOverBreakpoint::MischiefManaged() { void ThreadPlanStepOverBreakpoint::ReenableBreakpointSite() { if (!m_reenabled_breakpoint_site) { m_reenabled_breakpoint_site = true; - BreakpointSiteSP bp_site_sp( - m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr)); - if (bp_site_sp) { - m_process.EnableBreakpointSite(bp_site_sp.get()); + + if (m_defer_reenable_breakpoint_site) { + // Let ThreadList track all threads stepping over this breakpoint. + // It will re-enable the breakpoint only when ALL threads have finished. + m_process.GetThreadList().ThreadFinishedSteppingOverBreakpoint( + m_breakpoint_addr, GetThread().GetID()); + } else { + // Default behavior: re-enable the breakpoint directly + BreakpointSiteSP bp_site_sp( + m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr)); + if (bp_site_sp) { + m_process.EnableBreakpointSite(bp_site_sp.get()); + } } } } >From ecba9c7a234bae0de53a5c77548a7448b45a8301 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Fri, 6 Feb 2026 22:39:00 -0800 Subject: [PATCH 2/6] Recompute deferred breakpoint state on each resume Clear and recompute the deferred re-enable state at the beginning of WillResume rather than carrying it across stops. This prevents stale state if the user suspends a thread or an exception returns control between stops. The recomputation scans for threads with existing StepOverBreakpoint plans, groups them by breakpoint address, and sets deferred re-enable for groups with multiple threads. This ensures the breakpoint is only re-enabled once when ALL threads finish stepping over it, even if they run one at a time through the StopOthers path. --- lldb/source/Target/ThreadList.cpp | 52 +++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index b9ebfe871ce34..f863620321016 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -505,6 +505,58 @@ bool ThreadList::WillResume(RunDirection &direction) { collection::iterator pos, end = m_threads.end(); + // Clear any cached defer state from previous stops. This ensures we always + // compute fresh based on the current thread list state, handling edge cases + // like user suspending a thread or exceptions returning control to the user. + for (pos = m_threads.begin(); pos != end; ++pos) { + ThreadSP thread_sp(*pos); + ThreadPlan *plan = thread_sp->GetCurrentPlan(); + if (plan && plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + bp_plan->SetDeferReenableBreakpointSite(false); + } + } + // Also clear the tracking map - we'll rebuild it based on current state + m_threads_stepping_over_bp.clear(); + + // Recompute deferred state for threads with existing StepOverBreakpoint + // plans (incomplete batch members from the previous stop). Group them by + // breakpoint address and set deferred re-enable so the breakpoint is only + // re-enabled when ALL threads finish, even if they run one at a time. + { + std::map<lldb::addr_t, std::vector<ThreadSP>> existing_bp_groups; + for (pos = m_threads.begin(); pos != end; ++pos) { + ThreadSP thread_sp(*pos); + if (thread_sp->GetResumeState() == eStateSuspended) + continue; + if (thread_sp->IsOperatingSystemPluginThread() && + !thread_sp->GetBackingThread()) + continue; + ThreadPlan *plan = thread_sp->GetCurrentPlan(); + if (plan && plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + existing_bp_groups[bp_plan->GetBreakpointLoadAddress()].push_back( + thread_sp); + } + } + for (auto &group : existing_bp_groups) { + if (group.second.size() > 1) { + for (ThreadSP &thread_sp : group.second) { + RegisterThreadSteppingOverBreakpoint(group.first, + thread_sp->GetID()); + ThreadPlan *plan = thread_sp->GetCurrentPlan(); + if (plan && + plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + static_cast<ThreadPlanStepOverBreakpoint *>(plan) + ->SetDeferReenableBreakpointSite(true); + } + } + } + } + } + // Go through the threads and see if any thread wants to run just itself. // if so then pick one and run it. >From 61de159c01d79d87971c46c4b09824cfe057aef8 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Fri, 6 Feb 2026 22:39:49 -0800 Subject: [PATCH 3/6] Add GetKind guard before static_cast to ThreadPlanStepOverBreakpoint SetupToStepOverBreakpointIfNeeded may not always push a StepOverBreakpoint plan. Guard the static_cast with a GetKind check to prevent undefined behavior if the current plan is a different type. --- lldb/source/Target/ThreadList.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index f863620321016..a99473652263d 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -651,10 +651,14 @@ bool ThreadList::WillResume(RunDirection &direction) { // Get the breakpoint address from the step-over-breakpoint plan ThreadPlan *current_plan = thread_sp->GetCurrentPlan(); - ThreadPlanStepOverBreakpoint *bp_plan = - static_cast<ThreadPlanStepOverBreakpoint *>(current_plan); - lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress(); - breakpoint_groups[bp_addr].push_back(thread_sp); + if (current_plan && + current_plan->GetKind() == + ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(current_plan); + lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress(); + breakpoint_groups[bp_addr].push_back(thread_sp); + } thread_to_run = thread_sp; if (thread_sp->ShouldRunBeforePublicStop()) { @@ -678,11 +682,15 @@ bool ThreadList::WillResume(RunDirection &direction) { // Register this thread as stepping over the breakpoint RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID()); - // Set the plan to defer re-enabling (use callback instead) + // Set the plan to defer re-enabling (use callback instead). ThreadPlan *plan = thread_sp->GetCurrentPlan(); - ThreadPlanStepOverBreakpoint *bp_plan = - static_cast<ThreadPlanStepOverBreakpoint *>(plan); - bp_plan->SetDeferReenableBreakpointSite(true); + // Verify the plan is actually a StepOverBreakpoint plan. + if (plan && + plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + bp_plan->SetDeferReenableBreakpointSite(true); + } } } // Single thread at breakpoint - keeps default behavior (re-enable >From 180cee60d3a387497e9dc6d3889847eccadbae2e Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Fri, 6 Feb 2026 22:41:18 -0800 Subject: [PATCH 4/6] Enable batched vCont for threads stepping over the same breakpoint When multiple threads need to step over the same breakpoint, step them all together in a single vCont packet rather than one at a time. This reduces the number of resume/stop cycles. Also guard the batching optimization with a found_run_before_public_stop check: if the scan exits early due to a ShouldRunBeforePublicStop thread, the breakpoint groups are incomplete and the priority thread must run solo. --- lldb/source/Target/ThreadList.cpp | 88 ++++++--- ...TestConcurrentBatchedBreakpointStepOver.py | 179 ++++++++++++++++++ 2 files changed, 244 insertions(+), 23 deletions(-) create mode 100644 lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index a99473652263d..a9018c09243d4 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -560,6 +560,10 @@ bool ThreadList::WillResume(RunDirection &direction) { // Go through the threads and see if any thread wants to run just itself. // if so then pick one and run it. + // Collect threads for batched vCont - multiple threads at the same breakpoint + // can be stepped together in a single vCont packet. + std::vector<ThreadSP> batched_step_threads; + ThreadList run_me_only_list(m_process); run_me_only_list.SetStopID(m_process.GetStopID()); @@ -634,6 +638,7 @@ bool ThreadList::WillResume(RunDirection &direction) { // step multiple threads over the same breakpoint with minimal breakpoint // swaps - only the last thread in each group will re-enable the breakpoint. std::map<lldb::addr_t, std::vector<ThreadSP>> breakpoint_groups; + bool found_run_before_public_stop = false; for (pos = m_threads.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); @@ -663,38 +668,56 @@ bool ThreadList::WillResume(RunDirection &direction) { thread_to_run = thread_sp; if (thread_sp->ShouldRunBeforePublicStop()) { // This takes precedence, so if we find one of these, service it: + found_run_before_public_stop = true; break; } } } } - // For each group of threads at the same breakpoint, register them with - // ThreadList and set them to use deferred re-enable. The breakpoint will - // only be re-enabled when ALL threads have finished stepping over it. - for (auto &group : breakpoint_groups) { - lldb::addr_t bp_addr = group.first; - std::vector<ThreadSP> &threads = group.second; - - if (threads.size() > 1) { - // Multiple threads stepping over the same breakpoint - use tracking - for (ThreadSP &thread_sp : threads) { - // Register this thread as stepping over the breakpoint - RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID()); + // Only apply batching optimization if we have a complete picture of + // breakpoint groups. If a ShouldRunBeforePublicStop thread caused the + // scan to exit early, the groups are incomplete and the priority thread + // must run solo. Deferred state will be cleaned up on next WillResume(). + if (!found_run_before_public_stop) { + // For each group of threads at the same breakpoint, register them with + // ThreadList and set them to use deferred re-enable. The breakpoint will + // only be re-enabled when ALL threads have finished stepping over it. + // Also collect threads for batched vCont if multiple threads at same BP. + for (auto &group : breakpoint_groups) { + lldb::addr_t bp_addr = group.first; + std::vector<ThreadSP> &threads = group.second; + + if (threads.size() > 1) { + // Multiple threads stepping over the same breakpoint - use tracking + for (ThreadSP &thread_sp : threads) { + // Register this thread as stepping over the breakpoint + RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID()); + + // Set the plan to defer re-enabling (use callback instead). + ThreadPlan *plan = thread_sp->GetCurrentPlan(); + // Verify the plan is actually a StepOverBreakpoint plan. + if (plan && + plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + bp_plan->SetDeferReenableBreakpointSite(true); + } + } - // Set the plan to defer re-enabling (use callback instead). - ThreadPlan *plan = thread_sp->GetCurrentPlan(); - // Verify the plan is actually a StepOverBreakpoint plan. - if (plan && - plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { - ThreadPlanStepOverBreakpoint *bp_plan = - static_cast<ThreadPlanStepOverBreakpoint *>(plan); - bp_plan->SetDeferReenableBreakpointSite(true); + // Collect for batched vCont - pick the largest group + if (threads.size() > batched_step_threads.size()) { + batched_step_threads = threads; } } + // Single thread at breakpoint - keeps default behavior (re-enable + // directly) + } + + // If we found a batch, use the first thread as thread_to_run + if (!batched_step_threads.empty()) { + thread_to_run = batched_step_threads[0]; } - // Single thread at breakpoint - keeps default behavior (re-enable - // directly) } } @@ -714,7 +737,26 @@ bool ThreadList::WillResume(RunDirection &direction) { bool need_to_resume = true; - if (thread_to_run == nullptr) { + if (!batched_step_threads.empty()) { + // Batched stepping: all threads in the batch step together, + // all other threads stay suspended. + std::set<lldb::tid_t> batch_tids; + for (ThreadSP &thread_sp : batched_step_threads) { + batch_tids.insert(thread_sp->GetID()); + } + + for (pos = m_threads.begin(); pos != end; ++pos) { + ThreadSP thread_sp(*pos); + if (batch_tids.count(thread_sp->GetID()) > 0) { + // This thread is in the batch - let it step + if (!thread_sp->ShouldResume(thread_sp->GetCurrentPlan()->RunState())) + need_to_resume = false; + } else { + // Not in the batch - suspend it + thread_sp->ShouldResume(eStateSuspended); + } + } + } else if (thread_to_run == nullptr) { // Everybody runs as they wish: for (pos = m_threads.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py new file mode 100644 index 0000000000000..aacb3eaf4381c --- /dev/null +++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py @@ -0,0 +1,179 @@ +""" +Test that the batched breakpoint step-over optimization is used when multiple +threads hit the same breakpoint simultaneously. Verifies that when N threads +need to step over the same breakpoint, the breakpoint is only disabled once +and re-enabled once, rather than N times. +""" + +import os +import re + +from lldbsuite.test.decorators import * +from lldbsuite.test.concurrent_base import ConcurrentEventsBase +from lldbsuite.test.lldbtest import TestBase + + +@skipIfWindows +class ConcurrentBatchedBreakpointStepOver(ConcurrentEventsBase): + + @skipIf(triple="^mips") + @expectedFailureAll( + archs=["aarch64"], oslist=["freebsd"], bugnumber="llvm.org/pr49433" + ) + def test(self): + """Test that batched breakpoint step-over reduces breakpoint + toggle operations when multiple threads hit the same breakpoint.""" + self.build() + + # Enable logging to capture optimization messages and GDB packets. + # Both categories must be in a single command because successive + # 'log enable lldb <cat> -f <file>' calls redirect the entire 'lldb' + # channel to the new file, leaving the old file empty. + lldb_logfile = self.getBuildArtifact("lldb-log.txt") + self.runCmd( + "log enable lldb step break -f {}".format(lldb_logfile) + ) + + gdb_logfile = self.getBuildArtifact("gdb-remote-log.txt") + self.runCmd("log enable gdb-remote packets -f {}".format(gdb_logfile)) + + # Run with 10 breakpoint threads - the pseudo_barrier in main.cpp + # ensures they all race to the breakpoint simultaneously, making it + # very likely multiple threads are at the same address when stopped. + self.do_thread_actions(num_breakpoint_threads=10) + + # --- Analyze the lldb log --- + self.assertTrue( + os.path.isfile(lldb_logfile), "lldb log file not found" + ) + with open(lldb_logfile, "r") as f: + lldb_log = f.read() + + # 1) Find the thread breakpoint address from "Registered thread" + # messages, which tell us the optimization was used. + registered_matches = re.findall( + r"Registered thread 0x[0-9a-fA-F]+ stepping over " + r"breakpoint at (0x[0-9a-fA-F]+)", + lldb_log, + ) + self.assertGreater( + len(registered_matches), + 0, + "Expected batched breakpoint step-over optimization to be " + "used (no 'Registered thread' messages found in log).", + ) + thread_bp_addr = registered_matches[0] + + # 2) Verify all threads completed their step-over. + completed_count = lldb_log.count( + "Completed step over breakpoint plan." + ) + self.assertGreaterEqual( + completed_count, + 10, + "Expected at least 10 'Completed step over breakpoint plan.' " + "messages (one per thread), but got {}.".format(completed_count), + ) + + # 3) Verify the deferred re-enable path was used: "finished stepping + # over breakpoint" messages show threads completed via the tracking + # mechanism. The last thread may use the direct path (when only 1 + # thread remains, deferred is not set), so we expect at least N-1. + finished_matches = re.findall( + r"Thread 0x[0-9a-fA-F]+ finished stepping over breakpoint at " + r"(0x[0-9a-fA-F]+)", + lldb_log, + ) + self.assertGreaterEqual( + len(finished_matches), + 9, + "Expected at least 9 'finished stepping over breakpoint' " + "messages (deferred path), but got {}.".format( + len(finished_matches) + ), + ) + + # --- Count z0/Z0 packets for the thread breakpoint address --- + # z0 = remove (disable) software breakpoint + # Z0 = set (enable) software breakpoint + # Strip the "0x" prefix and leading zeros to match the GDB packet + # format (which uses lowercase hex without "0x" prefix). + bp_addr_hex = thread_bp_addr[2:].lstrip("0") if thread_bp_addr else "" + + z0_count = 0 # disable packets + Z0_count = 0 # enable packets (excluding the initial set) + initial_Z0_seen = False + max_vcont_step_threads = 0 # largest number of s: actions in one vCont + + self.assertTrue( + os.path.isfile(gdb_logfile), "gdb-remote log file not found" + ) + with open(gdb_logfile, "r") as f: + for line in f: + if "send packet: $" not in line: + continue + # Match z0,<addr> (disable) or Z0,<addr> (enable) + m = re.search( + r'send packet: \$([Zz])0,([0-9a-fA-F]+),', line + ) + if m and m.group(2) == bp_addr_hex: + if m.group(1) == "Z": + if not initial_Z0_seen: + # Skip the initial breakpoint set + initial_Z0_seen = True + else: + Z0_count += 1 + else: + z0_count += 1 + + # Count step actions in vCont packets to detect batching. + # A batched vCont looks like: vCont;s:tid1;s:tid2;... + vcont_m = re.search( + r'send packet: \$vCont((?:;[^#]+)*)', line + ) + if vcont_m: + actions = vcont_m.group(1) + step_count = len(re.findall(r';s:', actions)) + if step_count > max_vcont_step_threads: + max_vcont_step_threads = step_count + + print("\n--- Breakpoint packet summary for {} ---".format( + thread_bp_addr)) + print(" z0 (disable) packets: {}".format(z0_count)) + print(" Z0 (re-enable) packets: {}".format(Z0_count)) + print(" Max threads in a single vCont step: {}".format( + max_vcont_step_threads)) + + # 4) With the optimization: 1 z0 (disable once) + 1 Z0 (re-enable once) + # Without optimization: N z0 + N Z0 (one pair per thread) + self.assertEqual( + z0_count, + 1, + "Expected exactly 1 breakpoint disable (z0) for the thread " + "breakpoint at {}, but got {}. The optimization should disable " + "the breakpoint only once for all {} threads.".format( + thread_bp_addr, z0_count, 10 + ), + ) + self.assertEqual( + Z0_count, + 1, + "Expected exactly 1 breakpoint re-enable (Z0) for the thread " + "breakpoint at {}, but got {}. The optimization should re-enable " + "the breakpoint only once after all threads finish.".format( + thread_bp_addr, Z0_count + ), + ) + + # 5) Verify batched vCont: at least one vCont packet should contain + # multiple s: (step) actions, proving threads were stepped together + # in a single packet rather than one at a time. + self.assertGreater( + max_vcont_step_threads, + 1, + "Expected at least one batched vCont packet with multiple " + "step actions (s:), but the maximum was {}. The optimization " + "should step multiple threads in a single vCont.".format( + max_vcont_step_threads + ), + ) >From 849c934c1e5a2e7d673c34cd9683a3440af27d2d Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Fri, 6 Feb 2026 23:20:03 -0800 Subject: [PATCH 5/6] Fixed formating, changed the test, Added batching --- lldb/source/Target/ThreadList.cpp | 31 ++++++-- ...TestConcurrentBatchedBreakpointStepOver.py | 73 ++++++------------- 2 files changed, 48 insertions(+), 56 deletions(-) diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index a9018c09243d4..256ee558a8ed0 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -544,11 +544,9 @@ bool ThreadList::WillResume(RunDirection &direction) { for (auto &group : existing_bp_groups) { if (group.second.size() > 1) { for (ThreadSP &thread_sp : group.second) { - RegisterThreadSteppingOverBreakpoint(group.first, - thread_sp->GetID()); + RegisterThreadSteppingOverBreakpoint(group.first, thread_sp->GetID()); ThreadPlan *plan = thread_sp->GetCurrentPlan(); - if (plan && - plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + if (plan && plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { static_cast<ThreadPlanStepOverBreakpoint *>(plan) ->SetDeferReenableBreakpointSite(true); } @@ -583,6 +581,15 @@ bool ThreadList::WillResume(RunDirection &direction) { !thread_sp->GetBackingThread()) continue; + // Skip threads with existing StepOverBreakpoint plans — they are + // mid-step-over from an incomplete previous batch and will be + // re-batched in the else branch below. Exception: if a thread + // needs to run before public stop, it takes priority. + if (thread_sp->GetCurrentPlan()->GetKind() == + ThreadPlan::eKindStepOverBreakpoint && + !thread_sp->ShouldRunBeforePublicStop()) + continue; + // 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); @@ -657,8 +664,7 @@ bool ThreadList::WillResume(RunDirection &direction) { // Get the breakpoint address from the step-over-breakpoint plan ThreadPlan *current_plan = thread_sp->GetCurrentPlan(); if (current_plan && - current_plan->GetKind() == - ThreadPlan::eKindStepOverBreakpoint) { + current_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { ThreadPlanStepOverBreakpoint *bp_plan = static_cast<ThreadPlanStepOverBreakpoint *>(current_plan); lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress(); @@ -671,6 +677,19 @@ bool ThreadList::WillResume(RunDirection &direction) { found_run_before_public_stop = true; break; } + } else { + // Check for threads with existing StepOverBreakpoint plans from + // an incomplete previous batch. SetupToStepOverBreakpointIfNeeded + // returns false for these because the plan already exists. + ThreadPlan *current_plan = thread_sp->GetCurrentPlan(); + if (current_plan && + current_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(current_plan); + lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress(); + breakpoint_groups[bp_addr].push_back(thread_sp); + thread_to_run = thread_sp; + } } } } diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py index aacb3eaf4381c..39964813a59df 100644 --- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py +++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py @@ -26,31 +26,21 @@ def test(self): self.build() # Enable logging to capture optimization messages and GDB packets. - # Both categories must be in a single command because successive - # 'log enable lldb <cat> -f <file>' calls redirect the entire 'lldb' - # channel to the new file, leaving the old file empty. lldb_logfile = self.getBuildArtifact("lldb-log.txt") - self.runCmd( - "log enable lldb step break -f {}".format(lldb_logfile) - ) + self.runCmd("log enable lldb step break -f {}".format(lldb_logfile)) gdb_logfile = self.getBuildArtifact("gdb-remote-log.txt") self.runCmd("log enable gdb-remote packets -f {}".format(gdb_logfile)) - # Run with 10 breakpoint threads - the pseudo_barrier in main.cpp - # ensures they all race to the breakpoint simultaneously, making it - # very likely multiple threads are at the same address when stopped. + # Run with 10 breakpoint threads. self.do_thread_actions(num_breakpoint_threads=10) - # --- Analyze the lldb log --- - self.assertTrue( - os.path.isfile(lldb_logfile), "lldb log file not found" - ) + self.assertTrue(os.path.isfile(lldb_logfile), "lldb log file not found") with open(lldb_logfile, "r") as f: lldb_log = f.read() - # 1) Find the thread breakpoint address from "Registered thread" - # messages, which tell us the optimization was used. + # Find the thread breakpoint address from "Registered thread" + # messages, which tell us the optimization was used. registered_matches = re.findall( r"Registered thread 0x[0-9a-fA-F]+ stepping over " r"breakpoint at (0x[0-9a-fA-F]+)", @@ -64,10 +54,8 @@ def test(self): ) thread_bp_addr = registered_matches[0] - # 2) Verify all threads completed their step-over. - completed_count = lldb_log.count( - "Completed step over breakpoint plan." - ) + # Verify all threads completed their step-over. + completed_count = lldb_log.count("Completed step over breakpoint plan.") self.assertGreaterEqual( completed_count, 10, @@ -75,10 +63,10 @@ def test(self): "messages (one per thread), but got {}.".format(completed_count), ) - # 3) Verify the deferred re-enable path was used: "finished stepping - # over breakpoint" messages show threads completed via the tracking - # mechanism. The last thread may use the direct path (when only 1 - # thread remains, deferred is not set), so we expect at least N-1. + # Verify the deferred re-enable path was used: "finished stepping + # over breakpoint" messages show threads completed via the tracking + # mechanism. The last thread may use the direct path (when only 1 + # hread remains, deferred is not set), so we expect at least N-1. finished_matches = re.findall( r"Thread 0x[0-9a-fA-F]+ finished stepping over breakpoint at " r"(0x[0-9a-fA-F]+)", @@ -88,12 +76,10 @@ def test(self): len(finished_matches), 9, "Expected at least 9 'finished stepping over breakpoint' " - "messages (deferred path), but got {}.".format( - len(finished_matches) - ), + "messages (deferred path), but got {}.".format(len(finished_matches)), ) - # --- Count z0/Z0 packets for the thread breakpoint address --- + # Count z0/Z0 packets for the thread breakpoint address # z0 = remove (disable) software breakpoint # Z0 = set (enable) software breakpoint # Strip the "0x" prefix and leading zeros to match the GDB packet @@ -105,21 +91,17 @@ def test(self): initial_Z0_seen = False max_vcont_step_threads = 0 # largest number of s: actions in one vCont - self.assertTrue( - os.path.isfile(gdb_logfile), "gdb-remote log file not found" - ) + self.assertTrue(os.path.isfile(gdb_logfile), "gdb-remote log file not found") with open(gdb_logfile, "r") as f: for line in f: if "send packet: $" not in line: continue + # Match z0,<addr> (disable) or Z0,<addr> (enable) - m = re.search( - r'send packet: \$([Zz])0,([0-9a-fA-F]+),', line - ) + m = re.search(r"send packet: \$([Zz])0,([0-9a-fA-F]+),", line) if m and m.group(2) == bp_addr_hex: if m.group(1) == "Z": if not initial_Z0_seen: - # Skip the initial breakpoint set initial_Z0_seen = True else: Z0_count += 1 @@ -128,24 +110,15 @@ def test(self): # Count step actions in vCont packets to detect batching. # A batched vCont looks like: vCont;s:tid1;s:tid2;... - vcont_m = re.search( - r'send packet: \$vCont((?:;[^#]+)*)', line - ) + vcont_m = re.search(r"send packet: \$vCont((?:;[^#]+)*)", line) if vcont_m: actions = vcont_m.group(1) - step_count = len(re.findall(r';s:', actions)) + step_count = len(re.findall(r";s:", actions)) if step_count > max_vcont_step_threads: max_vcont_step_threads = step_count - print("\n--- Breakpoint packet summary for {} ---".format( - thread_bp_addr)) - print(" z0 (disable) packets: {}".format(z0_count)) - print(" Z0 (re-enable) packets: {}".format(Z0_count)) - print(" Max threads in a single vCont step: {}".format( - max_vcont_step_threads)) - - # 4) With the optimization: 1 z0 (disable once) + 1 Z0 (re-enable once) - # Without optimization: N z0 + N Z0 (one pair per thread) + # With the optimization: 1 z0 (disable once) + 1 Z0 (re-enable once) + # Without optimization: N z0 + N Z0 (one pair per thread) self.assertEqual( z0_count, 1, @@ -165,9 +138,9 @@ def test(self): ), ) - # 5) Verify batched vCont: at least one vCont packet should contain - # multiple s: (step) actions, proving threads were stepped together - # in a single packet rather than one at a time. + # Verify batched vCont: at least one vCont packet should contain + # multiple s: (step) actions, proving threads were stepped together + # in a single packet rather than one at a time. self.assertGreater( max_vcont_step_threads, 1, >From 090605a15811618a8ab6e35ad1c8057dce0831f0 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Fri, 6 Feb 2026 23:34:58 -0800 Subject: [PATCH 6/6] fixing darker format py --- .../concurrent_events/TestConcurrentBatchedBreakpointStepOver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py index 39964813a59df..51bc40b853eb9 100644 --- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py +++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py @@ -15,7 +15,6 @@ @skipIfWindows class ConcurrentBatchedBreakpointStepOver(ConcurrentEventsBase): - @skipIf(triple="^mips") @expectedFailureAll( archs=["aarch64"], oslist=["freebsd"], bugnumber="llvm.org/pr49433" _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
