[Lldb-commits] [lldb] [lldb] Remove unfiltered stop reason propagation from StopInfoMachException (PR #122817)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/122817

>From b42dc01256c8a433629d3594d54e942417705f83 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Wed, 18 Dec 2024 14:47:08 -0800
Subject: [PATCH 1/3] [lldb] Remove unfiltered stop reason propagation from
 StopInfoMachException

In the presence of OS plugins, StopInfoMachException currently
propagates breakpoint stop reasons even if those breakpoints were not
intended for a specific thread, effectively removing our ability to set
thread-specific breakpoints.

This was originally added in [1], but the motivation provided in the
comment does not seem strong enough to remove the ability to set
thread-specific breakpoints. The only way to break thread specific
breakpoints would be if a user set such a breakpoint and _then_ loaded
an OS plugin, a scenario which we would likely not want to support.

[1]: https://github.com/swiftlang/llvm-project/commit/ab745c2ad865c07f3905482fd071ef36c024713a#diff-8ec6e41b1dffa7ac4b5841aae24d66442ef7ebc62c8618f89354d84594f91050R501
---
 .../Process/Utility/StopInfoMachException.cpp       |  9 ++-------
 .../plugins/python_os_plugin/TestPythonOSPlugin.py  | 13 +++++++++++++
 .../functionalities/plugins/python_os_plugin/main.c |  2 ++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index d08e99f4844de3..698ba0f0f720f6 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -780,13 +780,8 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
         // but if it is for another thread, we can just report no reason.  We
         // don't need to worry about stepping over the breakpoint here, that
         // will be taken care of when the thread resumes and notices that
-        // there's a breakpoint under the pc. If we have an operating system
-        // plug-in, we might have set a thread specific breakpoint using the
-        // operating system thread ID, so we can't make any assumptions about
-        // the thread ID so we must always report the breakpoint regardless
-        // of the thread.
-        if (bp_site_sp->ValidForThisThread(thread) ||
-            thread.GetProcess()->GetOperatingSystem() != nullptr)
+        // there's a breakpoint under the pc.
+        if (bp_site_sp->ValidForThisThread(thread))
           return StopInfo::CreateStopReasonWithBreakpointSiteID(
               thread, bp_site_sp->GetID());
         else if (is_trace_if_actual_breakpoint_missing)
diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py b/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
index 479c94c2315407..4b8471d1cc32be 100644
--- a/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
+++ b/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
@@ -219,3 +219,16 @@ def run_python_os_step(self):
             6,
             "Make sure we stepped from line 5 to line 6 in main.c",
         )
+
+        thread_bp_number = lldbutil.run_break_set_by_source_regexp(
+            self, "Set tid-specific breakpoint here", num_expected_locations=1
+        )
+        breakpoint = target.FindBreakpointByID(thread_bp_number)
+        # This breakpoint should not be hit.
+        breakpoint.SetThreadID(123)
+        process.Continue()
+
+        frame = thread.GetFrameAtIndex(0)
+        self.assertFalse(
+            frame.IsValid(), "Should not stop, the breakpoint was not for this thread."
+        )
diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/main.c b/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
index faa6dd58ecd62b..248ecc8f650d2a 100644
--- a/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
+++ b/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
@@ -3,5 +3,7 @@
 int main (int argc, char const *argv[], char const *envp[])
 {
     puts("stop here"); // Set breakpoint here
+    puts("hello");
+    puts("don't stop here on tid-specific breakpoint"); // Set tid-specific breakpoint here
     return 0;
 }

>From 2050c68df4edc32bb24d6bb0123f0ab99c6dcd94 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Mon, 13 Jan 2025 16:06:21 -0800
Subject: [PATCH 2/3] fixup! change regex string in test

---
 lldb/test/API/functionalities/plugins/python_os_plugin/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/main.c b/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
index 248ecc8f650d2a..deb80027c5e6c2 100644
--- a/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
+++ b/lldb/test/API/functionalities/plugins/python_os_plugin/main.c
@@ -4,6 +4,6 @@ int main (int argc, char const *argv[], char const *envp[])
 {
     puts("stop here"); // Set breakpoint here
     puts("hello");
-    puts("don't stop here on tid-specific breakpoint"); // Set tid-specific breakpoint here
+    puts("Set tid-specific breakpoint here");
     return 0;
 }

>From 67043ae8b7c8282e209ef04e04cbc04cd3c45710 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 14 Jan 2025 08:19:43 -0800
Subject: [PATCH 3/3] fixup! Check process exited instead of valid frame for
 thread

---
 .../plugins/python_os_plugin/TestPythonOSPlugin.py          | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py b/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
index 4b8471d1cc32be..3ad7539018d5d8 100644
--- a/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
+++ b/lldb/test/API/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py
@@ -227,8 +227,4 @@ def run_python_os_step(self):
         # This breakpoint should not be hit.
         breakpoint.SetThreadID(123)
         process.Continue()
-
-        frame = thread.GetFrameAtIndex(0)
-        self.assertFalse(
-            frame.IsValid(), "Should not stop, the breakpoint was not for this thread."
-        )
+        self.assertState(process.GetState(), lldb.eStateExited)