Skip to content

Commit 9d60069

Browse files
committed
Simplify Target::RunStopHooks()
Introduce `StopHookResult::NoPreference` and simplify control flow in `Target::RunStopHooks()`. The algorithm is (in order): 1. "Auto continue" set on any hook -> continue 2. "Stop demanded" by any hook -> stop 3. "Continue requested" by any hook -> continue 4. No hooks, or "no preference" only (default stance) -> stop The new `NoPreference` lets us keep the default stance, distinguishing case 3. and 4.
1 parent 6c9a9d9 commit 9d60069

File tree

2 files changed

+24
-57
lines changed

2 files changed

+24
-57
lines changed

lldb/include/lldb/Target/Target.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,7 @@ class Target : public std::enable_shared_from_this<Target>,
13171317
enum class StopHookResult : uint32_t {
13181318
KeepStopped = 0,
13191319
RequestContinue,
1320+
NoPreference,
13201321
AlreadyContinued
13211322
};
13221323

lldb/source/Target/Target.cpp

Lines changed: 23 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
#include "llvm/ADT/SetVector.h"
7070
#include "llvm/Support/ThreadPool.h"
7171

72+
#include <algorithm>
7273
#include <memory>
7374
#include <mutex>
7475
#include <optional>
@@ -3033,15 +3034,9 @@ bool Target::RunStopHooks() {
30333034
if (m_stop_hooks.empty())
30343035
return false;
30353036

3036-
// If there aren't any active stop hooks, don't bother either.
3037-
bool any_active_hooks = false;
3038-
for (auto hook : m_stop_hooks) {
3039-
if (hook.second->IsActive()) {
3040-
any_active_hooks = true;
3041-
break;
3042-
}
3043-
}
3044-
if (!any_active_hooks)
3037+
bool no_active = std::none_of(m_stop_hooks.cbegin(), m_stop_hooks.cend(),
3038+
[](auto &p) { return p.second->IsActive(); });
3039+
if (no_active)
30453040
return false;
30463041

30473042
// Make sure we check that we are not stopped because of us running a user
@@ -3075,13 +3070,13 @@ bool Target::RunStopHooks() {
30753070
return false;
30763071

30773072
StreamSP output_sp = m_debugger.GetAsyncOutputStream();
3073+
auto on_exit = llvm::make_scope_exit([output_sp] { output_sp->Flush(); });
30783074

3079-
bool auto_continue = false;
3080-
bool hooks_ran = false;
30813075
bool print_hook_header = (m_stop_hooks.size() != 1);
30823076
bool print_thread_header = (num_exe_ctx != 1);
3077+
bool auto_continue = false;
30833078
bool should_stop = false;
3084-
bool somebody_restarted = false;
3079+
bool requested_continue = false;
30853080

30863081
for (auto stop_entry : m_stop_hooks) {
30873082
StopHookSP cur_hook_sp = stop_entry.second;
@@ -3090,21 +3085,13 @@ bool Target::RunStopHooks() {
30903085

30913086
bool any_thread_matched = false;
30923087
for (auto exc_ctx : exc_ctx_with_reasons) {
3093-
// We detect somebody restarted in the stop-hook loop, and broke out of
3094-
// that loop back to here. So break out of here too.
3095-
if (somebody_restarted)
3096-
break;
3097-
30983088
if (!cur_hook_sp->ExecutionContextPasses(exc_ctx))
30993089
continue;
31003090

31013091
// We only consult the auto-continue for a stop hook if it matched the
31023092
// specifier.
31033093
auto_continue |= cur_hook_sp->GetAutoContinue();
31043094

3105-
if (!hooks_ran)
3106-
hooks_ran = true;
3107-
31083095
if (print_hook_header && !any_thread_matched) {
31093096
StreamString s;
31103097
cur_hook_sp->GetDescription(s, eDescriptionLevelBrief);
@@ -3120,59 +3107,38 @@ bool Target::RunStopHooks() {
31203107
output_sp->Printf("-- Thread %d\n",
31213108
exc_ctx.GetThreadPtr()->GetIndexID());
31223109

3123-
StopHook::StopHookResult this_result =
3124-
cur_hook_sp->HandleStop(exc_ctx, output_sp);
3125-
bool this_should_stop = true;
3126-
3127-
switch (this_result) {
3110+
auto result = cur_hook_sp->HandleStop(exc_ctx, output_sp);
3111+
switch (result) {
31283112
case StopHook::StopHookResult::KeepStopped:
3129-
// If this hook is set to auto-continue that should override the
3130-
// HandleStop result...
3131-
if (cur_hook_sp->GetAutoContinue())
3132-
this_should_stop = false;
3133-
else
3134-
this_should_stop = true;
3135-
3113+
should_stop = true;
31363114
break;
31373115
case StopHook::StopHookResult::RequestContinue:
3138-
this_should_stop = false;
3116+
requested_continue = true;
3117+
break;
3118+
case StopHook::StopHookResult::NoPreference:
3119+
// Do nothing
31393120
break;
31403121
case StopHook::StopHookResult::AlreadyContinued:
31413122
// We don't have a good way to prohibit people from restarting the
31423123
// target willy nilly in a stop hook. If the hook did so, give a
3143-
// gentle suggestion here and bag out if the hook processing.
3124+
// gentle suggestion here and back out of the hook processing.
31443125
output_sp->Printf("\nAborting stop hooks, hook %" PRIu64
31453126
" set the program running.\n"
31463127
" Consider using '-G true' to make "
31473128
"stop hooks auto-continue.\n",
31483129
cur_hook_sp->GetID());
3149-
somebody_restarted = true;
3150-
break;
3130+
// FIXME: if we are doing non-stop mode for real, we would have to
3131+
// check that OUR thread was restarted, otherwise we should keep
3132+
// processing stop hooks.
3133+
return true;
31513134
}
3152-
// If we're already restarted, stop processing stop hooks.
3153-
// FIXME: if we are doing non-stop mode for real, we would have to
3154-
// check that OUR thread was restarted, otherwise we should keep
3155-
// processing stop hooks.
3156-
if (somebody_restarted)
3157-
break;
3158-
3159-
// If anybody wanted to stop, we should all stop.
3160-
if (!should_stop)
3161-
should_stop = this_should_stop;
31623135
}
31633136
}
31643137

3165-
output_sp->Flush();
3166-
3167-
// If one of the commands in the stop hook already restarted the target,
3168-
// report that fact.
3169-
if (somebody_restarted)
3170-
return true;
3171-
3172-
// Finally, if auto-continue was requested, do it now:
3173-
// We only compute should_stop against the hook results if a hook got to run
3174-
// which is why we have to do this conjoint test.
3175-
if ((hooks_ran && !should_stop) || auto_continue) {
3138+
// Resume iff:
3139+
// 1) At least one hook requested to continue and no hook asked to stop, or
3140+
// 2) at least one hook had auto continue on.
3141+
if ((requested_continue && !should_stop) || auto_continue) {
31763142
Log *log = GetLog(LLDBLog::Process);
31773143
Status error = m_process_sp->PrivateResume();
31783144
if (error.Success()) {

0 commit comments

Comments
 (0)