Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [1/2] RFC: reference count breakpoint commands
@ 2010-03-10  3:49 Tom Tromey
  2010-03-24 21:08 ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2010-03-10  3:49 UTC (permalink / raw)
  To: gdb-patches

I'd appreciate comments on this.  Barring comments I will commit it
soonish.

This adds reference counting for breakpoint commands.  This removes a
longstanding restriction: you can now change a breakpoint's commands
while the commands are running.  This patch also sets the stage for the
next page.

Built and regtested on x86-64 (compile farm).

Tom

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

	* breakpoint.h (struct counted_command_line): New struct.
	(struct breakpoint) <commands>: Change type.
	(struct bpstats) <commands>: Change type.
	<commands_left>: New field.
	* breakpoint.c (alloc_counted_command_line): New function.
	(incref_counted_command_line): Likewise.
	(decref_counted_command_line): Likewise.
	(do_cleanup_counted_command_line): Likewise.
	(make_cleanup_decref_counted_command_line): Likewise.
	(breakpoint_set_commands): Use decref_counted_command_line and
	alloc_counted_command_line.
	(commands_command): Don't error if breakpoint commands are
	executing.
	(commands_from_control_command): Likewise.
	(bpstat_free): Update.
	(bpstat_copy): Likewise.
	(bpstat_clear_actions): Likewise.
	(bpstat_do_actions_1): Likewise.
	(bpstat_stop_status): Likewise.
	(print_one_breakpoint_location): Likewise.
	(delete_breakpoint): Likewise.
	(bpstat_alloc): Initialize new field.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index bd3330f..e817a23 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -431,6 +431,63 @@ tracepoint_type (const struct breakpoint *b)
   return (b->type == bp_tracepoint || b->type == bp_fast_tracepoint);
 }
   
+/* Allocate a new counted_command_line with reference count of 1.
+   The new structure owns COMMANDS.  */
+
+static struct counted_command_line *
+alloc_counted_command_line (struct command_line *commands)
+{
+  struct counted_command_line *result
+    = xmalloc (sizeof (struct counted_command_line));
+  result->refc = 1;
+  result->commands = commands;
+  return result;
+}
+
+/* Increment reference count.  This does nothing if CMD is NULL.  */
+
+static void
+incref_counted_command_line (struct counted_command_line *cmd)
+{
+  if (cmd)
+    ++cmd->refc;
+}
+
+/* Decrement reference count.  If the reference count reaches 0,
+   destroy the counted_command_line.  Sets *CMDP to NULL.  This does
+   nothing if *CMDP is NULL.  */
+
+static void
+decref_counted_command_line (struct counted_command_line **cmdp)
+{
+  if (*cmdp)
+    {
+      if (--(*cmdp)->refc == 0)
+	{
+	  free_command_lines (&(*cmdp)->commands);
+	  xfree (*cmdp);
+	}
+      *cmdp = NULL;
+    }
+}
+
+/* A cleanup function that calls decref_counted_command_line.  */
+
+static void
+do_cleanup_counted_command_line (void *arg)
+{
+  decref_counted_command_line (arg);
+}
+
+/* Create a cleanup that calls decref_counted_command_line on the
+   argument.  */
+
+static struct cleanup *
+make_cleanup_decref_counted_command_line (struct counted_command_line **cmdp)
+{
+  return make_cleanup (do_cleanup_counted_command_line, cmdp);
+}
+
 /* Default address, symtab and line to put a breakpoint at
    for "break" command with no arg.
    if default_breakpoint_valid is zero, the other three are
@@ -684,8 +741,8 @@ condition_command (char *arg, int from_tty)
 void
 breakpoint_set_commands (struct breakpoint *b, struct command_line *commands)
 {
-  free_command_lines (&b->commands);
-  b->commands = commands;
+  decref_counted_command_line (&b->commands);
+  b->commands = alloc_counted_command_line (commands);
   breakpoints_changed ();
   observer_notify_breakpoint_modified (b->number);
 }
@@ -698,13 +755,6 @@ commands_command (char *arg, int from_tty)
   int bnum;
   struct command_line *l;
 
-  /* If we allowed this, we would have problems with when to
-     free the storage, if we change the commands currently
-     being read from.  */
-
-  if (executing_breakpoint_commands)
-    error (_("Can't use the \"commands\" command among a breakpoint's commands."));
-
   p = arg;
   bnum = get_number (&p);
 
@@ -737,13 +787,6 @@ commands_from_control_command (char *arg, struct command_line *cmd)
   char *p;
   int bnum;
 
-  /* If we allowed this, we would have problems with when to
-     free the storage, if we change the commands currently
-     being read from.  */
-
-  if (executing_breakpoint_commands)
-    error (_("Can't use the \"commands\" command among a breakpoint's commands."));
-
   /* An empty string for the breakpoint number means the last
      breakpoint, but get_number expects a NULL pointer.  */
   if (arg && !*arg)
