Re: [2/2] RFC: let "commands" affect multiple breakpoints

Tom Tromey
2010-03-23T04:23:50Z

>>>>> "Tom" == Tom Tromey <tromey@redhat.com> writes:

Pedro> I'm okay with change. I only skimmed throught the patch, but I
Pedro> was wondering if it should be generalized to not be specific to
Pedro> rbreak only, but to all cases we create more than one breakpoint
Pedro> with a single command?
Tom> [...]
Pedro> Just curious if you considered it, and decided against it.

Tom> Nope, I just didn't think of it.
Tom> I am looking into it.

Here is a replacement patch which implements this.

This needs another doc review, as I changed the text a bit relative to
the previous patch.

Tom

2010-03-12 Tom Tromey <tromey@redhat.com>

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_multi.
(multi_start, multi_end, last_was_multi): 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.
(break_command_really): Set last_was_multi when needed.
* breakpoint.h (start_rbreak_breakpoints, end_rbreak_breakpoints):
Declare.

2010-03-12 Tom Tromey <tromey@redhat.com>

PR breakpoints/9352:
* gdb.texinfo (Break Commands): Update.

2010-03-09 Tom Tromey <tromey@redhat.com>

PR breakpoints/9352:
* gdb.base/default.exp: Update.
* gdb.base/commands.exp: Update.

diff --git a/gdb/NEWS b/gdb/NEWS
index 64c48f6..b6ddf58 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -13,6 +13,10 @@
16-bit word register AX that are actually portions of the 32-bit
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'.
+
* 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 b50c190..807916f 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,
@@ -392,11 +388,19 @@ 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 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;

/* Number of last tracepoint made. */

-int tracepoint_count;
+static int tracepoint_count;

/* Return whether a breakpoint is an active enabled breakpoint. */
static int
@@ -407,13 +411,34 @@ breakpoint_enabled (struct breakpoint *b)

/* Set breakpoint count to NUM. */

-void
+static void
set_breakpoint_count (int num)
{
breakpoint_count = num;
+ last_was_multi = 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)
+{
+ multi_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 >= multi_start)
+ {
+ multi_end = breakpoint_count;
+ last_was_multi = 1;
+ }
+}
+
/* Used in run_command to zero the hit count when a new run starts. */

void
@@ -760,32 +785,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_multi)
+ arg = xstrprintf ("%d-%d", multi_start, multi_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
@@ -796,36 +875,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. */
@@ -7031,6 +7082,7 @@ break_command_really (struct gdbarch *gdbarch,
int not_found = 0;
enum bptype type_wanted;
int task = 0;
+ int first_bp_set = breakpoint_count + 1;

sals.sals = NULL;
sals.nelts = 0;
@@ -7186,8 +7238,14 @@ break_command_really (struct gdbarch *gdbarch,
}

if (sals.nelts > 1)
- warning (_("Multiple breakpoints were set.\n"
- "Use the \"delete\" command to delete unwanted breakpoints."));
+ {
+ 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;
+ }
+
/* That's it. Discard the cleanups for data inserted into the
breakpoint. */
discard_cleanups (bkpt_chain);
@@ -9059,6 +9117,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)
{
@@ -9106,7 +9173,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
@@ -9567,7 +9634,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;
@@ -9595,9 +9664,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)
@@ -9673,6 +9742,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)
{
@@ -9706,7 +9784,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
@@ -9763,6 +9841,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. */
@@ -9800,11 +9887,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);
}
@@ -9812,11 +9899,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);
}
@@ -9824,7 +9911,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);
}
\f
static void
@@ -10247,7 +10334,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 5db2df6..d1301b6 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1019,4 +1019,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 a1f3a78..dd1a53f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4328,19 +4328,23 @@ 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 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.
@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 aa0aae6..bec790b 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3155,23 +3155,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));
@@ -3185,8 +3203,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"
--
1.6.6.1