From 892943f9ddb6ff6f8e4b8eca56238503fece72f3 Mon Sep 17 00:00:00 2001 From: Johnny Chen Date: Thu, 23 Aug 2012 22:28:26 +0000 Subject: [PATCH] Cope with the case where the user-supplied callbacks want the watchpoint itself to be disabled! Previously we put a WatchpointSentry object within StopInfo.cpp to disable-and-then-enable the watchpoint itself while we are performing the actions associated with the triggered watchpoint, which can cause the user-initiated watchpoint disabling action to be negated. Add a test case to verify that a watchpoint can be disabled during the callbacks. llvm-svn: 162483 --- lldb/include/lldb/Breakpoint/Watchpoint.h | 7 ++ lldb/source/Breakpoint/Watchpoint.cpp | 12 ++++ .../Process/gdb-remote/ProcessGDBRemote.cpp | 4 ++ lldb/source/Target/StopInfo.cpp | 3 +- .../command/TestWatchpointCommandLLDB.py | 68 +++++++++++++++++++ 5 files changed, 93 insertions(+), 1 deletion(-) diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h index 13a37041af4c..7594ad91369b 100644 --- a/lldb/include/lldb/Breakpoint/Watchpoint.h +++ b/lldb/include/lldb/Breakpoint/Watchpoint.h @@ -156,6 +156,9 @@ public: void TurnOffEphemeralMode(); + bool + IsDisabledDuringEphemeralMode(); + private: friend class Target; friend class WatchpointList; @@ -170,6 +173,10 @@ private: bool m_is_ephemeral; // True if the watchpoint is in the ephemeral mode, meaning that it is // undergoing a pair of temporary disable/enable actions to avoid recursively // triggering further watchpoint events. + uint32_t m_disabled_count; // Keep track of the count that the watchpoint is disabled while in ephemeral mode. + // At the end of the ephemeral mode when the watchpoint is to be enabled agian, + // we check the count, if it is more than 1, it means the user-supplied actions + // actually want the watchpoint to be disabled! uint32_t m_watch_read:1, // 1 if we stop when the watched data is read from m_watch_write:1, // 1 if we stop when the watched data is written to m_watch_was_read:1, // Set to 1 when watchpoint is hit for a read access diff --git a/lldb/source/Breakpoint/Watchpoint.cpp b/lldb/source/Breakpoint/Watchpoint.cpp index 3736adb3b730..ca5503ff2d34 100644 --- a/lldb/source/Breakpoint/Watchpoint.cpp +++ b/lldb/source/Breakpoint/Watchpoint.cpp @@ -30,6 +30,7 @@ Watchpoint::Watchpoint (lldb::addr_t addr, size_t size, bool hardware) : m_is_hardware(hardware), m_is_watch_variable(false), m_is_ephemeral(false), + m_disabled_count(0), m_watch_read(0), m_watch_write(0), m_watch_was_read(0), @@ -322,6 +323,14 @@ void Watchpoint::TurnOffEphemeralMode() { m_is_ephemeral = false; + // Leaving ephemeral mode, reset the m_disabled_count! + m_disabled_count = 0; +} + +bool +Watchpoint::IsDisabledDuringEphemeralMode() +{ + return m_disabled_count > 1; } void @@ -331,6 +340,9 @@ Watchpoint::SetEnabled(bool enabled) { if (!m_is_ephemeral) SetHardwareIndex(LLDB_INVALID_INDEX32); + else + ++m_disabled_count; + // Don't clear the snapshots for now. // Within StopInfo.cpp, we purposely do disable/enable watchpoint while performing watchpoint actions. //ClearSnapshots(); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 07536c731f46..6089b83c785c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2281,6 +2281,10 @@ ProcessGDBRemote::DisableWatchpoint (Watchpoint *wp) { if (log) log->Printf ("ProcessGDBRemote::DisableWatchpoint (watchID = %llu) addr = 0x%8.8llx -- SUCCESS (already disabled)", watchID, (uint64_t)addr); + // See also 'class WatchpointSentry' within StopInfo.cpp. + // This disabling attempt might come from the user-supplied actions, we'll route it in order for + // the watchpoint object to intelligently process this action. + wp->SetEnabled(false); return error; } diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index 400829ff7824..01d8ba9c1f17 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -463,7 +463,8 @@ public: { if (process && watchpoint) { - process->EnableWatchpoint(watchpoint); + if (!watchpoint->IsDisabledDuringEphemeralMode()) + process->EnableWatchpoint(watchpoint); watchpoint->TurnOffEphemeralMode(); } } diff --git a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py index 5348737a3c51..4a174f956678 100644 --- a/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py +++ b/lldb/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandLLDB.py @@ -39,6 +39,21 @@ class WatchpointLLDBCommandTestCase(TestBase): self.setTearDownCleanup(dictionary=self.d) self.watchpoint_command() + @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin") + @dsym_test + def test_watchpoint_command_can_disable_a_watchpoint_with_dsym(self): + """Test that 'watchpoint command' action can disable a watchpoint after it is triggered.""" + self.buildDsym(dictionary=self.d) + self.setTearDownCleanup(dictionary=self.d) + self.watchpoint_command_can_disable_a_watchpoint() + + @dwarf_test + def test_watchpoint_command_can_disable_a_watchpoint_with_dwarf(self): + """Test that 'watchpoint command' action can disable a watchpoint after it is triggered.""" + self.buildDwarf(dictionary=self.d) + self.setTearDownCleanup(dictionary=self.d) + self.watchpoint_command_can_disable_a_watchpoint() + def watchpoint_command(self): """Do 'watchpoint command add'.""" exe = os.path.join(os.getcwd(), self.exe_name) @@ -90,6 +105,59 @@ class WatchpointLLDBCommandTestCase(TestBase): self.expect("frame variable -g cookie", substrs = ['(int32_t)', 'cookie = 777']) + def watchpoint_command_can_disable_a_watchpoint(self): + """Test that 'watchpoint command' action can disable a watchpoint after it is triggered.""" + exe = os.path.join(os.getcwd(), self.exe_name) + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + + # Add a breakpoint to set a watchpoint when stopped on the breakpoint. + 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) + + # We should be stopped again due to the breakpoint. + # The stop reason of the thread should be breakpoint. + self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT, + substrs = ['stopped', + 'stop reason = breakpoint']) + + # Now let's set a write-type watchpoint for 'global'. + self.expect("watchpoint set variable -w write global", WATCHPOINT_CREATED, + substrs = ['Watchpoint created', 'size = 4', 'type = w', + '%s:%d' % (self.source, self.decl)]) + + self.runCmd('watchpoint command add 1 -o "watchpoint disable 1"') + + # List the watchpoint command we just added. + self.expect("watchpoint command list 1", + substrs = ['watchpoint disable 1']) + + # Use the '-v' option to do verbose listing of the watchpoint. + # The hit count should be 0 initially. + self.expect("watchpoint list -v", + substrs = ['hit_count = 0']) + + self.runCmd("process continue") + + # We should be stopped again due to the watchpoint (write type). + # The stop reason of the thread should be watchpoint. + self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT, + substrs = ['stop reason = watchpoint']) + + # Check that the watchpoint has been disabled. + self.expect("watchpoint list -v", + substrs = ['disabled']) + + self.runCmd("process continue") + + # There should be no more watchpoint hit and the process status should + # be 'exited'. + self.expect("process status", + substrs = ['exited']) + if __name__ == '__main__': import atexit