From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9092 invoked by alias); 10 Mar 2010 17:05:25 -0000 Received: (qmail 8058 invoked by uid 22791); 10 Mar 2010 17:05:17 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,KAM_STOCKGEN 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; Wed, 10 Mar 2010 17:05:11 +0000 Received: (qmail 14560 invoked from network); 10 Mar 2010 17:05:09 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Mar 2010 17:05:09 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, tromey@redhat.com Subject: Re: [2/2] RFC: let "commands" affect multiple breakpoints Date: Wed, 10 Mar 2010 17:05:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-19-generic; KDE/4.3.2; x86_64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201003101705.06883.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-03/txt/msg00387.txt.bz2 On Wednesday 10 March 2010 03:54:28, Tom Tromey wrote: > I'd appreciate comments on this. Barring comments I will commit it > soonish. It needs a doc review. > This patch changes "commands" in two ways. First, it lets "commands" > accept a breakpoint range, like "enable". With no arguments it mostly > acts like it did before; except if the previous command was "rbreak" > then this form will affect all the breakpoints that were just set. I'm okay with change. I only skimmed throught the patch, but I was wondering if it should be generalized to not be specific to rbreak only, but to all cases we create more than one breakpoint with a single command? For example, using gdb.cp/overload, (gdb) b foo::foo Breakpoint 2 at 0x4007fc: file ../../../src/gdb/testsuite/gdb.cp/overload.cc, line 135. (2 locations) Breakpoint 3 at 0x4007b3: file ../../../src/gdb/testsuite/gdb.cp/overload.cc, line 134. (2 locations) Breakpoint 4 at 0x40076b: file ../../../src/gdb/testsuite/gdb.cp/overload.cc, line 133. (2 locations) warning: Multiple breakpoints were set. Use the "delete" command to delete unwanted breakpoints. (gdb) Just curious if you considered it, and decided against it. > > This is PR 9352. > > Built and regtested on x86-64 (compile farm). > > Tom > > 2010-03-09 Tom Tromey > > PR breakpoints/9352: > * NEWS: Mention changes to `commands' and `rbreak'. > * symtab.c (do_end_rbreak_breakpoints): New function. > (rbreak_command): Call start_rbreak_breakpoints; arrange to call > end_rbreak_breakpoints. > * breakpoint.c (breakpoint_count, tracepoint_count): Now static. > (set_breakpoint_count): Likewise. Clear last_was_rbreak. > (rbreak_start, rbreak_end, last_was_rbreak): New globals. > (start_rbreak_breakpoints, end_rbreak_breakpoints): New > functions. > (struct commands_info): New > (do_map_commands_command): New function. > (commands_command_1): New function. > (commands_command): Use it. > (commands_from_control_command): Likewise. > (do_delete_breakpoint): New function. > (delete_command): Use it. > (map_breakpoint_numbers): Add 'data' argument. Pass to callback. > (do_map_disable_breakpoint): New function. > (disable_command): Use it. > (do_map_enable_breakpoint): New function. > (enable_command): Use it. > (enable_once_breakpoint): Add argument. > (enable_once_command): Update. > (enable_delete_breakpoint): Add argument. > (enable_delete_command): Update. > * breakpoint.h (start_rbreak_breakpoints, end_rbreak_breakpoints): > Declare. > > 2010-03-09 Tom Tromey > > PR breakpoints/9352: > * gdb.texinfo (Set Breaks): Update. > > 2010-03-09 Tom Tromey > > PR breakpoints/9352: > * gdb.base/default.exp: Update. > * gdb.base/commands.exp: Update. > > diff --git a/gdb/NEWS b/gdb/NEWS > index 6cec32a..107aeba 100644 > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -3,6 +3,10 @@ > > *** Changes since GDB 7.1 > > +* 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'. > + > * Python scripting > > The GDB Python API now has access to symbols, symbol tables, and > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index e817a23..e7f5823 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -79,17 +79,15 @@ > > static void enable_delete_command (char *, int); > > -static void enable_delete_breakpoint (struct breakpoint *); > - > static void enable_once_command (char *, int); > > -static void enable_once_breakpoint (struct breakpoint *); > - > static void disable_command (char *, int); > > static void enable_command (char *, int); > > -static void map_breakpoint_numbers (char *, void (*)(struct breakpoint *)); > +static void map_breakpoint_numbers (char *, void (*) (struct breakpoint *, > + void *), > + void *); > > static void ignore_command (char *, int); > > @@ -146,8 +144,6 @@ static void condition_command (char *, int); > > static int get_number_trailer (char **, int); > > -void set_breakpoint_count (int); > - > typedef enum > { > mark_inserted, > @@ -390,11 +386,18 @@ VEC(bp_location_p) *moribund_locations = NULL; > > /* Number of last breakpoint made. */ > > -int breakpoint_count; > +static int breakpoint_count; > + > +/* If the last command to create a breakpoint was "rbreak", this holds > + the start and end breakpoint numbers. */ > +static int rbreak_start; > +static int rbreak_end; > +/* True if the last breakpoint was made with "rbreak". */ > +static int last_was_rbreak; > > /* Number of last tracepoint made. */ > > -int tracepoint_count; > +static int tracepoint_count; > > /* Return whether a breakpoint is an active enabled breakpoint. */ > static int > @@ -405,13 +408,34 @@ breakpoint_enabled (struct breakpoint *b) > > /* Set breakpoint count to NUM. */ > > -void > +static void > set_breakpoint_count (int num) > { > breakpoint_count = num; > + last_was_rbreak = 0; > set_internalvar_integer (lookup_internalvar ("bpnum"), num); > } > > +/* Called at the start an "rbreak" command to record the first > + breakpoint made. */ > +void > +start_rbreak_breakpoints (void) > +{ > + rbreak_start = breakpoint_count + 1; > +} > + > +/* Called at the end of an "rbreak" command to record the last > + breakpoint made. */ > +void > +end_rbreak_breakpoints (void) > +{ > + if (breakpoint_count >= rbreak_start) > + { > + rbreak_end = breakpoint_count; > + last_was_rbreak = 1; > + } > +} > + > /* Used in run_command to zero the hit count when a new run starts. */ > > void > @@ -747,32 +771,86 @@ breakpoint_set_commands (struct breakpoint *b, struct command_line *commands) > observer_notify_breakpoint_modified (b->number); > } > > +/* A structure used to pass information through > + map_breakpoint_numbers. */ > + > +struct commands_info > +{ > + /* True if the command was typed at a tty. */ > + int from_tty; > + /* 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; > +}; > + > +/* A callback for map_breakpoint_numbers that sets the commands for > + commands_command. */ > + > static void > -commands_command (char *arg, int from_tty) > +do_map_commands_command (struct breakpoint *b, void *data) > { > - struct breakpoint *b; > - char *p; > - int bnum; > - struct command_line *l; > + struct commands_info *info = data; > > - p = arg; > - bnum = get_number (&p); > + if (info->cmd == NULL) > + { > + struct command_line *l; > + if (info->control != NULL) > + l = copy_command_lines (info->control->body_list[0]); > + else > + l = read_command_lines (_("Type commands for all specified breakpoints"), > + info->from_tty, 1); > + info->cmd = alloc_counted_command_line (l); > + } > + > + /* If a breakpoint was on the list more than once, we don't need to > + do anything. */ > + if (b->commands != info->cmd) > + { > + incref_counted_command_line (info->cmd); > + decref_counted_command_line (&b->commands); > + b->commands = info->cmd; > + breakpoints_changed (); > + observer_notify_breakpoint_modified (b->number); > + } > +} > > - if (p && *p) > - error (_("Unexpected extra arguments following breakpoint number.")); > +static void > +commands_command_1 (char *arg, int from_tty, struct command_line *control) > +{ > + struct cleanup *cleanups; > + struct commands_info info; > > - ALL_BREAKPOINTS (b) > - if (b->number == bnum) > - { > - char *tmpbuf = xstrprintf ("Type commands for when breakpoint %d is hit, one per line.", > - bnum); > - struct cleanup *cleanups = make_cleanup (xfree, tmpbuf); > - l = read_command_lines (tmpbuf, from_tty, 1); > - do_cleanups (cleanups); > - breakpoint_set_commands (b, l); > - return; > + info.from_tty = from_tty; > + info.control = control; > + info.cmd = NULL; > + /* If we read command lines from the user, then `info' will hold an > + extra reference to the commands that we must clean up. */ > + cleanups = make_cleanup_decref_counted_command_line (&info.cmd); > + > + if (arg == NULL || !*arg) > + { > + if (last_was_rbreak) > + arg = xstrprintf ("%d-%d", rbreak_start, rbreak_end); > + else if (breakpoint_count > 0) > + arg = xstrprintf ("%d", breakpoint_count); > + make_cleanup (xfree, arg); > } > - error (_("No breakpoint number %d."), bnum); > + > + map_breakpoint_numbers (arg, do_map_commands_command, &info); > + > + if (info.cmd == NULL) > + error (_("No breakpoints specified.")); > + > + do_cleanups (cleanups); > +} > + > +static void > +commands_command (char *arg, int from_tty) > +{ > + commands_command_1 (arg, from_tty, NULL); > } > > /* Like commands_command, but instead of reading the commands from > @@ -783,36 +861,8 @@ commands_command (char *arg, int from_tty) > enum command_control_type > commands_from_control_command (char *arg, struct command_line *cmd) > { > - struct breakpoint *b; > - char *p; > - int bnum; > - > - /* An empty string for the breakpoint number means the last > - breakpoint, but get_number expects a NULL pointer. */ > - if (arg && !*arg) > - p = NULL; > - else > - p = arg; > - bnum = get_number (&p); > - > - if (p && *p) > - error (_("Unexpected extra arguments following breakpoint number.")); > - > - ALL_BREAKPOINTS (b) > - if (b->number == bnum) > - { > - decref_counted_command_line (&b->commands); > - if (cmd->body_count != 1) > - error (_("Invalid \"commands\" block structure.")); > - /* We need to copy the commands because if/while will free the > - list after it finishes execution. */ > - b->commands > - = alloc_counted_command_line (copy_command_lines (cmd->body_list[0])); > - breakpoints_changed (); > - observer_notify_breakpoint_modified (b->number); > - return simple_control; > - } > - error (_("No breakpoint number %d."), bnum); > + commands_command_1 (arg, 0, cmd); > + return simple_control; > } > > /* Return non-zero if BL->TARGET_INFO contains valid information. */ > @@ -8950,6 +9000,15 @@ make_cleanup_delete_breakpoint (struct breakpoint *b) > return make_cleanup (do_delete_breakpoint_cleanup, b); > } > > +/* A callback for map_breakpoint_numbers that calls > + delete_breakpoint. */ > + > +static void > +do_delete_breakpoint (struct breakpoint *b, void *ignore) > +{ > + delete_breakpoint (b); > +} > + > void > delete_command (char *arg, int from_tty) > { > @@ -8997,7 +9056,7 @@ delete_command (char *arg, int from_tty) > } > } > else > - map_breakpoint_numbers (arg, delete_breakpoint); > + map_breakpoint_numbers (arg, do_delete_breakpoint, NULL); > } > > static int > @@ -9458,7 +9517,9 @@ ignore_command (char *args, int from_tty) > whose numbers are given in ARGS. */ > > static void > -map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *)) > +map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *, > + void *), > + void *data) > { > char *p = args; > char *p1; > @@ -9486,9 +9547,9 @@ map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *)) > { > struct breakpoint *related_breakpoint = b->related_breakpoint; > match = 1; > - function (b); > + function (b, data); > if (related_breakpoint) > - function (related_breakpoint); > + function (related_breakpoint, data); > break; > } > if (match == 0) > @@ -9564,6 +9625,15 @@ disable_breakpoint (struct breakpoint *bpt) > observer_notify_breakpoint_modified (bpt->number); > } > > +/* A callback for map_breakpoint_numbers that calls > + disable_breakpoint. */ > + > +static void > +do_map_disable_breakpoint (struct breakpoint *b, void *ignore) > +{ > + disable_breakpoint (b); > +} > + > static void > disable_command (char *args, int from_tty) > { > @@ -9597,7 +9667,7 @@ disable_command (char *args, int from_tty) > update_global_location_list (0); > } > else > - map_breakpoint_numbers (args, disable_breakpoint); > + map_breakpoint_numbers (args, do_map_disable_breakpoint, NULL); > } > > static void > @@ -9654,6 +9724,15 @@ enable_breakpoint (struct breakpoint *bpt) > do_enable_breakpoint (bpt, bpt->disposition); > } > > +/* A callback for map_breakpoint_numbers that calls > + enable_breakpoint. */ > + > +static void > +do_map_enable_breakpoint (struct breakpoint *b, void *ignore) > +{ > + enable_breakpoint (b); > +} > + > /* The enable command enables the specified breakpoints (or all defined > breakpoints) so they once again become (or continue to be) effective > in stopping the inferior. */ > @@ -9691,11 +9770,11 @@ enable_command (char *args, int from_tty) > update_global_location_list (1); > } > else > - map_breakpoint_numbers (args, enable_breakpoint); > + map_breakpoint_numbers (args, do_map_enable_breakpoint, NULL); > } > > static void > -enable_once_breakpoint (struct breakpoint *bpt) > +enable_once_breakpoint (struct breakpoint *bpt, void *ignore) > { > do_enable_breakpoint (bpt, disp_disable); > } > @@ -9703,11 +9782,11 @@ enable_once_breakpoint (struct breakpoint *bpt) > static void > enable_once_command (char *args, int from_tty) > { > - map_breakpoint_numbers (args, enable_once_breakpoint); > + map_breakpoint_numbers (args, enable_once_breakpoint, NULL); > } > > static void > -enable_delete_breakpoint (struct breakpoint *bpt) > +enable_delete_breakpoint (struct breakpoint *bpt, void *ignore) > { > do_enable_breakpoint (bpt, disp_del); > } > @@ -9715,7 +9794,7 @@ enable_delete_breakpoint (struct breakpoint *bpt) > static void > enable_delete_command (char *args, int from_tty) > { > - map_breakpoint_numbers (args, enable_delete_breakpoint); > + map_breakpoint_numbers (args, enable_delete_breakpoint, NULL); > } > > static void > @@ -10138,7 +10217,7 @@ delete_trace_command (char *arg, int from_tty) > } > } > else > - map_breakpoint_numbers (arg, delete_breakpoint); > + map_breakpoint_numbers (arg, do_delete_breakpoint, NULL); > } > > /* Set passcount for tracepoint. > diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h > index 1480991..8e36f21 100644 > --- a/gdb/breakpoint.h > +++ b/gdb/breakpoint.h > @@ -1012,4 +1012,9 @@ extern struct breakpoint *get_tracepoint_by_number (char **arg, int multi_p, > is newly allocated; the caller should free when done with it. */ > extern VEC(breakpoint_p) *all_tracepoints (void); > > +/* Call at the start and end of an "rbreak" command to register > + breakpoint numbers for a later "commands" command. */ > +extern void start_rbreak_breakpoints (void); > +extern void end_rbreak_breakpoints (void); > + > #endif /* !defined (BREAKPOINT_H) */ > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index f6105b7..557316a 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -4326,19 +4326,21 @@ enable other breakpoints. > @table @code > @kindex commands > @kindex end@r{ (breakpoint commands)} > -@item commands @r{[}@var{bnum}@r{]} > +@item commands @r{[}@var{range}@dots{}@r{]} > @itemx @dots{} @var{command-list} @dots{} > @itemx end > -Specify a list of commands for breakpoint number @var{bnum}. The commands > +Specify a list of commands for the given breakpoints. The commands > themselves appear on the following lines. Type a line containing just > @code{end} to terminate the commands. > > To remove all commands from a breakpoint, type @code{commands} and > follow it immediately with @code{end}; that is, give no commands. > > -With no @var{bnum} argument, @code{commands} refers to the last > -breakpoint, watchpoint, or catchpoint set (not to the breakpoint most > -recently encountered). > +With no argument, @code{commands} refers to the last breakpoint, > +watchpoint, or catchpoint set (not to the breakpoint most recently > +encountered). If the most recent breakpoints were set with an > +@command{rbreak} command, then the @code{commands} will apply to all > +the breakpoints set by that @command{rbreak}. > @end table > > Pressing @key{RET} as a means of repeating the last @value{GDBN} command is > diff --git a/gdb/symtab.c b/gdb/symtab.c > index af4e501..e9a3c0f 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3610,23 +3610,41 @@ rbreak_command_wrapper (char *regexp, int from_tty) > rbreak_command (regexp, from_tty); > } > > +/* A cleanup function that calls end_rbreak_breakpoints. */ > + > +static void > +do_end_rbreak_breakpoints (void *ignore) > +{ > + end_rbreak_breakpoints (); > +} > + > static void > rbreak_command (char *regexp, int from_tty) > { > struct symbol_search *ss; > struct symbol_search *p; > struct cleanup *old_chain; > + char *string = NULL; > + int len = 0; > > search_symbols (regexp, FUNCTIONS_DOMAIN, 0, (char **) NULL, &ss); > old_chain = make_cleanup_free_search_symbols (ss); > + make_cleanup (free_current_contents, &string); > > + start_rbreak_breakpoints (); > + make_cleanup (do_end_rbreak_breakpoints, NULL); > for (p = ss; p != NULL; p = p->next) > { > if (p->msymbol == NULL) > { > - char *string = alloca (strlen (p->symtab->filename) > - + strlen (SYMBOL_LINKAGE_NAME (p->symbol)) > - + 4); > + int newlen = (strlen (p->symtab->filename) > + + strlen (SYMBOL_LINKAGE_NAME (p->symbol)) > + + 4); > + if (newlen > len) > + { > + string = xrealloc (string, newlen); > + len = newlen; > + } > strcpy (string, p->symtab->filename); > strcat (string, ":'"); > strcat (string, SYMBOL_LINKAGE_NAME (p->symbol)); > @@ -3640,8 +3658,13 @@ rbreak_command (char *regexp, int from_tty) > } > else > { > - char *string = alloca (strlen (SYMBOL_LINKAGE_NAME (p->msymbol)) > - + 3); > + int newlen = (strlen (SYMBOL_LINKAGE_NAME (p->msymbol)) > + + 3); > + if (newlen > len) > + { > + string = xrealloc (string, newlen); > + len = newlen; > + } > strcpy (string, "'"); > strcat (string, SYMBOL_LINKAGE_NAME (p->msymbol)); > strcat (string, "'"); > diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp > index b3257aa..6514e81 100644 > --- a/gdb/testsuite/gdb.base/commands.exp > +++ b/gdb/testsuite/gdb.base/commands.exp > @@ -299,7 +299,7 @@ proc watchpoint_command_test {} { > > send_gdb "commands $wp_id\n" > gdb_expect { > - -re "Type commands for when breakpoint $wp_id is hit, one per line.*>" { > + -re "Type commands for all specified breakpoints.*>" { > 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 when breakpoint .* is hit, one per line.*>" { > + -re "Type commands for all specified breakpoints.*>" { > 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 when breakpoint .* is hit, one per line.*>" { > + -re "Type commands for all specified breakpoints.*>" { > pass "begin commands in bp_deleted_in_command_test" > } > -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"} > diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp > index 9603fd4..3a7e1e8 100644 > --- a/gdb/testsuite/gdb.base/default.exp > +++ b/gdb/testsuite/gdb.base/default.exp > @@ -100,7 +100,7 @@ gdb_test "cd" "Argument required .new working directory.*" "cd" > gdb_test "clear" "No source file specified..*" "clear" > > #test commands > -gdb_test "commands" "No breakpoint number 0..*" "commands" > +gdb_test "commands" "Argument required .one or more breakpoint numbers...*" "commands" > > #test condition > gdb_test "condition" "Argument required .breakpoint number.*" "condition" > -- Pedro Alves