From patchwork Mon Jun 6 07:59:45 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kratochvil X-Patchwork-Id: 12786 Received: (qmail 86537 invoked by alias); 6 Jun 2016 08:00:08 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 86512 invoked by uid 89); 6 Jun 2016 08:00:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS, UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=Suite, temple, 6179, 4727 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 06 Jun 2016 07:59:51 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 06DCA7F0A4 for ; Mon, 6 Jun 2016 07:59:50 +0000 (UTC) Received: from host1.jankratochvil.net (ovpn-116-44.ams2.redhat.com [10.36.116.44]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u567xjsI003414 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Mon, 6 Jun 2016 03:59:47 -0400 Date: Mon, 6 Jun 2016 09:59:45 +0200 From: Jan Kratochvil To: gdb-patches@sourceware.org Subject: [patch] aarch64: PR 19806: watchpoints: false negatives -> false positives Message-ID: <20160606075945.GA19395@host1.jankratochvil.net> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.1 (2016-04-27) X-IsSubscribed: yes 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 gdb/ChangeLog 2016-06-06 Jan Kratochvil 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 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 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 . */ + +#include +#include + +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 + } + } + } + } +}