diff options
author | Jan Kratochvil <jan.kratochvil@redhat.com> | 2018-05-04 22:22:04 +0200 |
---|---|---|
committer | Jan Kratochvil <jan.kratochvil@redhat.com> | 2018-05-04 22:26:46 +0200 |
commit | a3b60e4588606354b93508a0008a5ca04b68fad8 (patch) | |
tree | 87962568d24528162603c7fd5f3be6a63f25f174 /gdb/gdbserver/linux-aarch64-low.c | |
parent | 45fe4a03b495f778013f0a0deb06512913e1955b (diff) | |
download | gdb-a3b60e4588606354b93508a0008a5ca04b68fad8.zip gdb-a3b60e4588606354b93508a0008a5ca04b68fad8.tar.gz gdb-a3b60e4588606354b93508a0008a5ca04b68fad8.tar.bz2 |
aarch64: PR 19806: watchpoints: false negatives + PR 20207 contiguous ones
Some unaligned watchpoints were currently missed.
On old kernels as specified in
kernel RFE: aarch64: ptrace: BAS: Support any contiguous range (edit)
https://sourceware.org/bugzilla/show_bug.cgi?id=20207
after this patch some other unaligned watchpoints will get reported as false
positives.
With new kernels all the watchpoints should work exactly.
There may be a regresion that it now less merges watchpoints so that with
multiple overlapping watchpoints it may run out of the 4 hardware watchpoint
registers. But as discussed in the original thread GDB needs some generic
watchpoints merging framework to be used by all the target specific code.
Even current FSF GDB code does not merge it perfectly. Also with the more
precise watchpoints one can technically merge them less. And I do not think
it matters too much to improve mergeability only for old kernels.
Still even on new kernels some better merging logic would make sense.
There remains one issue:
kernel-4.15.14-300.fc27.armv7hl
FAIL: gdb.base/watchpoint-unaligned.exp: continue
FAIL: gdb.base/watchpoint-unaligned.exp: continue
(gdb) continue
Continuing.
Unexpected error setting watchpoint: Invalid argument.
(gdb) FAIL: gdb.base/watchpoint-unaligned.exp: continue
But that looks as a kernel bug to me.
(1) It is not a regression by this patch.
(2) It is unrelated to this patch.
gdb/ChangeLog
2018-05-04 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
PR breakpoints/19806 and support for PR external/20207.
* NEWS: Mention Aarch64 watchpoint improvements.
* aarch64-linux-nat.c (aarch64_linux_stopped_data_address): Fix missed
watchpoints and PR external/20207 watchpoints.
* nat/aarch64-linux-hw-point.c
(kernel_supports_any_contiguous_range): New.
(aarch64_watchpoint_offset): New.
(aarch64_watchpoint_length): Support PR external/20207 watchpoints.
(aarch64_point_encode_ctrl_reg): New parameter offset, new asserts.
(aarch64_point_is_aligned): Support PR external/20207 watchpoints.
(aarch64_align_watchpoint): New parameters aligned_offset_p and
next_addr_orig_p. Support PR external/20207 watchpoints.
(aarch64_downgrade_regs): New.
(aarch64_dr_state_insert_one_point): New parameters offset and
addr_orig.
(aarch64_dr_state_remove_one_point): Likewise.
(aarch64_handle_breakpoint): Update caller.
(aarch64_handle_aligned_watchpoint): Likewise.
(aarch64_handle_unaligned_watchpoint): Support addr_orig and
aligned_offset.
(aarch64_linux_set_debug_regs): Remove const from state. Call
aarch64_downgrade_regs.
(aarch64_show_debug_reg_state): Print also dr_addr_orig_wp.
* nat/aarch64-linux-hw-point.h (DR_CONTROL_LENGTH): Rename to ...
(DR_CONTROL_MASK): ... this.
(struct aarch64_debug_reg_state): New field dr_addr_orig_wp.
(unsigned int aarch64_watchpoint_offset): New prototype.
(aarch64_linux_set_debug_regs): Remove const from state.
* utils.c (align_up, align_down): Move to ...
* common/common-utils.c (align_up, align_down): ... here.
* utils.h (align_up, align_down): Move to ...
* common/common-utils.h (align_up, align_down): ... here.
gdb/gdbserver/ChangeLog
2018-05-04 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
* linux-aarch64-low.c (aarch64_stopped_data_address):
Likewise.
gdb/testsuite/ChangeLog
2018-05-04 Jan Kratochvil <jan.kratochvil@redhat.com>
Pedro Alves <palves@redhat.com>
PR breakpoints/19806 and support for PR external/20207.
* gdb.base/watchpoint-unaligned.c: New file.
* gdb.base/watchpoint-unaligned.exp: New file.
Diffstat (limited to 'gdb/gdbserver/linux-aarch64-low.c')
-rw-r--r-- | gdb/gdbserver/linux-aarch64-low.c | 31 |
1 files changed, 28 insertions, 3 deletions
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c index eccac4d..7ea24c2 100644 --- a/gdb/gdbserver/linux-aarch64-low.c +++ b/gdb/gdbserver/linux-aarch64-low.c @@ -360,14 +360,39 @@ aarch64_stopped_data_address (void) state = aarch64_get_debug_reg_state (pid_of (current_thread)); for (i = aarch64_num_wp_regs - 1; i >= 0; --i) { + const unsigned int offset + = aarch64_watchpoint_offset (state->dr_ctrl_wp[i]); const unsigned int len = aarch64_watchpoint_length (state->dr_ctrl_wp[i]); const CORE_ADDR addr_trap = (CORE_ADDR) siginfo.si_addr; - const CORE_ADDR addr_watch = state->dr_addr_wp[i]; + const CORE_ADDR addr_watch = state->dr_addr_wp[i] + offset; + const CORE_ADDR addr_watch_aligned = align_down (state->dr_addr_wp[i], 8); + const CORE_ADDR addr_orig = state->dr_addr_orig_wp[i]; + if (state->dr_ref_count_wp[i] && DR_CONTROL_ENABLED (state->dr_ctrl_wp[i]) - && addr_trap >= addr_watch + && addr_trap >= addr_watch_aligned && addr_trap < addr_watch + len) - return addr_trap; + { + /* ADDR_TRAP reports the first address of the memory range + accessed by the CPU, regardless of what was the memory + range watched. Thus, a large CPU access that straddles + the ADDR_WATCH..ADDR_WATCH+LEN range may result in an + ADDR_TRAP that is lower than the + ADDR_WATCH..ADDR_WATCH+LEN range. E.g.: + + addr: | 4 | 5 | 6 | 7 | 8 | + |---- range watched ----| + |----------- range accessed ------------| + + In this case, ADDR_TRAP will be 4. + + To match a watchpoint known to GDB core, we must never + report *ADDR_P outside of any ADDR_WATCH..ADDR_WATCH+LEN + range. ADDR_WATCH <= ADDR_TRAP < ADDR_ORIG is a false + positive on kernels older than 4.10. See PR + external/20207. */ + return addr_orig; + } } return (CORE_ADDR) 0; |