[RFA] GDB/622 - clear current breakpoint in commands causes trouble

[Joel Brobecker <brobecker@gnat.com>]

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

As described in PR GDB/622:

Using the program in the gdb.base/commands.exp testcase, the following
session shows that clearing the current breakpoint inside its commands
list causes a memory-corruption problem:

       (gdb) b factorial
       Breakpoint 1 at 0x8048582: file ./gdb.base/run.c, line 77.
       (gdb) commands
       Type commands for when breakpoint 1 is hit, one per line.
       End with a line saying just "end".
       >silent
       >printf "factorial command-list executed\n"
       >clear factorial
       >cont
       >end
       (gdb) run 1
       Starting program: [...]/gdb.base/commands 1
       factorial command-list executed
       warning: Invalid control type in command structure.      <<<--- (1)
       (gdb)

(1) shows that the command-list becomes corrupted, and as a consequence,
the execution is not resumed.  Instead, the expected output from the run
command is:

       Starting program: [...]/gdb.base/run 1
       factorial command-list executed
       1

       Program exited normally.
       (gdb)

The fix consists into executing a copy of the commands list, to protect
this execution from using a list that has been freed. The attached patch
introduces no regression. A new test has also been added to commands.exp.

Ok to apply?

2002-07-31  Joel Brobecker  <brobecker@gnat.com>

        * cli/cli-scripts.c (copy_command_lines): New function.
        (make_cleanup_free_command_lines): Make this function non static.

        * defs.h (copy_command_lines): Add definition.
        (make_cleanup_free_command_lines): Add definition.

        * breakpoint.c (bpstat_do_actions): Execute a temporary copy of
        the command-list associated to each breakpoint hit, in order to
        avoid accessing a dangling pointer, in case one of the commands
        in the list causes the breakpoint to be deleted.

Thanks,
-- 
Joel

[-- Attachment #2: clear_in_commands.diff --]
[-- Type: text/plain, Size: 6914 bytes --]

Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.13
diff -c -3 -p -r1.13 cli-script.c
*** cli/cli-script.c	30 Jul 2002 13:45:14 -0000	1.13
--- cli/cli-script.c	31 Jul 2002 19:49:40 -0000
*************** extern void do_restore_instream_cleanup 
*** 42,49 ****
  
  /* 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);
--- 42,48 ----
  
  /* Prototypes for local functions */
  
! 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);
*************** do_free_command_lines_cleanup (void *arg
*** 1007,1016 ****
    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);
  }
  \f
  static void
--- 1006,1045 ----
    free_command_lines (arg);
  }
  
! struct cleanup *
  make_cleanup_free_command_lines (struct command_line **arg)
  {
    return make_cleanup (do_free_command_lines_cleanup, arg);
+ }
+ 
+ struct command_line *
+ copy_command_lines (struct command_line *cmds)
+ {
+   struct command_line *result = NULL;
+ 
+   if (cmds)
+     {
+       result = (struct command_line *) xmalloc (sizeof (struct command_line));
+ 
+       result->next = copy_command_lines (cmds->next);
+       result->line = strdup (cmds->line);
+       result->control_type = cmds->control_type;
+       result->body_count = cmds->body_count;
+       if (cmds->body_count > 0)
+         {
+           int i;
+ 
+           result->body_list = (struct command_line **)
+             xmalloc (sizeof (struct command_line *) * cmds->body_count);
+ 
+           for (i = 0; i < cmds->body_count; i++)
+             result->body_list[i] = copy_command_lines (cmds->body_list[i]);
+         }
+       else
+         result->body_list = NULL;
+     }
+ 
+   return result;
  }
  \f
  static void
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.93
diff -c -3 -p -r1.93 defs.h
*** defs.h	24 Jul 2002 22:46:48 -0000	1.93
--- defs.h	31 Jul 2002 19:49:40 -0000
*************** struct command_line
*** 653,658 ****
--- 653,661 ----
  extern struct command_line *read_command_lines (char *, int);
  
  extern void free_command_lines (struct command_line **);
