From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26632 invoked by alias); 7 Jun 2016 13:23:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 26620 invoked by uid 89); 7 Jun 2016 13:23:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS,UNSUBSCRIBE_BODY autolearn=no version=3.3.2 spammy=Suite, maker, secondly, Secondly X-HELO: mail-pf0-f195.google.com Received: from mail-pf0-f195.google.com (HELO mail-pf0-f195.google.com) (209.85.192.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 07 Jun 2016 13:23:44 +0000 Received: by mail-pf0-f195.google.com with SMTP id u67so4437144pfu.0 for ; Tue, 07 Jun 2016 06:23:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=WwyoTc9SkC6ra9pKA7gfWyQlhDwb92JiQwOpscCXujg=; b=D6kU3cWmoxZLLi7qw+fIz+5Z2xxPXMf784YvZ/8RTrJ68fa8MoyC3JK9s6ex9FVtyQ 68DHJr9r557jB7xdwYTr+8Vt3zLX8yMI8VoeCrZZvXx5J7WSKV2Wka/hH59C36d7bzPK iuGSJR4zkNY8mHfVvMct7UjeFJ8kzSaXs1DKju/R+OQOxHzOugIF+1CcvFXEN51u/ppL 4RBXj3w2+S/0vW6Fj0iZv5lcAc+JsMZXsNPD3Jl8CcHEsubSj3VC7/wf1EXjC6fXX9Fn V7+oCPz1xCB//bzW/Y00lvdaINlwEB9FXURTdWbC75WcfeVwhkXw2xP2JZZyzhIoS9sN QDRw== X-Gm-Message-State: ALyK8tJtJxF3uWryHmh8Kt4e4qJtpZUT6zcj4nOjgtCBaeyr7u9dz6UHuwvhuG9iFQ3hwg== X-Received: by 10.98.21.210 with SMTP id 201mr31914785pfv.51.1465305822838; Tue, 07 Jun 2016 06:23:42 -0700 (PDT) Received: from E107787-LIN (gcc113.osuosl.org. [140.211.9.71]) by smtp.gmail.com with ESMTPSA id s131sm35483179pfs.45.2016.06.07.06.23.39 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 07 Jun 2016 06:23:42 -0700 (PDT) From: Yao Qi To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: [patch] aarch64: PR 19806: watchpoints: false negatives -> false positives References: <20160606075945.GA19395@host1.jankratochvil.net> Date: Tue, 07 Jun 2016 13:23:00 -0000 In-Reply-To: <20160606075945.GA19395@host1.jankratochvil.net> (Jan Kratochvil's message of "Mon, 6 Jun 2016 09:59:45 +0200") Message-ID: <86eg89w2sr.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-06/txt/msg00131.txt.bz2 Jan Kratochvil writes: Hi Jan, > Aarch64: watchpoints set on non-8-byte-aligned addresses are always missed > https://sourceware.org/bugzilla/show_bug.cgi?id=3D19806 > > some unaligned watchpoints are currently missed. After this patch some o= ther > 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=3D20207 > -> > https://bugzilla.redhat.com/show_bug.cgi?id=3D1342821 > I have no idea if/when the kernel part gets fixed so I am posting this > hopefully-temporary GDB change. > I agree that false positives are better than false negatives. I don't think this problem is aarch64-linux specific, because other targets, such as mips and ppc have the similar restrictions. IMO, we need to fix it in a target-independent way. I've had some ideas, but still need some experiments to see how good/bad they are. In general, the trouble maker is target_stopped_data_address in watchpoints_triggered, because it returns the 8-byte-aligned address, which doesn't falls in the range of [loc->address, loc->address + loc->leng= th). However, watchpoints_triggered calls three target hooks and does two things, - return true if target is stopped by watchpoints, and return false otherwise, - update watchpoint.watchpoint_triggered, this leads me thinking that why do we need to get "inaccurate address" from target_stopped_data_address, and pass it to target_watchpoint_addr_within_range. Instead, we can pass the watchpoint to the (new) target hook, and set watchpoint.watchpoint_triggered in different target implementations. In each target implementation, we can set .watchpoint_triggered to watch_triggered_{yes,no,unknown} according to its hardware feature or capability. I'll give a try this way. > diff --git a/gdb/testsuite/gdb.base/watchpoint-unaligned.exp b/gdb/testsu= ite/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 > +} Can we generalize this test to any targets supports watchpoint? > + > +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 $decima= l 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\]" Can we test both read watch and write watch? Secondly, can you add some comments on the code? > + 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 =3D $rdsize" "" > + gdb_test_no_output "set variable offset =3D $rdoffset" "" > + set test "continue" > + set did_hit 0 > + gdb_test_multiple $test $test { > + -re "Hardware read watchpoint $wpnum:.*Value =3D .*\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=3D$wpsize offset=3D$wpoffset) rd(size=3D$rdsize offs= et=3D$rdoffset) expect=3D$expect_hit" > + if {$expect_hit =3D=3D 0 && $rdstart < $wpendaligned} { > + setup_kfail external/20207 "aarch64-*-*" It should be xfail rather than kfail. --=20 Yao (=E9=BD=90=E5=B0=A7)