Author: jingham Date: Thu Oct 20 19:06:38 2016 New Revision: 284795 URL: http://llvm.org/viewvc/llvm-project?rev=284795&view=rev Log: Fix a race condition between "ephemeral watchpoint disable/enable" and continue in commands.
Also, watchpoint commands, like breakpoint commands, need to run in async mode. This was causing intermittent failures in TestWatchpointCommandPython.py, which is now solid. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py lldb/trunk/source/Breakpoint/Watchpoint.cpp lldb/trunk/source/Target/StopInfo.cpp Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py?rev=284795&r1=284794&r2=284795&view=diff ============================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py Thu Oct 20 19:06:38 2016 @@ -54,9 +54,6 @@ class WatchpointPythonCommandTestCase(Te # 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) @@ -131,9 +128,6 @@ class WatchpointPythonCommandTestCase(Te # 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) @@ -177,3 +171,4 @@ class WatchpointPythonCommandTestCase(Te # second hit and set it to 999 self.expect("frame variable --show-globals cookie", substrs=['(int32_t)', 'cookie = 999']) + Modified: lldb/trunk/source/Breakpoint/Watchpoint.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Watchpoint.cpp?rev=284795&r1=284794&r2=284795&view=diff ============================================================================== --- lldb/trunk/source/Breakpoint/Watchpoint.cpp (original) +++ lldb/trunk/source/Breakpoint/Watchpoint.cpp Thu Oct 20 19:06:38 2016 @@ -232,7 +232,7 @@ void Watchpoint::TurnOffEphemeralMode() } bool Watchpoint::IsDisabledDuringEphemeralMode() { - return m_disabled_count > 1; + return m_disabled_count > 1 && m_is_ephemeral; } void Watchpoint::SetEnabled(bool enabled, bool notify) { Modified: lldb/trunk/source/Target/StopInfo.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StopInfo.cpp?rev=284795&r1=284794&r2=284795&view=diff ============================================================================== --- lldb/trunk/source/Target/StopInfo.cpp (original) +++ lldb/trunk/source/Target/StopInfo.cpp Thu Oct 20 19:06:38 2016 @@ -557,27 +557,45 @@ public: // performing watchpoint actions. class WatchpointSentry { public: - WatchpointSentry(Process *p, Watchpoint *w) : process(p), watchpoint(w) { - if (process && watchpoint) { + WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp), + watchpoint_sp(w_sp) { + if (process_sp && watchpoint_sp) { const bool notify = false; - watchpoint->TurnOnEphemeralMode(); - process->DisableWatchpoint(watchpoint, notify); + watchpoint_sp->TurnOnEphemeralMode(); + process_sp->DisableWatchpoint(watchpoint_sp.get(), notify); + process_sp->AddPreResumeAction(SentryPreResumeAction, this); } } - - ~WatchpointSentry() { - if (process && watchpoint) { - if (!watchpoint->IsDisabledDuringEphemeralMode()) { - const bool notify = false; - process->EnableWatchpoint(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); } - watchpoint->TurnOffEphemeralMode(); } } + + ~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; + } private: - Process *process; - Watchpoint *watchpoint; + ProcessSP process_sp; + WatchpointSP watchpoint_sp; + bool sentry_triggered = false; }; StopInfoWatchpoint(Thread &thread, break_id_t watch_id, @@ -659,12 +677,12 @@ protected: GetValue())); if (wp_sp) { ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0)); - Process *process = exe_ctx.GetProcessPtr(); + ProcessSP process_sp = exe_ctx.GetProcessSP(); // 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, wp_sp.get()); + WatchpointSentry sentry(process_sp, wp_sp); { // check if this process is running on an architecture where @@ -672,10 +690,10 @@ protected: // before the associated instruction runs. if so, disable the WP, // single-step and then // re-enable the watchpoint - if (process) { + if (process_sp) { uint32_t num; bool wp_triggers_after; - if (process->GetWatchpointSupportInfo(num, wp_triggers_after) + if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after) .Success()) { if (!wp_triggers_after) { StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo(); @@ -689,10 +707,10 @@ protected: new_plan_sp->SetIsMasterPlan(true); new_plan_sp->SetOkayToDiscard(false); new_plan_sp->SetPrivate(true); - process->GetThreadList().SetSelectedThreadByID( + process_sp->GetThreadList().SetSelectedThreadByID( thread_sp->GetID()); - process->ResumeSynchronous(nullptr); - process->GetThreadList().SetSelectedThreadByID( + process_sp->ResumeSynchronous(nullptr); + process_sp->GetThreadList().SetSelectedThreadByID( thread_sp->GetID()); thread_sp->SetStopInfo(stored_stop_info_sp); } @@ -739,6 +757,8 @@ 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 @@ -778,7 +798,6 @@ 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 "); @@ -800,8 +819,20 @@ 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. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits