From 17b475b7cc48ff1fd9eceffdaa13e09043cbeec5 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan@apple.com>
Date: Thu, 27 Nov 2025 12:41:18 +0000
Subject: [PATCH] [lldb] Fix ThreadPlanStepOut::DoPlanExplainsStop inspection
 of BreakpointSite

Suppose two threads are performing the exact same step out plan.
They will both have an internal breakpoint set at their parent frame.
Now supposed both of those breakpoints are in the same address (i.e. the
same BreakpointSite).

At the end of `ThreadPlanStepOut::DoPlanExplainsStop`, we see this:

```
// If there was only one owner, then we're done.  But if we also hit
// some user breakpoint on our way out, we should mark ourselves as
// done, but also not claim to explain the stop, since it is more
// important to report the user breakpoint than the step out
// completion.

if (site_sp->GetNumberOfConstituents() == 1)
  return true;
```

In other words, the plan looks at the name number of constituents of the
site to decide whether it explains the stop, the logic being that a
_user_ might have put a breakpoint there. However, the implementation is
not correct; in particular, it will fail in the situation described
above. We should only care about non-internal breakpoints that would
stop for the current thread.

It is tricky to test this, as it depends on the timing of threads, but I
was able to consistently reproduce the issue with a swift program using
concurrency.
---
 lldb/source/Target/ThreadPlanStepOut.cpp | 27 ++++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp
index d49a01bdbcef7..7970e731aa972 100644
--- a/lldb/source/Target/ThreadPlanStepOut.cpp
+++ b/lldb/source/Target/ThreadPlanStepOut.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Target/ThreadPlanStepOut.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Breakpoint/BreakpointLocation.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Symbol/Block.h"
 #include "lldb/Symbol/Function.h"
@@ -306,6 +307,21 @@ bool ThreadPlanStepOut::ValidatePlan(Stream *error) {
   return true;
 }
 
+/// Returns true if :
+/// 1. All breakpoints in this site are internal, and
+/// 2. All breakpoint locations in this site are NOT valid for `thread`.
+static bool NoUserBreakpointsHere(BreakpointSite &site, Thread &thread) {
+  for (unsigned bp_idx = 0; bp_idx < site.GetNumberOfConstituents(); bp_idx++) {
+    BreakpointLocation &bp_loc = *site.GetConstituentAtIndex(bp_idx);
+    const Breakpoint &bp = bp_loc.GetBreakpoint();
+    if (bp.IsInternal())
+      continue;
+    if (bp_loc.ValidForThisThread(thread))
+      return false;
+  }
+  return true;
+}
+
 bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) {
   // If the step out plan is done, then we just need to step through the
   // inlined frame.
@@ -356,13 +372,10 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) {
           }
         }
 
-        // If there was only one owner, then we're done.  But if we also hit
-        // some user breakpoint on our way out, we should mark ourselves as
-        // done, but also not claim to explain the stop, since it is more
-        // important to report the user breakpoint than the step out
-        // completion.
-
-        if (site_sp->GetNumberOfConstituents() == 1)
+        // If the thread also hit a user breakpoint on its way out, the plan is
+        // done but should not claim to explain the stop. It is more important
+        // to report the user breakpoint than the step out completion.
+        if (NoUserBreakpointsHere(*site_sp, GetThread()))
           return true;
       }
       return false;