[Lldb-commits] [lldb] r284706 - Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
Omair Javaid via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 20 02:07:27 PDT 2016
Author: omjavaid
Date: Thu Oct 20 04:07:26 2016
New Revision: 284706
URL: http://llvm.org/viewvc/llvm-project?rev=284706&view=rev
Log:
Fix ARM/AArch64 Step-Over watchpoint issue remove provision for duplicate watchpoints
This patch fixes ARM/AArch64 watchpoint bug which was taking inferior out of control while stepping over watchpoints.
Also adds a test case that tests above problem.
Differential revision: https://reviews.llvm.org/D25057
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile?rev=284706&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/Makefile Thu Oct 20 04:07:26 2016
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules
Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py?rev=284706&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/TestWatchpointMultipleSlots.py Thu Oct 20 04:07:26 2016
@@ -0,0 +1,97 @@
+"""
+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'])
+ # 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")
Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c?rev=284706&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multi_watchpoint_slots/main.c Thu Oct 20 04:07:26 2016
@@ -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;
+}
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp?rev=284706&r1=284705&r2=284706&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp Thu Oct 20 04:07:26 2016
@@ -570,42 +570,33 @@ uint32_t NativeRegisterContextLinux_arm:
// 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;
-
- // 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;
+ // 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;
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ // 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;
+
+ return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -628,36 +619,25 @@ bool NativeRegisterContextLinux_arm::Cle
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, wp_index);
+
+ 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, 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;
- }
-
- return true;
+ return false;
}
- return false;
+ return true;
}
Error NativeRegisterContextLinux_arm::ClearAllHardwareWatchpoints() {
@@ -675,19 +655,17 @@ Error NativeRegisterContextLinux_arm::Cl
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);
@@ -695,7 +673,6 @@ Error NativeRegisterContextLinux_arm::Cl
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 +727,8 @@ Error NativeRegisterContextLinux_arm::Ge
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();
}
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp?rev=284706&r1=284705&r2=284706&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp Thu Oct 20 04:07:26 2016
@@ -538,42 +538,33 @@ uint32_t NativeRegisterContextLinux_arm6
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;
-
- // 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;
+ // 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;
- return LLDB_INVALID_INDEX32;
- }
- } else
- m_hwp_regs[wp_index].refcount++;
+ // 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;
+
+ return LLDB_INVALID_INDEX32;
+ }
return wp_index;
}
@@ -596,36 +587,25 @@ bool NativeRegisterContextLinux_arm64::C
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,12 +630,10 @@ Error NativeRegisterContextLinux_arm64::
// 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);
@@ -663,7 +641,6 @@ Error NativeRegisterContextLinux_arm64::
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 +695,8 @@ Error NativeRegisterContextLinux_arm64::
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();
}
More information about the lldb-commits
mailing list