[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.

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);
   }

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 @@ public:
   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());
+}