diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -1145,6 +1145,11 @@ virtual ~StopHook() = default; enum class StopHookKind : uint32_t { CommandBased = 0, ScriptBased }; + enum class StopHookResult : uint32_t { + KeepStopped = 0, + RequestContinue, + AlreadyContinued + }; lldb::TargetSP &GetTarget() { return m_target_sp; } @@ -1160,8 +1165,8 @@ // with a reason" thread. It should add to the stream whatever text it // wants to show the user, and return False to indicate it wants the target // not to stop. - virtual bool HandleStop(ExecutionContext &exe_ctx, - lldb::StreamSP output) = 0; + virtual StopHookResult HandleStop(ExecutionContext &exe_ctx, + lldb::StreamSP output) = 0; // Set the Thread Specifier. The stop hook will own the thread specifier, // and is responsible for deleting it when we're done. @@ -1201,8 +1206,8 @@ void SetActionFromString(const std::string &strings); void SetActionFromStrings(const std::vector &strings); - bool HandleStop(ExecutionContext &exc_ctx, - lldb::StreamSP output_sp) override; + StopHookResult HandleStop(ExecutionContext &exc_ctx, + lldb::StreamSP output_sp) override; void GetSubclassDescription(Stream *s, lldb::DescriptionLevel level) const override; @@ -1219,7 +1224,8 @@ class StopHookScripted : public StopHook { public: virtual ~StopHookScripted() = default; - bool HandleStop(ExecutionContext &exc_ctx, lldb::StreamSP output) override; + StopHookResult HandleStop(ExecutionContext &exc_ctx, + lldb::StreamSP output) override; Status SetScriptCallback(std::string class_name, StructuredData::ObjectSP extra_args_sp); @@ -1254,7 +1260,9 @@ /// remove the stop hook, as it will also reset the stop hook counter. void UndoCreateStopHook(lldb::user_id_t uid); - void RunStopHooks(); + // Runs the stop hooks that have been registered for this target. + // Returns true if the stop hooks cause the target to resume. + bool RunStopHooks(); size_t GetStopHookSize(); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4178,8 +4178,7 @@ // 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) + if (process_sp->GetTarget().RunStopHooks()) SetRestarted(true); } } diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -2541,25 +2541,26 @@ } } -void Target::RunStopHooks() { +bool Target::RunStopHooks() { if (m_suppress_stop_hooks) - return; + return false; if (!m_process_sp) - return; + return false; // Somebody might have restarted the process: + // Still return false, the return value is about US restarting the target. if (m_process_sp->GetState() != eStateStopped) - return; + return false; // 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; + return false; if (m_stop_hooks.empty()) - return; + return false; // If there aren't any active stop hooks, don't bother either. bool any_active_hooks = false; @@ -2570,7 +2571,7 @@ } } if (!any_active_hooks) - return; + return false; std::vector exc_ctx_with_reasons; @@ -2588,7 +2589,7 @@ // If no threads stopped for a reason, don't run the stop-hooks. size_t num_exe_ctx = exc_ctx_with_reasons.size(); if (num_exe_ctx == 0) - return; + return false; StreamSP output_sp = m_debugger.GetAsyncOutputStream(); @@ -2636,22 +2637,27 @@ output_sp->Printf("-- Thread %d\n", exc_ctx.GetThreadPtr()->GetIndexID()); - bool this_should_stop = cur_hook_sp->HandleStop(exc_ctx, output_sp); - // If this hook is set to auto-continue that should override the - // HandleStop result... - if (cur_hook_sp->GetAutoContinue()) - this_should_stop = false; + StopHook::StopHookResult this_result = + cur_hook_sp->HandleStop(exc_ctx, output_sp); + bool this_should_stop = true; - // If anybody wanted to stop, we should all stop. - if (!should_stop) - should_stop = this_should_stop; + switch (this_result) { + case StopHook::StopHookResult::KeepStopped: + // If this hook is set to auto-continue that should override the + // HandleStop result... + if (cur_hook_sp->GetAutoContinue()) + this_should_stop = false; + else + this_should_stop = true; - // We don't have a good way to prohibit people from restarting the target - // willy nilly in a stop hook. So see if the private state is running - // here and bag out if it is. - // FIXME: when we are doing non-stop mode for realz we'll have to instead - // track each thread, and only bag out if a thread is set running. - if (m_process_sp->GetPrivateState() != eStateStopped) { + break; + case StopHook::StopHookResult::RequestContinue: + this_should_stop = false; + break; + case StopHook::StopHookResult::AlreadyContinued: + // We don't have a good way to prohibit people from restarting the + // target willy nilly in a stop hook. If the hook did so, give a + // gentle suggestion here and bag out if the hook processing. output_sp->Printf("\nAborting stop hooks, hook %" PRIu64 " set the program running.\n" " Consider using '-G true' to make " @@ -2660,16 +2666,42 @@ somebody_restarted = true; break; } + // If we're already restarted, stop processing stop hooks. + // FIXME: if we are doing non-stop mode for real, we would have to + // check that OUR thread was restarted, otherwise we should keep + // processing stop hooks. + if (somebody_restarted) + break; + + // If anybody wanted to stop, we should all stop. + if (!should_stop) + should_stop = this_should_stop; } } output_sp->Flush(); + // If one of the commands in the stop hook already restarted the target, + // report that fact. + if (somebody_restarted) + return true; + // Finally, if auto-continue was requested, do it now: // We only compute should_stop against the hook results if a hook got to run // which is why we have to do this conjoint test. - if (!somebody_restarted && ((hooks_ran && !should_stop) || auto_continue)) - m_process_sp->PrivateResume(); + if ((hooks_ran && !should_stop) || auto_continue) { + Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS)); + Status error = m_process_sp->PrivateResume(); + if (error.Success()) { + LLDB_LOG(log, "Resuming from RunStopHooks"); + return true; + } else { + LLDB_LOG(log, "Resuming from RunStopHooks failed: {0}", error); + return false; + } + } + + return false; } const TargetPropertiesSP &Target::GetGlobalProperties() { @@ -3235,13 +3267,14 @@ GetCommands().AppendString(string.c_str()); } -bool Target::StopHookCommandLine::HandleStop(ExecutionContext &exc_ctx, - StreamSP output_sp) { +Target::StopHook::StopHookResult +Target::StopHookCommandLine::HandleStop(ExecutionContext &exc_ctx, + StreamSP output_sp) { assert(exc_ctx.GetTargetPtr() && "Can't call PerformAction on a context " "with no target"); if (!m_commands.GetSize()) - return true; + return StopHookResult::KeepStopped; CommandReturnObject result(false); result.SetImmediateOutputStream(output_sp); @@ -3260,8 +3293,11 @@ debugger.GetCommandInterpreter().HandleCommands(GetCommands(), &exc_ctx, options, result); debugger.SetAsyncExecution(old_async); - - return true; + lldb::ReturnStatus status = result.GetStatus(); + if (status == eReturnStatusSuccessContinuingNoResult || + status == eReturnStatusSuccessContinuingResult) + return StopHookResult::AlreadyContinued; + return StopHookResult::KeepStopped; } // Target::StopHookScripted @@ -3289,20 +3325,22 @@ return error; } -bool Target::StopHookScripted::HandleStop(ExecutionContext &exc_ctx, - StreamSP output_sp) { +Target::StopHook::StopHookResult +Target::StopHookScripted::HandleStop(ExecutionContext &exc_ctx, + StreamSP output_sp) { assert(exc_ctx.GetTargetPtr() && "Can't call HandleStop on a context " "with no target"); ScriptInterpreter *script_interp = GetTarget()->GetDebugger().GetScriptInterpreter(); if (!script_interp) - return true; + return StopHookResult::KeepStopped; bool should_stop = script_interp->ScriptedStopHookHandleStop( m_implementation_sp, exc_ctx, output_sp); - return should_stop; + return should_stop ? StopHookResult::KeepStopped + : StopHookResult::RequestContinue; } void Target::StopHookScripted::GetSubclassDescription( diff --git a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py --- a/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py +++ b/lldb/test/API/commands/target/stop-hooks/TestStopHookScripted.py @@ -71,8 +71,6 @@ """Test that the returning False from a stop hook works""" self.do_test_auto_continue(True) - # Test is flakey on Linux. - @skipIfLinux def do_test_auto_continue(self, return_true): """Test that auto-continue works.""" # We set auto-continue to 1 but the stop hook only applies to step_out_of_me,