+ extern struct cleanup *
+   make_cleanup_free_command_lines (struct command_line **arg);
+ extern struct command_line *copy_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: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.78
diff -c -3 -p -r1.78 breakpoint.c
*** breakpoint.c	26 Jun 2002 05:20:04 -0000	1.78
--- breakpoint.c	31 Jul 2002 19:49:45 -0000
*************** top:
*** 1882,1888 ****
    breakpoint_proceeded = 0;
    for (; bs != NULL; bs = bs->next)
      {
!       cmd = bs->commands;
        while (cmd != NULL)
  	{
  	  execute_control_command (cmd);
--- 1882,1892 ----
    breakpoint_proceeded = 0;
    for (; bs != NULL; bs = bs->next)
      {
!       /* Use a temporary copy of the commands, as one command in the list
!          may cause this breakpoint and its commands to be deleted.  */
!       cmd = copy_command_lines (bs->commands);
!       make_cleanup_free_command_lines (&cmd);
! 
        while (cmd != NULL)
  	{
  	  execute_control_command (cmd);
Index: testsuite/gdb.base/commands.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/commands.exp,v
retrieving revision 1.10
diff -c -3 -p -r1.10 commands.exp
*** testsuite/gdb.base/commands.exp	13 Dec 2001 22:42:23 -0000	1.10
--- testsuite/gdb.base/commands.exp	31 Jul 2002 19:49:45 -0000
*************** proc deprecated_command_test {} {
*** 440,445 ****
--- 440,503 ----
  	    "deprecate with no arguments"
  }
  
+ proc bp_deleted_in_command_test {} {
+     global gdb_prompt
+     
+     gdb_test "set args 1" "" "set args in bp_deleted_in_command_test"
+     delete_breakpoints
+ 
+     # Create a breakpoint, and associate a command-list to it, with
+     # one command that deletes this breakpoint.
+     gdb_test "break factorial" \
+              "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \
+              "breakpoint in bp_deleted_in_command_test"
+     
+     send_gdb "commands\n"
+     gdb_expect {
+       -re "Type commands for when breakpoint .* is hit, one per line.*>" {
+           pass "begin commands in bp_deleted_in_command_test"
+       }
+       -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"}
+       timeout             {fail "(timeout) begin commands bp_deleted_in_command_test"}
+     }
+     send_gdb "silent\n"
+     gdb_expect {
+         -re ">"               {pass "add silent command"}
+         -re "$gdb_prompt $"   {fail "add silent command"}
+         timeout               {fail "(timeout) add silent command"}
+     }
+     send_gdb "printf \"factorial command-list executed\\n\"\n"
+     gdb_expect {
+         -re ">"               {pass "add printf command"}
+         -re "$gdb_prompt $"   {fail "add printf command"}
+         timeout               {fail "(timeout) add printf command"}
+     }
+     send_gdb "clear factorial\n"
+     gdb_expect {
+         -re ">"               {pass "add clear command"}
+         -re "$gdb_prompt $"   {fail "add clear command"}
+         timeout               {fail "(timeout) add clear command"} }
+     send_gdb "cont\n"
+     gdb_expect {
+         -re ">"               {pass "add cont command"}
+         -re "$gdb_prompt $"   {fail "add cont command"}
+         timeout               {fail "(timeout) add cont command"} }
+     send_gdb "end\n"
+     gdb_expect {
+         -re "$gdb_prompt $"   {pass "end commands"}
+         timeout               {fail "(timeout) end commands"}
+     }
+ 
+     gdb_run_cmd
+     gdb_expect {
+         -re ".*factorial command-list executed.*1.*Program exited normally.*" {
+             pass "run factorial until breakpoint"
+         }
+         timeout { fail "(timeout) run factorial until breakpoint" }
+     }
+ 
+ }
+ 
  
  gdbvar_simple_if_test
  gdbvar_simple_while_test
*************** user_defined_command_test
*** 454,456 ****
--- 512,515 ----
  watchpoint_command_test
  test_command_prompt_position
  deprecated_command_test
+ bp_deleted_in_command_test