Skip to content

Fix a bug copying the stop hooks from the dummy target. #129340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 3, 2025

Conversation

jimingham
Copy link
Collaborator

We didn't also copy over the next stop hook id, which meant we would overwrite the stop hooks from the dummy target with stop hooks set after they are copied over.

We didn't also copy over the next stop hook id, which meant we would
overwrite the stop hooks from the dummy target with stop hooks set
after they are copied over.
@llvmbot
Copy link
Member

llvmbot commented Feb 28, 2025

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

We didn't also copy over the next stop hook id, which meant we would overwrite the stop hooks from the dummy target with stop hooks set after they are copied over.


Full diff: https://github.com/llvm/llvm-project/pull/129340.diff

2 Files Affected:

  • (modified) lldb/source/Target/Target.cpp (+1)
  • (modified) lldb/test/API/commands/target/stop-hooks/TestStopHooks.py (+22-2)
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index db289fe9c4b64..550424720e095 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -211,6 +211,7 @@ Target::~Target() {
 
 void Target::PrimeFromDummyTarget(Target &target) {
   m_stop_hooks = target.m_stop_hooks;
+  m_stop_hook_next_id = target.m_stop_hook_next_id;
 
   for (const auto &breakpoint_sp : target.m_breakpoint_list.Breakpoints()) {
     if (breakpoint_sp->IsInternal())
diff --git a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
index fe59bd8a5d007..5215ec7258d14 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
@@ -26,10 +26,15 @@ def test_stop_hooks_step_out(self):
         self.step_out_test()
 
     def test_stop_hooks_after_expr(self):
-        """Test that a stop hook fires when hitting a breakpoint
-        that runs an expression"""
+        """Test that a stop hook fires when hitting a breakpoint that
+           runs an expression"""
         self.after_expr_test()
 
+    def test_stop_hooks_before_and_after_creation(self):
+        """Test that if we add a stop hook in the dummy target, we can
+           they don't collide with ones set directly in the target."""
+        self.before_and_after_target()
+
     def step_out_test(self):
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
             self, "Set a breakpoint here", self.main_source_file
@@ -85,3 +90,18 @@ def after_expr_test(self):
         var = target.FindFirstGlobalVariable("g_var")
         self.assertTrue(var.IsValid())
         self.assertEqual(var.GetValueAsUnsigned(), 1, "Updated g_var")
+
+    def before_and_after_target(self):
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        interp.HandleCommand("target stop-hook add -o 'expr g_var++'", result)
+        self.assertTrue(result.Succeeded(), "Set the target stop hook")
+
+        (target, process, thread, first_bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "Set a breakpoint here", self.main_source_file
+        )
+
+        interp.HandleCommand("target stop-hook add -o 'thread backtrace'", result)
+        self.assertTrue(result.Succeeded(), "Set the target stop hook")
+        self.expect("target stop-hook list", substrs=["expr g_var++", "thread backtrace"])
+

@jimingham jimingham requested a review from yln February 28, 2025 23:59
Copy link

github-actions bot commented Mar 1, 2025

✅ With the latest revision this PR passed the Python code formatter.

self.after_expr_test()

def test_stop_hooks_before_and_after_creation(self):
"""Test that if we add a stop hook in the dummy target, we can
they don't collide with ones set directly in the target."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot a word? we can they don't collide

Copy link
Collaborator

@yln yln left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jimingham jimingham merged commit 64c26c8 into llvm:main Mar 3, 2025
10 checks passed
@jimingham jimingham deleted the stop-hook-from-dummy branch March 3, 2025 18:30
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
We didn't also copy over the next stop hook id, which meant we would
overwrite the stop hooks from the dummy target with stop hooks set after
they are copied over.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants