omjavaid updated this revision to Diff 72723.
omjavaid added a comment.
Give this approach a rethink I dont see a lot of problems with this final
implementation unless it fails on other architectures.
We are already hacking our way to have these byte selection watchpoints working
in existing code. New code seems to be improving the hack in my opinion.
Let me explain what I am doing and may be you can provide your suggestion and
feedback.
Watchpoint Install => Register watchpoint into hardware watchpoint register
cache
Watchpoint Enable => Enable in cache and write registers using ptrace interface.
Ideally we should be able to install/uninstall watchpoints in cache and then
enable them all on a resume.
In case of arm If a watchpoint is hit we should be able to disable that
watchpoint.
Step over watchpoint instruction and then re-enable the watchpoint.
Our existing implementation will require a lot of changes to do that so thats
why here is what i am doing.
SetHardwareWatchpoint => Performs Install and Enable
- If a new watchpoint slot is going to be used we Install and enable.
- For new watchpoint we should be able to complete both Install and or we
report an error.
- If a duplicate slot is going to be used we Install and enable if required.
- Install means updating size if needed plus updating ref count.
- Enable means updating registers if size was updated.
ClearHardwareWatchpoint
- Disable and uinstall watchpoint means
- Decrement ref count and clear hardware watchpoint regsiters.
- Advantage of keeping ref counts is:
- If refcount is greater than zero then SetHardwareWatchpoint cannot use this
slot for a new watchpoint (new address).
- But SetHardwareWatchpoint can be use this slot to install duplicate
watchpoints (same address but different byte or word)
ClearAllHardwareWatchpoint -- Just clear the whole watchpoint cache and call
SetHardwareWatchpoint for all available watchpoints.
NativeThreadLinux:
On Watchpoint Remove -> Invalidate watchpoint cache
On Resume - > Re-validate watchpoints by creating a new cache and re-enabling
all watchpoints
So this fixes our step-over issue and also preserves watchpoint slot if it is
being used by multiple watchpoints.
Can you think of any scenarios which might fail for 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
source/Plugins/Process/Linux/NativeThreadLinux.h
Index: source/Plugins/Process/Linux/NativeThreadLinux.h
===================================================================
--- source/Plugins/Process/Linux/NativeThreadLinux.h
+++ source/Plugins/Process/Linux/NativeThreadLinux.h
@@ -108,6 +108,7 @@
using WatchpointIndexMap = std::map<lldb::addr_t, uint32_t>;
WatchpointIndexMap m_watchpoint_index_map;
cpu_set_t m_original_cpu_set; // For single-step workaround.
+ bool m_invalidate_watchpoints;
};
typedef std::shared_ptr<NativeThreadLinux> NativeThreadLinuxSP;
Index: source/Plugins/Process/Linux/NativeThreadLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -87,7 +87,8 @@
NativeThreadLinux::NativeThreadLinux(NativeProcessLinux *process,
lldb::tid_t tid)
: NativeThreadProtocol(process, tid), m_state(StateType::eStateInvalid),
- m_stop_info(), m_reg_context_sp(), m_stop_description() {}
+ m_stop_info(), m_reg_context_sp(), m_stop_description(),
+ m_invalidate_watchpoints(false) {}
std::string NativeThreadLinux::GetName() {
NativeProcessProtocolSP process_sp = m_process_wp.lock();
@@ -185,8 +186,10 @@
return Error();
uint32_t wp_index = wp->second;
m_watchpoint_index_map.erase(wp);
- if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index))
+ if (GetRegisterContext()->ClearHardwareWatchpoint(wp_index)) {
+ m_invalidate_watchpoints = true;
return Error();
+ }
return Error("Clearing hardware watchpoint failed.");
}
@@ -198,8 +201,13 @@
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
+ if (m_invalidate_watchpoints) {
+ m_invalidate_watchpoints = false;
+ 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,71 @@
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);
+ lldb::addr_t tempAddr = 0;
+ uint32_t tempControl = 0;
- // 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
- }
- }
+ // Create a backup we can revert to in case of failure.
+ tempAddr = m_hwp_regs[wp_index].address;
+ tempControl = m_hwp_regs[wp_index].control;
+
+ // Update watchpoint in local cache
+ 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);
+
+ if (error.Fail()) {
+ m_hwp_regs[wp_index].control = tempControl;
+ m_hwp_regs[wp_index].address = tempAddr;
- 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].address = 0;
- m_hwp_regs[wp_index].control &= ~1;
- m_hwp_regs[wp_index].refcount = 0;
-
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ m_hwp_regs[wp_index].real_addr = real_addr;
+ m_hwp_regs[wp_index].refcount++;
return wp_index;
}
@@ -628,36 +624,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 +669,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 +684,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 +739,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,135 @@
+"""
+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'])
+ # This is a arm specific test case. No other architectures tested.
+ @skipIf(archs=no_match(['arm']))
+ 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'])
+ # This is a arm specific test case. No other architectures tested.
+ @skipIf(archs=no_match(['arm']))
+ 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'], error=True)
+
+ 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
