From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10442 invoked by alias); 13 Aug 2010 18:25:31 -0000 Received: (qmail 10417 invoked by uid 22791); 13 Aug 2010 18:25:28 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 13 Aug 2010 18:25:20 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o7DIPHbv013510 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 13 Aug 2010 14:25:18 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o7DIPHab018223; Fri, 13 Aug 2010 14:25:17 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o7DIPG9A008501; Fri, 13 Aug 2010 14:25:16 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 204CB3781C7; Fri, 13 Aug 2010 12:25:15 -0600 (MDT) From: Tom Tromey To: gdb-patches@sourceware.org Subject: Re: RFA: implement "watch -location" References: Date: Fri, 13 Aug 2010 18:25:00 -0000 In-Reply-To: (Tom Tromey's message of "Wed, 11 Aug 2010 16:04:05 -0600") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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: 2010-08/txt/msg00195.txt.bz2 >>>>> "Tom" == Tom Tromey writes: Tom> This patch steals an idea from Apple's gdb fork, namely "watch -location". Tom> I wrote this from scratch but in the end it looks pretty similar to what Tom> they did. Here is a new version of the patch. I have addressed most comments. Michael> Syntax quibble. "-location"? Don't we traditionally use Michael> command option syntax such as "print /x"? Hate to be Michael> inconsistant... I kept this as-is. Michael> Also, what about help? Did I miss it? I couldn't see it at a Michael> glance. I updated the help and added "usage" lines as well. Hui> I am not sure about "-location" or "/x". But if you choose Hui> "-location", could you add a alias "-l"? I added this. Eli> I suggest a slight rewording here: Eli> ... take the address of the result, and watch the memory at that Eli> address. Eli> Please also say something about the size of the memory area watched by Eli> such a watchpoint. I did these. Phil> I guess we could get around it by massaging the data in Python before Phil> passing it as an expression to the Breakpoint API. It would be nice, Phil> however, to maintain parity with the command line. I'm on the fence Phil> too, but if you don't implement it I probably will (eventually). I didn't do this. Jan> this patch has a regression (guessing due to -lmcheck) on The bug was that watch_maybe_just_location did not check for arg==NULL. So, a plain "watch" crashed gdb. Tom b/gdb/ChangeLog: 2010-08-13 Tom Tromey * breakpoint.c (watch_command_1): Add 'just_location' argument. (watch_command_wrapper): Update. (watch_maybe_just_location): New function. (watch_command): Update. (rwatch_command_wrapper): Update. (rwatch_command): Update. (awatch_command_wrapper): Update. (awatch_command): Update. (check_for_argument): New function. (_initialize_breakpoint): Update help text. b/gdb/doc/ChangeLog: 2010-08-11 Tom Tromey * gdb.texinfo (Set Watchpoints): Document -location option. b/gdb/testsuite/ChangeLog: 2010-08-11 Tom Tromey * gdb.base/watchpoint.exp (test_watchpoint_and_breakpoint): Delete watchpoint. (test_watch_location): New proc. (test_watchpoint_in_big_blob): Delete watchpoint. * gdb.base/watchpoint.c (func5): New function. (main): Call it. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4affe0a..a58a9bb 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -98,8 +98,6 @@ static void clear_command (char *, int); static void catch_command (char *, int); -static void watch_command (char *, int); - static int can_use_hardware_watchpoint (struct value *); static void break_command_1 (char *, int, int); @@ -173,12 +171,6 @@ static void hbreak_command (char *, int); static void thbreak_command (char *, int); -static void watch_command_1 (char *, int, int); - -static void rwatch_command (char *, int); - -static void awatch_command (char *, int); - static void do_enable_breakpoint (struct breakpoint *, enum bpdisp); static void stop_command (char *arg, int from_tty); @@ -7981,7 +7973,7 @@ watchpoint_exp_is_const (const struct expression *exp) hw_read: watch read, hw_access: watch access (read or write) */ static void -watch_command_1 (char *arg, int accessflag, int from_tty) +watch_command_1 (char *arg, int accessflag, int from_tty, int just_location) { struct breakpoint *b, *scope_breakpoint = NULL; struct expression *exp; @@ -8086,7 +8078,15 @@ watch_command_1 (char *arg, int accessflag, int from_tty) exp_valid_block = innermost_block; mark = value_mark (); fetch_subexp_value (exp, &pc, &val, NULL, NULL); - if (val != NULL) + + if (just_location) + { + exp_valid_block = NULL; + val = value_addr (val); + release_value (val); + value_free_to_mark (mark); + } + else if (val != NULL) release_value (val); tok = arg; @@ -8188,7 +8188,24 @@ watch_command_1 (char *arg, int accessflag, int from_tty) b->exp = exp; b->exp_valid_block = exp_valid_block; b->cond_exp_valid_block = cond_exp_valid_block; - b->exp_string = savestring (exp_start, exp_end - exp_start); + if (just_location) + { + struct type *t = value_type (val); + CORE_ADDR addr = value_as_address (val); + char *name; + + t = check_typedef (TYPE_TARGET_TYPE (check_typedef (t))); + name = type_to_string (t); + + b->exp_string = xstrprintf ("* (%s *) %s", name, + core_addr_to_string (addr)); + xfree (name); + + /* The above expression is in C. */ + b->language = language_c; + } + else + b->exp_string = savestring (exp_start, exp_end - exp_start); b->val = val; b->val_valid = 1; if (cond_start) @@ -8215,7 +8232,8 @@ watch_command_1 (char *arg, int accessflag, int from_tty) scope_breakpoint->related_breakpoint = b; } - value_free_to_mark (mark); + if (!just_location) + value_free_to_mark (mark); /* Finally update the new watchpoint. This creates the locations that should be inserted. */ @@ -8305,37 +8323,73 @@ can_use_hardware_watchpoint (struct value *v) void watch_command_wrapper (char *arg, int from_tty) { - watch_command (arg, from_tty); + watch_command_1 (arg, hw_write, from_tty, 0); +} + +/* A helper function that looks for an argument at the start of a + string. The argument must also either be at the end of the string, + or be followed by whitespace. Returns 1 if it finds the argument, + 0 otherwise. If the argument is found, it updates *STR. */ + +static int +check_for_argument (char **str, char *arg, int arg_len) +{ + if (strncmp (*str, arg, arg_len) == 0 + && ((*str)[arg_len] == '\0' || isspace ((*str)[arg_len]))) + { + *str += arg_len; + return 1; + } + return 0; +} + +/* A helper function that looks for the "-location" argument and then + calls watch_command_1. */ + +static void +watch_maybe_just_location (char *arg, int accessflag, int from_tty) +{ + int just_location = 0; + + if (arg + && (check_for_argument (&arg, "-location", sizeof ("-location") - 1) + || check_for_argument (&arg, "-l", sizeof ("-l") - 1))) + { + ep_skip_leading_whitespace (&arg); + just_location = 1; + } + + watch_command_1 (arg, accessflag, from_tty, just_location); } static void watch_command (char *arg, int from_tty) { - watch_command_1 (arg, hw_write, from_tty); + watch_maybe_just_location (arg, hw_write, from_tty); } void rwatch_command_wrapper (char *arg, int from_tty) { - rwatch_command (arg, from_tty); + watch_command_1 (arg, hw_read, from_tty, 0); } static void rwatch_command (char *arg, int from_tty) { - watch_command_1 (arg, hw_read, from_tty); + watch_maybe_just_location (arg, hw_read, from_tty); } void awatch_command_wrapper (char *arg, int from_tty) { - awatch_command (arg, from_tty); + watch_command_1 (arg, hw_access, from_tty, 0); } static void awatch_command (char *arg, int from_tty) { - watch_command_1 (arg, hw_access, from_tty); + watch_maybe_just_location (arg, hw_access, from_tty); } @@ -11823,20 +11877,29 @@ With an argument, catch only exceptions with the given name."), c = add_com ("watch", class_breakpoint, watch_command, _("\ Set a watchpoint for an expression.\n\ +Usage: watch [-l|-location] EXPRESSION\n\ A watchpoint stops execution of your program whenever the value of\n\ -an expression changes.")); +an expression changes.\n\ +If -l or -location is given, this evaluates EXPRESSION and watches\n\ +the memory to which it refers.")); set_cmd_completer (c, expression_completer); c = add_com ("rwatch", class_breakpoint, rwatch_command, _("\ Set a read watchpoint for an expression.\n\ +Usage: rwatch [-l|-location] EXPRESSION\n\ A watchpoint stops execution of your program whenever the value of\n\ -an expression is read.")); +an expression is read.\n\ +If -l or -location is given, this evaluates EXPRESSION and watches\n\ +the memory to which it refers.")); set_cmd_completer (c, expression_completer); c = add_com ("awatch", class_breakpoint, awatch_command, _("\ Set a watchpoint for an expression.\n\ +Usage: awatch [-l|-location] EXPRESSION\n\ A watchpoint stops execution of your program whenever the value of\n\ -an expression is either read or written.")); +an expression is either read or written.\n\ +If -l or -location is given, this evaluates EXPRESSION and watches\n\ +the memory to which it refers.")); set_cmd_completer (c, expression_completer); add_info ("watchpoints", watchpoints_info, _("\ diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 7abb9ed..d4249c8 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -3699,7 +3699,7 @@ watchpoints, which do not slow down the running of your program. @table @code @kindex watch -@item watch @var{expr} @r{[}thread @var{threadnum}@r{]} +@item watch @r{[}-l@r{|}-location@r{]} @var{expr} @r{[}thread @var{threadnum}@r{]} Set a watchpoint for an expression. @value{GDBN} will break when the expression @var{expr} is written into by the program and its value changes. The simplest (and the most popular) use of this command is @@ -3716,13 +3716,22 @@ change the value of @var{expr}, @value{GDBN} will not break. Note that watchpoints restricted to a single thread in this way only work with Hardware Watchpoints. +Ordinarily a watchpoint respects the scope of variables in @var{expr} +(see below). The @code{-location} argument tells @value{GDBN} to +instead watch the memory referred to by @var{expr}. In this case, +@value{GDBN} will evaluate @var{expr}, take the address of the result, +and watch the memory at that address. The type of the result is used +to determine the size of the watched memory. If the expression's +result does not have an address, then @value{GDBN} will print an +error. + @kindex rwatch -@item rwatch @var{expr} @r{[}thread @var{threadnum}@r{]} +@item rwatch @r{[}-l@r{|}-location@r{]} @var{expr} @r{[}thread @var{threadnum}@r{]} Set a watchpoint that will break when the value of @var{expr} is read by the program. @kindex awatch -@item awatch @var{expr} @r{[}thread @var{threadnum}@r{]} +@item awatch @r{[}-l@r{|}-location@r{]} @var{expr} @r{[}thread @var{threadnum}@r{]} Set a watchpoint that will break when @var{expr} is either read from or written into by the program. diff --git a/gdb/testsuite/gdb.base/watchpoint.c b/gdb/testsuite/gdb.base/watchpoint.c index 8c212c1..9ef9253 100644 --- a/gdb/testsuite/gdb.base/watchpoint.c +++ b/gdb/testsuite/gdb.base/watchpoint.c @@ -126,6 +126,17 @@ func4 () global_ptr++; } +void +func5 () +{ + int val = 0, val2 = 23; + int *x = &val; + + /* func5 breakpoint here */ + x = &val2; + val = 27; +} + int main () { #ifdef usestubs @@ -203,5 +214,7 @@ int main () func4 (); + func5 (); + return 0; } diff --git a/gdb/testsuite/gdb.base/watchpoint.exp b/gdb/testsuite/gdb.base/watchpoint.exp index 6029b5b..edc7ea0 100644 --- a/gdb/testsuite/gdb.base/watchpoint.exp +++ b/gdb/testsuite/gdb.base/watchpoint.exp @@ -615,6 +615,8 @@ proc test_watchpoint_and_breakpoint {} { kfail "gdb/38" "next after watch x" } } + + gdb_test_no_output "delete \$bpnum" "delete watch x" } } @@ -628,6 +630,19 @@ proc test_constant_watchpoint {} { gdb_test_no_output "delete \$bpnum" "delete watchpoint `7 + count'" } +proc test_watch_location {} { + gdb_breakpoint [gdb_get_line_number "func5 breakpoint here"] + gdb_continue_to_breakpoint "func5 breakpoint here" + + gdb_test "watch -location *x" "atchpoint .*: .*" "watch -location .x" + + gdb_test "continue" \ + "Continuing.*\[Ww\]atchpoint .*: .*New value = 27.*" \ + "continue with watch -location" + + gdb_test_no_output "delete \$bpnum" "delete watch -location" +} + proc test_inaccessible_watchpoint {} { global gdb_prompt @@ -678,6 +693,8 @@ proc test_watchpoint_in_big_blob {} { gdb_test "watch buf" ".*atchpoint \[0-9\]+: buf" gdb_test "cont" "Continuing.*atchpoint \[0-9\]+: buf\r\n\r\nOld value = .*testte\".*" "watchpoint on buf hit" + + gdb_test_no_output "delete \$bpnum" "delete watch buf" } # Start with a fresh gdb. @@ -853,6 +870,8 @@ if [initialize] then { } test_constant_watchpoint + + test_watch_location } # Restore old timeout