Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFC/RFA] GDB crash when using command lines due to memory corruption
@ 2002-12-11 10:01 Joel Brobecker
  2002-12-11 10:20 ` Klee Dienes
  0 siblings, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2002-12-11 10:01 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3394 bytes --]

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 #2: commands2.diff --]
[-- Type: text/plain, Size: 3678 bytes --]

Index: breakpoint.c
===================================================================
RCS file: /nile.c/cvs/Dev/gdb/gdb-5/gdb/breakpoint.c,v
retrieving revision 1.1.1.1.2.3.2.9
diff -c -3 -p -r1.1.1.1.2.3.2.9 breakpoint.c
*** breakpoint.c	29 Aug 2002 16:41:28 -0000	1.1.1.1.2.3.2.9
--- breakpoint.c	11 Dec 2002 17:08:52 -0000
*************** bpstat_clear (bpstat *bsp)
*** 1686,1692 ****
        q = p->next;
        if (p->old_val != NULL)
  	value_free (p->old_val);
-       free_command_lines (&p->commands);
        xfree (p);
        p = q;
      }
--- 1686,1691 ----
*************** bpstat_do_actions (bpstat *bsp)
*** 1827,1832 ****
--- 1826,1832 ----
    bpstat bs;
    struct cleanup *old_chain;
    struct command_line *cmd;
+   struct command_line *saved_cmd;
  
    /* Avoid endless recursion if a `source' command is contained
       in bs->commands.  */
*************** top:
*** 1852,1857 ****
--- 1852,1858 ----
    for (; bs != NULL; bs = bs->next)
      {
        cmd = bs->commands;
+       saved_cmd = cmd;
        while (cmd != NULL)
  	{
  	  execute_control_command (cmd);
*************** top:
*** 1861,1866 ****
--- 1862,1877 ----
  	  else
  	    cmd = cmd->next;
  	}
+ 
+       /* Deallocate the commands line that we just executed.  We can not
+          deallocate directly the commands line in bs, as bs may have
+          changed while inside excecute_command_control if the command
+          caused the inferior to resume.  Note that there is no memory
+          leak when an error is raised inside execute_control_command,
+          because the error catching mechanism automatically deallocates
+          it from the global stop_bpstat.  */
+       free_command_lines (&saved_cmd);
+ 
        if (breakpoint_proceeded)
  	/* The inferior is proceeded by the command; bomb out now.
  	   The bpstat chain has been blown away by wait_for_inferior.
*************** top:
*** 1868,1874 ****
  	   to look at, so start over.  */
  	goto top;
        else
! 	free_command_lines (&bs->commands);
      }
    do_cleanups (old_chain);
  }
--- 1879,1889 ----
  	   to look at, so start over.  */
  	goto top;
        else
!         /* Since we deallocated the command_lines above, make sure
!            to reset the commands in the bpstat, so make sure we do
!            not leave any dangling pointer.  */
!         bs->commands = NULL;
! 
      }
    do_cleanups (old_chain);
  }
*************** bpstat_stop_status (CORE_ADDR *pc, int n
*** 2643,2651 ****
  	    /* We will stop here */
  	    if (b->disposition == disp_disable)
  	      b->enable_state = bp_disabled;
- 	    bs->commands = copy_command_lines (b->commands);
  	    if (b->silent)
  	      bs->print = 0;
  	    if (bs->commands &&
  		(STREQ ("silent", bs->commands->line) ||
  		 (xdb_commands && STREQ ("Q", bs->commands->line))))
--- 2658,2666 ----
  	    /* We will stop here */
  	    if (b->disposition == disp_disable)
  	      b->enable_state = bp_disabled;
  	    if (b->silent)
  	      bs->print = 0;
+ 	    bs->commands = b->commands;
  	    if (bs->commands &&
  		(STREQ ("silent", bs->commands->line) ||
  		 (xdb_commands && STREQ ("Q", bs->commands->line))))
*************** bpstat_stop_status (CORE_ADDR *pc, int n
*** 2653,2658 ****
--- 2668,2677 ----
  		bs->commands = bs->commands->next;
  		bs->print = 0;
  	      }
+             /* Copy the part of the command lines that will be executed,
+                as the breakpoint may disappear before or during its
+                execution.  */
+             bs->commands = copy_command_lines (bs->commands);
  	  }
        }
      /* Print nothing for this entry if we dont stop or if we dont print.  */

^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2003-03-10 23:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-12-11 10:01 [RFC/RFA] GDB crash when using command lines due to memory corruption Joel Brobecker
2002-12-11 10:20 ` Klee Dienes
2002-12-11 15:24   ` Michael Snyder
2002-12-12  2:04     ` Joel Brobecker
2002-12-12  2:41   ` Joel Brobecker
2002-12-12 13:03     ` Daniel Jacobowitz
2003-01-13  3:21   ` [RFA] (ping) " Joel Brobecker
2003-02-11  9:38     ` Joel Brobecker
2003-02-11 16:55       ` Andrew Cagney
2003-02-24  7:37         ` Andrew Cagney
2003-03-10 23:38   ` [RFC/RFA] " Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox