https://github.com/medismailben created https://github.com/llvm/llvm-project/pull/181409
StopCondition::SetText was computing the hash from the moved-from text parameter instead of the stored m_text member. After std::move(text), the source parameter becomes empty, causing the hash to always be computed from an empty string. This caused breakpoint condition updates to fail silently. When a user modified a condition (e.g., from "x == y" to "x > y"), the hash remained unchanged. Breakpoint locations use this hash to detect when conditions need re-evaluation, so with a stale hash they would continue using cached state for the old condition, triggering at incorrect locations. The patch fixes this issue by computing the hash from m_text after the move operation, ensuring it reflects the actual stored condition text. It also adds API test that updates a breakpoint condition and verifies the new condition is properly evaluated. rdar://170191229 >From f90b62b5a4e32445f9d127fe2565d6d12f2ba51a Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Fri, 13 Feb 2026 11:57:29 -0800 Subject: [PATCH] [lldb/Breakpoint] Fix condition hash after updating condition text StopCondition::SetText was computing the hash from the moved-from text parameter instead of the stored m_text member. After std::move(text), the source parameter becomes empty, causing the hash to always be computed from an empty string. This caused breakpoint condition updates to fail silently. When a user modified a condition (e.g., from "x == y" to "x > y"), the hash remained unchanged. Breakpoint locations use this hash to detect when conditions need re-evaluation, so with a stale hash they would continue using cached state for the old condition, triggering at incorrect locations. The patch fixes this issue by computing the hash from m_text after the move operation, ensuring it reflects the actual stored condition text. It also adds API test that updates a breakpoint condition and verifies the new condition is properly evaluated. rdar://170191229 Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Breakpoint/StopCondition.h | 2 +- .../breakpoint/update_condition/Makefile | 3 + .../TestUpdateBreakpointCondition.py | 127 ++++++++++++++++++ .../breakpoint/update_condition/main.c | 16 +++ 4 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 lldb/test/API/functionalities/breakpoint/update_condition/Makefile create mode 100644 lldb/test/API/functionalities/breakpoint/update_condition/TestUpdateBreakpointCondition.py create mode 100644 lldb/test/API/functionalities/breakpoint/update_condition/main.c diff --git a/lldb/include/lldb/Breakpoint/StopCondition.h b/lldb/include/lldb/Breakpoint/StopCondition.h index 485a615368400..9106d57435af5 100644 --- a/lldb/include/lldb/Breakpoint/StopCondition.h +++ b/lldb/include/lldb/Breakpoint/StopCondition.h @@ -30,7 +30,7 @@ class StopCondition { void SetText(std::string text) { static std::hash<std::string> hasher; m_text = std::move(text); - m_hash = hasher(text); + m_hash = hasher(m_text); } size_t GetHash() const { return m_hash; } diff --git a/lldb/test/API/functionalities/breakpoint/update_condition/Makefile b/lldb/test/API/functionalities/breakpoint/update_condition/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/update_condition/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/update_condition/TestUpdateBreakpointCondition.py b/lldb/test/API/functionalities/breakpoint/update_condition/TestUpdateBreakpointCondition.py new file mode 100644 index 0000000000000..ec23b8876638a --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/update_condition/TestUpdateBreakpointCondition.py @@ -0,0 +1,127 @@ +""" +Test that updating a breakpoint condition correctly invalidates cached state. + +This test verifies that when a breakpoint condition is changed, the new condition +is properly evaluated. Previously, due to a bug in StopCondition::SetText where +the hash was computed from a moved-from string, updating conditions could fail +to invalidate cached condition state at breakpoint locations. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class UpdateBreakpointConditionTestCase(TestBase): + + def setUp(self): + TestBase.setUp(self) + self.source = "main.c" + + @add_test_categories(["pyapi"]) + def test_update_condition_python_api(self): + """Test that updating a breakpoint condition works correctly using Python API.""" + self.build() + target, process, thread, breakpoint = lldbutil.run_to_source_breakpoint( + self, "Set breakpoint here", lldb.SBFileSpec(self.source) + ) + + # Set initial condition: x == y + breakpoint.SetCondition("x == y") + self.assertEqual(breakpoint.GetCondition(), "x == y") + + # Need to continue since we're already stopped, but the condition wasn't set initially + # First hit should be at foo(5, 5) where x == y + process.Continue() + thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue(thread.IsValid(), "Should stop at first x == y condition") + + frame = thread.GetFrameAtIndex(0) + x_val = frame.FindVariable("x") + y_val = frame.FindVariable("y") + self.assertEqual(x_val.GetValueAsSigned(), 5, "x should be 5") + self.assertEqual(y_val.GetValueAsSigned(), 5, "y should be 5") + self.assertEqual(breakpoint.GetHitCount(), 2) + + # Continue to second hit with x == y + process.Continue() + thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue(thread.IsValid(), "Should stop at second x == y condition") + + frame = thread.GetFrameAtIndex(0) + x_val = frame.FindVariable("x") + y_val = frame.FindVariable("y") + self.assertEqual(x_val.GetValueAsSigned(), 6, "x should be 6") + self.assertEqual(y_val.GetValueAsSigned(), 6, "y should be 6") + self.assertEqual(breakpoint.GetHitCount(), 3) + + # Now update the condition to x > y + # This tests the fix for the bug where the hash wasn't updated correctly + breakpoint.SetCondition("x > y") + self.assertEqual(breakpoint.GetCondition(), "x > y") + + # Continue - should now hit at foo(3, 1) where x > y (3 > 1) + # Without the fix, it would incorrectly hit at foo(7, 7) due to stale condition hash + process.Continue() + thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue(thread.IsValid(), "Should stop at x > y condition") + + frame = thread.GetFrameAtIndex(0) + x_val = frame.FindVariable("x") + y_val = frame.FindVariable("y") + self.assertEqual(x_val.GetValueAsSigned(), 3, "x should be 3") + self.assertEqual(y_val.GetValueAsSigned(), 1, "y should be 1") + self.assertTrue( + x_val.GetValueAsSigned() > y_val.GetValueAsSigned(), + "Condition x > y should be true", + ) + self.assertEqual(breakpoint.GetHitCount(), 4) + + def test_update_condition_command(self): + """Test that updating a breakpoint condition works correctly using breakpoint modify.""" + self.build() + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( + self, "Set breakpoint here", lldb.SBFileSpec(self.source) + ) + + # Set initial condition: x == y + self.runCmd("breakpoint modify -c 'x == y' 1") + self.expect( + "breakpoint list", + substrs=["Condition: x == y"], + ) + + # Continue to first hit at foo(5, 5) + self.runCmd("continue") + self.expect("process status", PROCESS_STOPPED, patterns=["Process .* stopped"]) + self.expect( + "frame variable x y", + substrs=["x = 5", "y = 5"], + ) + + # Continue to second hit + self.runCmd("continue") + self.expect("process status", PROCESS_STOPPED, patterns=["Process .* stopped"]) + self.expect( + "frame variable x y", + substrs=["x = 6", "y = 6"], + ) + + # Update condition to x > y + self.runCmd("breakpoint modify -c 'x > y' 1") + self.expect( + "breakpoint list", + substrs=["Condition: x > y"], + ) + + # Continue - should hit at foo(3, 1) where x > y + self.runCmd("continue") + self.expect("process status", PROCESS_STOPPED, patterns=["Process .* stopped"]) + self.expect( + "frame variable x y", + substrs=["x = 3", "y = 1"], + ) + + # Verify x > y is actually true + self.expect("expr x > y", substrs=["true"]) diff --git a/lldb/test/API/functionalities/breakpoint/update_condition/main.c b/lldb/test/API/functionalities/breakpoint/update_condition/main.c new file mode 100644 index 0000000000000..7900f9fff8bb1 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/update_condition/main.c @@ -0,0 +1,16 @@ +int foo(int x, int y) { + return x - y + 5; // Set breakpoint here. +} + +int main() { + foo(1, 4); + foo(5, 1); + foo(5, 5); + foo(3, -1); + foo(6, 6); + foo(7, 7); + foo(1, 3); + foo(3, 1); + + return 0; +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
