Skip to content

Commit df28c81

Browse files
committed
[lldb][debugserver] Fix an off-by-one error in watchpoint identification (#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
1 parent 3756ba3 commit df28c81

File tree

4 files changed

+122
-1
lines changed

4 files changed

+122
-1
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
C_SOURCES := main.c
2+
3+
include Makefile.rules
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
"""
2+
Watch contiguous memory regions with separate watchpoints, check that lldb
3+
correctly detect which watchpoint was hit for each one.
4+
"""
5+
6+
import lldb
7+
from lldbsuite.test.decorators import *
8+
from lldbsuite.test.lldbtest import *
9+
from lldbsuite.test import lldbutil
10+
11+
12+
class ConsecutiveWatchpointsTestCase(TestBase):
13+
NO_DEBUG_INFO_TESTCASE = True
14+
15+
def continue_and_report_stop_reason(self, process, iter_str):
16+
process.Continue()
17+
self.assertIn(
18+
process.GetState(), [lldb.eStateStopped, lldb.eStateExited], iter_str
19+
)
20+
thread = process.GetSelectedThread()
21+
return thread.GetStopReason()
22+
23+
# debugserver only gained the ability to watch larger regions
24+
# with this patch.
25+
def test_consecutive_watchpoints(self):
26+
"""Test watchpoint that covers a large region of memory."""
27+
self.build()
28+
self.main_source_file = lldb.SBFileSpec("main.c")
29+
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
30+
self, "break here", self.main_source_file
31+
)
32+
33+
frame = thread.GetFrameAtIndex(0)
34+
35+
field2_wp = (
36+
frame.locals["var"][0]
37+
.GetChildMemberWithName("field2")
38+
.Watch(True, False, True)
39+
)
40+
field3_wp = (
41+
frame.locals["var"][0]
42+
.GetChildMemberWithName("field3")
43+
.Watch(True, False, True)
44+
)
45+
field4_wp = (
46+
frame.locals["var"][0]
47+
.GetChildMemberWithName("field4")
48+
.Watch(True, False, True)
49+
)
50+
field5_wp = (
51+
frame.locals["var"][0]
52+
.GetChildMemberWithName("field5")
53+
.Watch(True, False, True)
54+
)
55+
56+
# Require that the first two watchpoints
57+
# are set -- hopefully every machine running
58+
# the testsuite can support two watchpoints.
59+
self.assertTrue(field2_wp.IsValid())
60+
self.assertTrue(field3_wp.IsValid())
61+
62+
reason = self.continue_and_report_stop_reason(process, "continue to field2 wp")
63+
self.assertEqual(reason, lldb.eStopReasonWatchpoint)
64+
stop_reason_watchpoint_id = (
65+
process.GetSelectedThread().GetStopReasonDataAtIndex(0)
66+
)
67+
self.assertEqual(stop_reason_watchpoint_id, field2_wp.GetID())
68+
69+
reason = self.continue_and_report_stop_reason(process, "continue to field3 wp")
70+
self.assertEqual(reason, lldb.eStopReasonWatchpoint)
71+
stop_reason_watchpoint_id = (
72+
process.GetSelectedThread().GetStopReasonDataAtIndex(0)
73+
)
74+
self.assertEqual(stop_reason_watchpoint_id, field3_wp.GetID())
75+
76+
# If we were able to set the second two watchpoints,
77+
# check that they are hit. Some CI bots can only
78+
# create two watchpoints.
79+
if field4_wp.IsValid() and field5_wp.IsValid():
80+
reason = self.continue_and_report_stop_reason(
81+
process, "continue to field4 wp"
82+
)
83+
self.assertEqual(reason, lldb.eStopReasonWatchpoint)
84+
stop_reason_watchpoint_id = (
85+
process.GetSelectedThread().GetStopReasonDataAtIndex(0)
86+
)
87+
self.assertEqual(stop_reason_watchpoint_id, field4_wp.GetID())
88+
89+
reason = self.continue_and_report_stop_reason(
90+
process, "continue to field5 wp"
91+
)
92+
self.assertEqual(reason, lldb.eStopReasonWatchpoint)
93+
stop_reason_watchpoint_id = (
94+
process.GetSelectedThread().GetStopReasonDataAtIndex(0)
95+
)
96+
self.assertEqual(stop_reason_watchpoint_id, field5_wp.GetID())
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include <stdint.h>
2+
struct fields {
3+
uint32_t field1;
4+
uint32_t field2; // offset +4
5+
uint16_t field3; // offset +8
6+
uint16_t field4; // offset +10
7+
uint16_t field5; // offset +12
8+
uint16_t field6; // offset +14
9+
};
10+
11+
int main() {
12+
struct fields var = {0, 0, 0, 0, 0, 0};
13+
14+
var.field1 = 5; // break here
15+
var.field2 = 6;
16+
var.field3 = 7;
17+
var.field4 = 8;
18+
var.field5 = 9;
19+
var.field6 = 10;
20+
21+
return var.field1 + var.field2 + var.field3;
22+
}

lldb/tools/debugserver/source/DNBBreakpoint.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ DNBBreakpointList::FindNearestWatchpoint(nub_addr_t addr) const {
9898
if (pos.second.IsEnabled()) {
9999
nub_addr_t start_addr = pos.second.Address();
100100
nub_addr_t end_addr = start_addr + pos.second.ByteSize();
101-
if (addr >= start_addr && addr <= end_addr)
101+
if (addr >= start_addr && addr < end_addr)
102102
return &pos.second;
103103
}
104104
}

0 commit comments

Comments
 (0)