[Lldb-commits] [lldb] 64799fb - [debugserver/ARM64] Make sure watchpoints hit are attributed correctly.

[Davide Italiano via lldb-commits]

Author: Davide Italiano
Date: 2020-03-31T15:56:20-07:00
New Revision: 64799fbebddc9877f78c7501b0b986b7afe84d6b

URL: https://github.com/llvm/llvm-project/commit/64799fbebddc9877f78c7501b0b986b7afe84d6b
DIFF: https://github.com/llvm/llvm-project/commit/64799fbebddc9877f78c7501b0b986b7afe84d6b.diff

LOG: [debugserver/ARM64] Make sure watchpoints hit are attributed correctly.

This didn't happen for arm64 if you have watches for variables
that are contigous in memory.

<rdar://problem/55135006>

Added: 
    lldb/test/API/commands/watchpoints/watchpoint_count/Makefile
    lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
    lldb/test/API/commands/watchpoints/watchpoint_count/main.c

Modified: 
    lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_count/Makefile b/lldb/test/API/commands/watchpoints/watchpoint_count/Makefile
new file mode 100644
index 000000000000..10495940055b
--- /dev/null
+++ b/lldb/test/API/commands/watchpoints/watchpoint_count/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules

diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py b/lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
new file mode 100644
index 000000000000..18667e913a94
--- /dev/null
+++ b/lldb/test/API/commands/watchpoints/watchpoint_count/TestWatchpointCount.py
@@ -0,0 +1,43 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestWatchpointCount(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    def test_watchpoint_count(self):
+        self.build()
+        (_, process, thread, _) = lldbutil.run_to_source_breakpoint(self, "patatino", lldb.SBFileSpec("main.c"))
+        frame = thread.GetFrameAtIndex(0)
+        first_var = frame.FindVariable("x1")
+        second_var = frame.FindVariable("x2")
+
+        error = lldb.SBError()
+        first_watch = first_var.Watch(True, False, True, error)
+        if not error.Success():
+            self.fail(
+                "Failed to make watchpoint for x1: %s" %
+                (error.GetCString()))
+
+        second_watch = second_var.Watch(True, False, True, error)
+        if not error.Success():
+            self.fail(
+                "Failed to make watchpoint for x2: %s" %
+                (error.GetCString()))
+        process.Continue()
+
+        stop_reason = thread.GetStopReason()
+        self.assertEqual(stop_reason, lldb.eStopReasonWatchpoint, "watchpoint for x1 not hit")
+        stop_reason_descr = thread.GetStopDescription(256)
+        self.assertEqual(stop_reason_descr, "watchpoint 1")
+
+        process.Continue()
+        stop_reason = thread.GetStopReason()
+        self.assertEqual(stop_reason, lldb.eStopReasonWatchpoint, "watchpoint for x2 not hit")
+        stop_reason_descr = thread.GetStopDescription(256)
+        self.assertEqual(stop_reason_descr, "watchpoint 2")

diff  --git a/lldb/test/API/commands/watchpoints/watchpoint_count/main.c b/lldb/test/API/commands/watchpoints/watchpoint_count/main.c
new file mode 100644
index 000000000000..fc9a370e41f3
--- /dev/null
+++ b/lldb/test/API/commands/watchpoints/watchpoint_count/main.c
@@ -0,0 +1,13 @@
+#include <stdint.h>
+#include <stdio.h>
+
+int main() {
+  uint8_t x1 = 0;
+  uint16_t x2 = 0;
+
+  printf("patatino\n");
+
+  x1 += 1;
+  x2 += 2;
+  return 0;
+}

diff  --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index e5d4b05d987c..3e7bda88e6af 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -1067,31 +1067,34 @@ uint32_t DNBArchMachARM64::GetHardwareWatchpointHit(nub_addr_t &addr) {
                    "DNBArchMachARM64::GetHardwareWatchpointHit() addr = 0x%llx",
                    (uint64_t)addr);
 
-  // This is the watchpoint value to match against, i.e., word address.
-  nub_addr_t wp_val = addr & ~((nub_addr_t)3);
   if (kret == KERN_SUCCESS) {
     DBG &debug_state = m_state.dbg;
     uint32_t i, num = NumSupportedHardwareWatchpoints();
     for (i = 0; i < num; ++i) {
       nub_addr_t wp_addr = GetWatchAddress(debug_state, i);
-      DNBLogThreadedIf(LOG_WATCHPOINTS, "DNBArchMachARM64::"
-                                        "GetHardwareWatchpointHit() slot: %u "
-                                        "(addr = 0x%llx).",
-                       i, (uint64_t)wp_addr);
-      if (wp_val == wp_addr) {
-        uint32_t byte_mask = bits(debug_state.__wcr[i], 12, 5);
-
-        // Sanity check the byte_mask, first.
-        if (LowestBitSet(byte_mask) < 0)
-          continue;
-
-        // Check that the watchpoint is enabled.
-        if (!IsWatchpointEnabled(debug_state, i))
-          continue;
-
-        // Compute the starting address (from the point of view of the
-        // debugger).
-        addr = wp_addr + LowestBitSet(byte_mask);
+      uint32_t byte_mask = bits(debug_state.__wcr[i], 12, 5);
+
+      DNBLogThreadedIf(LOG_WATCHPOINTS, "DNBArchImplX86_64::"
+                       "GetHardwareWatchpointHit() slot: %u "
+                       "(addr = 0x%llx; byte_mask = 0x%x)",
+                       i, static_cast<uint64_t>(wp_addr),
+                       byte_mask);
+
+      if (!IsWatchpointEnabled(debug_state, i))
+        continue;
+
+      if (bits(wp_addr, 48, 3) != bits(addr, 48, 3))
+        continue;
+
+      // Sanity check the byte_mask
+      uint32_t lsb = LowestBitSet(byte_mask);
+      if (lsb < 0)
+        continue;
+
+      uint64_t byte_to_match = bits(addr, 2, 0);
+
+      if (byte_mask & (1 << byte_to_match)) {
+        addr = wp_addr + lsb;
         return i;
       }
     }