[RFA] deleting breakpoints inside of 'commands'

[Don Howard <dhoward at redhat dot com>]

I'm revisiting this again, as I've not received feedback the last two
times that I've reposted it.

This patch addresses a crash that can occur when a breakpoint's command
list deletes it's own breakpoint.  In this solution, breakpoint command
lists are walked (recursively, so as to examine compound statements and
user defined commands).  If a 'clear' or 'delete' command is found
anywhere in the definition of the command list, then the command list is
duplicated and that duplicate is executed.  Once execution of the command
list is complete, the duplicate is freed.


This patch does not address any situation where a user defined command
deletes itself. (is that even possible?  I don't see a way to remove a
user-defined command, only a way to redefine one.)


This version of contains 2 corrections to
bpstat_actions_delete_breakpoints()

1) The call to lookup_cmd() allows unknown commands. 

2) The body of the command is examined regardless of the result of 
   lookup_cmd().


Testing on i686 RH 7.2 shows no regressions:


                === gdb Summary ===
 
# of expected passes            8272
# of unexpected failures        31
# of unexpected successes       11
# of expected failures          170
# of untested testcases         7
/home/dhoward/work/sources/build/gdb/testsuite/../../gdb/gdb version  
2002-04-27-cvs -nx



After:
                === gdb Summary ===
 
# of expected passes            8272
# of unexpected failures        31
# of unexpected successes       11
# of expected failures          170
# of untested testcases         7
/home/dhoward/work/sources/rebuild/gdb/testsuite/../../gdb/gdb version  
2002-04-27-cvs -nx


Comments?

Ok to commit? 




2002-04-27  Don Howard  <dhoward@redhat.com>

        * breakpoint.c (bpstat_do_actions): Avoid deleting a 'commands'
        list while executing that list.  Thanks to Pierre Muller
        <muller@ics.u-strasbg.fr> for suggesting this implementation.
        (cleanup_dup_command_lines): New function.
        (bpstat_actions_delete_breakpoints): Ditto.
        * cli/cli-script.c (dup_command_lines): New function.
        * defs.h: Added declaration of new function.



Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.74
diff -p -u -w -r1.74 breakpoint.c
--- gdb/breakpoint.c	24 Apr 2002 16:28:15 -0000	1.74
+++ gdb/breakpoint.c	27 Apr 2002 19:18:57 -0000
@@ -47,6 +47,7 @@
 #include "ui-out.h"
 
 #include "gdb-events.h"
+#include "cli/cli-decode.h"
 
 /* Prototypes for local functions. */
 
@@ -1851,6 +1852,100 @@ cleanup_executing_breakpoints (PTR ignor
   executing_breakpoint_commands = 0;
 }
 
+static void
+cleanup_dup_command_lines (PTR cmds)
+{
+  free_command_lines (cmds);
+}
+
+
+/* Walk a list of struct command_lines and try to determine if any
+   command deletes breakpoints */
+
+static int 
+bpstat_actions_delete_breakpoints (struct command_line * cmd)
+{
+  for (; cmd; cmd = cmd->next)
+    {
+      struct cmd_list_element *ce;
+      char *line = cmd->line;
+      int i;
+      int ret;
+
+      ce = lookup_cmd (&line, cmdlist, "", 1, 1);
+
+      if (ce != NULL)
+	{
+	  /* Check the command */
+	  if (ce->class == class_user && !ce->hook_in)
+	    {
+	      ce->hook_in = 1;
+	      ret = bpstat_actions_delete_breakpoints (ce->user_commands);
+	      ce->hook_in = 0;
+
+	      if (ret)
+		return 1;
+	    }
+	  else if (strcmp (ce->name, "delete") == 0 
+		   || strcmp (ce->name, "clear") == 0)
+	    {
+	      return 1;
+	    }
+
+
+	  /* Check the pre-hook */
+	  if (ce->hook_pre)
+	    {
+	      if (ce->hook_pre->class == class_user && !ce->hook_in)
+		{
+		  ce->hook_in = 1;
+		  ret = bpstat_actions_delete_breakpoints (ce->hook_pre->user_commands);
+		  ce->hook_in = 0;
+	      
+		  if (ret)
+		    return 1;
+		}
+	      else if (strcmp (ce->hook_pre->name, "delete") == 0
+		       || strcmp (ce->hook_pre->name, "clear") == 0)
+		{
+		  return 1;
+		}
+	    }
+
+
+	  /* Check the post-hook */
+	  if (ce->hook_post)
+	    {
+	      if (ce->hook_post->class == class_user && !ce->hook_in)
+		{
+		  ce->hook_in = 1;
+		  ret = bpstat_actions_delete_breakpoints (ce->hook_post->user_commands);
+		  ce->hook_in = 0;
+	      
+		  if (ret)
+		    return 1;
+		}
+	      else if (strcmp (ce->hook_post->name, "delete") == 0
+		       || strcmp (ce->hook_post->name, "clear") == 0)
+		{
+		  return 1;
+		}
+	    }
+	}
+      
+      /* If this is a multi-part command (while, if, etc), check the
+	 body. */
+      for (i=0; i<cmd->body_count; i++)
+	if (bpstat_actions_delete_breakpoints (cmd->body_list[i]))
+	  return 1;
+
+    }
+
+  return 0;
+
+}
+
+
 /* Execute all the commands associated with all the breakpoints at this
    location.  Any of these commands could cause the process to proceed
    beyond this point, etc.  We look out for such changes by checking
@@ -1861,7 +1956,6 @@ bpstat_do_actions (bpstat *bsp)
 {
   bpstat bs;
   struct cleanup *old_chain;
-  struct command_line *cmd;
 
   /* Avoid endless recursion if a `source' command is contained
      in bs->commands.  */
