From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27319 invoked by alias); 26 Jul 2011 19:46:39 -0000 Received: (qmail 27286 invoked by uid 22791); 26 Jul 2011 19:46:37 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 26 Jul 2011 19:46:22 +0000 Received: (qmail 17329 invoked from network); 26 Jul 2011 19:46:21 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 26 Jul 2011 19:46:21 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: software watchpoints bug (was: Re: x86 watchpoints bug) Date: Tue, 26 Jul 2011 20:02:00 -0000 User-Agent: KMail/1.13.6 (Linux/2.6.38-8-generic; KDE/4.6.2; x86_64; ; ) Cc: Thiago Jung Bauermann , Philippe Waroquiers , yao@codesourcery.com References: <201107221740.07110.pedro@codesourcery.com> <1311388735.3205.29.camel@hactar> In-Reply-To: <1311388735.3205.29.camel@hactar> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201107262046.19451.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2011-07/txt/msg00741.txt.bz2 On Saturday 23 July 2011 03:38:55, Thiago Jung Bauermann wrote: > On Fri, 2011-07-22 at 17:40 +0100, Pedro Alves wrote: > > Maybe better would be to change works_in_software_mode_watchpoint to: > > > > int > > works_in_software_mode_watchpoint (const struct breakpoint *b) > > { > > - return b->type == bp_hardware_watchpoint; > > + return (b->type == bp_watchpoint || b->type == bp_hardware_watchpoint); > > } > > Agreed. I would only comment that the parenthesis are not necessary. :-) Alright! :-) I've applied the patch below, which does that, and adds a new test. > > Theoretically resources_needed_watchpoint would have to be adapted for > software watchpoints too, but in practice that function is only called > in hw_watchpoint_used_count, which is never called with bp_watchpoint as > an argument. > > FWIW, my local branch with my rework of debug registers accounting > doesn't have hw_watchpoint_used_count anymore. Oooh! Is that branch somewhere public? > > > The error string could also be enhanced to include the real > > watchpoint type (so a user of masked watchpoints doesn't get > > confused). > > I tried to keep that code agnostic to the type of watchpoint at hand > (hence the breakpoint_ops methods), so what about a more generic error > message, like "There is no hardware debug support for this watchpoint." > or "Expression cannot be implemented with hardware debug resources."? ... > error (_("Expression cannot be implemented with this type of watchpoint.")); Yeah, I'd go the path of making the error string more generic. Perhaps s/implemented/watched/. -- Pedro Alves 2011-07-26 Pedro Alves gdb/ * breakpoint.c (works_in_software_mode_watchpoint): Also return true for software watchpoints. gdb/testsuite/ * gdb.base/watchpoint.exp (test_disable_enable_software_watchpoint): New procedure. (top level): Run it. --- gdb/breakpoint.c | 3 ++- gdb/testsuite/gdb.base/watchpoint.exp | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) Index: src/gdb/testsuite/gdb.base/watchpoint.exp =================================================================== --- src.orig/gdb/testsuite/gdb.base/watchpoint.exp 2011-07-26 20:34:05.271448492 +0100 +++ src/gdb/testsuite/gdb.base/watchpoint.exp 2011-07-26 20:38:49.661448542 +0100 @@ -630,6 +630,23 @@ proc test_constant_watchpoint {} { gdb_test_no_output "delete \$bpnum" "delete watchpoint `7 + count'" } +proc test_disable_enable_software_watchpoint {} { + # This is regression test for a bug that caused `enable' to fail + # for software watchpoints. + + # Watch something not memory to force a software watchpoint. + gdb_test {watch $pc} ".*atchpoint \[0-9\]+: .pc" + + gdb_test_no_output "disable \$bpnum" "disable watchpoint `\$pc'" + gdb_test_no_output "enable \$bpnum" "reenable watchpoint `\$pc'" + + gdb_test "info watchpoint \$bpnum" \ + ".*watchpoint\[ \t\]+keep\[ \t\]+y\[ \t\]+.pc.*" \ + "watchpoint `\$pc' is enabled" + + gdb_test_no_output "delete \$bpnum" "delete watchpoint `\$pc'" +} + proc test_watch_location {} { gdb_breakpoint [gdb_get_line_number "func5 breakpoint here"] gdb_continue_to_breakpoint "func5 breakpoint here" @@ -903,6 +920,8 @@ if [initialize] then { test_constant_watchpoint + test_disable_enable_software_watchpoint + test_watch_location } Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2011-07-26 20:34:05.271448492 +0100 +++ src/gdb/breakpoint.c 2011-07-26 20:36:48.211448520 +0100 @@ -8637,7 +8637,8 @@ resources_needed_watchpoint (const struc static int works_in_software_mode_watchpoint (const struct breakpoint *b) { - return b->type == bp_hardware_watchpoint; + /* Read and access watchpoints only work with hardware support. */ + return b->type == bp_watchpoint || b->type == bp_hardware_watchpoint; } static enum print_stop_action