Re: execute_control_command may not remove its cleanups

[Dave Allan <da_gdb at egenera dot com>]

> > However, it seems from code inspection and the gdb internals
> > documentation that the call to do_cleanups ought to be unconditional. 
> > Does that seem right?  
> 
> No, instead, the cleanup chain should always have an item on it.  If
> make_cleanup is not called then old_chain will remain NULL, and
> do_cleanups (NULL) means "do all cleanups", not "do nothing".  It looks
> to me like command_handler is responsible for there always being a
> cleanup on the chain:
>   old_chain = make_cleanup (null_cleanup, 0);
> but maybe I'm mistaken about that; it's a bit far down the tree.

I definitely understand that do_cleanups(NULL) will do all cleanups
which is not what's wanted here.  The call is do_cleanups(old_chain),
though, so if there are cleanups on the chain already, they are
preserved.  The problem isn't the do_cleanups call, it's the fact that
the do_cleanups call is conditional.  The solution is to remove the if
(old_chain) statement and always do the cleanup.  

Given what's stated in the docs, that a function must always remove the
cleanups it creates, it would seem to me that regardless of the state of
cleanup_chain at the beginning of execute_control_command, whether it's
NULL or contains cleanups, we want to get back to that state before we
return.  

Looking at what cleanups execute_control_command puts on cleanup_chain,
that is correct.  Either one or two cleanups are put on the chain where
arg is an automatic variable and function is free_current_contents.  If
these cleanups aren't done before the stack frame is destroyed,
something undefined will later be freed when the cleanups are done.  

For example (and this output is from debugging the gdb I compiled from
the CVS tree this morning), immediately prior to the call to do_cleanups
at line 431 cleanup_chain contains the following:

$11 = {next = 0x82a3d50, function = 0x8080310 <free_current_contents>,
arg = 0xbffff1f4}
$12 = {next = 0x82cf628, function = 0x8080310 <free_current_contents>,
arg = 0xbffff1f8}
$13 = {next = 0x0, function = 0x8080354 <null_cleanup>, arg = 0x0}

So, in this case, as you point out, we return to one entry on
cleanup_chain, however, it seems unsafe to me to count on there always
being an entry on the chain rather than explicitly removing whatever we
put on the chain, which doesn't affect prior entries.  

Dave