Skip to content

Commit c21cd52

Browse files
authored
Fix a crash when a stop hook deletes itself in its callback. (#160416)
We're iterating over the stop hooks so if one of them changes the stop hook list by deleting itself or another stop hook, that invalidates our iterator. I chose to fix this by making a copy of the stop hook list and iterating over that. That's a cheap operation since this is just an array of shared pointers. But more importantly doing it this way means that if on a stop where one stop hook deletes another, the deleted hook will always get a chance to run. If you iterated over the list itself, then whether that to be deleted hook gets to run would be dependent on whether it was before or after the deleting stop hook, which would be confusing.
1 parent ec620bf commit c21cd52

File tree

4 files changed

+72
-0
lines changed

4 files changed

+72
-0
lines changed

lldb/source/Commands/CommandObjectTarget.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5121,6 +5121,15 @@ class CommandObjectTargetStopHookDelete : public CommandObjectParsed {
51215121
: CommandObjectParsed(interpreter, "target stop-hook delete",
51225122
"Delete a stop-hook.",
51235123
"target stop-hook delete [<idx>]") {
5124+
SetHelpLong(
5125+
R"(
5126+
Deletes the stop hook by index.
5127+
5128+
At any given stop, all enabled stop hooks that pass the stop filter will
5129+
get a chance to run. That means if one stop-hook deletes another stop hook
5130+
while executing, the deleted stop hook will still fire for the stop at which
5131+
it was deleted.
5132+
)");
51245133
AddSimpleArgumentList(eArgTypeStopHookID, eArgRepeatStar);
51255134
}
51265135

lldb/source/Target/Target.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3207,6 +3207,11 @@ bool Target::RunStopHooks(bool at_initial_stop) {
32073207
bool should_stop = false;
32083208
bool requested_continue = false;
32093209

3210+
// A stop hook might get deleted while running stop hooks.
3211+
// We have to decide what that means. We will follow the rule that deleting
3212+
// a stop hook while processing these stop hooks will delete it for FUTURE
3213+
// stops but not this stop. Fortunately, copying the m_stop_hooks to the
3214+
// active_hooks list before iterating over the hooks has this effect.
32103215
for (auto cur_hook_sp : active_hooks) {
32113216
bool any_thread_matched = false;
32123217
for (auto exc_ctx : exc_ctx_with_reasons) {

lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,39 @@ def test_bad_handler(self):
4848
"Got the right error",
4949
)
5050

51+
def test_self_deleting(self):
52+
"""Test that we can handle a stop hook that deletes itself"""
53+
self.script_setup()
54+
# Run to the first breakpoint before setting the stop hook
55+
# so we don't have to figure out where it showed up in the new
56+
# target.
57+
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
58+
self, "Stop here first", self.main_source_file
59+
)
60+
61+
# Now add our stop hook and register it:
62+
result = lldb.SBCommandReturnObject()
63+
command = "target stop-hook add -P stop_hook.self_deleting_stop"
64+
self.interp.HandleCommand(command, result)
65+
self.assertCommandReturn(result, f"Added my stop hook: {result.GetError()}")
66+
67+
result_str = result.GetOutput()
68+
p = re.compile("Stop hook #([0-9]+) added.")
69+
m = p.match(result_str)
70+
current_stop_hook_id = m.group(1)
71+
command = "command script add -o -f stop_hook.handle_stop_hook_id handle_id"
72+
self.interp.HandleCommand(command, result)
73+
self.assertCommandReturn(result, "Added my command")
74+
75+
command = f"handle_id {current_stop_hook_id}"
76+
self.interp.HandleCommand(command, result)
77+
self.assertCommandReturn(result, "Registered my stop ID")
78+
79+
# Now step the process and make sure the stop hook was deleted.
80+
thread.StepOver()
81+
self.interp.HandleCommand("target stop-hook list", result)
82+
self.assertEqual(result.GetOutput().rstrip(), "No stop hooks.", "Deleted hook")
83+
5184
def test_stop_hooks_scripted(self):
5285
"""Test that a scripted stop hook works with no specifiers"""
5386
self.stop_hooks_scripted(5, "-I false")

lldb/test/API/commands/target/stop-hooks/stop_hook.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,28 @@ def handle_stop(self):
4848
class no_handle_stop:
4949
def __init__(self, target, extra_args, dict):
5050
print("I am okay")
51+
52+
53+
class self_deleting_stop:
54+
def __init__(self, target, extra_args, dict):
55+
self.target = target
56+
57+
def handle_stop(self, exe_ctx, stream):
58+
interp = exe_ctx.target.debugger.GetCommandInterpreter()
59+
result = lldb.SBCommandReturnObject()
60+
interp.HandleCommand("handle_id", result)
61+
id_str = result.GetOutput().rstrip()
62+
63+
command = f"target stop-hook delete {id_str}"
64+
interp.HandleCommand(command, result)
65+
66+
67+
stop_hook_id = 0
68+
69+
70+
def handle_stop_hook_id(debugger, command, exe_ctx, result, extra_args):
71+
global stop_hook_id
72+
if command == "":
73+
result.AppendMessage(str(stop_hook_id))
74+
else:
75+
stop_hook_id = int(command)

0 commit comments

Comments
 (0)