jingham created this revision.
jingham added reviewers: labath, jgorbe.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Auto-continuing a stop hook by adding a "continue" command to the hook didn't
work very well for multi-threaded programs. The first hook in kept all the
others from running, so the behavior was unpredictable.
Also, the stop hooks were running in synchronous mode, which meant that
'continue' in a stop hook would start nesting hook execution, which they were
not designed to do.
This patch forces Async execution for the stop-hooks, and adds an auto-continue
flag.
I also made it possible to add more than one command at a time to the stop hook
by repeating the -o command. I can't remember why I thought it was a good idea
originally to have only ONE --one-liner option here & in breakpoint commands,
etc. It's really convenient to be able to build these on a single line.
Also fixed up a few tests. The plain stop-hook tests were failing for me, even
though I didn't change any relevant output. Adding the
'interpreter.echo-comment-commands false' made them start passing again...
I didn't uncomment the UNSUPPORTED on Linux for stop-hook-threads.test, I'd
appreciate somebody giving this a whirl there first to see if this works now.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D58394
Files:
include/lldb/Target/Process.h
include/lldb/Target/Target.h
lit/ExecControl/StopHook/Inputs/stop-hook-1.lldbinit
lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
lit/ExecControl/StopHook/stop-hook-threads.test
source/Commands/CommandObjectTarget.cpp
source/Target/Process.cpp
source/Target/Target.cpp
Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2554,12 +2554,14 @@
StopHookCollection::iterator pos, end = m_stop_hooks.end();
- // If there aren't any active stop hooks, don't bother either:
+ // If there aren't any active stop hooks, don't bother either.
+ // Also see if any of the active hooks want to auto-continue.
bool any_active_hooks = false;
- for (pos = m_stop_hooks.begin(); pos != end; pos++) {
- if ((*pos).second->IsActive()) {
+ bool auto_continue = false;
+ for (auto hook : m_stop_hooks) {
+ if (hook.second->IsActive()) {
any_active_hooks = true;
- break;
+ auto_continue |= hook.second->GetAutoContinue();
}
}
if (!any_active_hooks)
@@ -2595,6 +2597,7 @@
bool hooks_ran = false;
bool print_hook_header = (m_stop_hooks.size() != 1);
bool print_thread_header = (num_exe_ctx != 1);
+ bool did_restart = false;
for (pos = m_stop_hooks.begin(); keep_going && pos != end; pos++) {
// result.Clear();
@@ -2639,10 +2642,13 @@
options.SetPrintResults(true);
options.SetAddToHistory(false);
+ // Force Async:
+ bool old_async = GetDebugger().GetAsyncExecution();
+ GetDebugger().SetAsyncExecution(true);
GetDebugger().GetCommandInterpreter().HandleCommands(
cur_hook_sp->GetCommands(), &exc_ctx_with_reasons[i], options,
result);
-
+ GetDebugger().SetAsyncExecution(old_async);
// If the command started the target going again, we should bag out of
// running the stop hooks.
if ((result.GetStatus() == eReturnStatusSuccessContinuingNoResult) ||
@@ -2651,13 +2657,19 @@
StopHookCollection::iterator tmp = pos;
if (++tmp != end)
result.AppendMessageWithFormat("\nAborting stop hooks, hook %" PRIu64
- " set the program running.\n",
+ " set the program running.\n"
+ " Consider using '-G true' to make "
+ "stop hooks auto-continue.\n",
cur_hook_sp->GetID());
keep_going = false;
+ did_restart = true;
}
}
}
}
+ // Finally, if auto-continue was requested, do it now:
+ if (!did_restart && auto_continue)
+ m_process_sp->PrivateResume();
result.GetImmediateOutputStream()->Flush();
result.GetImmediateErrorStream()->Flush();
@@ -3143,12 +3155,13 @@
//--------------------------------------------------------------
Target::StopHook::StopHook(lldb::TargetSP target_sp, lldb::user_id_t uid)
: UserID(uid), m_target_sp(target_sp), m_commands(), m_specifier_sp(),
- m_thread_spec_up(), m_active(true) {}
+ m_thread_spec_up() {}
Target::StopHook::StopHook(const StopHook &rhs)
: UserID(rhs.GetID()), m_target_sp(rhs.m_target_sp),
m_commands(rhs.m_commands), m_specifier_sp(rhs.m_specifier_sp),
- m_thread_spec_up(), m_active(rhs.m_active) {
+ m_thread_spec_up(), m_active(rhs.m_active),
+ m_auto_continue(rhs.m_auto_continue) {
if (rhs.m_thread_spec_up)
m_thread_spec_up.reset(new ThreadSpec(*rhs.m_thread_spec_up));
}
@@ -3175,6 +3188,9 @@
else
s->Indent("State: disabled\n");
+ if (m_auto_continue)
+ s->Indent("AutoContinue on\n");
+
if (m_specifier_sp) {
s->Indent();
s->PutCString("Specifier:\n");
Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -1615,6 +1615,8 @@
return error;
}
+static const char *g_resume_sync_name = "lldb.Process.ResumeSynchronous.hijack";
+
Status Process::ResumeSynchronous(Stream *stream) {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
LIBLLDB_LOG_PROCESS));
@@ -1628,7 +1630,7 @@
}
ListenerSP listener_sp(
- Listener::MakeListener("lldb.Process.ResumeSynchronous.hijack"));
+ Listener::MakeListener(g_resume_sync_name));
HijackProcessEvents(listener_sp);
Status error = PrivateResume();
@@ -1652,6 +1654,11 @@
return error;
}
+bool Process::IsHijackedForSynchronousResume() {
+ const char *hijacker_name = GetHijackingListenerName();
+ return strcmp(hijacker_name, g_resume_sync_name) == 0;
+}
+
StateType Process::GetPrivateState() { return m_private_state.GetValue(); }
void Process::SetPrivateState(StateType new_state) {
@@ -4260,11 +4267,20 @@
// public resume.
process_sp->PrivateResume();
} else {
- // If we didn't restart, run the Stop Hooks here: They might also
- // restart the target, so watch for that.
- process_sp->GetTarget().RunStopHooks();
- if (process_sp->GetPrivateState() == eStateRunning)
- SetRestarted(true);
+ bool hijacked =
+ process_sp->IsHijackedForEvent(eBroadcastBitStateChanged)
+ && !process_sp->IsHijackedForSynchronousResume();
+
+ if (!hijacked) {
+ // If we didn't restart, run the Stop Hooks here.
+ // Don't do that if state changed events aren't hooked up to the
+ // public (or SyncResume) broadcasters. StopHooks are just for
+ // real public stops. They might also restart the target,
+ // so watch for that.
+ process_sp->GetTarget().RunStopHooks();
+ if (process_sp->GetPrivateState() == eStateRunning)
+ SetRestarted(true);
+ }
}
}
}
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -4555,7 +4555,7 @@
static constexpr OptionDefinition g_target_stop_hook_add_options[] = {
// clang-format off
- { LLDB_OPT_SET_ALL, false, "one-liner", 'o', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeOneLiner, "Specify a one-line breakpoint command inline. Be sure to surround it with quotes." },
+ { LLDB_OPT_SET_ALL, false, "one-liner", 'o', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeOneLiner, "Add a command for the stop hook. Can be specified more than once, and commands will be run in the order they appear." },
{ LLDB_OPT_SET_ALL, false, "shlib", 's', OptionParser::eRequiredArgument, nullptr, {}, CommandCompletions::eModuleCompletion, eArgTypeShlibName, "Set the module within which the stop-hook is to be run." },
{ LLDB_OPT_SET_ALL, false, "thread-index", 'x', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeThreadIndex, "The stop hook is run only for the thread whose index matches this argument." },
{ LLDB_OPT_SET_ALL, false, "thread-id", 't', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeThreadID, "The stop hook is run only for the thread whose TID matches this argument." },
@@ -4566,6 +4566,7 @@
{ LLDB_OPT_SET_1, false, "end-line", 'e', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeLineNum, "Set the end of the line range for which the stop-hook is to be run." },
{ LLDB_OPT_SET_2, false, "classname", 'c', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeClassName, "Specify the class within which the stop-hook is to be run." },
{ LLDB_OPT_SET_3, false, "name", 'n', OptionParser::eRequiredArgument, nullptr, {}, CommandCompletions::eSymbolCompletion, eArgTypeFunctionName, "Set the function name within which the stop hook will be run." },
+ { LLDB_OPT_SET_ALL, false, "auto-continue",'G', OptionParser::eRequiredArgument, nullptr, {}, 0, eArgTypeBoolean, "The breakpoint will auto-continue after running its commands." },
// clang-format on
};
@@ -4606,6 +4607,17 @@
m_sym_ctx_specified = true;
break;
+ case 'G': {
+ bool value, success;
+ value = OptionArgParser::ToBoolean(option_arg, false, &success);
+ if (success) {
+ m_auto_continue = value;
+ } else
+ error.SetErrorStringWithFormat(
+ "invalid boolean value '%s' passed for -G option",
+ option_arg.str().c_str());
+ }
+ break;
case 'l':
if (option_arg.getAsInteger(0, m_line_start)) {
error.SetErrorStringWithFormat("invalid start line number: \"%s\"",
@@ -4661,7 +4673,7 @@
case 'o':
m_use_one_liner = true;
- m_one_liner = option_arg;
+ m_one_liner.push_back(option_arg);
break;
default:
@@ -4690,6 +4702,7 @@
m_use_one_liner = false;
m_one_liner.clear();
+ m_auto_continue = false;
}
std::string m_class_name;
@@ -4708,7 +4721,8 @@
bool m_thread_specified;
// Instance variables to hold the values for one_liner options.
bool m_use_one_liner;
- std::string m_one_liner;
+ std::vector<std::string> m_one_liner;
+ bool m_auto_continue;
};
CommandObjectTargetStopHookAdd(CommandInterpreter &interpreter)
@@ -4833,10 +4847,13 @@
new_hook_sp->SetThreadSpecifier(thread_spec);
}
+
+ new_hook_sp->SetAutoContinue(m_options.m_auto_continue);
if (m_options.m_use_one_liner) {
- // Use one-liner.
- new_hook_sp->GetCommandPointer()->AppendString(
- m_options.m_one_liner.c_str());
+ // Use one-liners.
+ for (auto cmd : m_options.m_one_liner)
+ new_hook_sp->GetCommandPointer()->AppendString(
+ cmd.c_str());
result.AppendMessageWithFormat("Stop hook #%" PRIu64 " added.\n",
new_hook_sp->GetID());
} else {
Index: lit/ExecControl/StopHook/stop-hook-threads.test
===================================================================
--- lit/ExecControl/StopHook/stop-hook-threads.test
+++ lit/ExecControl/StopHook/stop-hook-threads.test
@@ -12,6 +12,7 @@
# CHECK: Hook: 1
# CHECK-NEXT: State: enabled
+# CHECK-NO-FILTER-NEXT: AutoContinue on
# CHECK-FILTER-NEXT: Thread
# CHECK-FILTER-NEXT: index: 2
# CHECK-NEXT: Commands:
@@ -19,16 +20,17 @@
# CHECK-FILTER: Hook: 2
# CHECK-FILTER-NEXT: State: enabled
+# CHECK-FILTER-NEXT: AutoContinue on
# CHECK-FILTER-NEXT: Commands:
-# CHECK-FILTER-NEXT: continue
+# CHECK-FILTER-NEXT: script print('Hit stop hook')
# Get the threads going
continue
+quit
# When we filter per thread, we expect exactly 4 identical "frame var" results
# CHECK-FILTER: (uint32_t) thread_index = [[THREAD_INDEX:[0-9]*]]
-# CHECK-FILTER-COUNT-3: (uint32_t) thread_index = [[THREAD_INDEX]]
-# CHECK-FILTER-NOT: thread_index
+# CHECK-FILTER-COUNT-3: (uint32_t) thread_index = [[THREAD_INDEX]]
# When we don't filter, we expect to count 12 stopped threads in the thread list output
-# CHECK-NO-FILTER-COUNT-12: at stop-hook-threads.cpp{{.*}} stop reason = breakpoint
\ No newline at end of file
+# CHECK-NO-FILTER-COUNT-12: , stop reason = breakpoint 3.1
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads.cpp
@@ -62,7 +62,8 @@
int main (int argc, char const *argv[])
{
std::thread threads[3];
-
+ // Break here to set up the stop hook
+ printf("Stop hooks engaged.\n");
// Create 3 threads
for (auto &thread : threads)
thread = std::thread{thread_func, std::distance(threads, &thread)};
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads-2.lldbinit
@@ -1,4 +1,5 @@
+break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
break set -f stop-hook-threads.cpp -p "Break here to test that the stop-hook"
run
target stop-hook add -x 2 -o "frame variable thread_index"
-target stop-hook add -o continue
+target stop-hook add -G true -o "script print('Hit stop hook')
Index: lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-threads-1.lldbinit
@@ -1,7 +1,7 @@
+break set -f stop-hook-threads.cpp -p "Break here to set up the stop hook"
break set -f stop-hook-threads.cpp -p "Break here to test that the stop-hook"
run
-target stop-hook add
+target stop-hook add -G true
frame variable --show-globals g_val
thread list
-continue
DONE
Index: lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-3.lldbinit
@@ -1,3 +1,4 @@
+settings set interpreter.echo-comment-commands false
target stop-hook add -f stop-hook.c -l 29 -e 34
expr ptr
-DONE
\ No newline at end of file
+DONE
Index: lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-2.lldbinit
@@ -1 +1,2 @@
-target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
\ No newline at end of file
+settings set interpreter.echo-comment-commands false
+target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
Index: lit/ExecControl/StopHook/Inputs/stop-hook-1.lldbinit
===================================================================
--- lit/ExecControl/StopHook/Inputs/stop-hook-1.lldbinit
+++ lit/ExecControl/StopHook/Inputs/stop-hook-1.lldbinit
@@ -1 +1,2 @@
+settings set interpreter.echo-comment-commands false
target stop-hook add -n b -o "expr ptr"
Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -1153,6 +1153,10 @@
void SetIsActive(bool is_active) { m_active = is_active; }
+ void SetAutoContinue(bool auto_continue) {m_auto_continue = auto_continue;}
+
+ bool GetAutoContinue() const { return m_auto_continue; }
+
void GetDescription(Stream *s, lldb::DescriptionLevel level) const;
private:
@@ -1160,7 +1164,8 @@
StringList m_commands;
lldb::SymbolContextSpecifierSP m_specifier_sp;
std::unique_ptr<ThreadSpec> m_thread_spec_up;
- bool m_active;
+ bool m_active = true;
+ bool m_auto_continue = false;
// Use CreateStopHook to make a new empty stop hook. The GetCommandPointer
// and fill it with commands, and SetSpecifier to set the specifier shared
Index: include/lldb/Target/Process.h
===================================================================
--- include/lldb/Target/Process.h
+++ include/lldb/Target/Process.h
@@ -2519,6 +2519,8 @@
///
//------------------------------------------------------------------
void RestoreProcessEvents();
+
+ bool IsHijackedForSynchronousResume();
const lldb::ABISP &GetABI();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
