https://github.com/igorkudrin created https://github.com/llvm/llvm-project/pull/174264
This fixes stepping out for a case when two threads reach the stepping-out target breakpoint simultaneously, but a concurrent thread executes the breakpoint first. The issue affects platforms with software breakpoints. The scenario is as follows: * The `step-out` command is executed for thread `A`. * `ThreadPlanStepOut` creates a breakpoint at the target location. * All threads are resumed, because the `step-out` command does not suspend other threads. * Threads `A` and `B` reach the stepping-out address at the same time, but `B` executes the breakpoint instruction first. * `SetThreadStoppedAtUnexecutedBP()` is called for thread `A`, and `SetThreadHitBreakpointSite()` is called for thread `B`. * Thread `B` has no plans to stop at this location, so `ThreadPlanStepOverBreakpoint` is scheduled. * The plan disables the breakpoint and resumes thread `B` with `eStateStepping`; for thread `A`, `ShouldResume(eStateSuspended)` is called, which clears `m_stopped_at_unexecuted_bp`. * After the stepping, `ThreadPlanStepOverBreakpoint` finishes, the breakpoint is re-enabled, and all threads are resumed. * At this moment, thread `A` is still at the location of the breakpoint, but `m_stopped_at_unexecuted_bp` is cleared, so `Thread::SetupToStepOverBreakpointIfNeeded()` schedules `ThreadPlanStepOverBreakpoint` for it. * `ThreadPlanStepOverBreakpoint` steps over the target breakpoint, so `ThreadPlanStepOut` can't catch the execution there. >From 023b50fe84d10b0723ab076d092c74dd88278c47 Mon Sep 17 00:00:00 2001 From: Igor Kudrin <[email protected]> Date: Fri, 2 Jan 2026 19:27:42 -0800 Subject: [PATCH] [lldb] Keep the unexpected b/p state for suspended threads This fixes stepping out for a case when two threads reach the stepping-out target breakpoint simultaneously, but a concurrent thread executes the breakpoint first. The issue affects platforms with software breakpoints. The scenario is as follows: * The `step-out` command is executed for thread `A`. * `ThreadPlanStepOut` creates a breakpoint at the target location. * All threads are resumed, because the `step-out` command does not suspend other threads. * Threads `A` and `B` reach the stepping-out address at the same time, but `B` executes the breakpoint instruction first. * `SetThreadStoppedAtUnexecutedBP()` is called for thread `A`, and `SetThreadHitBreakpointSite()` is called for thread `B`. * Thread `B` has no plans to stop at this location, so `ThreadPlanStepOverBreakpoint` is scheduled. * The plan disables the breakpoint and resumes thread `B` with `eStateStepping`; for thread `A`, `ShouldResume(eStateSuspended)` is called, which clears `m_stopped_at_unexecuted_bp`. * After the stepping, `ThreadPlanStepOverBreakpoint` finishes, the breakpoint is re-enabled, and all threads are resumed. * At this moment, thread `A` is still at the location of the breakpoint, but `m_stopped_at_unexecuted_bp` is cleared, so `Thread::SetupToStepOverBreakpointIfNeeded()` schedules `ThreadPlanStepOverBreakpoint` for it. * `ThreadPlanStepOverBreakpoint` steps over the target breakpoint, so `ThreadPlanStepOut` can't catch the execution there. --- lldb/source/Target/Thread.cpp | 2 +- lldb/unittests/Thread/ThreadTest.cpp | 54 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index b833918c27818..8856d3505c5f5 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -739,12 +739,12 @@ bool Thread::ShouldResume(StateType resume_state) { if (need_to_resume && resume_state != eStateSuspended) { m_stop_info_sp.reset(); + m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS; } } if (need_to_resume) { ClearStackFrames(); - m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS; // Let Thread subclasses do any special work they need to prior to resuming WillResume(resume_state); } diff --git a/lldb/unittests/Thread/ThreadTest.cpp b/lldb/unittests/Thread/ThreadTest.cpp index ef3a73cd3de87..4f4a5558db8df 100644 --- a/lldb/unittests/Thread/ThreadTest.cpp +++ b/lldb/unittests/Thread/ThreadTest.cpp @@ -24,6 +24,7 @@ #include "lldb/Host/HostThread.h" #include "lldb/Target/Process.h" #include "lldb/Target/StopInfo.h" +#include "lldb/Target/ThreadPlanBase.h" #include "lldb/Utility/ArchSpec.h" #include "gtest/gtest.h" @@ -105,6 +106,18 @@ class DummyThread : public Thread { bool CalculateStopInfo() override { return false; } bool IsStillAtLastBreakpointHit() override { return true; } + + using Thread::PushPlan; + + lldb::addr_t GetStoppedAtUnexpectedBP() const { + return m_stopped_at_unexecuted_bp; + } +}; + +class DummyThreadPlan : public ThreadPlanBase { +public: + DummyThreadPlan(Thread &thread) : ThreadPlanBase(thread) {} + ~DummyThreadPlan() override = default; }; } // namespace @@ -228,3 +241,44 @@ TEST_F(ThreadTest, GetPrivateStopInfo) { StopInfoSP new_stopinfo_sp = thread_sp->GetPrivateStopInfo(); ASSERT_TRUE(new_stopinfo_sp && stopinfo_sp->IsValid() == true); } + +TEST_F(ThreadTest, ShouldResume) { + ArchSpec arch("x86_64-pc-linux"); + + Platform::SetHostPlatform( + platform_linux::PlatformLinux::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + auto thread_sp = std::make_shared<DummyThread>(*process_sp.get(), 0); + ASSERT_TRUE(thread_sp); + + thread_sp->PushPlan(std::make_shared<DummyThreadPlan>(*thread_sp)); + ASSERT_TRUE(thread_sp->GetCurrentPlan()); + + const lldb::addr_t unexpected_bp_addr = 0x1234; + + // If a thread is resumed with 'eStateRunning' or 'eStateStepping', it should + // clear the address of an unexpected breakpoint. + thread_sp->SetThreadStoppedAtUnexecutedBP(unexpected_bp_addr); + EXPECT_TRUE(thread_sp->ShouldResume(eStateRunning)); + EXPECT_EQ(LLDB_INVALID_ADDRESS, thread_sp->GetStoppedAtUnexpectedBP()); + + thread_sp->SetThreadStoppedAtUnexecutedBP(unexpected_bp_addr); + EXPECT_TRUE(thread_sp->ShouldResume(eStateStepping)); + EXPECT_EQ(LLDB_INVALID_ADDRESS, thread_sp->GetStoppedAtUnexpectedBP()); + + // If a thread is resumed with 'eStateSuspended', the address of an unexpected + // breakpoint should be preserved. + thread_sp->SetThreadStoppedAtUnexecutedBP(unexpected_bp_addr); + EXPECT_TRUE(thread_sp->ShouldResume(eStateSuspended)); + EXPECT_EQ(unexpected_bp_addr, thread_sp->GetStoppedAtUnexpectedBP()); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
