* RFA: protect breakpoint commands from being freed
@ 2003-12-13 5:59 Jim Blandy
2003-12-13 18:56 ` Andrew Cagney
2003-12-17 18:32 ` Daniel Jacobowitz
0 siblings, 2 replies; 6+ messages in thread
From: Jim Blandy @ 2003-12-13 5:59 UTC (permalink / raw)
To: gdb-patches
2003-12-02 Jim Blandy <jimb@redhat.com>
* breakpoint.c (bpstat_do_actions): To ensure that
clear_proceed_status doesn't free the command tree we're
evaluating out from under us, zero the bpstat's pointer to it, and
take care of freeing it ourselves.
* cli/cli-script.c (make_cleanup_free_command_lines): Make this
function externally visible.
* cli/cli-script.h (make_cleanup_free_command_lines): New
declaration.
Index: gdb/breakpoint.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/breakpoint.c,v
retrieving revision 1.361.2.1
diff -c -r1.361.2.1 breakpoint.c
*** gdb/breakpoint.c 22 Oct 2003 00:34:07 -0000 1.361.2.1
--- gdb/breakpoint.c 3 Dec 2003 04:31:28 -0000
***************
*** 1943,1949 ****
{
bpstat bs;
struct cleanup *old_chain;
- struct command_line *cmd;
/* Avoid endless recursion if a `source' command is contained
in bs->commands. */
--- 1943,1948 ----
***************
*** 1968,1974 ****
--- 1967,1989 ----
breakpoint_proceeded = 0;
for (; bs != NULL; bs = bs->next)
{
+ struct command_line *cmd;
+ struct cleanup *this_cmd_tree_chain;
+
+ /* Take ownership of the BSP's command tree, if it has one.
+
+ The command tree could legitimately contain commands like
+ 'step' and 'next', which call clear_proceed_status, which
+ frees stop_bpstat's command tree. To make sure this doesn't
+ free the tree we're executing out from under us, we need to
+ take ownership of the tree ourselves. Since a given bpstat's
+ 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);
+
while (cmd != NULL)
{
execute_control_command (cmd);
***************
*** 1978,1991 ****
else
cmd = cmd->next;
}
if (breakpoint_proceeded)
/* The inferior is proceeded by the command; bomb out now.
The bpstat chain has been blown away by wait_for_inferior.
But since execution has stopped again, there is a new bpstat
to look at, so start over. */
goto top;
- else
- free_command_lines (&bs->commands);
}
do_cleanups (old_chain);
}
--- 1993,2008 ----
else
cmd = cmd->next;
}
+
+ /* We can free this command tree now. */
+ do_cleanups (this_cmd_tree_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.
But since execution has stopped again, there is a new bpstat
to look at, so start over. */
goto top;
}
do_cleanups (old_chain);
}
Index: gdb/cli/cli-script.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/cli/cli-script.c,v
retrieving revision 1.16
diff -c -r1.16 cli-script.c
*** gdb/cli/cli-script.c 15 Oct 2003 17:30:27 -0000 1.16
--- gdb/cli/cli-script.c 3 Dec 2003 04:31:31 -0000
***************
*** 36,44 ****
/* Prototypes for local functions */
- static struct cleanup *
- make_cleanup_free_command_lines (struct command_line **arg);
-
static enum command_control_type
recurse_read_control_structure (struct command_line *current_cmd);
--- 36,41 ----
***************
*** 1001,1007 ****
free_command_lines (arg);
}
! static struct cleanup *
make_cleanup_free_command_lines (struct command_line **arg)
{
return make_cleanup (do_free_command_lines_cleanup, arg);
--- 998,1004 ----
free_command_lines (arg);
}
! struct cleanup *
make_cleanup_free_command_lines (struct command_line **arg)
{
return make_cleanup (do_free_command_lines_cleanup, arg);
Index: gdb/cli/cli-script.h
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/cli/cli-script.h,v
retrieving revision 1.5
diff -c -r1.5 cli-script.h
*** gdb/cli/cli-script.h 16 Apr 2003 21:18:50 -0000 1.5
--- gdb/cli/cli-script.h 3 Dec 2003 04:31:31 -0000
***************
*** 47,52 ****
--- 47,54 ----
extern struct command_line * copy_command_lines (struct command_line *cmds);
+ struct cleanup *make_cleanup_free_command_lines (struct command_line **arg);
+
/* Exported to gdb/infrun.c */
extern void execute_user_command (struct cmd_list_element *c, char *args);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: protect breakpoint commands from being freed
2003-12-13 5:59 RFA: protect breakpoint commands from being freed Jim Blandy
@ 2003-12-13 18:56 ` Andrew Cagney
2003-12-13 20:13 ` Jim Blandy
2003-12-17 18:32 ` Daniel Jacobowitz
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Cagney @ 2003-12-13 18:56 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches
> 2003-12-02 Jim Blandy <jimb@redhat.com>
>
> * breakpoint.c (bpstat_do_actions): To ensure that
> clear_proceed_status doesn't free the command tree we're
> evaluating out from under us, zero the bpstat's pointer to it, and
> take care of freeing it ourselves.
> * cli/cli-script.c (make_cleanup_free_command_lines): Make this
> function externally visible.
> * cli/cli-script.h (make_cleanup_free_command_lines): New
> declaration.
>
Didn't Don Howard fix this?
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: protect breakpoint commands from being freed
2003-12-13 18:56 ` Andrew Cagney
@ 2003-12-13 20:13 ` Jim Blandy
2003-12-13 22:42 ` Jim Blandy
0 siblings, 1 reply; 6+ messages in thread
From: Jim Blandy @ 2003-12-13 20:13 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
Andrew Cagney <cagney@gnu.org> writes:
> > 2003-12-02 Jim Blandy <jimb@redhat.com>
> > * breakpoint.c (bpstat_do_actions): To ensure that
> > clear_proceed_status doesn't free the command tree we're
> > evaluating out from under us, zero the bpstat's pointer to it, and
> > take care of freeing it ourselves.
> > * cli/cli-script.c (make_cleanup_free_command_lines): Make this
> > function externally visible.
> > * cli/cli-script.h (make_cleanup_free_command_lines): New
> > declaration.
>
> Didn't Don Howard fix this?
I remember Don Howard doing something related to this, but I it must
have been a separate case. The regression test I posted makes today's
GDB crash.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: protect breakpoint commands from being freed
2003-12-13 20:13 ` Jim Blandy
@ 2003-12-13 22:42 ` Jim Blandy
0 siblings, 0 replies; 6+ messages in thread
From: Jim Blandy @ 2003-12-13 22:42 UTC (permalink / raw)
To: Andrew Cagney; +Cc: gdb-patches
Jim Blandy <jimb@redhat.com> writes:
> Andrew Cagney <cagney@gnu.org> writes:
>
> > > 2003-12-02 Jim Blandy <jimb@redhat.com>
> > > * breakpoint.c (bpstat_do_actions): To ensure that
> > > clear_proceed_status doesn't free the command tree we're
> > > evaluating out from under us, zero the bpstat's pointer to it, and
> > > take care of freeing it ourselves.
> > > * cli/cli-script.c (make_cleanup_free_command_lines): Make this
> > > function externally visible.
> > > * cli/cli-script.h (make_cleanup_free_command_lines): New
> > > declaration.
> >
> > Didn't Don Howard fix this?
>
> I remember Don Howard doing something related to this, but I it must
> have been a separate case. The regression test I posted makes today's
> GDB crash.
Okay, I looked this up. Don Howard posted a fix for a slightly
different problem --- the case where the breakpoint is deleted by its
own script. The eventual solution entailed giving the bpstat its own
copy of the command tree.
The problem my patch addresses is distinct: GDB is freeing the
bpstat's copy before it is done with it.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: protect breakpoint commands from being freed
2003-12-13 5:59 RFA: protect breakpoint commands from being freed Jim Blandy
2003-12-13 18:56 ` Andrew Cagney
@ 2003-12-17 18:32 ` Daniel Jacobowitz
2003-12-22 3:43 ` Jim Blandy
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2003-12-17 18:32 UTC (permalink / raw)
To: Jim Blandy; +Cc: gdb-patches
On Sat, Dec 13, 2003 at 12:56:39AM -0500, Jim Blandy wrote:
>
> 2003-12-02 Jim Blandy <jimb@redhat.com>
>
> * breakpoint.c (bpstat_do_actions): To ensure that
> clear_proceed_status doesn't free the command tree we're
> evaluating out from under us, zero the bpstat's pointer to it, and
> take care of freeing it ourselves.
> * cli/cli-script.c (make_cleanup_free_command_lines): Make this
> function externally visible.
> * cli/cli-script.h (make_cleanup_free_command_lines): New
> declaration.
I don't believe anyone has objected to this patch, and it looks good to
me. Please check this in.
--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: protect breakpoint commands from being freed
2003-12-17 18:32 ` Daniel Jacobowitz
@ 2003-12-22 3:43 ` Jim Blandy
0 siblings, 0 replies; 6+ messages in thread
From: Jim Blandy @ 2003-12-22 3:43 UTC (permalink / raw)
To: Daniel Jacobowitz; +Cc: gdb-patches
Daniel Jacobowitz <drow@mvista.com> writes:
> I don't believe anyone has objected to this patch, and it looks good to
> me. Please check this in.
Committed; thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-12-22 3:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-12-13 5:59 RFA: protect breakpoint commands from being freed Jim Blandy
2003-12-13 18:56 ` Andrew Cagney
2003-12-13 20:13 ` Jim Blandy
2003-12-13 22:42 ` Jim Blandy
2003-12-17 18:32 ` Daniel Jacobowitz
2003-12-22 3:43 ` Jim Blandy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox