[RFC/RFA] GDB crash when using command lines due to memory corruption

[Joel Brobecker <brobecker at gnat dot com>]

This problem was reproduced using GDB freshly rebuilt from trunk on
x86-linux, but is not platform dependent. 

Using the sources below, the following causes some problems...

        (gdb) b stopme
        Breakpoint 1 at 0x8048513: file foo.c, line 9.
        (gdb) commands
        Type commands for when breakpoint 1 is hit, one per line.
        End with a line saying just "end".
        >call callme ()
        >end
        (gdb) run
        Starting program: foo 
        
        Breakpoint 1, stopme () at foo.c:9
        9       }
        warning: Invalid control type in command structure.
        warning: Invalid control type in command structure.
        warning: Invalid control type in command structure.
        zsh: 29624 segmentation fault  ./gdb-public.ref/gdb/gdb foo

I would guess that the same happens on 5.3 too, but I haven't tried yet.
Anyone with a handy recent 5.3 build?

What happens is that the "call" command causes the inferior to resume
execution. According to the documentation, any command that causes the
inferior to resume automatically ends the execution of the commands list.
The following patch tries to fix this problem (and other minor nits), but
does not try to enforce this rule. 

2002-12-11  J. Brobecker  <brobecker@gnat.com>

        * breakpoint.c (bpstat_clear): Do not deallocate the commands
        here, as the associated bpstat_copy does not perform a deep
        copy of this field. This is more symetrical, and it makes sure
        we don't have a double deallocation while execution a "call"
        command inside a commands list, as this command clears the
        global stop_bpstat after having "saved" it.
        (bpstat_do_actions): Deallocate the commands that we just
        executed. This is now needed, as this is no longer done by
        bpstat_clear. Make sure to not leave any dangling pointer
        behind.
        (bpstat_stop_status): Only copy the part of the commands list
        that will be executed. Otherwise, this would result in a
        small memory leak when using silent breakpoint commands.
        This change is probably independent from the other changes,
        but I did not have the courage to test it independently.

This change does not introduce any regression. I am willing to
contribute a new test for this case when a fix for this crash
is checked in.

Please accept my appologies for submitting the change in bpstat_stop_status
together with the other changes. I seriously lack time right now :-(.
This is really a small additional change, so hopefully it is ok if it
tags along with the other more serious one.

I verified as much as I could with all the weird scenarios I could think
of using the gdb testsuite that there was no memory leak by using traces
inside copy_commands_line and free_commands_lines.

As for 5.3, it took me a while to figure out all (really all?) the possible
scenarios that could happen, and the consequences on bpstat_do_actions.
The fact that the bpstat given is the __global__ bpstat is really nasty,
and makes this change risky, in my opinion. I would there simply add
a note in the PROBLEMS file. Unless, of course, somebody with more GDB
experience than me could confidently approve it...

Ok to apply?
-- 
Joel

----  foo.c  ----
void
callme (void)
{
}

void
stopme (void)
{
}

int
main (void)
{
  int i;
  for (i=0; i < 3; i++)
    stopme ();
}

Attachment: commands2.diff
Description: Text document