Fix handling of auto_continue for stop hooks

[yln]

Follow-up fix discussed here: #129578 (comment)

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Julian Lettner (yln)

Changes

Follow-up fix discussed here: #129578 (comment)

---

Full diff: https://github.com/llvm/llvm-project/pull/129622.diff

1 Files Affected:

• (modified) lldb/source/Target/Target.cpp (+7-10)

diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index bbc2110dada5b..a48586dfd9197 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3073,7 +3073,6 @@ bool Target::RunStopHooks() {
 
   bool print_hook_header = (m_stop_hooks.size() != 1);
   bool print_thread_header = (num_exe_ctx != 1);
-  bool auto_continue = false;
   bool should_stop = false;
   bool requested_continue = false;
 
@@ -3087,10 +3086,6 @@ bool Target::RunStopHooks() {
       if (!cur_hook_sp->ExecutionContextPasses(exc_ctx))
         continue;
 
-      // We only consult the auto-continue for a stop hook if it matched the
-      // specifier.
-      auto_continue |= cur_hook_sp->GetAutoContinue();
-
       if (print_hook_header && !any_thread_matched) {
         StreamString s;
         cur_hook_sp->GetDescription(s, eDescriptionLevelBrief);
@@ -3109,7 +3104,10 @@ bool Target::RunStopHooks() {
       auto result = cur_hook_sp->HandleStop(exc_ctx, output_sp);
       switch (result) {
       case StopHook::StopHookResult::KeepStopped:
-        should_stop = true;
+        if (cur_hook_sp->GetAutoContinue())
+          requested_continue = true;
+        else
+          should_stop = true;
         break;
       case StopHook::StopHookResult::RequestContinue:
         requested_continue = true;
@@ -3134,10 +3132,9 @@ bool Target::RunStopHooks() {
     }
   }
 
-  // Resume iff:
-  // 1) At least one hook requested to continue and no hook asked to stop, or
-  // 2) at least one hook had auto continue on.
-  if ((requested_continue && !should_stop) || auto_continue) {
+  // Resume iff at least one hook requested to continue and no hook asked to
+  // stop.
+  if (requested_continue && !should_stop) {
     Log *log = GetLog(LLDBLog::Process);
     Status error = m_process_sp->PrivateResume();
     if (error.Success()) {

[jimingham]

This needs tests to assert this better behavior, but I agreed to write those. I'll do that when this lands.

Other than that, this LGTM.

[yln]

This needs tests to assert this better behavior, but I agreed to write those. I'll do that when this lands.

Thanks! Feel free to push to this PR and I will run them locally before merging the PR.

[yln]

Should the "auto continue" override only apply to KeepStopped or all potential HandleStop() -> StopHookResult return values, or a specific set?
@jimingham

[jimingham]

Only KeepStopped. If any thread reports AlreadyContinued, then we aren't doing this negotiation anymore, someone called continue directly. Threads reporting NoPreference shouldn't be consulted here. And RequestContinue means that thread goes along with continuing.
GetAutoContinue is formally different, since that is a stop-hook that's asking to auto-continue regardless of what value it returns from the stop hook callback, but in practice is the same as RequestContinue.

[jimingham]

LGTM