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 <jmole...@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 <jmole...@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()) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits