Revert "Fix a race condition between "ephemeral watchpoint disable/enable" and continue in commands."

This reverts commit r284795, as it breaks watchpoint handling on arm (and
presumable all architectures that report watchpoint hits without executing the
tripping instruction).

There seems to be something fundamentally wrong with this patch: it uses
process_sp->AddPreResumeAction to re-enable the watchpoint, but the whole point
of the step-over-watchpoint logic (which AFAIK is the only user of this class) is
to disable the watchpoint *after* we resume to do the single step.

I have no idea how to fix this except by reverting the offending patch.

llvm-svn: 284817
This commit is contained in:
Pavel Labath 2016-10-21 10:52:11 +00:00
parent f447fbf913
commit 2e8fe80447
3 changed files with 29 additions and 55 deletions

View File

@ -54,6 +54,9 @@ class WatchpointPythonCommandTestCase(TestBase):
# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
lldbutil.run_break_set_by_file_and_line(
self, None, self.line, num_expected_locations=1)
# self.expect("breakpoint set -l %d" % self.line, BREAKPOINT_CREATED,
# startstr = "Breakpoint created: 1: file ='%s', line = %d, locations = 1" %
# (self.source, self.line))#
# Run the program.
self.runCmd("run", RUN_SUCCEEDED)
@ -128,6 +131,9 @@ class WatchpointPythonCommandTestCase(TestBase):
# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
lldbutil.run_break_set_by_file_and_line(
self, None, self.line, num_expected_locations=1)
# self.expect("breakpoint set -l %d" % self.line, BREAKPOINT_CREATED,
# startstr = "Breakpoint created: 1: file ='%s', line = %d, locations = 1" %
# (self.source, self.line))#
# Run the program.
self.runCmd("run", RUN_SUCCEEDED)
@ -171,4 +177,3 @@ class WatchpointPythonCommandTestCase(TestBase):
# second hit and set it to 999
self.expect("frame variable --show-globals cookie",
substrs=['(int32_t)', 'cookie = 999'])

View File

@ -232,7 +232,7 @@ void Watchpoint::TurnOffEphemeralMode() {
}
bool Watchpoint::IsDisabledDuringEphemeralMode() {
return m_disabled_count > 1 && m_is_ephemeral;
return m_disabled_count > 1;
}
void Watchpoint::SetEnabled(bool enabled, bool notify) {

View File

@ -557,45 +557,27 @@ public:
// performing watchpoint actions.
class WatchpointSentry {
public:
WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp),
watchpoint_sp(w_sp) {
if (process_sp && watchpoint_sp) {
WatchpointSentry(Process *p, Watchpoint *w) : process(p), watchpoint(w) {
if (process && watchpoint) {
const bool notify = false;
watchpoint_sp->TurnOnEphemeralMode();
process_sp->DisableWatchpoint(watchpoint_sp.get(), notify);
process_sp->AddPreResumeAction(SentryPreResumeAction, this);
watchpoint->TurnOnEphemeralMode();
process->DisableWatchpoint(watchpoint, notify);
}
}
void DoReenable() {
if (process_sp && watchpoint_sp) {
bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode();
watchpoint_sp->TurnOffEphemeralMode();
const bool notify = false;
if (was_disabled) {
process_sp->DisableWatchpoint(watchpoint_sp.get(), notify);
} else {
process_sp->EnableWatchpoint(watchpoint_sp.get(), notify);
}
}
}
~WatchpointSentry() {
DoReenable();
if (process_sp)
process_sp->ClearPreResumeAction(SentryPreResumeAction, this);
}
static bool SentryPreResumeAction(void *sentry_void) {
WatchpointSentry *sentry = (WatchpointSentry *) sentry_void;
sentry->DoReenable();
return true;
if (process && watchpoint) {
if (!watchpoint->IsDisabledDuringEphemeralMode()) {
const bool notify = false;
process->EnableWatchpoint(watchpoint, notify);
}
watchpoint->TurnOffEphemeralMode();
}
}
private:
ProcessSP process_sp;
WatchpointSP watchpoint_sp;
bool sentry_triggered = false;
Process *process;
Watchpoint *watchpoint;
};
StopInfoWatchpoint(Thread &thread, break_id_t watch_id,
@ -677,12 +659,12 @@ protected:
GetValue()));
if (wp_sp) {
ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
ProcessSP process_sp = exe_ctx.GetProcessSP();
Process *process = exe_ctx.GetProcessPtr();
// This sentry object makes sure the current watchpoint is disabled
// while performing watchpoint actions,
// and it is then enabled after we are finished.
WatchpointSentry sentry(process_sp, wp_sp);
WatchpointSentry sentry(process, wp_sp.get());
{
// check if this process is running on an architecture where
@ -690,10 +672,10 @@ protected:
// before the associated instruction runs. if so, disable the WP,
// single-step and then
// re-enable the watchpoint
if (process_sp) {
if (process) {
uint32_t num;
bool wp_triggers_after;
if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
if (process->GetWatchpointSupportInfo(num, wp_triggers_after)
.Success()) {
if (!wp_triggers_after) {
StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo();
@ -707,10 +689,10 @@ protected:
new_plan_sp->SetIsMasterPlan(true);
new_plan_sp->SetOkayToDiscard(false);
new_plan_sp->SetPrivate(true);
process_sp->GetThreadList().SetSelectedThreadByID(
process->GetThreadList().SetSelectedThreadByID(
thread_sp->GetID());
process_sp->ResumeSynchronous(nullptr);
process_sp->GetThreadList().SetSelectedThreadByID(
process->ResumeSynchronous(nullptr);
process->GetThreadList().SetSelectedThreadByID(
thread_sp->GetID());
thread_sp->SetStopInfo(stored_stop_info_sp);
}
@ -757,8 +739,6 @@ protected:
if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount())
m_should_stop = false;
Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
if (m_should_stop && wp_sp->GetConditionText() != nullptr) {
// We need to make sure the user sees any parse errors in their
// condition, so we'll hook the
@ -798,6 +778,7 @@ protected:
}
}
} else {
Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
StreamSP error_sp = debugger.GetAsyncErrorStream();
error_sp->Printf(
"Stopped due to an error evaluating condition of watchpoint ");
@ -819,20 +800,8 @@ protected:
// If the condition says to stop, we run the callback to further decide
// whether to stop.
if (m_should_stop) {
// FIXME: For now the callbacks have to run in async mode - the
// first time we restart we need
// to get out of there. So set it here.
// When we figure out how to nest watchpoint hits then this will
// change.
bool old_async = debugger.GetAsyncExecution();
debugger.SetAsyncExecution(true);
StoppointCallbackContext context(event_ptr, exe_ctx, false);
bool stop_requested = wp_sp->InvokeCallback(&context);
debugger.SetAsyncExecution(old_async);
// Also make sure that the callback hasn't continued the target.
// If it did, when we'll set m_should_stop to false and get out of
// here.