From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15811 invoked by alias); 1 Apr 2010 11:47:01 -0000 Received: (qmail 15762 invoked by uid 22791); 1 Apr 2010 11:46:56 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=BAYES_00,T_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; Thu, 01 Apr 2010 11:46:44 +0000 Received: (qmail 28108 invoked from network); 1 Apr 2010 11:46:42 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 1 Apr 2010 11:46:42 -0000 From: Pedro Alves To: tromey@redhat.com, Eli Zaretskii Subject: Re: [2/2] RFC: let "commands" affect multiple breakpoints Date: Thu, 01 Apr 2010 11:47:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) Cc: gdb-patches@sourceware.org References: <201003251709.32951.pedro@codesourcery.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201004011246.40641.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: 2010-04/txt/msg00007.txt.bz2 On Tuesday 30 March 2010 21:42:42, Tom Tromey wrote: > >>>>> "Pedro" == Pedro Alves writes: > > Pedro> I have a suggestion for this, in form of a patch. How > Pedro> about we just print the breakpoint range? > > It makes sense to me. > > I think I didn't do this because I thought it would look weird if the > user entered an invalid range specifier. But on reflection I'm not > concerned about that. That was my early concern as well, but, trying it out removed the concern. See: (top-gdb) commands 213-2 inverted range (top-gdb) commands 213-2as0fa inverted range (top-gdb) commands asdf warning: bad breakpoint number at or near 'asdf' No breakpoints specified. (top-gdb) commands asdf342-3 warning: bad breakpoint number at or near 'asdf342-3' No breakpoints specified. (top-gdb) commands main warning: bad breakpoint number at or near 'main' No breakpoints specified. (top-gdb) set $foo=1 (top-gdb) commands $foo Type commands for breakpoint(s) $foo, one per line. End with a line saying just "end". Only the last case made me stop for a second and consider expanding the variable, but I quickly gave on the idea (about 10 seconds later). > Pedro> WDYT? > > It looks reasonable to me, though be sure to incorporate the subsequent > bug fix: > 2010-03-26 Tom Tromey > > * breakpoint.c (commands_command_1): Duplicate 'arg'. Thanks, will do. Below's what I'm re-testing. Eli, want to take a look at the proposed docs/NEWS changes? I can split those into a separate patch if you'd prefer: > > * NEWS: Mention changes to `commands' and `rbreak'. > > * gdb.texinfo (Break Commands): Update. > > I'm also suggesting to adjusting these to better explain that > this applies when "break" (or similars) creates more than one > breakpoint. That is different to creating a breakpoint with > multiple locations ("multiple locations" are described both > in the manual and in NEWS); in the multiple locations case, > "commands" already applied to all locations. I'm adding > a cross reference to where the case where multiple breakpoints > are created is described; that section has a nice example. -- Pedro Alves 2010-04-01 Pedro Alves gdb/ * breakpoint.c (multi_start, multi_end, last_was_multi): Delete. (prev_breakpoint_count): New. (set_breakpoint_count): Adjust. (rbreak_start_breakpoint_count): New. (start_rbreak_breakpoints): Adjust. (end_rbreak_breakpoints): Adjust. (struct commands_info) : New field. (do_map_commands_command): Tweak output to include breakpoint spec range. (commands_command_1): Adjust. Avoid setting an xfree cleanup if ARG was empty on entry. Set INFO's arg. (create_breakpoint): Adjust. * NEWS: Clarify `commands' changes. gdb/doc/ * gdb.texinfo (Break Commands): Clarify `commands' changes, and add cross reference. gdb/testsuite/ * gdb.base/commands.exp: Adjust. * gdb.cp/extern-c.exp: Adjust. --- gdb/NEWS | 7 ++- gdb/breakpoint.c | 77 +++++++++++++++++++++++------------- gdb/doc/gdb.texinfo | 5 +- gdb/testsuite/gdb.base/commands.exp | 6 +- gdb/testsuite/gdb.cp/extern-c.exp | 2 5 files changed, 63 insertions(+), 34 deletions(-) Index: src/gdb/breakpoint.c =================================================================== --- src.orig/gdb/breakpoint.c 2010-04-01 01:46:33.000000000 +0100 +++ src/gdb/breakpoint.c 2010-04-01 12:41:21.000000000 +0100 @@ -388,13 +388,11 @@ VEC(bp_location_p) *moribund_locations = static int breakpoint_count; -/* If the last command to create a breakpoint created multiple - breakpoints, this holds the start and end breakpoint numbers. */ -static int multi_start; -static int multi_end; -/* True if the last breakpoint set was part of a group set with a - single command, e.g., "rbreak". */ -static int last_was_multi; +/* The value of `breakpoint_count' before the last command that + created breakpoints. If the last (break-like) command created more + than one breakpoint, then the difference between BREAKPOINT_COUNT + and PREV_BREAKPOINT_COUNT is more than one. */ +static int prev_breakpoint_count; /* Number of last tracepoint made. */ @@ -412,29 +410,31 @@ breakpoint_enabled (struct breakpoint *b static void set_breakpoint_count (int num) { + prev_breakpoint_count = breakpoint_count; breakpoint_count = num; - last_was_multi = 0; set_internalvar_integer (lookup_internalvar ("bpnum"), num); } +/* Used by `start_rbreak_breakpoints' below, to record the current + breakpoint count before "rbreak" creates any breakpoint. */ +static int rbreak_start_breakpoint_count; + /* Called at the start an "rbreak" command to record the first breakpoint made. */ + void start_rbreak_breakpoints (void) { - multi_start = breakpoint_count + 1; + rbreak_start_breakpoint_count = breakpoint_count; } /* Called at the end of an "rbreak" command to record the last breakpoint made. */ + void end_rbreak_breakpoints (void) { - if (breakpoint_count >= multi_start) - { - multi_end = breakpoint_count; - last_was_multi = 1; - } + prev_breakpoint_count = rbreak_start_breakpoint_count; } /* Used in run_command to zero the hit count when a new run starts. */ @@ -894,9 +894,14 @@ struct commands_info { /* True if the command was typed at a tty. */ int from_tty; + + /* The breakpoint range spec. */ + char *arg; + /* Non-NULL if the body of the commands are being read from this already-parsed command. */ struct command_line *control; + /* The command lines read from the user, or NULL if they have not yet been read. */ struct counted_command_line *cmd; @@ -917,12 +922,23 @@ do_map_commands_command (struct breakpoi if (info->control != NULL) l = copy_command_lines (info->control->body_list[0]); else + { + struct cleanup *old_chain; + char *str; + + str = xstrprintf (_("Type commands for breakpoint(s) %s, one per line."), + info->arg); - l = read_command_lines (_("Type commands for all specified breakpoints"), - info->from_tty, 1, - (breakpoint_is_tracepoint (b) - ? check_tracepoint_command : 0), - b); + old_chain = make_cleanup (xfree, str); + + l = read_command_lines (str, + info->from_tty, 1, + (breakpoint_is_tracepoint (b) + ? check_tracepoint_command : 0), + b); + + do_cleanups (old_chain); + } info->cmd = alloc_counted_command_line (l); } @@ -955,16 +971,27 @@ commands_command_1 (char *arg, int from_ if (arg == NULL || !*arg) { - if (last_was_multi) - arg = xstrprintf ("%d-%d", multi_start, multi_end); + if (breakpoint_count - prev_breakpoint_count > 1) + arg = xstrprintf ("%d-%d", prev_breakpoint_count + 1, breakpoint_count); else if (breakpoint_count > 0) arg = xstrprintf ("%d", breakpoint_count); + else + { + /* So that we don't try to free the incoming non-NULL + argument in the cleanup below. Mapping breakpoint + numbers will fail in this case. */ + arg = NULL; + } } else /* The command loop has some static state, so we need to preserve our argument. */ arg = xstrdup (arg); - make_cleanup (xfree, arg); + + if (arg != NULL) + make_cleanup (xfree, arg); + + info.arg = arg; map_breakpoint_numbers (arg, do_map_commands_command, &info); @@ -7239,7 +7266,7 @@ create_breakpoint (struct gdbarch *gdbar int not_found = 0; enum bptype type_wanted; int task = 0; - int first_bp_set = breakpoint_count + 1; + int prev_bkpt_count = breakpoint_count; sals.sals = NULL; sals.nelts = 0; @@ -7399,9 +7426,7 @@ create_breakpoint (struct gdbarch *gdbar { warning (_("Multiple breakpoints were set.\n" "Use the \"delete\" command to delete unwanted breakpoints.")); - multi_start = first_bp_set; - multi_end = breakpoint_count; - last_was_multi = 1; + prev_breakpoint_count = prev_bkpt_count; } /* That's it. Discard the cleanups for data inserted into the Index: src/gdb/doc/gdb.texinfo =================================================================== --- src.orig/gdb/doc/gdb.texinfo 2010-03-31 13:13:56.000000000 +0100 +++ src/gdb/doc/gdb.texinfo 2010-04-01 12:27:19.000000000 +0100 @@ -4343,8 +4343,9 @@ watchpoint, or catchpoint set (not to th encountered). If the most recent breakpoints were set with a single command, then the @code{commands} will apply to all the breakpoints set by that command. This applies to breakpoints set by -@code{rbreak}, and also breakpoints set with @code{break} that have -multiple locations. +@code{rbreak}, and also applies when a single @code{break} command +creates multiple breakpoints (@pxref{Ambiguous Expressions,,Ambiguous +Expressions}). @end table Pressing @key{RET} as a means of repeating the last @value{GDBN} command is Index: src/gdb/testsuite/gdb.base/commands.exp =================================================================== --- src.orig/gdb/testsuite/gdb.base/commands.exp 2010-03-25 18:22:02.000000000 +0000 +++ src/gdb/testsuite/gdb.base/commands.exp 2010-04-01 12:27:19.000000000 +0100 @@ -299,7 +299,7 @@ proc watchpoint_command_test {} { send_gdb "commands $wp_id\n" gdb_expect { - -re "Type commands for all specified breakpoints.*>" { + -re "Type commands for breakpoint.*, one per line.*>" { pass "begin commands on watch" } -re "$gdb_prompt $" {fail "begin commands on watch"} @@ -452,7 +452,7 @@ proc bp_deleted_in_command_test {} { send_gdb "commands\n" gdb_expect { - -re "Type commands for all specified breakpoints.*>" { + -re "Type commands for breakpoint.*>" { pass "begin commands in bp_deleted_in_command_test" } -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"} @@ -519,7 +519,7 @@ proc temporary_breakpoint_commands {} { send_gdb "commands\n" gdb_expect { - -re "Type commands for all specified breakpoints.*>" { + -re "Type commands for breakpoint.*>" { pass "begin commands in bp_deleted_in_command_test" } -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"} Index: src/gdb/testsuite/gdb.cp/extern-c.exp =================================================================== --- src.orig/gdb/testsuite/gdb.cp/extern-c.exp 2010-03-25 18:22:02.000000000 +0000 +++ src/gdb/testsuite/gdb.cp/extern-c.exp 2010-04-01 12:27:19.000000000 +0100 @@ -48,7 +48,7 @@ gdb_test "rbreak c_funcs" \ # Test that "commands" without an argument puts commands on both # breakpoints. gdb_test_multiple "commands" "set commands on multiple breakpoints" { - -re "Type commands for all specified breakpoints\r\nEnd with a line saying just \"end\".\r\n>$" { + -re "Type commands for breakpoint\\(s\\) 3-4, one per line\.\r\nEnd with a line saying just \"end\".\r\n>$" { gdb_test_multiple "set \$counter = \$counter + 1\nend" \ "command details for multiple breakpoints" { -re "$gdb_prompt $" { Index: src/gdb/NEWS =================================================================== --- src.orig/gdb/NEWS 2010-03-30 22:45:15.000000000 +0100 +++ src/gdb/NEWS 2010-04-01 12:27:19.000000000 +0100 @@ -17,8 +17,11 @@ register EAX or 64-bit register RAX. * The `commands' command now accepts a range of breakpoints to modify. - A plain `commands' following an `rbreak' will affect all the - breakpoints set by `rbreak'. + A plain `commands' following a command that creates multiple + breakpoints affects all the breakpoints set by that command. This + applies to breakpoints set by `rbreak', and also applies when a + single `break' command creates multiple breakpoints (e.g., + breakpoints on overloaded c++ functions). * Python scripting