Re: [RFA] deleting breakpoints inside of 'commands' (again)

[Don Howard <dhoward@redhat.com>]

[patch ping]

I never received any feedback on this, and had forgotten about it.  
Comments?



On Fri, 14 Dec 2001, Don Howard wrote:

> On Tue, 6 Nov 2001, Andrew Cagney wrote:
> 
> > > 
> > > Doesn't the function below catch that?  There is a recursive call to
> > > look for just the problem that you mention.  Is there some other way of
> > > deleting commands that I need to look out for?
> > 
> > I'm not sure.
> > 
> > Consider:
> > 
> > 	command callee
> >            delete command callee
> > 	end
> > 	breakpoint foo
> > 	commands $bp
> > 	  callee
> > 	end
> > 
> > it will realise that there is a delete but will it protect its self from 
> > a command deleting its self.
> > 
> > > [BTW, I personally think that the unconditional copy is much less
> > > error prone (and therefore better) even if it's slower. Just my $0.02]
> > 
> > Agreed (And slower is relative - what happens if there are 100's or 
> > 1000's of commands - that search becomes the bottle neck.)
> > 
> > Andrew
> 
> 
> 
> 
> I've added a call to lookup_cmd().  This allows user-defined commands to 
> be examined also, which should address your concern.
> 
> I've also added checks that examine the pre and post hook commands.
> 
> 
> 
> 2001-12-13  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.57
> diff -p -u -w -r1.57 breakpoint.c
> --- breakpoint.c	2001/11/11 16:39:59	1.57
> +++ breakpoint.c	2001/12/14 08:13:07
> @@ -1790,6 +1790,102 @@ 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, "", 0, 1);
> +
> +      if (ce == NULL)
> +	return 0;
> +      
> +
> +      /* 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
> @@ -1800,7 +1896,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.  */
> @@ -1825,16 +1920,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.
> @@ -1844,6 +1960,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.67
> diff -p -u -w -r1.67 defs.h
> --- defs.h	2001/12/06 20:59:11	1.67
> +++ defs.h	2001/12/14 08:13:08
> @@ -839,6 +839,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.8
> diff -p -u -w -r1.8 cli-script.c
> --- cli-script.c	2001/12/13 22:42:23	1.8
> +++ cli-script.c	2001/12/14 08:13:08
> @@ -1003,6 +1003,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