@@ -1886,16 +1980,37 @@ top:
   breakpoint_proceeded = 0;
   for (; bs != NULL; bs = bs->next)
     {
-      cmd = bs->commands;
-      while (cmd != NULL)
+      struct command_line *cmd;
+      struct cleanup *new_old_chain;
+
+      cmd = 0;
+      new_old_chain = 0;
+
+      /* If the command list for this breakpoint includes a statement
+	 that deletes breakpoints, we assume that the target may be
+	 this breakpoint, so we make a copy of the command list to
+	 avoid walking a list that has been deleted. */
+      
+      for (cmd = bs->commands; cmd; cmd = cmd->next)
+	{
+	  if (!new_old_chain && bpstat_actions_delete_breakpoints (cmd))
 	{
+	      cmd = dup_command_lines (cmd);
+	      new_old_chain = make_cleanup (cleanup_dup_command_lines, cmd);
+	    }
+
 	  execute_control_command (cmd);
 
 	  if (breakpoint_proceeded)
 	    break;
-	  else
-	    cmd = cmd->next;
 	}
+
+      if (new_old_chain)
+	{
+	  free_command_lines (&cmd);
+	  discard_cleanups (new_old_chain);
+	}
+
       if (breakpoint_proceeded)
 	/* The inferior is proceeded by the command; bomb out now.
 	   The bpstat chain has been blown away by wait_for_inferior.
@@ -1905,6 +2020,7 @@ top:
       else
 	bs->commands = NULL;
     }
+
 
   executing_breakpoint_commands = 0;
   discard_cleanups (old_chain);
Index: gdb/defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.88
diff -p -u -w -r1.88 defs.h
--- gdb/defs.h	18 Apr 2002 18:08:59 -0000	1.88
+++ gdb/defs.h	27 Apr 2002 19:18:57 -0000
@@ -641,6 +641,7 @@ struct command_line
 extern struct command_line *read_command_lines (char *, int);
 
 extern void free_command_lines (struct command_line **);
+extern struct command_line * dup_command_lines (struct command_line *);
 
 /* To continue the execution commands when running gdb asynchronously. 
    A continuation structure contains a pointer to a function to be called 
Index: gdb/cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.12
diff -p -u -w -r1.12 cli-script.c
--- gdb/cli/cli-script.c	12 Apr 2002 22:31:23 -0000	1.12
+++ gdb/cli/cli-script.c	27 Apr 2002 19:18:58 -0000
@@ -974,6 +974,59 @@ read_command_lines (char *prompt_arg, in
   return (head);
 }
 
+/* Duplicate a chain of struct command_line's */
+
+struct command_line *
+dup_command_lines (struct command_line *l)
+{
+  struct command_line *dup = NULL;
+  register struct command_line *next = NULL;
+
+
+  for (; l ; l = l->next)
+    {
+      if (next == NULL)
+       {
+         dup = next = (struct command_line *)
+           xmalloc (sizeof (struct command_line));
+       }
+      else
+       {
+         next->next = (struct command_line *)
+           xmalloc (sizeof (struct command_line));
+
+         next = next->next;
+       }
+
+
+      if (next == NULL)
+       return NULL;
+
+
+      next->next = NULL;
+      next->line = xstrdup (l->line);
+      next->control_type = l->control_type;
+      next->body_count = l->body_count;
+
+
+      if (l->body_count > 0)
+       {
+         int i;
+         struct command_line **blist = l->body_list;
+
+         next->body_list =
+           (struct command_line **) xmalloc (sizeof (struct command_line *)
+                                             * l->body_count);
+
+         for (i = 0; i < l->body_count; i++, blist++)
+           next->body_list[i] = dup_command_lines (*blist);
+       }
+    }
+
+  return dup;
+}
+
+
 /* Free a chain of struct command_line's.  */
 
 void


-- 
dhoward@redhat.com
gdb engineering