mgorny created this revision.
mgorny added reviewers: labath, krytarowski.

Fix handling concurrent watchpoint events so that they are reported
correctly in LLDB.

If multiple watchpoints are hit concurrently, the NetBSD kernel reports
them as series of SIGTRAPs with a thread specified, and the debugger
investigates DR6 in order to establish which watchpoint was hit.  This
is normally fine.

However, LLDB disables and reenables the watchpoint on all threads after
each hit, which results in the hit status from DR6 being wiped.
As a result, it can't establish which watchpoint was hit in successive
SIGTRAP processing.

In order to workaround this problem, clear DR6 only if the breakpoint
is overwritten with a new one.  More specifically, move cleaning DR6
from ClearHardwareWatchpoint() to SetHardwareWatchpointWithIndex(),
and do that only if the newly requested watchpoint is different
from the one being set previously.  This ensures that the disable-enable
logic of LLDB does not clear watchpoint hit status for the remaining
threads.

This also involves refactoring of watchpoint logic.  With the old logic,
clearing watchpoint involved wiping dr6 & dr7, and setting it setting
dr{0..3} & dr7.  With the new logic, only enable bit is cleared
from dr7, and the remaining bits are cleared/overwritten while setting
new watchpoint.


https://reviews.llvm.org/D70025

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
@@ -61,6 +61,8 @@
 
   bool ClearHardwareWatchpoint(uint32_t wp_index) override;
 
+  Status ClearWatchpointHit(uint32_t wp_index) override;
+
   Status ClearAllHardwareWatchpoints() override;
 
   Status SetHardwareWatchpointWithIndex(lldb::addr_t addr, size_t size,
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -860,10 +860,10 @@
   if (!is_vacant)
     return Status("Watchpoint index not vacant");
 
-  RegisterValue reg_value;
   const RegisterInfo *const reg_info_dr7 =
       GetRegisterInfoAtIndex(lldb_dr7_x86_64);
-  error = ReadRegister(reg_info_dr7, reg_value);
+  RegisterValue dr7_value;
+  error = ReadRegister(reg_info_dr7, dr7_value);
   if (error.Fail())
     return error;
 
@@ -881,16 +881,41 @@
 
   uint64_t bit_mask = (0x3 << (2 * wp_index)) | (0xF << (16 + 4 * wp_index));
 
-  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
+  uint64_t control_bits = dr7_value.GetAsUInt64() & ~bit_mask;
 
   control_bits |= enable_bit | rw_bits | size_bits;
 
   const RegisterInfo *const reg_info_drN =
       GetRegisterInfoAtIndex(lldb_dr0_x86_64 + wp_index);
-  error = WriteRegister(reg_info_drN, RegisterValue(addr));
+  RegisterValue drN_value;
+  error = ReadRegister(reg_info_drN, drN_value);
   if (error.Fail())
     return error;
 
+  // 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)) {
+    // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0, 1, 2, or 3 of
+    // the debug status register (DR6)
+    const RegisterInfo *const reg_info_dr6 =
+        GetRegisterInfoAtIndex(lldb_dr6_x86_64);
+    RegisterValue dr6_value;
+    error = ReadRegister(reg_info_dr6, dr6_value);
+    if (error.Fail())
+      return error;
+
+    uint64_t dr6_bit_mask = 1 << wp_index;
+    uint64_t dr6_bits = dr6_value.GetAsUInt64() & ~dr6_bit_mask;
+    error = WriteRegister(reg_info_dr6, RegisterValue(dr6_bits));
+    if (error.Fail())
+      return error;
+
+    error = WriteRegister(reg_info_drN, RegisterValue(addr));
+    if (error.Fail())
+      return error;
+  }
+
   error = WriteRegister(reg_info_dr7, RegisterValue(control_bits));
   if (error.Fail())
     return error;
@@ -904,32 +929,36 @@
   if (wp_index >= NumSupportedHardwareWatchpoints())
     return false;
 
+  // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0-1, 2-3, 4-5
+  // or 6-7 of the debug control register (DR7)
+  const RegisterInfo *const reg_info_dr7 =
+      GetRegisterInfoAtIndex(lldb_dr7_x86_64);
   RegisterValue reg_value;
