[Lldb-commits] [lldb] Disable ThreadPlanSingleThreadTimeout during step over breakpoint (PR #104532)

[via lldb-commits]

https://github.com/jeffreytan81 created https://github.com/llvm/llvm-project/pull/104532

This PR fixes another potential race condition in https://github.com/llvm/llvm-project/pull/90930. The failure is found by @labath with this log: https://paste.debian.net/hidden/30235a5c/.

The investigation shows the failure happens when timeout is short and async interrupt is sent to lldb-server immediately after vCont so ptrace() resumes and then async interrupts debuggee immediately so debuggee does not get a chance to execute and move PC. So it enters stop mode immediately at original PC. `ThreadPlanStepOverBreakpoint` does not expect PC not moving and reports stop at the original place. 

To fix this, the PR prevents `ThreadPlanSingleThreadTimeout` from being created during `ThreadPlanStepOverBreakpoint` by introduces a new `SupportsResumeOthers()` method and `ThreadPlanStepOverBreakpoint` returns false for it. This makes sense because we should never resume threads during step over breakpoint anyway otherwise it might cause other threads to miss breakpoint. 




>From e86454235bf3bd20be1ddc394683d3f475fad218 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Thu, 15 Aug 2024 18:12:04 -0700
Subject: [PATCH] Fix async interrupt for step over breakpoint

---
 lldb/include/lldb/Target/ThreadPlan.h                   | 8 +++++++-
 lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h | 1 +
 lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp    | 6 ++++++
 lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp     | 6 ++++++
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index c336b6bb37df1b..d6da484f4fc137 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -385,7 +385,13 @@ class ThreadPlan : public std::enable_shared_from_this<ThreadPlan>,
   virtual void SetStopOthers(bool new_value);
 
   virtual bool StopOthers();
-  
+
+  // Returns true if the thread plan supports ThreadPlanSingleThreadTimeout to
+  // resume other threads after timeout. If the thread plan returns false it
+  // will prevent ThreadPlanSingleThreadTimeout from being created when this
+  // thread plan is alive.
+  virtual bool SupportsResumeOthers() { return true; }
+
   virtual bool ShouldRunBeforePublicStop() { return false; }
 
   // This is the wrapper for DoWillResume that does generic ThreadPlan logic,
diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
index 1f3aff45c49abe..0da8dbf44ffd8a 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
@@ -23,6 +23,7 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan {
   void GetDescription(Stream *s, lldb::DescriptionLevel level) override;
   bool ValidatePlan(Stream *error) override;
   bool ShouldStop(Event *event_ptr) override;
+  bool SupportsResumeOthers() override;
   bool StopOthers() override;
   lldb::StateType GetPlanRunState() override;
   bool WillStop() override;
diff --git a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
index 806ba95c508b7c..71be81365a2668 100644
--- a/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
+++ b/lldb/source/Target/ThreadPlanSingleThreadTimeout.cpp
@@ -76,6 +76,9 @@ void ThreadPlanSingleThreadTimeout::PushNewWithTimeout(Thread &thread,
   if (!thread.GetCurrentPlan()->StopOthers())
     return;
 
+  if (!thread.GetCurrentPlan()->SupportsResumeOthers())
+    return;
+
   auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info);
   ThreadPlanSP thread_plan_sp(timeout_plan);
   auto status = thread.QueueThreadPlan(thread_plan_sp,
@@ -102,6 +105,9 @@ void ThreadPlanSingleThreadTimeout::ResumeFromPrevState(Thread &thread,
   if (!thread.GetCurrentPlan()->StopOthers())
     return;
 
+  if (!thread.GetCurrentPlan()->SupportsResumeOthers())
+    return;
+
   auto timeout_plan = new ThreadPlanSingleThreadTimeout(thread, info);
   ThreadPlanSP thread_plan_sp(timeout_plan);
   auto status = thread.QueueThreadPlan(thread_plan_sp,
diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
index f88a2b895931cd..97c27ad4cd0493 100644
--- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
@@ -103,6 +103,12 @@ bool ThreadPlanStepOverBreakpoint::ShouldStop(Event *event_ptr) {
 
 bool ThreadPlanStepOverBreakpoint::StopOthers() { return true; }
 
+// The ThreadPlanSingleThreadTimeout can interrupt and resume all threads during
+// stepping, which may cause them to miss breakpoint. Therefore, we should
+// prevent the creation of ThreadPlanSingleThreadTimeout during a step-over
+// breakpoint.
+bool ThreadPlanStepOverBreakpoint::SupportsResumeOthers() { return false; }
+
 StateType ThreadPlanStepOverBreakpoint::GetPlanRunState() {
   return eStateStepping;
 }