From b42dc01256c8a433629d3594d54e942417705f83 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan 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 d08e99f4844de..698ba0f0f720f 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 479c94c231540..4b8471d1cc32b 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 faa6dd58ecd62..248ecc8f650d2 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 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 248ecc8f650d2..deb80027c5e6c 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 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 4b8471d1cc32b..3ad7539018d5d 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)