Author: Igor Kudrin Date: 2026-01-08T14:11:35-08:00 New Revision: f59d12001fd877e44e25f260db888a352d5ab755
URL: https://github.com/llvm/llvm-project/commit/f59d12001fd877e44e25f260db888a352d5ab755 DIFF: https://github.com/llvm/llvm-project/commit/f59d12001fd877e44e25f260db888a352d5ab755.diff LOG: [lldb] Keep the unexpected b/p state for suspended threads (#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. Added: Modified: lldb/source/Target/Thread.cpp lldb/unittests/Thread/ThreadTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index d1127bb2ded30..ed65e304fc5f2 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -744,7 +744,16 @@ bool Thread::ShouldResume(StateType resume_state) { if (need_to_resume) { ClearStackFrames(); - m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS; + + // Only reset m_stopped_at_unexecuted_bp if the thread is actually being + // resumed. Otherwise, the state of a suspended thread may not be restored + // correctly at the next stop. For example, this could happen if the thread + // is suspended by ThreadPlanStepOverBreakpoint in another thread, which + // temporarily disables the breakpoint that the suspended thread has reached + // but not yet executed. + if (resume_state != eStateSuspended) + 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
