[lldb][debugserver] Fix an off-by-one error in watchpoint identification #134314
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
|
@llvm/pr-subscribers-lldb Author: Jason Molenda (jasonmolenda) Changesdebugserver 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 Full diff: https://github.com/llvm/llvm-project/pull/134314.diff 4 Files Affected:
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;
}
}
|
additional watchpoint.
JDevlieghere
approved these changes
Apr 4, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/15581 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/14116 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/7267 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/3684 Here is the relevant piece of the build log for the reference |
jasonmolenda
added a commit
that referenced
this pull request
Apr 7, 2025
…ion (#134314) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575
jasonmolenda
added a commit
to jasonmolenda/llvm-project
that referenced
this pull request
Apr 8, 2025
…ion (llvm#134314) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575 (cherry picked from commit df28c81)
jasonmolenda
added a commit
to jasonmolenda/llvm-project
that referenced
this pull request
Apr 8, 2025
…ion (llvm#134314) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575 (cherry picked from commit df28c81) (cherry picked from commit 4cf806a)
jasonmolenda
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Apr 14, 2025
…ion (llvm#134314) (#10450) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575 (cherry picked from commit df28c81) (cherry picked from commit 4cf806a)
jasonmolenda
added a commit
to jasonmolenda/llvm-project
that referenced
this pull request
Apr 14, 2025
…ion (llvm#134314) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575 (cherry picked from commit df28c81)
jasonmolenda
added a commit
to jasonmolenda/llvm-project
that referenced
this pull request
Apr 15, 2025
…ion (llvm#134314) 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. The result is that lldb wouldn't show the second watchpoint as ever being hit. Re-landing this test with a modification to only require two watchpoints in the test, instead of four. If four watchpoints can be set, it will test them. rdar://145107575 (cherry picked from commit df28c81)
adrian-prantl
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Apr 15, 2025
…cent-watchpoints-correctly-62 [lldb][debugserver] Fix an off-by-one error in watchpoint identification (llvm#134314)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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. The result is that lldb wouldn't show the second watchpoint as ever being hit.
rdar://145107575