omjavaid created this revision.
omjavaid added reviewers: tberghammer, clayborg.
omjavaid added a subscriber: lldb-commits.
Herald added subscribers: rengolin, aemerson.
There were some bugs that needed to be fixed in watchpoint handling code on
arm64.
Watchpoints were being written to all watchpoint registers and cache was not
being maintained correctly.
This patch fixes up all these issues. Watchpoints now work on arm64 except that
there is race condition which is not allowing lldb to step out of a watchpoint
hit before we can continue again so inferior gets stuck at that point. Manually
disabling the watchpoint then issuing a step and then enabling the watchpoint
again fixes shows that watchpoints are being installed and removed correctly.
http://reviews.llvm.org/D11899
Files:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -172,10 +172,10 @@
IsFPR(unsigned reg) const;
Error
- ReadHardwareDebugInfo(unsigned int &watch_count , unsigned int &break_count);
+ ReadHardwareDebugInfo();
Error
- WriteHardwareDebugRegs(lldb::addr_t *addr_buf, uint32_t *cntrl_buf, int type, int count);
+ WriteHardwareDebugRegs(int type);
};
} // namespace process_linux
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -391,14 +391,10 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
- NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
- if (!process_sp)
- return false;
-
// Check if our hardware breakpoint and watchpoint information is updated.
if (m_refresh_hwdebug_info)
{
- ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
+ ReadHardwareDebugInfo ();
m_refresh_hwdebug_info = false;
}
@@ -443,7 +439,8 @@
m_hbr_regs[bp_index].control = control_value;
m_hbr_regs[bp_index].refcount = 1;
- //TODO: PTRACE CALL HERE for an UPDATE
+ // PTRACE call to set corresponding hardware breakpoint register.
+ WriteHardwareDebugRegs(1);
}
else
m_hbr_regs[bp_index].refcount++;
@@ -459,6 +456,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+ // Check if our hardware breakpoint and watchpoint information is updated.
+ if (m_refresh_hwdebug_info)
+ {
+ ReadHardwareDebugInfo ();
+ m_refresh_hwdebug_info = false;
+ }
+
if (hw_idx >= m_max_hbp_supported)
return false;
@@ -474,8 +478,8 @@
m_hbr_regs[hw_idx].address = 0;
m_hbr_regs[hw_idx].refcount = 0;
- //TODO: PTRACE CALL HERE for an UPDATE
- return true;
+ // PTRACE call to clear corresponding hardware breakpoint register.
+ WriteHardwareDebugRegs(1);
}
return false;
@@ -489,6 +493,13 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
+ // Check if our hardware breakpoint and watchpoint information is updated.
+ if (m_refresh_hwdebug_info)
+ {
+ ReadHardwareDebugInfo ();
+ m_refresh_hwdebug_info = false;
+ }
+
return m_max_hwp_supported;
}
@@ -499,33 +510,31 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
-
- NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
- if (!process_sp)
- return false;
-
// Check if our hardware breakpoint and watchpoint information is updated.
if (m_refresh_hwdebug_info)
{
- ReadHardwareDebugInfo (m_max_hwp_supported, m_max_hbp_supported);
+ ReadHardwareDebugInfo ();
m_refresh_hwdebug_info = false;
}
uint32_t control_value, wp_index;
-
+ // Check if we are setting watchpoint other than read/write/access
if (watch_flags != 0x1 && watch_flags != 0x2 && watch_flags != 0x3)
- return 0;//Error ("Invalid read/write bits for watchpoint");
+ return LLDB_INVALID_INDEX32;
// Check if size has a valid hardware watchpoint length.
if (size != 1 && size != 2 && size != 4 && size != 8)
- return 0;//Error ("Invalid size for watchpoint");
+ return LLDB_INVALID_INDEX32;
// Check 8-byte alignment for hardware watchpoint target address.
// TODO: Add support for watching un-aligned addresses
if (addr & 0x07)
- return 0;//Error ("LLDB for AArch64 currently supports 8-byte alignment for hardware watchpoint target address.");
+ return LLDB_INVALID_INDEX32;
+
+ // Flip watchpoint flag bits to match AArch64 write-read bit configuration.
+ watch_flags = ((watch_flags >> 1) | (watch_flags << 1));
// Setup control value
control_value = watch_flags << 3;
@@ -554,12 +563,13 @@
// 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].address = addr;
m_hwp_regs[wp_index].control = control_value;
m_hwp_regs[wp_index].refcount = 1;
// PTRACE call to set corresponding watchpoint register.
- WriteHardwareDebugRegs(&addr, &control_value, 0, wp_index);
+ WriteHardwareDebugRegs(0);
}
else
m_hwp_regs[wp_index].refcount++;
@@ -575,9 +585,12 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
- NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
- if (!process_sp)
- return false;
+ // Check if our hardware breakpoint and watchpoint information is updated.
+ if (m_refresh_hwdebug_info)
+ {
+ ReadHardwareDebugInfo ();
+ m_refresh_hwdebug_info = false;
+ }
if (wp_index >= m_max_hwp_supported)
return false;
@@ -590,12 +603,13 @@
}
else if (m_hwp_regs[wp_index].refcount == 1)
{
+ // 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;
- //TODO: PTRACE CALL HERE for an UPDATE
- WriteHardwareDebugRegs(&m_hwp_regs[wp_index].address, &m_hwp_regs[wp_index].control, 0, wp_index);
+ // Ptrace call to update hardware debug registers
+ WriteHardwareDebugRegs(0);
return true;
}
@@ -610,22 +624,24 @@
if (log)
log->Printf ("NativeRegisterContextLinux_arm64::%s()", __FUNCTION__);
- NativeProcessProtocolSP process_sp (m_thread.GetProcess ());
-
- Error ml_error;
- ml_error.SetErrorToErrno();
- if (!process_sp)
- return ml_error;
+ // Check if our hardware breakpoint and watchpoint information is updated.
+ if (m_refresh_hwdebug_info)
+ {
+ ReadHardwareDebugInfo ();
+ m_refresh_hwdebug_info = false;
+ }
for (uint32_t i = 0; i < m_max_hwp_supported; i++)
{
if (m_hwp_regs[i].control & 0x01)
{
+ // Clear watchpoints in local cache
m_hwp_regs[i].control &= ~1;
m_hwp_regs[i].address = 0;
m_hwp_regs[i].refcount = 0;
- WriteHardwareDebugRegs(&m_hwp_regs[i].address, &m_hwp_regs[i].control, 0, i);
+ // Ptrace call to update hardware debug registers
+ WriteHardwareDebugRegs(0);
}
}
@@ -712,8 +728,7 @@
}
Error
-NativeRegisterContextLinux_arm64::ReadHardwareDebugInfo(unsigned int &watch_count,
- unsigned int &break_count)
+NativeRegisterContextLinux_arm64::ReadHardwareDebugInfo()
{
::pid_t tid = m_thread.GetID();
@@ -725,20 +740,17 @@
ioVec.iov_base = &dreg_state;
ioVec.iov_len = sizeof (dreg_state);
error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set, &ioVec, ioVec.iov_len);
- watch_count = dreg_state.dbg_info & 0xff;
+ m_max_hwp_supported = dreg_state.dbg_info & 0xff;
regset = NT_ARM_HW_BREAK;
error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set, &ioVec, ioVec.iov_len);
- break_count = dreg_state.dbg_info & 0xff;
+ m_max_hbp_supported = dreg_state.dbg_info & 0xff;
return error;
}
Error
-NativeRegisterContextLinux_arm64::WriteHardwareDebugRegs(lldb::addr_t *addr_buf,
- uint32_t *cntrl_buf,
- int type,
- int count)
+NativeRegisterContextLinux_arm64::WriteHardwareDebugRegs(int type)
{
struct iovec ioVec;
struct user_hwdebug_state dreg_state;
@@ -749,14 +761,24 @@
ioVec.iov_len = sizeof (dreg_state);
if (type == 0)
+ {
type = NT_ARM_HW_WATCH;
+
+ for (uint32_t i = 0; i < m_max_hwp_supported; i++)
+ {
+ dreg_state.dbg_regs[i].addr = m_hwp_regs[i].address;
+ dreg_state.dbg_regs[i].ctrl = m_hwp_regs[i].control;
+ }
+ }
else
+ {
type = NT_ARM_HW_BREAK;
- for (int i = 0; i < count; i++)
- {
- dreg_state.dbg_regs[i].addr = addr_buf[i];
- dreg_state.dbg_regs[i].ctrl = cntrl_buf[i];
+ for (uint32_t i = 0; i < m_max_hbp_supported; i++)
+ {
+ dreg_state.dbg_regs[i].addr = m_hbr_regs[i].address;
+ dreg_state.dbg_regs[i].ctrl = m_hbr_regs[i].control;
+ }
}
return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, m_thread.GetID(), &type, &ioVec, ioVec.iov_len);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
