This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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  <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_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  <tromey@redhat.com>
> 
> 	PR breakpoints/9352:
> 	* gdb.texinfo (Set Breaks): 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 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]