From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3293 invoked by alias); 29 Oct 2013 16:44:34 -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 3205 invoked by uid 89); 29 Oct 2013 16:44:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mms3.broadcom.com Received: from mms3.broadcom.com (HELO mms3.broadcom.com) (216.31.210.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Oct 2013 16:44:31 +0000 Received: from [10.9.208.57] by mms3.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.5)); Tue, 29 Oct 2013 09:44:02 -0700 X-Server-Uuid: B86B6450-0931-4310-942E-F00ED04CA7AF Received: from IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) by IRVEXCHCAS08.corp.ad.broadcom.com (10.9.208.57) with Microsoft SMTP Server (TLS) id 14.1.438.0; Tue, 29 Oct 2013 09:44:26 -0700 Received: from mail-irva-13.broadcom.com (10.10.10.20) by IRVEXCHSMTP1.corp.ad.broadcom.com (10.9.207.51) with Microsoft SMTP Server id 14.1.438.0; Tue, 29 Oct 2013 09:44:26 -0700 Received: from [10.177.73.52] (unknown [10.177.73.52]) by mail-irva-13.broadcom.com (Postfix) with ESMTP id D2F37246A4; Tue, 29 Oct 2013 09:44:25 -0700 (PDT) Message-ID: <526FE5E9.3000909@broadcom.com> Date: Tue, 29 Oct 2013 16:44:00 -0000 From: "Andrew Burgess" User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: gdb-patches@sourceware.org cc: "Pedro Alves" Subject: Re: [PATCH] Extra error message from update_watchpoint References: <5260FD66.7090506@broadcom.com> <52616D4B.3020209@redhat.com> In-Reply-To: <52616D4B.3020209@redhat.com> Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-10/txt/msg00899.txt.bz2 On 18/10/2013 6:18 PM, Pedro Alves wrote: > On 10/18/2013 10:20 AM, Andrew Burgess wrote: >> @@ -1946,8 +1946,13 @@ update_watchpoint (struct watchpoint *b, int reparse) >> } >> } >> else if (!b->base.ops->works_in_software_mode (&b->base)) >> - error (_("Expression cannot be implemented with " >> - "read/access watchpoint.")); >> + { >> + if (!can_use_hw_watchpoints) >> + error (_("Software read/access watchpoints not supported.")); > > Hmm, is this really "not supported", or rather "explicitly disabled"? > I think I'd prefer: > > error (_("Hardware watchpoints support disabled. " > "See set/show can-use-hw-watchpoints.")); > > which also hints towards the toggle. WDYT? I used that error message as it matches the one I already added in: https://sourceware.org/ml/gdb-patches/2013-10/msg00563.html but I don't really mind. Ideally you might want both bits of the error message, you've turned off H/W watchpoints so you need to be told that read/access watchpoints are not supported in S/W, but I agree, we should also let the user know that H/W watchpoints have been turned off and could be turned back on again. That seems like too much for one error message though... I'm happy to go with your suggestion. Updated patch changes both error messages to match, OK to apply? Thanks, Andrew gdb/ChangeLog 2013-10-29 Andrew Burgess * breakpoint.c (update_watchpoint): Update error message and add an additional error message. gdb/testsuite/ChangeLog 2013-10-29 Andrew Burgess * gdb.base/watchpoint.exp (test_no_hw_watchpoints): Add additional tests and update expected error message. (test_watch_register_location): New tests. (do_tests): Call test_watch_register_location. * gdb.base/watchpoints.exp: Update expected error message. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 608463d..68b348d 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1805,7 +1805,8 @@ update_watchpoint (struct watchpoint *b, int reparse) if (b->base.ops->works_in_software_mode (&b->base)) b->base.type = bp_watchpoint; else - error (_("Software read/access watchpoints not supported.")); + error (_("Hardware watchpoint support disabled. " + "See set/show can-use-hw-watchpoints.")); } } else if (within_current_scope && b->exp) @@ -1946,8 +1947,14 @@ update_watchpoint (struct watchpoint *b, int reparse) } } else if (!b->base.ops->works_in_software_mode (&b->base)) - error (_("Expression cannot be implemented with " - "read/access watchpoint.")); + { + if (!can_use_hw_watchpoints) + error (_("Hardware watchpoint support disabled. " + "See set/show can-use-hw-watchpoints.")); + else + error (_("Expression cannot be implemented with " + "read/access watchpoint.")); + } else b->base.type = bp_watchpoint; diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp index e0d4f81..0ff5802 100644 --- a/gdb/testsuite/gdb.base/watchpoint.exp +++ b/gdb/testsuite/gdb.base/watchpoint.exp @@ -825,8 +825,12 @@ proc test_no_hw_watchpoints {} { # refrains from using them. # gdb_test "rwatch ival3" \ - "Expression cannot be implemented with read/access watchpoint..*" \ + "Hardware watchpoint support disabled\. See set/show can-use-hw-watchpoints\." \ "rwatch disallowed when can-set-hw-watchpoints cleared" + gdb_test "awatch ival3" \ + "Hardware watchpoint support disabled\. See set/show can-use-hw-watchpoints\." \ + "awatch disallowed when can-set-hw-watchpoints cleared" + # Re-enable hardware watchpoints if necessary. if ![target_info exists gdb,no_hardware_watchpoints] { @@ -879,6 +883,22 @@ proc test_watchpoint_in_big_blob {} { gdb_test_no_output "delete \$bpnum" "delete watch buf" } +proc test_watch_register_location {} { + global no_hw + + if {!$no_hw && ![target_info exists gdb,no_hardware_watchpoints]} { + # Non-memory read/access watchpoints are not supported, they would + # require software read/access watchpoint support (which is not + # currently available). + gdb_test "rwatch \$pc" \ + "Expression cannot be implemented with read/access watchpoint..*" \ + "rwatch disallowed for register based expression" + gdb_test "awatch \$pc" \ + "Expression cannot be implemented with read/access watchpoint..*" \ + "awatch disallowed for register based expression" + } +} + # Start with a fresh gdb. set prev_timeout $timeout @@ -940,6 +960,8 @@ proc do_tests {} { test_wide_location_1 test_wide_location_2 + + test_watch_register_location } # On targets that can do hardware watchpoints, run the tests twice: diff --git a/gdb/testsuite/gdb.base/watchpoints.exp b/gdb/testsuite/gdb.base/watchpoints.exp index b70e86c..e429f91 100644 --- a/gdb/testsuite/gdb.base/watchpoints.exp +++ b/gdb/testsuite/gdb.base/watchpoints.exp @@ -41,10 +41,10 @@ with_test_prefix "before inferior start" { # and read watchpoints require hardware watchpoint support, with this # turned off these can't be created. gdb_test "awatch ival1" \ - "Software read/access watchpoints not supported." \ + "Hardware watchpoint support disabled\. See set/show can-use-hw-watchpoints\." \ "create access watchpoint" gdb_test "rwatch ival1" \ - "Software read/access watchpoints not supported." \ + "Hardware watchpoint support disabled\. See set/show can-use-hw-watchpoints\." \ "create read watchpoint" }