[patch] aarch64: PR 19806: watchpoints: false negatives -> false positives
Jan Kratochvil
jan.kratochvil@redhat.com
Mon Jun 6 08:00:00 GMT 2016
Hi,
Aarch64: watchpoints set on non-8-byte-aligned addresses are always missed
https://sourceware.org/bugzilla/show_bug.cgi?id=19806
some unaligned watchpoints are currently missed. After this patch some other
unaligned watchpoints will get reported as false positives.
It could be probably all fixed on the kernel side, filed a RFE for it:
kernel RFE: aarch64: ptrace: BAS: Support any contiguous range
https://sourceware.org/bugzilla/show_bug.cgi?id=20207
->
https://bugzilla.redhat.com/show_bug.cgi?id=1342821
I have no idea if/when the kernel part gets fixed so I am posting this
hopefully-temporary GDB change.
No regressions on aarch64-fedora23-linux-gnu in native mode.
No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu in native
and gdbserver mode.
Jan
-------------- next part --------------
gdb/ChangeLog
2016-06-06 Jan Kratochvil <jan.kratochvil@redhat.com>
PR breakpoints/19806
* aarch64-linux-nat.c (aarch64_linux_stopped_data_address): Use
dr_aligned_addr_wp and dr_orig_addr_wp. Return dr_orig_addr_wp instead
of addr_trap.
* nat/aarch64-linux-hw-point.c (aarch64_dr_state_insert_one_point): Add
parameter orig_addr. Use dr_aligned_addr_wp and dr_orig_addr_wp.
(aarch64_dr_state_remove_one_point): Use dr_aligned_addr_wp.
(aarch64_handle_breakpoint, aarch64_handle_aligned_watchpoint)
(aarch64_handle_unaligned_watchpoint): Update
aarch64_dr_state_insert_one_point caller.
(aarch64_linux_set_debug_regs): Use dr_aligned_addr_wp.
(aarch64_show_debug_reg_state): Use dr_aligned_addr_wp and
dr_orig_addr_wp.
* nat/aarch64-linux-hw-point.h (struct aarch64_debug_reg_state): Rename
dr_addr_wp to dr_orig_addr_wp, add dr_orig_addr_wp.
gdb/gdbserver/ChangeLog
2016-06-06 Jan Kratochvil <jan.kratochvil@redhat.com>
PR breakpoints/19806
* linux-aarch64-low.c (aarch64_init_debug_reg_state): Use
dr_aligned_addr_wp and dr_orig_addr_wp.
(aarch64_stopped_data_address): Use dr_aligned_addr_wp and
dr_orig_addr_wp. Return dr_orig_addr_wp instead of addr_trap.
gdb/testsuite/ChangeLog
2016-06-06 Jan Kratochvil <jan.kratochvil@redhat.com>
PR breakpoints/19806
* gdb.base/watchpoint-unaligned.c: New file.
* gdb.base/watchpoint-unaligned.exp: New file.
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index fe1631d..33a0a4f 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -765,14 +765,15 @@ aarch64_linux_stopped_data_address (struct target_ops *target,
{
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_aligned_watch = state->dr_aligned_addr_wp[i];
+ const CORE_ADDR addr_orig = state->dr_orig_addr_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 + len)
+ && addr_trap >= addr_aligned_watch
+ && addr_trap < addr_aligned_watch + len)
{
- *addr_p = addr_trap;
+ *addr_p = addr_orig;
return 1;
}
}
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index d237bde..d9edd36 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -220,7 +220,8 @@ aarch64_init_debug_reg_state (struct aarch64_debug_reg_state *state)
for (i = 0; i < AARCH64_HWP_MAX_NUM; ++i)
{
- state->dr_addr_wp[i] = 0;
+ state->dr_aligned_addr_wp[i] = 0;
+ state->dr_orig_addr_wp[i] = 0;
state->dr_ctrl_wp[i] = 0;
state->dr_ref_count_wp[i] = 0;
}
@@ -376,12 +377,13 @@ aarch64_stopped_data_address (void)
{
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_aligned_watch = state->dr_aligned_addr_wp[i];
+ const CORE_ADDR addr_orig = state->dr_orig_addr_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 + len)
- return addr_trap;
+ && addr_trap >= addr_aligned_watch
+ && addr_trap < addr_aligned_watch + len)
+ return addr_orig;
}
return (CORE_ADDR) 0;
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index a06a6e6..ddba270 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -330,11 +330,11 @@ aarch64_notify_debug_reg_change (const struct aarch64_debug_reg_state *state,
static int
aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
enum target_hw_bp_type type,
- CORE_ADDR addr, int len)
+ CORE_ADDR addr, int len, CORE_ADDR orig_addr)
{
int i, idx, num_regs, is_watchpoint;
unsigned int ctrl, *dr_ctrl_p, *dr_ref_count;
- CORE_ADDR *dr_addr_p;
+ CORE_ADDR *dr_aligned_addr_p, *dr_orig_addr_p;
/* Set up state pointers. */
is_watchpoint = (type != hw_execute);
@@ -342,14 +342,16 @@ aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
if (is_watchpoint)
{
num_regs = aarch64_num_wp_regs;
- dr_addr_p = state->dr_addr_wp;
+ dr_aligned_addr_p = state->dr_aligned_addr_wp;
+ dr_orig_addr_p = state->dr_orig_addr_wp;
dr_ctrl_p = state->dr_ctrl_wp;
dr_ref_count = state->dr_ref_count_wp;
}
else
{
num_regs = aarch64_num_bp_regs;
- dr_addr_p = state->dr_addr_bp;
+ dr_aligned_addr_p = state->dr_addr_bp;
+ dr_orig_addr_p = NULL;
dr_ctrl_p = state->dr_ctrl_bp;
dr_ref_count = state->dr_ref_count_bp;
}
@@ -366,7 +368,7 @@ aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
idx = i;
/* no break; continue hunting for an exising one. */
}
- else if (dr_addr_p[i] == addr && dr_ctrl_p[i] == ctrl)
+ else if (dr_aligned_addr_p[i] == addr && dr_ctrl_p[i] == ctrl)
{
gdb_assert (dr_ref_count[i] != 0);
idx = i;
@@ -382,7 +384,9 @@ aarch64_dr_state_insert_one_point (struct aarch64_debug_reg_state *state,
if ((dr_ctrl_p[idx] & 1) == 0)
{
/* new entry */
- dr_addr_p[idx] = addr;
+ dr_aligned_addr_p[idx] = addr;
+ if (dr_orig_addr_p != NULL)
+ dr_orig_addr_p[idx] = orig_addr;
dr_ctrl_p[idx] = ctrl;
dr_ref_count[idx] = 1;
/* Notify the change. */
@@ -414,7 +418,7 @@ aarch64_dr_state_remove_one_point (struct aarch64_debug_reg_state *state,
if (is_watchpoint)
{
num_regs = aarch64_num_wp_regs;
- dr_addr_p = state->dr_addr_wp;
+ dr_addr_p = state->dr_aligned_addr_wp;
dr_ctrl_p = state->dr_ctrl_wp;
dr_ref_count = state->dr_ref_count_wp;
}
@@ -472,7 +476,7 @@ aarch64_handle_breakpoint (enum target_hw_bp_type type, CORE_ADDR addr,
if (!aarch64_point_is_aligned (0 /* is_watchpoint */ , addr, len))
return -1;
- return aarch64_dr_state_insert_one_point (state, type, addr, len);
+ return aarch64_dr_state_insert_one_point (state, type, addr, len, 0);
}
else
return aarch64_dr_state_remove_one_point (state, type, addr, len);
@@ -487,7 +491,7 @@ aarch64_handle_aligned_watchpoint (enum target_hw_bp_type type,
struct aarch64_debug_reg_state *state)
{
if (is_insert)
- return aarch64_dr_state_insert_one_point (state, type, addr, len);
+ return aarch64_dr_state_insert_one_point (state, type, addr, len, addr);
else
return aarch64_dr_state_remove_one_point (state, type, addr, len);
}
@@ -504,6 +508,8 @@ aarch64_handle_unaligned_watchpoint (enum target_hw_bp_type type,
CORE_ADDR addr, int len, int is_insert,
struct aarch64_debug_reg_state *state)
{
+ const CORE_ADDR orig_addr = addr;
+
while (len > 0)
{
CORE_ADDR aligned_addr;
@@ -514,7 +520,7 @@ aarch64_handle_unaligned_watchpoint (enum target_hw_bp_type type,
if (is_insert)
ret = aarch64_dr_state_insert_one_point (state, type, aligned_addr,
- aligned_len);
+ aligned_len, orig_addr);
else
ret = aarch64_dr_state_remove_one_point (state, type, aligned_addr,
aligned_len);
@@ -564,7 +570,7 @@ aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
memset (®s, 0, sizeof (regs));
iov.iov_base = ®s;
count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
- addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
+ addr = watchpoint ? state->dr_aligned_addr_wp : state->dr_addr_bp;
ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
if (count == 0)
return;
@@ -611,8 +617,9 @@ aarch64_show_debug_reg_state (struct aarch64_debug_reg_state *state,
debug_printf ("\tWATCHPOINTs:\n");
for (i = 0; i < aarch64_num_wp_regs; i++)
- debug_printf ("\tWP%d: addr=%s, ctrl=0x%08x, ref.count=%d\n",
- i, core_addr_to_string_nz (state->dr_addr_wp[i]),
+ debug_printf ("\tWP%d: addr=%s (orig=%s), ctrl=0x%08x, ref.count=%d\n",
+ i, core_addr_to_string_nz (state->dr_aligned_addr_wp[i]),
+ core_addr_to_string_nz (state->dr_orig_addr_wp[i]),
state->dr_ctrl_wp[i], state->dr_ref_count_wp[i]);
}
diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h
index acf0a49..5d17f92 100644
--- a/gdb/nat/aarch64-linux-hw-point.h
+++ b/gdb/nat/aarch64-linux-hw-point.h
@@ -143,7 +143,8 @@ struct aarch64_debug_reg_state
unsigned int dr_ref_count_bp[AARCH64_HBP_MAX_NUM];
/* hardware watchpoint */
- CORE_ADDR dr_addr_wp[AARCH64_HWP_MAX_NUM];
+ CORE_ADDR dr_aligned_addr_wp[AARCH64_HWP_MAX_NUM];
+ CORE_ADDR dr_orig_addr_wp[AARCH64_HWP_MAX_NUM];
unsigned int dr_ctrl_wp[AARCH64_HWP_MAX_NUM];
unsigned int dr_ref_count_wp[AARCH64_HWP_MAX_NUM];
};
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.c b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
new file mode 100644
index 0000000..b76bc1c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.c
@@ -0,0 +1,68 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2016 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include <stdint.h>
+#include <assert.h>
+
+static int again;
+
+static volatile struct
+{
+ uint64_t alignment;
+ union
+ {
+ uint64_t size8[1];
+ uint32_t size4[2];
+ uint16_t size2[4];
+ uint8_t size1[8];
+ }
+ u;
+} data;
+
+static int size = 0;
+static int offset;
+
+int
+main (void)
+{
+ volatile uint64_t local;
+
+ assert (sizeof (data) == 8 + 8);
+ while (size)
+ {
+ switch (size)
+ {
+ case 8:
+ local = data.u.size8[offset];
+ break;
+ case 4:
+ local = data.u.size4[offset];
+ break;
+ case 2:
+ local = data.u.size2[offset];
+ break;
+ case 1:
+ local = data.u.size1[offset];
+ break;
+ default:
+ assert (0);
+ }
+ size = 0;
+ size = size; /* again_start */
+ }
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
new file mode 100644
index 0000000..623314a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-unaligned.exp
@@ -0,0 +1,82 @@
+# Copyright 2016 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+# This file is part of the gdb testsuite.
+
+if {![is_aarch64_target] && ![istarget "x86_64-*-*"] && ![istarget i?86-*]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+ return -1
+}
+
+if ![runto_main] {
+ untested "could not run to main"
+ return -1
+}
+
+gdb_breakpoint "[gdb_get_line_number "again_start"]" "Breakpoint $decimal at $hex" "again_start"
+
+set sizes {1 2 4 8}
+array set alignedend {1 1 2 2 3 4 4 4 5 8 6 8 7 8 8 8}
+
+foreach wpsize $sizes {
+ for {set wpoffset 0} {$wpoffset < 8/$wpsize} {incr wpoffset} {
+ set wpstart [expr $wpoffset * $wpsize]
+ set wpend [expr ($wpoffset + 1) * $wpsize]
+ set wpendaligned $alignedend($wpend)
+ foreach rdsize $sizes {
+ for {set rdoffset 0} {$rdoffset < 8/$rdsize} {incr rdoffset} {
+ set rdstart [expr $rdoffset * $rdsize]
+ set rdend [expr ($rdoffset + 1) * $rdsize]
+ set expect_hit [expr max($wpstart,$rdstart) < min($wpend,$rdend)]
+ set test "rwatch data.u.size$wpsize\[$wpoffset\]"
+ set wpnum ""
+ gdb_test_multiple $test $test {
+ -re "Hardware read watchpoint (\[0-9\]+): .*\r\n$gdb_prompt $" {
+ set wpnum $expect_out(1,string)
+ }
+ }
+ gdb_test_no_output "set variable size = $rdsize" ""
+ gdb_test_no_output "set variable offset = $rdoffset" ""
+ set test "continue"
+ set did_hit 0
+ gdb_test_multiple $test $test {
+ -re "Hardware read watchpoint $wpnum:.*Value = .*\r\n$gdb_prompt $" {
+ set did_hit 1
+ send_gdb "continue\n"
+ exp_continue
+ }
+ -re " again_start .*\r\n$gdb_prompt $" {
+ }
+ }
+ gdb_test_no_output "delete $wpnum" ""
+ set test "wp(size=$wpsize offset=$wpoffset) rd(size=$rdsize offset=$rdoffset) expect=$expect_hit"
+ if {$expect_hit == 0 && $rdstart < $wpendaligned} {
+ setup_kfail external/20207 "aarch64-*-*"
+ }
+ if {$expect_hit == $did_hit} {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ }
+ }
+}
More information about the Gdb-patches
mailing list