+  Status error = ReadRegister(reg_info_dr7, reg_value);
+  if (error.Fail())
+    return false;
+  uint64_t bit_mask = 0x3 << (2 * wp_index);
+  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
+
+  return WriteRegister(reg_info_dr7, RegisterValue(control_bits)).Success();
+}
 
-  // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0, 1, 2, or 3 of
+Status NativeRegisterContextNetBSD_x86_64::ClearWatchpointHit(uint32_t wp_index) {
+  if (wp_index >= NumSupportedHardwareWatchpoints())
+    return Status("Watchpoint index out of range");
+
+  // for watchpoints 0, 1, 2, or 3, respectively, check bits 0, 1, 2, or 3 of
   // the debug status register (DR6)
   const RegisterInfo *const reg_info_dr6 =
       GetRegisterInfoAtIndex(lldb_dr6_x86_64);
+  RegisterValue reg_value;
   Status error = ReadRegister(reg_info_dr6, reg_value);
   if (error.Fail())
-    return false;
+    return error;
+
   uint64_t bit_mask = 1 << wp_index;
   uint64_t status_bits = reg_value.GetAsUInt64() & ~bit_mask;
-  error = WriteRegister(reg_info_dr6, RegisterValue(status_bits));
-  if (error.Fail())
-    return false;
-
-  // for watchpoints 0, 1, 2, or 3, respectively, clear bits {0-1,16-19},
-  // {2-3,20-23}, {4-5,24-27}, or {6-7,28-31} of the debug control register
-  // (DR7)
-  const RegisterInfo *const reg_info_dr7 =
-      GetRegisterInfoAtIndex(lldb_dr7_x86_64);
-  error = ReadRegister(reg_info_dr7, reg_value);
-  if (error.Fail())
-    return false;
-  bit_mask = (0x3 << (2 * wp_index)) | (0xF << (16 + 4 * wp_index));
-  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
-  return WriteRegister(reg_info_dr7, RegisterValue(control_bits)).Success();
+  return WriteRegister(reg_info_dr6, RegisterValue(status_bits));
 }
 
 Status NativeRegisterContextNetBSD_x86_64::ClearAllHardwareWatchpoints() {
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
@@ -33,6 +33,8 @@
   virtual Status
   CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) = 0;
 
+  virtual Status ClearWatchpointHit(uint32_t wp_index) = 0;
+
 protected:
   Status DoRegisterSet(int req, void *buf);
   virtual NativeProcessNetBSD &GetProcess();
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===================================================================
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -8,7 +8,7 @@
 
 #include "NativeProcessNetBSD.h"
 
-
+#include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h"
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
@@ -299,9 +299,11 @@
     if (!thread)
       break;
 
+    auto &regctx = static_cast<NativeRegisterContextNetBSD &>(
+        thread->GetRegisterContext());
     uint32_t wp_index = LLDB_INVALID_INDEX32;
-    Status error = thread->GetRegisterContext().GetWatchpointHitIndex(
-        wp_index, (uintptr_t)info.psi_siginfo.si_addr);
+    Status error = regctx.GetWatchpointHitIndex(wp_index,
+        (uintptr_t)info.psi_siginfo.si_addr);
     if (error.Fail())
       LLDB_LOG(log,
                "received error while checking for watchpoint hits, pid = "
@@ -309,14 +311,15 @@
                GetID(), info.psi_lwpid, error);
     if (wp_index != LLDB_INVALID_INDEX32) {
       thread->SetStoppedByWatchpoint(wp_index);
+      regctx.ClearWatchpointHit(wp_index);
       SetState(StateType::eStateStopped, true);
       break;
     }
 
     // If a breakpoint was hit, report it
     uint32_t bp_index = LLDB_INVALID_INDEX32;
-    error = thread->GetRegisterContext().GetHardwareBreakHitIndex(
-        bp_index, (uintptr_t)info.psi_siginfo.si_addr);
+    error = regctx.GetHardwareBreakHitIndex(bp_index,
+        (uintptr_t)info.psi_siginfo.si_addr);
     if (error.Fail())
       LLDB_LOG(log,
                "received error while checking for hardware "
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to