Skip to content

Commit

Permalink
Fix the condition for watchpoint matching on arm64
Browse files Browse the repository at this point in the history
The language from the ARM manual previously quoted in the comment in
fact only pertains to Memory Copy and Memory Set instructions. According
to the section describing the behavior for "other instructions", the
recorded address can be > the watchpoint address. Therefore, remove the
check that the watchpoint address >= the recorded address, and update
the comment for accuracy.

Fixes #3736
  • Loading branch information
pcc authored and rocallahan committed May 1, 2024
1 parent 42ae00b commit 4ed1b81
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 16 deletions.
20 changes: 9 additions & 11 deletions src/AddressSpace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1295,20 +1295,18 @@ bool AddressSpace::notify_watchpoint_fired(uintptr_t debug_status,
watchpoint_triggered(debug_status,
it.second.debug_regs_for_exec_read);
} else {
// The reported address may not match our watchpoint exactly.
// The ARM manual says:
// The address recorded is within an address range of the size defined by the
// DCZID_EL0.BS field. The start of the range is aligned to the size defined
// by the DCZID_EL0.BS field and its end is not greater than the address that
// triggered the watchpoint.
// So we construct a range spanning the whole block, then test that the range
// intersects a watchpoint range *and* that hit_addr is not past the first byte
// of the watched region.
// The reported address may not match our watchpoint exactly. The ARM
// manual specifies a number of conditions that apply to the address for
// various types of instructions (see D2.10.5, "Determining the memory
// location that caused a Watchpoint exception"), but at a minimum it is
// required that the reported address is in the same block as one of the
// addresses monitored by the watchpoint, where the block size is defined
// by DCZID_EL0.BS. So we construct a range spanning the whole block, then
// test that the range intersects a watchpoint range.
auto block_size = dczid_el0_block_size();
auto slop = hit_addr.as_int() % block_size;
auto hit_range = MemoryRange(hit_addr - slop, block_size);
watchpoint_in_range = it.first.intersects(hit_range) &&
it.first.start() >= hit_addr;
watchpoint_in_range = it.first.intersects(hit_range);
}
if (write_triggered || read_triggered || exec_triggered || watchpoint_in_range) {
it.second.changed = true;
Expand Down
6 changes: 1 addition & 5 deletions src/test/watchpoint_unaligned.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ int main(void) {
uintptr_t aligned_addr = ((uintptr_t)m | 0xff) + 1;
test(aligned_addr - 1, aligned_addr - 1);
test(aligned_addr + 16, aligned_addr + 15);

/* FIXME: Currently fails on arm64. */
if (0) {
test(aligned_addr + 15, aligned_addr + 16);
}
test(aligned_addr + 15, aligned_addr + 16);

atomic_puts("EXIT-SUCCESS");
return 0;
Expand Down
1 change: 1 addition & 0 deletions src/test/watchpoint_unaligned.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def test():
expect_gdb(f'watchpoint {wp}')
send_gdb(f'delete {wp}')

test()
test()
test()
ok()

0 comments on commit 4ed1b81

Please sign in to comment.