This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbcce8e0fccc1: Fix the logic so stop-hooks get run after a 
breakpoint that ran an expression (authored by jingham).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D106514?vs=360661&id=360990#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106514/new/

https://reviews.llvm.org/D106514

Files:
  lldb/include/lldb/Target/Target.h
  lldb/source/Target/Target.cpp
  lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
  lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
  lldb/test/API/commands/target/stop-hooks/main.c

Index: lldb/test/API/commands/target/stop-hooks/main.c
===================================================================
--- lldb/test/API/commands/target/stop-hooks/main.c
+++ lldb/test/API/commands/target/stop-hooks/main.c
@@ -7,9 +7,15 @@
   return g_var; // Set a breakpoint here and step out.
 }
 
+void
+increment_gvar() {
+  g_var++;
+}
+
 int
 main()
 {
   int result = step_out_of_me(); // Stop here first
+  increment_gvar(); // Continue to here
   return result;
 }
Index: lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
===================================================================
--- lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
+++ lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
@@ -29,6 +29,11 @@
         """Test that stop hooks fire on step-out."""
         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"""
+        self.after_expr_test()
+
     def step_out_test(self):
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                    "Set a breakpoint here", self.main_source_file)
@@ -42,3 +47,44 @@
         self.assertTrue(var.IsValid())
         self.assertEqual(var.GetValueAsUnsigned(), 1, "Updated g_var")
 
+    def after_expr_test(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)
+
+        var = target.FindFirstGlobalVariable("g_var")
+        self.assertTrue(var.IsValid())
+        self.assertEqual(var.GetValueAsUnsigned(), 1, "Updated g_var")
+
+        bkpt = target.BreakpointCreateBySourceRegex("Continue to here", self.main_source_file)
+        self.assertNotEqual(bkpt.GetNumLocations(), 0, "Set the second breakpoint")
+        commands = lldb.SBStringList()
+        commands.AppendString("expr increment_gvar()")
+        bkpt.SetCommandLineCommands(commands);
+        
+        threads = lldbutil.continue_to_breakpoint(process, bkpt)
+        self.assertEqual(len(threads), 1, "Hit my breakpoint")
+        
+        self.assertTrue(var.IsValid())
+        self.assertEqual(var.GetValueAsUnsigned(), 3, "Updated g_var")
+
+        # Make sure running an expression does NOT run the stop hook.
+        # Our expression will increment it by one, but the stop shouldn't
+        # have gotten it to 5.
+        threads[0].frames[0].EvaluateExpression("increment_gvar()")
+        self.assertTrue(var.IsValid())
+        self.assertEqual(var.GetValueAsUnsigned(), 4, "Updated g_var")
+        
+
+        # Make sure a rerun doesn't upset the state we've set up:
+        process.Kill()
+        lldbutil.run_to_breakpoint_do_run(self, target, first_bkpt)
+        var = target.FindFirstGlobalVariable("g_var")
+        self.assertTrue(var.IsValid())
+        self.assertEqual(var.GetValueAsUnsigned(), 1, "Updated g_var")
+        
+        
Index: lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
===================================================================
--- lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
+++ lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
@@ -96,12 +96,12 @@
         # Now set the breakpoint on step_out_of_me, and make sure we run the
         # expression, then continue back to main.
         bkpt = target.BreakpointCreateBySourceRegex("Set a breakpoint here and step out", self.main_source_file)
-        self.assertTrue(bkpt.GetNumLocations() > 0, "Got breakpoints in step_out_of_me")
+        self.assertNotEqual(bkpt.GetNumLocations(), 0, "Got breakpoints in step_out_of_me")
         process.Continue()
 
         var = target.FindFirstGlobalVariable("g_var")
         self.assertTrue(var.IsValid())
-        self.assertEqual(var.GetValueAsUnsigned(), 5, "Updated g_var")
+        self.assertEqual(var.GetValueAsUnsigned(), 6, "Updated g_var")
         
         func_name = process.GetSelectedThread().frames[0].GetFunctionName()
         self.assertEqual("main", func_name, "Didn't stop at the expected function.")
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -95,6 +95,7 @@
       m_watchpoint_list(), m_process_sp(), m_search_filter_sp(),
       m_image_search_paths(ImageSearchPathsChanged, this),
       m_source_manager_up(), m_stop_hooks(), m_stop_hook_next_id(0),
+      m_latest_stop_hook_id(0),
       m_valid(true), m_suppress_stop_hooks(false),
       m_is_dummy_target(is_dummy_target),
       m_frame_recognizer_manager_up(
@@ -181,6 +182,7 @@
   DisableAllWatchpoints(false);
   ClearAllWatchpointHitCounts();
   ClearAllWatchpointHistoricValues();
+  m_latest_stop_hook_id = 0;
 }
 
 void Target::DeleteCurrentProcess() {
@@ -2609,12 +2611,6 @@
   if (m_process_sp->GetState() != eStateStopped)
     return false;
 
-  // <rdar://problem/12027563> make sure we check that we are not stopped
-  // because of us running a user expression since in that case we do not want
-  // to run the stop-hooks
-  if (m_process_sp->GetModIDRef().IsLastResumeForUserExpression())
-    return false;
-
   if (m_stop_hooks.empty())
     return false;
 
@@ -2629,6 +2625,18 @@
   if (!any_active_hooks)
     return false;
 
+  // <rdar://problem/12027563> make sure we check that we are not stopped
+  // because of us running a user expression since in that case we do not want
+  // to run the stop-hooks.  Note, you can't just check whether the last stop
+  // was for a User Expression, because breakpoint commands get run before
+  // stop hooks, and one of them might have run an expression.  You have
+  // to ensure you run the stop hooks once per natural stop.
+  uint32_t last_natural_stop = m_process_sp->GetModIDRef().GetLastNaturalStopID();
+  if (last_natural_stop != 0 && m_latest_stop_hook_id == last_natural_stop)
+    return false;
+
+  m_latest_stop_hook_id = last_natural_stop;
+
   std::vector<ExecutionContext> exc_ctx_with_reasons;
 
   ThreadList &cur_threadlist = m_process_sp->GetThreadList();
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1429,8 +1429,10 @@
   typedef std::map<lldb::user_id_t, StopHookSP> StopHookCollection;
   StopHookCollection m_stop_hooks;
   lldb::user_id_t m_stop_hook_next_id;
+  uint32_t m_latest_stop_hook_id; /// This records the last natural stop at
+                                  /// which we ran a stop-hook.
   bool m_valid;
-  bool m_suppress_stop_hooks;
+  bool m_suppress_stop_hooks; /// Used to not run stop hooks for expressions
   bool m_is_dummy_target;
   unsigned m_next_persistent_variable_index = 0;
   /// An optional \a lldb_private::Trace object containing processor trace
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to