tberghammer added a comment.
Generally looks good to me. I am happy to push the 2 cleanup change to a
separate CL but please check that the read/write flag calculation is correct.
================
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:514-519
@@ -507,8 +513,8 @@
// 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;
}
----------------
You use this pattern several times in this class. It would be nice if you can
move it out to a separate function to avoid code duplication.
================
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:537
@@ -529,1 +536,3 @@
+ // Flip watchpoint flag bits to match AArch64 write-read bit configuration.
+ watch_flags = ((watch_flags >> 1) | (watch_flags << 1));
----------------
I think this expression is incorrect. Based on
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500d/CEGDIEBB.html
I don't see why you need this at all and if watch_flags==0x3 then it will set
watch_flags to 0x7 what looks wrong for me.
================
Comment at:
source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:753
@@ -737,6 +752,3 @@
Error
-NativeRegisterContextLinux_arm64::WriteHardwareDebugRegs(lldb::addr_t
*addr_buf,
- uint32_t *cntrl_buf,
- int type,
- int count)
+NativeRegisterContextLinux_arm64::WriteHardwareDebugRegs(int type)
{
----------------
The 0/1 for the type isn't too descriptive. Please use an enum here (or the
NT_ARM_HW_WATCH/NT_ARM_HW_BREAK constants)
http://reviews.llvm.org/D11899
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
