[Lldb-commits] [PATCH] D89874: [lldb] Unify x86 watchpoint implementation on Linux and NetBSD (and future FreeBSD plugin) [WIP]

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:296
     const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
-    : NativeRegisterContextLinux(native_thread,
+    : NativeRegisterContextRegisterInfo(native_thread, CreateRegisterInfoInterface(target_arch)),
+      NativeRegisterContextLinux(native_thread,
----------------
I'm wondering if we can eliminate the init inside `NativeRegisterContextRegisterInfo`


================
Comment at: lldb/source/Plugins/Process/Utility/WatchpointMixin_x86.cpp:146-155
+  // clear dr6 if address or bits changed (i.e. we're not reenabling the same
+  // watchpoint)
+  if (drN_value.GetAsUInt64() != addr ||
+      (dr7_value.GetAsUInt64() & bit_mask) != (rw_bits | size_bits)) {
+    ClearWatchpointHit(wp_index);
+
+    error = WriteRegister(reg_info_drN, RegisterValue(addr));
----------------
This is the DR6 cleanup I was talking about.


================
Comment at: lldb/source/Plugins/Process/Utility/WatchpointMixin_x86.cpp:183
+
+Status WatchpointMixin_x86::ClearWatchpointHit(uint32_t wp_index) {
+  if (wp_index >= NumSupportedHardwareWatchpoints())
----------------
On NetBSD, we have to call this explicitly when processing watchpoint hit (in SIGTRAP handling). Not sure if Linux would need that too (in which case we'd have to add a virtual method to the base class, I guess), or if the kernel takes care of it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89874/new/

https://reviews.llvm.org/D89874