Skip to content

Commit 6dafd13

Browse files
ylnfrederik-h
authored andcommitted
Simplify Target::RunStopHooks() (llvm#129578)
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 272db00 commit 6dafd13

File tree

2 files changed

+23
-57
lines changed

2 files changed

+23
-57
lines changed

lldb/include/lldb/Target/Target.h

+1
Original file line numberDiff line numberDiff line change
@@ -1331,6 +1331,7 @@ class Target : public std::enable_shared_from_this<Target>,
13311331
enum class StopHookResult : uint32_t {
13321332
KeepStopped = 0,
13331333
RequestContinue,
1334+
NoPreference,
13341335
AlreadyContinued
13351336
};
13361337

lldb/source/Target/Target.cpp

+22-57
Original file line numberDiff line numberDiff line change
@@ -3033,15 +3033,9 @@ bool Target::RunStopHooks() {
30333033
if (m_stop_hooks.empty())
30343034
return false;
30353035

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)
3036+
bool no_active_hooks =
3037+
llvm::none_of(m_stop_hooks, [](auto &p) { return p.second->IsActive(); });
3038+
if (no_active_hooks)
30453039
return false;
30463040

30473041
// Make sure we check that we are not stopped because of us running a user
@@ -3075,13 +3069,13 @@ bool Target::RunStopHooks() {
30753069
return false;
30763070

30773071
StreamSP output_sp = m_debugger.GetAsyncOutputStream();
3072+
auto on_exit = llvm::make_scope_exit([output_sp] { output_sp->Flush(); });
30783073

3079-
bool auto_continue = false;
3080-
bool hooks_ran = false;
30813074
bool print_hook_header = (m_stop_hooks.size() != 1);
30823075
bool print_thread_header = (num_exe_ctx != 1);
3076+
bool auto_continue = false;
30833077
bool should_stop = false;
3084-
bool somebody_restarted = false;
3078+
bool requested_continue = false;
30853079

30863080
for (auto stop_entry : m_stop_hooks) {
30873081
StopHookSP cur_hook_sp = stop_entry.second;
@@ -3090,21 +3084,13 @@ bool Target::RunStopHooks() {
30903084

30913085
bool any_thread_matched = false;
30923086
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-
30983087
if (!cur_hook_sp->ExecutionContextPasses(exc_ctx))
30993088
continue;
31003089

31013090
// We only consult the auto-continue for a stop hook if it matched the
31023091
// specifier.
31033092
auto_continue |= cur_hook_sp->GetAutoContinue();
31043093

3105-
if (!hooks_ran)
3106-
hooks_ran = true;
3107-
31083094
if (print_hook_header && !any_thread_matched) {
31093095
StreamString s;
31103096
cur_hook_sp->GetDescription(s, eDescriptionLevelBrief);
@@ -3120,59 +3106,38 @@ bool Target::RunStopHooks() {
31203106
output_sp->Printf("-- Thread %d\n",
31213107
exc_ctx.GetThreadPtr()->GetIndexID());
31223108

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) {
3109+
auto result = cur_hook_sp->HandleStop(exc_ctx, output_sp);
3110+
switch (result) {
31283111
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-
3112+
should_stop = true;
31363113
break;
31373114
case StopHook::StopHookResult::RequestContinue:
3138-
this_should_stop = false;
3115+
requested_continue = true;
3116+
break;
3117+
case StopHook::StopHookResult::NoPreference:
3118+
// Do nothing
31393119
break;
31403120
case StopHook::StopHookResult::AlreadyContinued:
31413121
// We don't have a good way to prohibit people from restarting the
31423122
// 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.
3123+
// gentle suggestion here and back out of the hook processing.
31443124
output_sp->Printf("\nAborting stop hooks, hook %" PRIu64
31453125
" set the program running.\n"
31463126
" Consider using '-G true' to make "
31473127
"stop hooks auto-continue.\n",
31483128
cur_hook_sp->GetID());
3149-
somebody_restarted = true;
3150-
break;
3129+
// FIXME: if we are doing non-stop mode for real, we would have to
3130+
// check that OUR thread was restarted, otherwise we should keep
3131+
// processing stop hooks.
3132+
return true;
31513133
}
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;
31623134
}
31633135
}
31643136

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) {
3137+
// Resume iff:
3138+
// 1) At least one hook requested to continue and no hook asked to stop, or
3139+
// 2) at least one hook had auto continue on.
3140+
if ((requested_continue && !should_stop) || auto_continue) {
31763141
Log *log = GetLog(LLDBLog::Process);
31773142
Status error = m_process_sp->PrivateResume();
31783143
if (error.Success()) {

0 commit comments

Comments
 (0)