jingham created this revision.
jingham added a reviewer: labath.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.
I was running the lit tests by hand (since you can't actually run them with the
Xcode build) and missed one crucial fact. The lit tests are all run in batch
mode with command files. Normally, in lldb, the main interpreter in in Async
mode, and individual commands choose to be in sync or async mode based, for
instance, on whether we are handing the inferior's stdout in our terminal or
not. But when sourcing command files, the interpreter runs in Sync mode -
since it never makes sense for batch files to run in Async mode.
Normally that doesn't make any difference, but there was an error in how "run
by starting stopped and then attaching" in sync mode was implemented. It
hijacked the process state events so that it could handle the initial SIGSTOP
that came from the attach. Then when it decided to continue synchronously, it
didn't do so by calling ResumeSynchronous, but just kept the old hijack
listener in place and waited for the events by hand. So other code couldn't
tell that a sync resume was in progress. That was the difference that was
causing the failure Pavel saw (and I should have seen had I been more careful
in running the lit tests by hand.)
This is the same patch as D58394 <https://reviews.llvm.org/D58394> except that
I changed how Target::Launch handles the sync case, just replacing the
hand-rolled Resume with a call to ResumeSynchronous.
BTW, is it possible to re-open a closed review, I didn't see a way to do that,
so I had to make another.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D58727
Files:
include/lldb/Target/Process.h
include/lldb/Target/Target.h
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();
@@ -2914,17 +2926,11 @@
if (state == eStateStopped) {
if (!launch_info.GetFlags().Test(eLaunchFlagStopAtEntry)) {
if (synchronous_execution) {
- error = m_process_sp->PrivateResume();
- if (error.Success()) {
- state = m_process_sp->WaitForProcessToStop(
- llvm::None, nullptr, true, hijack_listener_sp, stream);
- const bool must_be_alive =
- false; // eStateExited is ok, so this must be false
- if (!StateIsStoppedState(state, must_be_alive)) {
- error.SetErrorStringWithFormat("process isn't stopped: %s",
- StateAsCString(state));
- }
- }
+ // Now we have handled the stop-from-attach, and we are just switching
+ // to a synchronous resume. So we should switch to the SyncResume
+ // hijacker.
+ m_process_sp->RestoreProcessEvents();
+ m_process_sp->ResumeSynchronous(stream);
} else {
m_process_sp->RestoreProcessEvents();
error = m_process_sp->PrivateResume();
@@ -3143,12 +3149,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 +3182,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
@@ -1552,16 +1552,6 @@
return m_public_state.GetValue();
}
-bool Process::StateChangedIsExternallyHijacked() {
- if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
- const char *hijacking_name = GetHijackingListenerName();
- if (hijacking_name &&
- strcmp(hijacking_name, "lldb.Process.ResumeSynchronous.hijack"))
- return true;
- }
- return false;
-}
-
void Process::SetPublicState(StateType new_state, bool restarted) {
Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STATE |
LIBLLDB_LOG_PROCESS));
@@ -1615,6 +1605,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 +1620,7 @@
}
ListenerSP listener_sp(
- Listener::MakeListener("lldb.Process.ResumeSynchronous.hijack"));
+ Listener::MakeListener(g_resume_sync_name));
HijackProcessEvents(listener_sp);
Status error = PrivateResume();
@@ -1652,6 +1644,26 @@
return error;
}
+bool Process::StateChangedIsExternallyHijacked() {
+ if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
+ const char *hijacking_name = GetHijackingListenerName();
+ if (hijacking_name &&
+ strcmp(hijacking_name, g_resume_sync_name))
+ return true;
+ }
+ return false;
+}
+
+bool Process::StateChangedIsHijackedForSynchronousResume() {
+ if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
+ const char *hijacking_name = GetHijackingListenerName();
+ if (hijacking_name &&
+ strcmp(hijacking_name, g_resume_sync_name) == 0)
+ return true;
+ }
+ return false;
+}
+
StateType Process::GetPrivateState() { return m_private_state.GetValue(); }
void Process::SetPrivateState(StateType new_state) {
@@ -4260,11 +4272,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->StateChangedIsHijackedForSynchronousResume();
+
+ 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
@@ -4,7 +4,6 @@
# RUN: %lldb -b -s %p/Inputs/stop-hook-threads-2.lldbinit -s %s -f %t \
# RUN: | FileCheck --check-prefix=CHECK --check-prefix=CHECK-FILTER %s
# XFAIL: system-windows
-# UNSUPPORTED: linux
thread list
break set -f stop-hook-threads.cpp -p "Set break point at this line"
@@ -12,23 +11,22 @@
# 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:
-# CHECK-NEXT: frame variable
+# CHECK-NEXT: expr lldb_val += 1
+# CHECK-NEXT: thread list
# 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
-# 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
-
-# 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
+# Now make sure we hit the command the right number of times:
+# CHECK-NO-FILTER: lldb_val was set to: 15.
+# CHECK-FILTER: lldb_val was set to: 5.
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
@@ -16,6 +16,7 @@
std::uniform_int_distribution<> g_distribution{0, 3000};
uint32_t g_val = 0;
+uint32_t lldb_val = 0;
uint32_t
access_pool (bool flag = false)
@@ -62,7 +63,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)};
@@ -71,5 +73,7 @@
for (auto &thread : threads)
thread.join();
+ // print lldb_val so we can check it here.
+ printf ("lldb_val was set to: %d.\n", lldb_val);
return 0;
}
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 -x 2 -o "expr lldb_val += 1" -o "thread list"
+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
-frame variable --show-globals g_val
+target stop-hook add -G true
+expr lldb_val += 1
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,3 @@
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 @@
-target stop-hook add -f stop-hook.c -l 29 -e 34 -o "expr ptr"
\ No newline at end of file
+target stop-hook add -f stop-hook.c -l 29 -e 34 -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,10 @@
///
//------------------------------------------------------------------
void RestoreProcessEvents();
+
+ bool StateChangedIsHijackedForSynchronousResume();
+
+ bool StateChangedIsExternallyHijacked();
const lldb::ABISP &GetABI();
@@ -3211,8 +3215,6 @@
virtual Status UpdateAutomaticSignalFiltering();
- bool StateChangedIsExternallyHijacked();
-
void LoadOperatingSystemPlugin(bool flush);
private:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
