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 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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" >From b33d1fb4acc9b64ea802f869ef6cdb3e966eec1c Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Mon, 9 Feb 2026 14:51:30 -0800 Subject: [PATCH 07/12] Added another test with GDBserver simulation --- .../TestBatchedBreakpointStepOver.py | 222 ++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py new file mode 100644 index 0000000000000..9734396e20717 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py @@ -0,0 +1,222 @@ +""" +Test that when multiple threads are stopped at the same breakpoint, LLDB sends +a batched vCont with multiple step actions and only one breakpoint disable/ +re-enable pair, rather than stepping each thread individually with repeated +breakpoint toggles. + +Uses a mock GDB server to directly verify the packets LLDB sends. +""" + +import re + +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import * +from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase + + +class TestBatchedBreakpointStepOver(GDBRemoteTestBase): + @skipIfXmlSupportMissing + def test(self): + BP_ADDR = 0x0000000000401020 + # PC after stepping past the breakpoint instruction. + STEPPED_PC = BP_ADDR + 1 + NUM_THREADS = 10 + TIDS = [0x101 + i for i in range(NUM_THREADS)] + + class MyResponder(MockGDBServerResponder): + def __init__(self): + MockGDBServerResponder.__init__(self) + self.resume_count = 0 + # Track which threads have completed their step + self.stepped_threads = set() + + def qSupported(self, client_supported): + return ( + "PacketSize=3fff;QStartNoAckMode+;" + "qXfer:features:read+;swbreak+;hwbreak+" + ) + + def qfThreadInfo(self): + return "m" + ",".join("{:x}".format(t) for t in TIDS) + + def qsThreadInfo(self): + return "l" + + def haltReason(self): + # All threads stopped at the breakpoint address. + threads_str = ",".join("{:x}".format(t) for t in TIDS) + pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS) + return ( + "T05thread:{:x};threads:{};thread-pcs:{};" + "swbreak:;".format(TIDS[0], threads_str, pcs_str) + ) + + def threadStopInfo(self, threadnum): + threads_str = ",".join("{:x}".format(t) for t in TIDS) + pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS) + return ( + "T05thread:{:x};threads:{};thread-pcs:{};" + "swbreak:;".format(threadnum, threads_str, pcs_str) + ) + + def setBreakpoint(self, packet): + return "OK" + + def readRegisters(self): + return "00" * 160 + + def readRegister(self, regno): + return "00" * 8 + + def qXferRead(self, obj, annex, offset, length): + if annex == "target.xml": + return ( + """<?xml version="1.0"?> + <target version="1.0"> + <architecture>i386:x86-64</architecture> + <feature name="org.gnu.gdb.i386.core"> + <reg name="rax" bitsize="64" regnum="0" type="int" group="general"/> + <reg name="rbx" bitsize="64" regnum="1" type="int" group="general"/> + <reg name="rcx" bitsize="64" regnum="2" type="int" group="general"/> + <reg name="rdx" bitsize="64" regnum="3" type="int" group="general"/> + <reg name="rsi" bitsize="64" regnum="4" type="int" group="general"/> + <reg name="rdi" bitsize="64" regnum="5" type="int" group="general"/> + <reg name="rbp" bitsize="64" regnum="6" type="data_ptr" group="general"/> + <reg name="rsp" bitsize="64" regnum="7" type="data_ptr" group="general"/> + <reg name="r8" bitsize="64" regnum="8" type="int" group="general"/> + <reg name="r9" bitsize="64" regnum="9" type="int" group="general"/> + <reg name="r10" bitsize="64" regnum="10" type="int" group="general"/> + <reg name="r11" bitsize="64" regnum="11" type="int" group="general"/> + <reg name="r12" bitsize="64" regnum="12" type="int" group="general"/> + <reg name="r13" bitsize="64" regnum="13" type="int" group="general"/> + <reg name="r14" bitsize="64" regnum="14" type="int" group="general"/> + <reg name="r15" bitsize="64" regnum="15" type="int" group="general"/> + <reg name="rip" bitsize="64" regnum="16" type="code_ptr" group="general"/> + <reg name="eflags" bitsize="32" regnum="17" type="int" group="general"/> + <reg name="cs" bitsize="32" regnum="18" type="int" group="general"/> + <reg name="ss" bitsize="32" regnum="19" type="int" group="general"/> + </feature> + </target>""", + False, + ) + return None, False + + def other(self, packet): + if packet == "vCont?": + return "vCont;c;C;s;S" + if packet.startswith("vCont;"): + return self._handle_vCont(packet) + if packet.startswith("z"): + return "OK" + return "" + + def _handle_vCont(self, packet): + self.resume_count += 1 + # Parse step actions from vCont. + stepping_tids = [] + for action in packet[6:].split(";"): + if not action: + continue + if action.startswith("s:"): + tid_str = action[2:] + if "." in tid_str: + tid_str = tid_str.split(".")[1] + stepping_tids.append(int(tid_str, 16)) + + # All stepping threads complete their step. + for tid in stepping_tids: + self.stepped_threads.add(tid) + + all_done = self.stepped_threads >= set(TIDS) + + # Report stop, use the first stepping thread as the reporter. + report_tid = stepping_tids[0] if stepping_tids else TIDS[0] + threads_str = ",".join("{:x}".format(t) for t in TIDS) + if all_done: + # All threads moved past breakpoint + pcs_str = ",".join( + "{:x}".format(STEPPED_PC) for _ in TIDS + ) + else: + # Stepped threads moved, others still at breakpoint. + pcs_str = ",".join( + "{:x}".format( + STEPPED_PC if t in self.stepped_threads else BP_ADDR + ) + for t in TIDS + ) + return "T05thread:{:x};threads:{};thread-pcs:{};".format( + report_tid, threads_str, pcs_str + ) + + self.server.responder = MyResponder() + self.runCmd("platform select remote-linux") + target = self.createTarget("a.yaml") + process = self.connect(target) + + self.assertEqual(process.GetNumThreads(), NUM_THREADS) + + # Set a breakpoint at BP_ADDR, all threads are already stopped there. + bkpt = target.BreakpointCreateByAddress(BP_ADDR) + self.assertTrue(bkpt.IsValid()) + + # Continue, LLDB should step all threads over the breakpoint. + process.Continue() + + # Collect packets from the log + received = self.server.responder.packetLog.get_received() + + bp_addr_hex = "{:x}".format(BP_ADDR) + + # Count z0 (disable) and Z0 (enable) packets for our breakpoint. + z0_packets = [] + Z0_packets = [] + vcont_step_packets = [] + + for pkt in received: + if pkt.startswith("z0,{},".format(bp_addr_hex)): + z0_packets.append(pkt) + elif pkt.startswith("Z0,{},".format(bp_addr_hex)): + Z0_packets.append(pkt) + elif pkt.startswith("vCont;"): + step_count = len(re.findall(r";s:", pkt)) + if step_count > 0: + vcont_step_packets.append((step_count, pkt)) + + # Verify: exactly 1 breakpoint disable (z0) + self.assertEqual( + len(z0_packets), + 1, + "Expected 1 z0 (disable) packet, got {}: {}".format( + len(z0_packets), z0_packets + ), + ) + + # The initial Z0 is the breakpoint set. After stepping, there should + # be exactly 1 re-enable Z0 (total Z0 count = 2: set + re-enable). + # But we set the breakpoint via SB API, so count Z0 packets with + # our address, initial set + 1 re-enable = 2. + self.assertEqual( + len(Z0_packets), + 2, + "Expected 2 Z0 packets (1 set + 1 re-enable), got {}: {}".format( + len(Z0_packets), Z0_packets + ), + ) + + # At least one batched vCont with multiple step actions. + max_batch = max( + (count for count, _ in vcont_step_packets), default=0 + ) + self.assertGreaterEqual( + max_batch, + NUM_THREADS, + "Expected a vCont with {} step actions (batched), " + "but max was {}. Packets: {}".format( + NUM_THREADS, + max_batch, + [(c, p) for c, p in vcont_step_packets], + ), + ) >From 877b8b77d3f6d65ff7179a22ba3cc441e7dcda4c Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Mon, 9 Feb 2026 15:20:51 -0800 Subject: [PATCH 08/12] formatting for test --- .../TestBatchedBreakpointStepOver.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py index 9734396e20717..4954037565c76 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py @@ -48,17 +48,15 @@ def haltReason(self): # All threads stopped at the breakpoint address. threads_str = ",".join("{:x}".format(t) for t in TIDS) pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS) - return ( - "T05thread:{:x};threads:{};thread-pcs:{};" - "swbreak:;".format(TIDS[0], threads_str, pcs_str) + return "T05thread:{:x};threads:{};thread-pcs:{};" "swbreak:;".format( + TIDS[0], threads_str, pcs_str ) def threadStopInfo(self, threadnum): threads_str = ",".join("{:x}".format(t) for t in TIDS) pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS) - return ( - "T05thread:{:x};threads:{};thread-pcs:{};" - "swbreak:;".format(threadnum, threads_str, pcs_str) + return "T05thread:{:x};threads:{};thread-pcs:{};" "swbreak:;".format( + threadnum, threads_str, pcs_str ) def setBreakpoint(self, packet): @@ -136,9 +134,7 @@ def _handle_vCont(self, packet): threads_str = ",".join("{:x}".format(t) for t in TIDS) if all_done: # All threads moved past breakpoint - pcs_str = ",".join( - "{:x}".format(STEPPED_PC) for _ in TIDS - ) + pcs_str = ",".join("{:x}".format(STEPPED_PC) for _ in TIDS) else: # Stepped threads moved, others still at breakpoint. pcs_str = ",".join( @@ -207,9 +203,7 @@ def _handle_vCont(self, packet): ) # At least one batched vCont with multiple step actions. - max_batch = max( - (count for count, _ in vcont_step_packets), default=0 - ) + max_batch = max((count for count, _ in vcont_step_packets), default=0) self.assertGreaterEqual( max_batch, NUM_THREADS, >From fa81189386546f1c9bd792d9841566d2a90312f4 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Mon, 9 Feb 2026 15:21:23 -0800 Subject: [PATCH 09/12] alternative approach --- .../Target/ThreadPlanStepOverBreakpoint.h | 5 ++ lldb/source/Target/ThreadList.cpp | 76 +++---------------- 2 files changed, 16 insertions(+), 65 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h index d2cea98318ba7..0f0bbc0be4142 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h @@ -49,6 +49,11 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan { return m_defer_reenable_breakpoint_site; } + /// Mark the breakpoint site as already re-enabled, suppressing any + /// re-enable in DidPop()/ThreadDestroyed(). Used when discarding plans + /// during WillResume cleanup to avoid spurious breakpoint toggles. + void SetReenabledBreakpointSite() { m_reenabled_breakpoint_site = true; } + protected: bool DoPlanExplainsStop(Event *event_ptr) override; bool DoWillResume(lldb::StateType resume_state, bool current_plan) override; diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 256ee558a8ed0..fcfcf88e7fe15 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -505,53 +505,21 @@ 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. + // Clear tracking state from the previous stop and pop any leftover + // StepOverBreakpoint plans. This gives us a clean slate: plans will be + // recreated fresh by SetupToStepOverBreakpointIfNeeded below, and the + // batching logic will recompute deferred state from scratch. + m_threads_stepping_over_bp.clear(); 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); - } - } - } + // Suppress the re-enable side effect in DidPop() — the breakpoint + // may still be disabled from the previous batch, and we don't want + // to toggle it. The new plans will handle disable/re-enable correctly. + static_cast<ThreadPlanStepOverBreakpoint *>(plan) + ->SetReenabledBreakpointSite(); + thread_sp->DiscardPlan(); } } @@ -581,15 +549,6 @@ 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); @@ -677,19 +636,6 @@ 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; - } } } } >From 4a4372bb9c81eb947ac0c70131fb3f8ad4173435 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Tue, 10 Feb 2026 11:39:15 -0800 Subject: [PATCH 10/12] Fixed comments, braces and followed LLVM rules --- lldb/include/lldb/Target/ThreadList.h | 8 ++- lldb/source/Target/ThreadList.cpp | 57 +++++++++---------- .../Target/ThreadPlanStepOverBreakpoint.cpp | 5 +- 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h index 97541ed83b200..6920cefc20fd9 100644 --- a/lldb/include/lldb/Target/ThreadList.h +++ b/lldb/include/lldb/Target/ThreadList.h @@ -9,9 +9,7 @@ #ifndef LLDB_TARGET_THREADLIST_H #define LLDB_TARGET_THREADLIST_H -#include <map> #include <mutex> -#include <set> #include <vector> #include "lldb/Target/Thread.h" @@ -20,6 +18,9 @@ #include "lldb/Utility/UserID.h" #include "lldb/lldb-private.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" + namespace lldb_private { // This is a thread list with lots of functionality for use only by the process @@ -173,7 +174,8 @@ class ThreadList : public ThreadCollection { /// 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; + llvm::DenseMap<lldb::addr_t, llvm::DenseSet<lldb::tid_t>> + m_threads_stepping_over_bp; private: ThreadList() = delete; diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index fcfcf88e7fe15..6cb655c87d9f0 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -20,6 +20,8 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/State.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" using namespace lldb; using namespace lldb_private; @@ -526,9 +528,9 @@ 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; + // Collect threads for batched vCont for multiple threads at the same + // breakpoint. + llvm::SmallVector<ThreadSP> batched_step_threads; ThreadList run_me_only_list(m_process); @@ -602,8 +604,8 @@ bool ThreadList::WillResume(RunDirection &direction) { // 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; + // swaps, only the last thread in each group will re-enable the breakpoint. + llvm::DenseMap<lldb::addr_t, llvm::SmallVector<ThreadSP>> breakpoint_groups; bool found_run_before_public_stop = false; for (pos = m_threads.begin(); pos != end; ++pos) { @@ -620,7 +622,7 @@ bool ThreadList::WillResume(RunDirection &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 + // Get the breakpoint address from the step-over-breakpoint plan. ThreadPlan *current_plan = thread_sp->GetCurrentPlan(); if (current_plan && current_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { @@ -651,12 +653,13 @@ bool ThreadList::WillResume(RunDirection &direction) { // 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; + llvm::SmallVector<ThreadSP> &threads = group.second; if (threads.size() > 1) { - // Multiple threads stepping over the same breakpoint - use tracking + // Use tracking since multiple threads are stepping over the same + // breakpoint. for (ThreadSP &thread_sp : threads) { - // Register this thread as stepping over the breakpoint + // Register this thread as stepping over the breakpoint. RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID()); // Set the plan to defer re-enabling (use callback instead). @@ -670,19 +673,16 @@ bool ThreadList::WillResume(RunDirection &direction) { } } - // Collect for batched vCont - pick the largest group - if (threads.size() > batched_step_threads.size()) { + // Pick the largest group for batched vCont. + if (threads.size() > batched_step_threads.size()) batched_step_threads = threads; - } } - // Single thread at breakpoint - keeps default behavior (re-enable - // directly) + // Keeps default behavior for a single thread at breakpoint. } - // If we found a batch, use the first thread as thread_to_run - if (!batched_step_threads.empty()) { + // 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]; - } } } @@ -705,19 +705,18 @@ bool ThreadList::WillResume(RunDirection &direction) { 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) { + llvm::DenseSet<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 + // 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 + // Suspend it since it's not in the batch. thread_sp->ShouldResume(eStateSuspended); } } @@ -929,7 +928,7 @@ void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, 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 + // No threads registered for this breakpoint, re-enable directly. LLDB_LOGF(log, "ThreadList::%s: Thread 0x%" PRIx64 " finished stepping over breakpoint at 0x%" PRIx64 @@ -937,13 +936,12 @@ void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, __FUNCTION__, tid, breakpoint_addr); BreakpointSiteSP bp_site_sp( m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr)); - if (bp_site_sp) { + if (bp_site_sp) m_process.EnableBreakpointSite(bp_site_sp.get()); - } return; } - // Remove this thread from the set + // Remove this thread from the set. it->second.erase(tid); LLDB_LOGF(log, @@ -952,7 +950,7 @@ void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, " (%zu threads remaining)", __FUNCTION__, tid, breakpoint_addr, it->second.size()); - // If no more threads are stepping over this breakpoint, re-enable it + // 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 " @@ -961,11 +959,10 @@ void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, BreakpointSiteSP bp_site_sp( m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr)); - if (bp_site_sp) { + if (bp_site_sp) m_process.EnableBreakpointSite(bp_site_sp.get()); - } - // Clean up the entry + // 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 70538e21153b6..f7655d200641a 100644 --- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -163,12 +163,11 @@ void ThreadPlanStepOverBreakpoint::ReenableBreakpointSite() { m_process.GetThreadList().ThreadFinishedSteppingOverBreakpoint( m_breakpoint_addr, GetThread().GetID()); } else { - // Default behavior: re-enable the breakpoint directly + // Default behavior: re-enable the breakpoint directly. BreakpointSiteSP bp_site_sp( m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr)); - if (bp_site_sp) { + if (bp_site_sp) m_process.EnableBreakpointSite(bp_site_sp.get()); - } } } } >From aed827d987220ac23d53b7c59fa9c60f06e6c8d6 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Tue, 10 Feb 2026 12:36:31 -0800 Subject: [PATCH 11/12] fixed failing test and format --- lldb/source/Target/ThreadList.cpp | 14 ++++++++------ .../TestBatchedBreakpointStepOver.py | 6 +++--- .../TestConcurrentBatchedBreakpointStepOver.py | 8 ++++---- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 6cb655c87d9f0..7207128881220 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -913,11 +913,12 @@ void ThreadList::RegisterThreadSteppingOverBreakpoint(addr_t breakpoint_addr, 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()); + LLDB_LOGF( + log, + "ThreadList::%s: Registered thread 0x%" PRIx64 + " stepping over breakpoint at 0x%" PRIx64 " (now %zu threads)", + __FUNCTION__, tid, breakpoint_addr, + static_cast<size_t>(m_threads_stepping_over_bp[breakpoint_addr].size())); } void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, @@ -948,7 +949,8 @@ void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr, "ThreadList::%s: Thread 0x%" PRIx64 " finished stepping over breakpoint at 0x%" PRIx64 " (%zu threads remaining)", - __FUNCTION__, tid, breakpoint_addr, it->second.size()); + __FUNCTION__, tid, breakpoint_addr, + static_cast<size_t>(it->second.size())); // If no more threads are stepping over this breakpoint, re-enable it. if (it->second.empty()) { diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py index 4954037565c76..953294a77f658 100644 --- a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py @@ -29,7 +29,7 @@ class MyResponder(MockGDBServerResponder): def __init__(self): MockGDBServerResponder.__init__(self) self.resume_count = 0 - # Track which threads have completed their step + # Track which threads have completed their step. self.stepped_threads = set() def qSupported(self, client_supported): @@ -133,7 +133,7 @@ def _handle_vCont(self, packet): report_tid = stepping_tids[0] if stepping_tids else TIDS[0] threads_str = ",".join("{:x}".format(t) for t in TIDS) if all_done: - # All threads moved past breakpoint + # All threads moved past breakpoint. pcs_str = ",".join("{:x}".format(STEPPED_PC) for _ in TIDS) else: # Stepped threads moved, others still at breakpoint. @@ -161,7 +161,7 @@ def _handle_vCont(self, packet): # Continue, LLDB should step all threads over the breakpoint. process.Continue() - # Collect packets from the log + # Collect packets from the log. received = self.server.responder.packetLog.get_received() bp_addr_hex = "{:x}".format(BP_ADDR) diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py index 51bc40b853eb9..83bba388ecc7e 100644 --- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py +++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py @@ -65,7 +65,7 @@ def test(self): # 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. + # 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]+)", @@ -78,9 +78,9 @@ def test(self): "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 + # 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 "" >From cf43ca61bd4011fa488feeac4941ade657d1ef2c Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Tue, 17 Feb 2026 16:04:36 -0800 Subject: [PATCH 12/12] fix recent comments --- .../lldb/Target/ThreadPlanStepOverBreakpoint.h | 2 +- lldb/source/Target/ThreadList.cpp | 6 ++---- .../Target/ThreadPlanStepOverBreakpoint.cpp | 16 ++++++++-------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h index 0f0bbc0be4142..6537ac91af449 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h @@ -65,7 +65,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; + bool m_defer_reenable_breakpoint_site; ThreadPlanStepOverBreakpoint(const ThreadPlanStepOverBreakpoint &) = delete; const ThreadPlanStepOverBreakpoint & diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp index 7207128881220..f71d42b71e16b 100644 --- a/lldb/source/Target/ThreadList.cpp +++ b/lldb/source/Target/ThreadList.cpp @@ -512,8 +512,7 @@ bool ThreadList::WillResume(RunDirection &direction) { // recreated fresh by SetupToStepOverBreakpointIfNeeded below, and the // batching logic will recompute deferred state from scratch. m_threads_stepping_over_bp.clear(); - for (pos = m_threads.begin(); pos != end; ++pos) { - ThreadSP thread_sp(*pos); + for (const auto &thread_sp : m_threads) { ThreadPlan *plan = thread_sp->GetCurrentPlan(); if (plan && plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) { // Suppress the re-enable side effect in DidPop() — the breakpoint @@ -709,8 +708,7 @@ bool ThreadList::WillResume(RunDirection &direction) { 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); + for (const auto &thread_sp : m_threads) { 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())) diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp index f7655d200641a..c3347263595c9 100644 --- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp +++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp @@ -22,14 +22,14 @@ using namespace lldb_private; // the pc. ThreadPlanStepOverBreakpoint::ThreadPlanStepOverBreakpoint(Thread &thread) - : ThreadPlan( - ThreadPlan::eKindStepOverBreakpoint, "Step over breakpoint trap", - thread, eVoteNo, - eVoteNoOpinion), // We need to report the run since this happens - // first in the thread plan stack when stepping over - // a breakpoint - m_breakpoint_addr(LLDB_INVALID_ADDRESS), - m_auto_continue(false), m_reenabled_breakpoint_site(false) + : ThreadPlan(ThreadPlan::eKindStepOverBreakpoint, + "Step over breakpoint trap", thread, eVoteNo, + eVoteNoOpinion), // We need to report the run since this + // happens first in the thread plan stack when + // stepping over a breakpoint + m_breakpoint_addr(LLDB_INVALID_ADDRESS), m_auto_continue(false), + m_reenabled_breakpoint_site(false), + m_defer_reenable_breakpoint_site(false) { m_breakpoint_addr = thread.GetRegisterContext()->GetPC(); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
