omjavaid updated this revision to Diff 72589.
omjavaid added a comment.
This is a new version of what seems to me fully implementing functionality we
intend to have here.
On a second thought nuking ClearHardwareWatchpoint function seems to be the
wrong approach here. I spent some time taking different approaches and it turns
out that if we do not ClearHardwareWatchpoint when back-end asks us to remove
it then we wont be able to step over watchpoints. On ARM targets we have to
first clear and then reinstall watchpoints to step over the watchpoint
instruction.
On the other hand if we call
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint then that watchpoint
stands removed if call is just to delete watch on one of the bytes. And if we
follow up with creating a new watchpoint on a different word the slot being
used may appear vaccant which is actually inconsistent behavior.
So I have a new approach that does clear watchpoint registers if
NativeRegisterContextLinux_arm::ClearHardwareWatchpoint is called but we still
track reference counts by re-introducing refcount that I removed in my last
patch. This will mean that a follow up create may fail just because there are
still references to disabled watchpoint and watchpoint slots are still not
vaccant. I have made changes to the test to reflect this behaviour.
Please comment if you have any reservation about this approach.
https://reviews.llvm.org/D24610
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/NativeThreadLinux.cpp
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -198,8 +198,10 @@
m_stop_info.reason = StopReason::eStopReasonNone;
m_stop_description.clear();
- // If watchpoints have been set, but none on this thread,
- // then this is a new thread. So set all existing watchpoints.
+ // Invalidate watchpoint index map for a re-sync
+ m_watchpoint_index_map.clear();
+
+ // Re-sync all available watchpoints.
if (m_watchpoint_index_map.empty()) {
NativeProcessLinux &process = GetProcess();
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
@@ -508,8 +508,19 @@
if (error.Fail())
return LLDB_INVALID_INDEX32;
- uint32_t control_value = 0, wp_index = 0, addr_word_offset = 0, byte_mask = 0;
+ uint32_t control_value = 0;
lldb::addr_t real_addr = addr;
+ uint32_t wp_index = LLDB_INVALID_INDEX32;
+
+ // Find out how many bytes we need to watch after 4-byte alignment boundary.
+ uint8_t watch_size = (addr & 0x03) + size;
+
+ // We cannot watch zero or more than 4 bytes after 4-byte alignment boundary.
+ if (size == 0 || watch_size > 4)
+ return LLDB_INVALID_INDEX32;
+
+ // Strip away last two bits of address for byte/half-word/word selection.
+ addr &= ~((lldb::addr_t)3);
// Check if we are setting watchpoint other than read/write/access
// Also update watchpoint flag to match Arm write-read bit configuration.
@@ -526,86 +537,63 @@
return LLDB_INVALID_INDEX32;
}
- // Can't watch zero bytes
- // Can't watch more than 4 bytes per WVR/WCR pair
+ // Iterate over stored watchpoints and find a free or duplicate wp_index
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+ if ((m_hwp_regs[i].control & 1) == 0 && (m_hwp_regs[i].refcount <= 0)) {
+ wp_index = i; // Mark last free slot
+ } else if (m_hwp_regs[i].address == addr) {
+ wp_index = i; // Mark duplicate index
+ break; // Stop searching here
+ }
+ }
- if (size == 0 || size > 4)
+ // No vaccant slot available and no duplicate slot found.
+ if (wp_index == LLDB_INVALID_INDEX32)
return LLDB_INVALID_INDEX32;
- // Check 4-byte alignment for hardware watchpoint target address.
- // Below is a hack to recalculate address and size in order to
- // make sure we can watch non 4-byte alligned addresses as well.
- if (addr & 0x03) {
- uint8_t watch_mask = (addr & 0x03) + size;
-
- if (watch_mask > 0x04)
- return LLDB_INVALID_INDEX32;
- else if (watch_mask <= 0x02)
- size = 2;
- else if (watch_mask <= 0x04)
- size = 4;
-
- addr = addr & (~0x03);
+ uint8_t current_watch_size, new_watch_size;
+ // Calculate overall size width to be watched by current hardware watchpoint slot.
+ current_watch_size = GetWatchpointSize(wp_index);
+ new_watch_size = std::max(current_watch_size, watch_size);
+
+ // Make new_watch_size a power of 2.
+ if (new_watch_size == 3)
+ new_watch_size++;
+
+ // There is no need to update watchpoint registers
+ // if requested byte range is already covered by exiting watchpoint.
+ if (current_watch_size == new_watch_size &&
+ m_hwp_regs[wp_index].control & 1) {
+ m_hwp_regs[wp_index].refcount++;
+ return wp_index;
}
- // We can only watch up to four bytes that follow a 4 byte aligned address
- // per watchpoint register pair, so make sure we can properly encode this.
- addr_word_offset = addr % 4;
- byte_mask = ((1u << size) - 1u) << addr_word_offset;
-
- // Check if we need multiple watchpoint register
- if (byte_mask > 0xfu)
- return LLDB_INVALID_INDEX32;
-
// Setup control value
+ // Create byte mask for control register
// Make the byte_mask into a valid Byte Address Select mask
- control_value = byte_mask << 5;
+ control_value = ((1u << new_watch_size) - 1u) << 5;
// Turn on appropriate watchpoint flags read or write
control_value |= (watch_flags << 3);
// Enable this watchpoint and make it stop in privileged or user mode;
control_value |= 7;
- // Make sure bits 1:0 are clear in our address
- addr &= ~((lldb::addr_t)3);
+ // 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;
- // Iterate over stored watchpoints
- // Find a free wp_index or update reference count if duplicate.
- 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
- }
- }
+ // PTRACE call to set corresponding watchpoint register.
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
- 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;
-
- // PTRACE call to set corresponding watchpoint register.
- error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control &= ~1;
- if (error.Fail()) {
- m_hwp_regs[wp_index].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
+ return LLDB_INVALID_INDEX32;
+ }
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ m_hwp_regs[wp_index].refcount++;
return wp_index;
}
@@ -628,36 +616,24 @@
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;
+ // Create a backup we can revert to in case of failure.
+ uint32_t tempControl = m_hwp_regs[wp_index].control;
- // Ptrace call to update hardware debug registers
- error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
+ // Update watchpoint in local cache
+ m_hwp_regs[wp_index].control &= ~1;
+ m_hwp_regs[wp_index].refcount--;
- if (error.Fail()) {
- m_hwp_regs[wp_index].control = tempControl;
- m_hwp_regs[wp_index].address = tempAddr;
- m_hwp_regs[wp_index].refcount = tempRefCount;
+ // Ptrace call to update hardware debug registers
+ error = WriteHardwareDebugRegs(eDREGTypeWATCH, wp_index);
- return false;
- }
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control = tempControl;
+ m_hwp_regs[wp_index].refcount++;
- return true;
+ return false;
}
- return false;
+ return true;
}
Error NativeRegisterContextLinux_arm::ClearAllHardwareWatchpoints() {
@@ -685,7 +661,7 @@
tempRefCount = m_hwp_regs[i].refcount;
// Clear watchpoints in local cache
- m_hwp_regs[i].control &= ~1;
+ m_hwp_regs[i].control = 0;
m_hwp_regs[i].address = 0;
m_hwp_regs[i].refcount = 0;
@@ -700,6 +676,11 @@
return error;
}
}
+ else {
+ m_hwp_regs[i].control = 0;
+ m_hwp_regs[i].address = 0;
+ m_hwp_regs[i].refcount = 0;
+ }
}
return Error();
@@ -750,8 +731,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,40 @@
+//===-- 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[16] = {0};
+uint64_t pad1 = 0;
+uint16_t wordArray[8] = {0};
+uint32_t dWordVar = 0;
+
+int main(int argc, char** argv) {
+
+ int i;
+
+ for (i = 0; i < 16; i++)
+ {
+ printf("About to write byteArray[%d] ...\n", i); // About to write byteArray
+ pad0++;
+ byteArray[i] = 7;
+ pad1++;
+ }
+
+ for (i = 0; i < 8; i++)
+ {
+ printf("About to write wordArray[%d] ...\n", i); // About to write wordArray
+ pad0++;
+ wordArray[i] = 7;
+ pad1++;
+ }
+
+ dWordVar = 5; // Write dWordVar to check if a stale watchpoint is active
+ 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,131 @@
+"""
+Test watchpoint slots (1-byte/2-byte watchpoints with selective deletion)
+Make sure we can watch all installed byte/word size watchpoints,
+when we have installed multiple byte/word size watchpoints within same dword.
+Also make sure we hit only the ones which are left after we selective deletion.
+
+"""
+
+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'])
+ def test_byte_size_watchpoints_multi_slots(self):
+ """Test to selectively watch different bytes in a 8-byte array."""
+ self.run_watchpoint_slot_test('byteArray', 16, '1',
+ ['2', '4', '6', '8'], ['1', '3', '5', '7'])
+
+ # 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'])
+ def test_two_byte_watchpoints_multi_slots(self):
+ """Test to randomly watch different words in an 8-byte word array."""
+ self.run_watchpoint_slot_test('wordArray', 8, '2',
+ ['2', '4', '6', '8'], ['1', '3', '5', '7'])
+
+ def run_watchpoint_slot_test(self, arrayName, array_size, watchsize,
+ watch_hit_list, watch_del_list):
+ 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 ' + arrayName)
+
+ # 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'])
+
+ for i in range(array_size):
+ # Set a read_write type watchpoint arrayName
+ watch_loc = arrayName + "[" + str(i) + "]"
+ self.expect(
+ "watchpoint set variable -w read_write " +
+ watch_loc,
+ WATCHPOINT_CREATED,
+ substrs=[
+ 'Watchpoint created',
+ 'size = ' +
+ watchsize,
+ 'type = rw'])
+
+ # 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'])
+
+ 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'])
+
+ # Use the '-v' option to do verbose listing of the watchpoint.
+ # The hit count should now be 1.
+ self.expect("watchpoint list -v 1", substrs=['hit_count = 1'])
+
+ # Selectively delete watchpoints.
+ for idel in watch_del_list:
+ self.expect("watchpoint delete " + idel,
+ substrs=['1 watchpoints deleted'])
+
+ # Try setting watchpoint here and it should fail.
+ self.expect("watchpoint set variable -w read_write dWordVar",
+ substrs=['Watchpoint creation failed'])
+
+ for ihit in watch_hit_list:
+ # 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", STOPPED_DUE_TO_WATCHPOINT,
+ substrs=['stopped', 'stop reason = watchpoint ' + ihit])
+
+ # 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
