From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27671 invoked by alias); 16 May 2002 22:54:36 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 27526 invoked from network); 16 May 2002 22:54:32 -0000 Received: from unknown (HELO zwingli.cygnus.com) (208.245.165.35) by sources.redhat.com with SMTP; 16 May 2002 22:54:32 -0000 Received: by zwingli.cygnus.com (Postfix, from userid 442) id CFA2F5EA11; Thu, 16 May 2002 17:54:30 -0500 (EST) To: Don Howard Cc: Michael Snyder , Subject: Re: Patch ping References: From: Jim Blandy Date: Thu, 16 May 2002 15:54:00 -0000 In-Reply-To: Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.1 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-05/txt/msg00676.txt.bz2 I'm sorry it's taken a long time to get to this patch. Scanning for commands that could delete breakpoints doesn't seem very robust. If I add a new command that happens to delete breakpoints, this won't catch that, right? Why not add a reference count field to struct command_line? It would be set to 1 when allocated. You'd increment it when you begin executing a command list, and decrement it when you were done, or aborted execution due to an error. You'd also decrement it when the breakpoint gets deleted. I think it might be less code overall. Don Howard writes: > I still need reveiw/approval for the breakpoint portion of this. > > > > -- > dhoward@redhat.com > gdb engineering > > > ---------- Forwarded message ---------- > Date: Sun, 28 Apr 2002 12:06:11 -0400 > From: Fernando Nasser > To: Don Howard > Cc: Andrew Cagney , Michael Snyder , > gdb-patches@sources.redhat.com > Subject: Re: [RFA] deleting breakpoints inside of 'commands' > > The CLI part is approved. This is somewhat mixed with breakpoints code > (in the breakpoint.c file) so Michael Snyder would have to approve it as > well. > > Regards, > Fernando > > Don Howard wrote: > > > > 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 > > > > * breakpoint.c (bpstat_do_actions): Avoid deleting a 'commands' > > list while executing that list. Thanks to Pierre Muller > > 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; ibody_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 > > -- > Fernando Nasser > Red Hat Canada Ltd. E-Mail: fnasser@redhat.com > 2323 Yonge Street, Suite #300 > Toronto, Ontario M4P 2C9