[Lldb-commits] [lldb] bcce8e0 - Fix the logic so stop-hooks get run after a breakpoint that ran an expression
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 22 15:06:48 PDT 2021
Author: Jim Ingham
Date: 2021-07-22T15:06:41-07:00
New Revision: bcce8e0fccc1d6c2f18ade0ffa7039fb705bade2
URL: https://github.com/llvm/llvm-project/commit/bcce8e0fccc1d6c2f18ade0ffa7039fb705bade2
DIFF: https://github.com/llvm/llvm-project/commit/bcce8e0fccc1d6c2f18ade0ffa7039fb705bade2.diff
LOG: Fix the logic so stop-hooks get run after a breakpoint that ran an expression
Code was added to Target::RunStopHook to make sure that we don't run stop hooks when
you stop after an expression evaluation. But the way it was done was to check that we
hadn't run an expression since the last natural stop. That failed in the case where you
stopped for a breakpoint which had run an expression, because the stop-hooks get run
after the breakpoint actions, and so by the time we got to running the stop-hooks,
we had already run a user expression.
I fixed this by adding a target ivar tracking the last natural stop ID at which we had
run a stop-hook. Then we keep track of this and make sure we run the stop-hooks only
once per natural stop.
Differential Revision: https://reviews.llvm.org/D106514
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 0db5209cd1f35..ac8d002b09a12 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1429,8 +1429,10 @@ class Target : public std::enable_shared_from_this<Target>,
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
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 396f77dfedb83..1f8e8c54fa9e1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -95,6 +95,7 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch,
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 @@ void Target::CleanupProcess() {
DisableAllWatchpoints(false);
ClearAllWatchpointHitCounts();
ClearAllWatchpointHistoricValues();
+ m_latest_stop_hook_id = 0;
}
void Target::DeleteCurrentProcess() {
@@ -2609,12 +2611,6 @@ bool Target::RunStopHooks() {
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 @@ bool Target::RunStopHooks() {
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();
diff --git a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
index 7ef5a72b9f6fa..53c2c2e07a332 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py
@@ -96,12 +96,12 @@ def do_test_auto_continue(self, return_true):
# 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.")
diff --git a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
index 43447a845156d..6f5df860db57c 100644
--- a/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
+++ b/lldb/test/API/commands/target/stop-hooks/TestStopHooks.py
@@ -29,6 +29,11 @@ def test_stop_hooks_step_out(self):
"""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 @@ def step_out_test(self):
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")
+
+
diff --git a/lldb/test/API/commands/target/stop-hooks/main.c b/lldb/test/API/commands/target/stop-hooks/main.c
index 16bfc0ce5db6b..f760cd9598323 100644
--- a/lldb/test/API/commands/target/stop-hooks/main.c
+++ b/lldb/test/API/commands/target/stop-hooks/main.c
@@ -7,9 +7,15 @@ int step_out_of_me()
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;
}
More information about the lldb-commits
mailing list