From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7258 invoked by alias); 16 Oct 2013 16:28:28 -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 7246 invoked by uid 89); 16 Oct 2013 16:28:27 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 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 ESMTP; Wed, 16 Oct 2013 16:28:27 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r9GGSP47002584 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 16 Oct 2013 12:28:25 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r9GGSOp0009877; Wed, 16 Oct 2013 12:28:24 -0400 Message-ID: <525EBEA7.5060205@redhat.com> Date: Wed, 16 Oct 2013 16:28:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Andrew Burgess CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH] hardware watchpoints turned off, inferior not yet started References: <525E5EB6.4070305@broadcom.com> <525E8761.8060405@redhat.com> <525EB424.6010407@broadcom.com> In-Reply-To: <525EB424.6010407@broadcom.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-SW-Source: 2013-10/txt/msg00490.txt.bz2 On 10/16/2013 04:43 PM, Andrew Burgess wrote: > On 16/10/2013 1:32 PM, Pedro Alves wrote: >> ... this change I think makes it so that access/read >> watchpoints get converted to software watchpoints, which is wrong. > > OK I see that now, I got confused by code later within > update_watchpoint that seems to unconditionally fallback > to bp_watchpoint, but I see now how the read/access watchpoints > actually result in an error. Thanks for pointing this out. > > I extended this code to cover this case and added tests. > gdb/ChangeLog > > * breakpoint.c (update_watchpoint): Create software watchpoints if > the inferior has not yet started and hardware watchpoints are > turned off. "Create" isn't really accurate. This function is called for resets too. So, something like "file foo; start; watch; kill; set can-use-hw-watchpoint 0; file foo" will trigger that code path too, which I think will end up resulting in access/read watchpoints being disabled, with something like: "file foo; start; awatch; kill; set can-use-hw-watchpoint 0; file foo" (Replace the last "file" with anything that triggers a reset.) BTW, could you confirm that? Anyway, I suggest: * breakpoint.c (update_watchpoint): If hardware watchpoints are forced off, downgrade them to software watchpoints if possible, and error out if not possible. > + # Ensure that if we turn off hardware watchpoints and set a watch point > + # before starting the inferior the watchpoint created will not be a > + # hardware watchpoint. > + gdb_test_no_output "set can-use-hw-watchpoints 0" "" > + gdb_test "watch ival1" "Watchpoint \[0-9\]+: ival1" \ > + "Set software watchpoint before starting the inferior" > + > + # The next tests are written to match the current state of gdb: access > + # and read watchpoints require hardware watchpoint support, with this > + # turned off these can't be created. > + gdb_test "awatch ival1" \ > + "Target does not support software watchpoints of this type." \ > + "Set software watchpoint before starting the inferior" > + gdb_test "rwatch ival1" \ > + "Target does not support software watchpoints of this type." \ > + "Set software watchpoint before starting the inferior" The "Set software watchpoint before starting the inferior" string will appear in gdb.sum for the three tests. Please make those unique per test. Otherwise OK. Thanks, -- Pedro Alves