[Lldb-commits] [lldb] [lldb][debugserver] Fix an off-by-one error in watchpoint identification (PR #134314)
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 3 15:45:57 PDT 2025
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/134314
>From 780fc8f5081f97234749c70a139ad6034f48f3ae Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Thu, 3 Apr 2025 15:38:23 -0700
Subject: [PATCH 1/2] [lldb][debugserver] Fix an off-by-one error in watchpoint
identification
debugserver takes the address of a watchpoint exception and calculates
which watchpoint was responsible for it. There was an off-by-one
error in the range calculation which causes two watchpoints on
consecutive ranges to not correctly identify hits to the second
watchpoint.
rdar://145107575
---
.../consecutive-watchpoints/Makefile | 3 +
.../TestConsecutiveWatchpoints.py | 74 +++++++++++++++++++
.../watchpoint/consecutive-watchpoints/main.c | 22 ++++++
.../debugserver/source/DNBBreakpoint.cpp | 2 +-
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile
create mode 100644 lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
create mode 100644 lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/main.c
diff --git a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile
new file mode 100644
index 0000000000000..10495940055b6
--- /dev/null
+++ b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
new file mode 100644
index 0000000000000..63b41e32ad4f7
--- /dev/null
+++ b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
@@ -0,0 +1,74 @@
+"""
+Watch larger-than-8-bytes regions of memory, confirm that
+writes to those regions are detected.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ConsecutiveWatchpointsTestCase(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def continue_and_report_stop_reason(self, process, iter_str):
+ process.Continue()
+ self.assertIn(
+ process.GetState(), [lldb.eStateStopped, lldb.eStateExited], iter_str
+ )
+ thread = process.GetSelectedThread()
+ return thread.GetStopReason()
+
+ # debugserver only gained the ability to watch larger regions
+ # with this patch.
+ def test_large_watchpoint(self):
+ """Test watchpoint that covers a large region of memory."""
+ self.build()
+ self.main_source_file = lldb.SBFileSpec("main.c")
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "break here", self.main_source_file
+ )
+
+ frame = thread.GetFrameAtIndex(0)
+
+ field2_wp = (
+ frame.locals["var"][0]
+ .GetChildMemberWithName("field2")
+ .Watch(True, False, True)
+ )
+ field3_wp = (
+ frame.locals["var"][0]
+ .GetChildMemberWithName("field3")
+ .Watch(True, False, True)
+ )
+ field4_wp = (
+ frame.locals["var"][0]
+ .GetChildMemberWithName("field4")
+ .Watch(True, False, True)
+ )
+
+ self.assertTrue(field2_wp.IsValid())
+ self.assertTrue(field3_wp.IsValid())
+ self.assertTrue(field4_wp.IsValid())
+
+ reason = self.continue_and_report_stop_reason(process, "continue to field2 wp")
+ self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+ stop_reason_watchpoint_id = (
+ process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+ )
+ self.assertEqual(stop_reason_watchpoint_id, field2_wp.GetID())
+
+ reason = self.continue_and_report_stop_reason(process, "continue to field3 wp")
+ self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+ stop_reason_watchpoint_id = (
+ process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+ )
+ self.assertEqual(stop_reason_watchpoint_id, field3_wp.GetID())
+
+ reason = self.continue_and_report_stop_reason(process, "continue to field4 wp")
+ self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+ stop_reason_watchpoint_id = (
+ process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+ )
+ self.assertEqual(stop_reason_watchpoint_id, field4_wp.GetID())
diff --git a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/main.c b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/main.c
new file mode 100644
index 0000000000000..c0a3530be9f5e
--- /dev/null
+++ b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/main.c
@@ -0,0 +1,22 @@
+#include <stdint.h>
+struct fields {
+ uint32_t field1;
+ uint32_t field2; // offset +4
+ uint16_t field3; // offset +8
+ uint16_t field4; // offset +10
+ uint16_t field5; // offset +12
+ uint16_t field6; // offset +14
+};
+
+int main() {
+ struct fields var = {0, 0, 0, 0, 0, 0};
+
+ var.field1 = 5; // break here
+ var.field2 = 6;
+ var.field3 = 7;
+ var.field4 = 8;
+ var.field5 = 9;
+ var.field6 = 10;
+
+ return var.field1 + var.field2 + var.field3;
+}
diff --git a/lldb/tools/debugserver/source/DNBBreakpoint.cpp b/lldb/tools/debugserver/source/DNBBreakpoint.cpp
index f63ecf24222bd..e41bf9b4fd905 100644
--- a/lldb/tools/debugserver/source/DNBBreakpoint.cpp
+++ b/lldb/tools/debugserver/source/DNBBreakpoint.cpp
@@ -98,7 +98,7 @@ DNBBreakpointList::FindNearestWatchpoint(nub_addr_t addr) const {
if (pos.second.IsEnabled()) {
nub_addr_t start_addr = pos.second.Address();
nub_addr_t end_addr = start_addr + pos.second.ByteSize();
- if (addr >= start_addr && addr <= end_addr)
+ if (addr >= start_addr && addr < end_addr)
return &pos.second;
}
}
>From f3a81e57eb81418d70dbe04f6981bed839894feb Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Thu, 3 Apr 2025 15:45:22 -0700
Subject: [PATCH 2/2] Fix description of test, also test one additional
watchpoint.
---
.../TestConsecutiveWatchpoints.py | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
index 63b41e32ad4f7..229172e6ce0aa 100644
--- a/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
+++ b/lldb/test/API/functionalities/watchpoint/consecutive-watchpoints/TestConsecutiveWatchpoints.py
@@ -1,6 +1,6 @@
"""
-Watch larger-than-8-bytes regions of memory, confirm that
-writes to those regions are detected.
+Watch contiguous memory regions with separate watchpoints, check that lldb
+correctly detect which watchpoint was hit for each one.
"""
import lldb
@@ -47,10 +47,16 @@ def test_large_watchpoint(self):
.GetChildMemberWithName("field4")
.Watch(True, False, True)
)
+ field5_wp = (
+ frame.locals["var"][0]
+ .GetChildMemberWithName("field5")
+ .Watch(True, False, True)
+ )
self.assertTrue(field2_wp.IsValid())
self.assertTrue(field3_wp.IsValid())
self.assertTrue(field4_wp.IsValid())
+ self.assertTrue(field5_wp.IsValid())
reason = self.continue_and_report_stop_reason(process, "continue to field2 wp")
self.assertEqual(reason, lldb.eStopReasonWatchpoint)
@@ -72,3 +78,10 @@ def test_large_watchpoint(self):
process.GetSelectedThread().GetStopReasonDataAtIndex(0)
)
self.assertEqual(stop_reason_watchpoint_id, field4_wp.GetID())
+
+ reason = self.continue_and_report_stop_reason(process, "continue to field5 wp")
+ self.assertEqual(reason, lldb.eStopReasonWatchpoint)
+ stop_reason_watchpoint_id = (
+ process.GetSelectedThread().GetStopReasonDataAtIndex(0)
+ )
+ self.assertEqual(stop_reason_watchpoint_id, field5_wp.GetID())
More information about the lldb-commits
mailing list