omjavaid created this revision.
omjavaid added a reviewer: labath.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: samparker, srhines, danalbert, tberghammer, rengolin,
aemerson.
On ARM Linux targets watchpoints are reported by PTrace before the instruction
causing watchpoint to trigger is executed.
This means we cannot step-over watchpoint causing instruction as long as there
is watchpoint installed on that location.
For this reason we cannot have multiple watchpoint slots watching same region
word/byte/halfword etc as we have to disable watchpoint for stepping over and
only watchpoint index reported by lldb-server is disabled.
Similarly we cannot keep a single hardware watchpoint slot referring to
multiple watchpoints in LLDB. This actually means that hware watchpoint indexes
are exclusively assigned to a watchpoint and cannot be shared by other
watchpoint. Also no other watchpoint should watch same address already watched
by any other index.
More discussion on this can be found here:
https://reviews.llvm.org/D24610
This patch fixes issue with stepping over watchpoint by removing the provision
to enable multiple watchpoints referring to same hardware watchpoint slot (same
HW index). Also we restrict one watchpoint per word aligned address so
duplication of address will not be allowed to avoid any step-over failures.
I have also attached a test-case that tests this scenario.
https://reviews.llvm.org/D25057
Files:
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -538,42 +538,34 @@
control_value |= ((1 << size) - 1) << 5;
control_value |= (2 << 1) | 1;
- // Iterate over stored watchpoints
- // Find a free wp_index or update reference count if duplicate.
+ // Iterate over stored watchpoints and find a free wp_index
wp_index = LLDB_INVALID_INDEX32;
for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
if ((m_hwp_regs[i].control & 1) == 0) {
wp_index = i; // Mark last free slot
- } else if (m_hwp_regs[i].address == addr &&
- m_hwp_regs[i].control == control_value) {
- wp_index = i; // Mark duplicate index
- break; // Stop searching here
+ }
+ else if (m_hwp_regs[i].address == addr) {
+ return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
}
}
if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Add new or update existing watchpoint
- if ((m_hwp_regs[wp_index].control & 1) == 0) {
- // Update watchpoint in local cache
- m_hwp_regs[wp_index].real_addr = real_addr;
- m_hwp_regs[wp_index].address = addr;
- m_hwp_regs[wp_index].control = control_value;
- m_hwp_regs[wp_index].refcount = 1;
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].real_addr = real_addr;
+ m_hwp_regs[wp_index].address = addr;
+ m_hwp_regs[wp_index].control = control_value;
- // PTRACE call to set corresponding watchpoint register.
- error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
- if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].address = 0;
+ m_hwp_regs[wp_index].control &= ~1;
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -596,36 +588,25 @@
if (wp_index >= m_max_hwp_supported)
return false;
- // Update reference count if multiple references.
- if (m_hwp_regs[wp_index].refcount > 1) {
- m_hwp_regs[wp_index].refcount--;
- return true;
- } else if (m_hwp_regs[wp_index].refcount == 1) {
- // Create a backup we can revert to in case of failure.
- lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
- uint32_t tempControl = m_hwp_regs[wp_index].control;
- uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control &= ~1;
+ m_hwp_regs[wp_index].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH);
+
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control = tempControl;
+ m_hwp_regs[wp_index].address = tempAddr;
- // Update watchpoint in local cache
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].refcount = 0;
-
- // Ptrace call to update hardware debug registers
- error = WriteHardwareDebugRegs(eDREGTypeWATCH);
-
- if (error.Fail()) {
- m_hwp_regs[wp_index].control = tempControl;
- m_hwp_regs[wp_index].address = tempAddr;
- m_hwp_regs[wp_index].refcount = tempRefCount;
-
- return false;
- }
-
- return true;
+ return false;
}
- return false;
+ return true;
}
Error NativeRegisterContextLinux_arm64::ClearAllHardwareWatchpoints() {
@@ -650,20 +631,17 @@
// Create a backup we can revert to in case of failure.
tempAddr = m_hwp_regs[i].address;
tempControl = m_hwp_regs[i].control;
- tempRefCount = m_hwp_regs[i].refcount;
// Clear watchpoints in local cache
m_hwp_regs[i].control &= ~1;
m_hwp_regs[i].address = 0;
- m_hwp_regs[i].refcount = 0;
// Ptrace call to update hardware debug registers
error = WriteHardwareDebugRegs(eDREGTypeWATCH);
if (error.Fail()) {
m_hwp_regs[i].control = tempControl;
m_hwp_regs[i].address = tempAddr;
- m_hwp_regs[i].refcount = tempRefCount;
return error;
}
@@ -718,8 +696,8 @@
watch_size = GetWatchpointSize(wp_index);
watch_addr = m_hwp_regs[wp_index].address;
- if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index) &&
- trap_addr >= watch_addr && trap_addr < watch_addr + watch_size) {
+ if (WatchpointIsEnabled(wp_index) && trap_addr >= watch_addr &&
+ trap_addr < watch_addr + watch_size) {
m_hwp_regs[wp_index].hit_addr = trap_addr;
return Error();
}
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -570,42 +570,34 @@
// Make sure bits 1:0 are clear in our address
addr &= ~((lldb::addr_t)3);
- // Iterate over stored watchpoints
- // Find a free wp_index or update reference count if duplicate.
+ // Iterate over stored watchpoints and find a free wp_index
wp_index = LLDB_INVALID_INDEX32;
for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
if ((m_hwp_regs[i].control & 1) == 0) {
wp_index = i; // Mark last free slot
- } else if (m_hwp_regs[i].address == addr &&
- m_hwp_regs[i].control == control_value) {
- wp_index = i; // Mark duplicate index
- break; // Stop searching here
+ }
+ else if (m_hwp_regs[i].address == addr) {
+ return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
}
}
if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Add new or update existing watchpoint
- if ((m_hwp_regs[wp_index].control & 1) == 0) {
- // Update watchpoint in local cache
- m_hwp_regs[wp_index].real_addr = real_addr;
- m_hwp_regs[wp_index].address = addr;
- m_hwp_regs[wp_index].control = control_value;
- m_hwp_regs[wp_index].refcount = 1;
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].real_addr = real_addr;
+ m_hwp_regs[wp_index].address = addr;
+ m_hwp_regs[wp_index].control = control_value;
- // PTRACE call to set corresponding watchpoint register.
- error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
- if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].address = 0;
+ m_hwp_regs[wp_index].control &= ~1;
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -628,36 +620,25 @@
if (wp_index >= m_max_hwp_supported)
return false;
- // Update reference count if multiple references.
- if (m_hwp_regs[wp_index].refcount > 1) {
- m_hwp_regs[wp_index].refcount--;
- return true;
- } else if (m_hwp_regs[wp_index].refcount == 1) {
- // Create a backup we can revert to in case of failure.
- lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
- uint32_t tempControl = m_hwp_regs[wp_index].control;
- uint32_t tempRefCount = m_hwp_regs[wp_index].refcount;
-
- // Update watchpoint in local cache
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].refcount = 0;
-
- // Ptrace call to update hardware debug registers
- error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
-
- if (error.Fail()) {
- m_hwp_regs[wp_index].control = tempControl;
- m_hwp_regs[wp_index].address = tempAddr;
- m_hwp_regs[wp_index].refcount = tempRefCount;
-
- return false;
- }
+ // Create a backup we can revert to in case of failure.
+ lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control &= ~1;
+ m_hwp_regs[wp_index].address = 0;
+
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
+
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control = tempControl;
+ m_hwp_regs[wp_index].address = tempAddr;
- return true;
+ return false;
}
- return false;
+ return true;
}
Error NativeRegisterContextLinux_arm::ClearAllHardwareWatchpoints() {
@@ -675,27 +656,24 @@
return error;
lldb::addr_t tempAddr = 0;
- uint32_t tempControl = 0, tempRefCount = 0;
+ uint32_t tempControl = 0;
for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
if (m_hwp_regs[i].control & 0x01) {
// Create a backup we can revert to in case of failure.
tempAddr = m_hwp_regs[i].address;
tempControl = m_hwp_regs[i].control;
- tempRefCount = m_hwp_regs[i].refcount;
// Clear watchpoints in local cache
m_hwp_regs[i].control &= ~1;
m_hwp_regs[i].address = 0;
- m_hwp_regs[i].refcount = 0;
// Ptrace call to update hardware debug registers
error = WriteHardwareDebugRegs(eDREGTypeWATCH, i);
if (error.Fail()) {
m_hwp_regs[i].control = tempControl;
m_hwp_regs[i].address = tempAddr;
- m_hwp_regs[i].refcount = tempRefCount;
return error;
}
@@ -750,8 +728,8 @@
watch_size = GetWatchpointSize(wp_index);
watch_addr = m_hwp_regs[wp_index].address;
- if (m_hwp_regs[wp_index].refcount >= 1 && WatchpointIsEnabled(wp_index) &&
- trap_addr >= watch_addr && trap_addr < watch_addr + watch_size) {
+ if (WatchpointIsEnabled(wp_index) && trap_addr >= watch_addr &&
+ trap_addr < watch_addr + watch_size) {
m_hwp_regs[wp_index].hit_addr = trap_addr;
return Error();
}
Index: packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
===================================================================
--- packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
+++ packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
@@ -0,0 +1,29 @@
+//===-- main.c --------------------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#include <stdio.h>
+#include <stdint.h>
+
+uint64_t pad0 = 0;
+uint8_t byteArray[4] = {0};
+uint64_t pad1 = 0;
+
+int main(int argc, char** argv) {
+
+ int i;
+
+ for (i = 0; i < 4; i++)
+ {
+ printf("About to write byteArray[%d] ...\n", i); // About to write byteArray
+ pad0++;
+ byteArray[i] = 7;
+ pad1++;
+ }
+
+ return 0;
+}
Index: packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
+++ packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
@@ -0,0 +1,102 @@
+"""
+Test watchpoint slots we should not be able to install multiple watchpoints
+within same word boundary. We should be able to install individual watchpoints
+on any of the bytes, half-word, or word. This is only for ARM/AArch64 targets.
+"""
+
+from __future__ import print_function
+
+import os
+import time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class WatchpointSlotsTestCase(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+
+ # Source filename.
+ self.source = 'main.c'
+
+ # Output filename.
+ self.exe_name = 'a.out'
+ self.d = {'C_SOURCES': self.source, 'EXE': self.exe_name}
+
+ # Watchpoints not supported
+ @expectedFailureAndroid(archs=['arm', 'aarch64'])
+ @expectedFailureAll(
+ oslist=["windows"],
+ bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
+ # Read-write watchpoints not supported on SystemZ
+ @expectedFailureAll(archs=['s390x'])
+ # This is a arm and aarch64 specific test case. No other architectures tested.
+ @skipIf(archs=no_match(['arm', 'aarch64']))
+ def test_multiple_watchpoints_on_same_word(self):
+
+ self.build(dictionary=self.d)
+ self.setTearDownCleanup(dictionary=self.d)
+
+ exe = os.path.join(os.getcwd(), self.exe_name)
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+ # Detect line number after which we are going to increment arrayName.
+ loc_line = line_number('main.c', '// About to write byteArray')
+
+ # Set a breakpoint on the line detected above.
+ lldbutil.run_break_set_by_file_and_line(
+ self, "main.c", loc_line, num_expected_locations=1, loc_exact=True)
+
+ # Run the program.
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # The stop reason of the thread should be breakpoint.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=['stopped', 'stop reason = breakpoint'])
+
+ # Delete breakpoint we just hit.
+ self.expect("breakpoint delete 1", substrs=['1 breakpoints deleted'])
+
+ # Set a watchpoint at byteArray[0]
+ self.expect("watchpoint set variable byteArray[0]", WATCHPOINT_CREATED,
+ substrs=['Watchpoint created','size = 1'])
+
+ # Use the '-v' option to do verbose listing of the watchpoint.
+ # The hit count should be 0 initially.
+ self.expect("watchpoint list -v 1", substrs=['hit_count = 0'])
+
+ # Try setting a watchpoint at byteArray[1]
+ self.expect("watchpoint set variable byteArray[1]", error=True,
+ substrs=['Watchpoint creation failed'])
+
+ self.runCmd("process continue")
+
+ # We should be stopped due to the watchpoint.
+ # The stop reason of the thread should be watchpoint.
+ self.expect("thread list", STOPPED_DUE_TO_WATCHPOINT,
+ substrs=['stopped', 'stop reason = watchpoint 1'])
+
+ # Delete the watchpoint we hit above successfully.
+ self.expect("watchpoint delete 1", substrs=['1 watchpoints deleted'])
+
+ # Set a watchpoint at byteArray[3]
+ self.expect("watchpoint set variable byteArray[3]", WATCHPOINT_CREATED,
+ substrs=['Watchpoint created','size = 1'])
+
+ # Resume inferior.
+ self.runCmd("process continue")
+
+ # We should be stopped due to the watchpoint.
+ # The stop reason of the thread should be watchpoint.
+ self.expect("thread list -v", STOPPED_DUE_TO_WATCHPOINT,
+ substrs=['stopped', 'stop reason = watchpoint 3'])
+
+ # Resume inferior.
+ self.runCmd("process continue")
Index: packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
===================================================================
--- packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
+++ packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