@@ -758,12 +801,13 @@ commands_from_control_command (char *arg, struct command_line *cmd)
   ALL_BREAKPOINTS (b)
     if (b->number == bnum)
       {
-	free_command_lines (&b->commands);
+	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 = copy_command_lines (cmd->body_list[0]);
+	b->commands
+	  = alloc_counted_command_line (copy_command_lines (cmd->body_list[0]));
 	breakpoints_changed ();
 	observer_notify_breakpoint_modified (b->number);
 	return simple_control;
@@ -2553,7 +2597,7 @@ bpstat_free (bpstat bs)
 {
   if (bs->old_val != NULL)
     value_free (bs->old_val);
-  free_command_lines (&bs->commands);
+  decref_counted_command_line (&bs->commands);
   xfree (bs);
 }
 
@@ -2595,8 +2639,7 @@ bpstat_copy (bpstat bs)
     {
       tmp = (bpstat) xmalloc (sizeof (*tmp));
       memcpy (tmp, bs, sizeof (*tmp));
-      if (bs->commands != NULL)
-	tmp->commands = copy_command_lines (bs->commands);
+      incref_counted_command_line (tmp->commands);
       if (bs->old_val != NULL)
 	{
 	  tmp->old_val = value_copy (bs->old_val);
@@ -2697,7 +2740,7 @@ bpstat_clear_actions (bpstat bs)
 {
   for (; bs != NULL; bs = bs->next)
     {
-      free_command_lines (&bs->commands);
+      decref_counted_command_line (&bs->commands);
       if (bs->old_val != NULL)
 	{
 	  value_free (bs->old_val);
@@ -2763,6 +2806,7 @@ bpstat_do_actions_1 (bpstat *bsp)
   breakpoint_proceeded = 0;
   for (; bs != NULL; bs = bs->next)
     {
+      struct counted_command_line *ccmd;
       struct command_line *cmd;
       struct cleanup *this_cmd_tree_chain;
 
@@ -2776,9 +2820,12 @@ bpstat_do_actions_1 (bpstat *bsp)
          commands are only executed once, we don't need to copy it; we
          can clear the pointer in the bpstat, and make sure we free
          the tree when we're done.  */
-      cmd = bs->commands;
-      bs->commands = 0;
-      this_cmd_tree_chain = make_cleanup_free_command_lines (&cmd);
+      ccmd = bs->commands;
+      bs->commands = NULL;
+      this_cmd_tree_chain
+	= make_cleanup_decref_counted_command_line (&ccmd);
+      cmd = bs->commands_left;
+      bs->commands_left = NULL;
 
       while (cmd != NULL)
 	{
@@ -3161,6 +3208,7 @@ bpstat_alloc (const struct bp_location *bl, bpstat cbs /* Current "bs" value */
   bs->breakpoint_at = bl;
   /* If the condition is false, etc., don't do the commands.  */
   bs->commands = NULL;
+  bs->commands_left = NULL;
   bs->old_val = NULL;
   bs->print_it = print_it_normal;
   return bs;
@@ -3747,15 +3795,17 @@ bpstat_stop_status (struct address_space *aspace,
 	      if (b->silent)
 		bs->print = 0;
 	      bs->commands = b->commands;
-	      if (bs->commands
-		  && (strcmp ("silent", bs->commands->line) == 0
-		      || (xdb_commands && strcmp ("Q",
-						  bs->commands->line) == 0)))
+	      incref_counted_command_line (bs->commands);
+	      bs->commands_left = bs->commands ? bs->commands->commands : NULL;
+	      if (bs->commands_left
+		  && (strcmp ("silent", bs->commands_left->line) == 0
+		      || (xdb_commands
+			  && strcmp ("Q",
+				     bs->commands_left->line) == 0)))
 		{
-		  bs->commands = bs->commands->next;
+		  bs->commands_left = bs->commands_left->next;
 		  bs->print = 0;
 		}
-	      bs->commands = copy_command_lines (bs->commands);
 	    }
 
 	  /* Print nothing for this entry if we dont stop or dont print.  */
@@ -4422,7 +4472,7 @@ print_one_breakpoint_location (struct breakpoint *b,
       ui_out_text (uiout, " hits\n");
     }
 
-  l = b->commands;
+  l = b->commands ? b->commands->commands : NULL;
   if (!part_of_multiple && l)
     {
       struct cleanup *script_chain;
@@ -8841,7 +8891,7 @@ delete_breakpoint (struct breakpoint *bpt)
       break;
     }
 
-  free_command_lines (&bpt->commands);
+  decref_counted_command_line (&bpt->commands);
   if (bpt->cond_string != NULL)
     xfree (bpt->cond_string);
   if (bpt->addr_string != NULL)
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 6b373a3..1480991 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -374,6 +374,17 @@ DEF_VEC_I(int);
 typedef struct bp_location *bp_location_p;
 DEF_VEC_P(bp_location_p);
 
+/* A reference-counted struct command_line.  This lets multiple
+   breakpoints share a single command list.  */
+struct counted_command_line
+{
+  /* The reference count.  */
+  int refc;
+
+  /* The command list.  */
+  struct command_line *commands;
+};
+
 /* Note that the ->silent field is not currently used by any commands
    (though the code is in there if it was to be, and set_raw_breakpoint
    does set it to 0).  I implemented it because I thought it would be
@@ -412,7 +423,7 @@ struct breakpoint
        be continued automatically before really stopping.  */
     int ignore_count;
     /* Chain of command lines to execute when this breakpoint is hit.  */
-    struct command_line *commands;
+    struct counted_command_line *commands;
     /* Stack depth (address of frame).  If nonzero, break only if fp
        equals this.  */
     struct frame_id frame_id;
@@ -694,8 +705,11 @@ struct bpstats
     bpstat next;
     /* Breakpoint that we are at.  */
     const struct bp_location *breakpoint_at;
-    /* Commands left to be done.  */
-    struct command_line *commands;
+    /* The associated command list.  */
+    struct counted_command_line *commands;
+    /* Commands left to be done.  This points somewhere in
+       base_command.  */
+    struct command_line *commands_left;
     /* Old value associated with a watchpoint.  */
     struct value *old_val;
 


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-26 17:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-10  3:49 [1/2] RFC: reference count breakpoint commands Tom Tromey
2010-03-24 21:08 ` Tom Tromey
2010-03-26 16:43   ` [commit] Fix GDB crash (Re: [1/2] RFC: reference count breakpoint commands) Ulrich Weigand
2010-03-26 17:04     ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox