This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] GDB/622, GDB/644, bp deletion and cmds


Michael Snyder wrote:
> 
> OK, I think I have an elegant solution for this.  I'm going to use
> Joel's copy_command_lines function, and Joel's commands.exp tests,
> but here's what we'll do in breakpoint.c:
> 
>   1) bpstat_stop_status will make a copy of the bp commands
> and put it in the bpstat (instead of merely copying the pointer).
> 
>   2) bpstat_clear, bpstat_clear_actions, and bpstat_do_actions
> will free the command lines instead of simply discarding them.
> 
> Since all bpstat commands are copies instead of pointers, they
> won't be affected by the deletion of the breakpoint -- and we
> get to plug a memory leak as a bonus.
> 
> Here's the patch as checked in (tested on linux, no regressions):

[forgot the change log entry:]
2002-08-26  Joel Brobecker  <brobecker@gnat.com>

	* cli/cli-script.c (copy_command_lines): New function.
	* defs.h (copy_command_lines): Export.
	* testsuite/gdb.base/commands.exp: New tests for commands
	attached to a temporary breakpoint, and for commands that
	delete the breakpoint they are attached to.
	
2002-08-26  Michael Snyder  <msnyder@redhat.com>

	* breakpoint.c (bpstat_stop_status): Instead of copying the 
	pointer to the breakpoint commands struct, make a new copy
	of the struct and point to that.
	(bpstat_clear): Free the commands struct.
	(bpstat_clear_actions): Free the commands struct.
	(bpstat_do_actions): Free the command actions.  Also execute
	the local cleanups, instead of deleting them.
	(delete_breakpoint): Leave the commands field of the bpstat
	chain alone -- it will be freed later.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.94
diff -p -r1.94 defs.h
*** defs.h	1 Aug 2002 17:18:32 -0000	1.94
--- defs.h	27 Aug 2002 00:43:32 -0000
*************** struct command_line
*** 653,658 ****
--- 653,659 ----
  extern struct command_line *read_command_lines (char *, int);
  
  extern void free_command_lines (struct command_line **);
+ 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.82
diff -p -r1.82 breakpoint.c
*** breakpoint.c	23 Aug 2002 20:49:38 -0000	1.82
--- breakpoint.c	27 Aug 2002 00:43:32 -0000
*************** bpstat_clear (bpstat *bsp)
*** 1763,1768 ****
--- 1763,1769 ----
        q = p->next;
        if (p->old_val != NULL)
  	value_free (p->old_val);
+       free_command_lines (&p->commands);
        xfree (p);
        p = q;
      }
*************** bpstat_clear_actions (bpstat bs)
*** 1875,1881 ****
  {
    for (; bs != NULL; bs = bs->next)
      {
!       bs->commands = NULL;
        if (bs->old_val != NULL)
  	{
  	  value_free (bs->old_val);
--- 1876,1882 ----
  {
    for (; bs != NULL; bs = bs->next)
      {
!       free_command_lines (&bs->commands);
        if (bs->old_val != NULL)
  	{
  	  value_free (bs->old_val);
*************** top:
*** 1944,1954 ****
  	   to look at, so start over.  */
  	goto top;
        else
! 	bs->commands = NULL;
      }
! 
!   executing_breakpoint_commands = 0;
!   discard_cleanups (old_chain);
  }
  
  /* This is the normal print function for a bpstat.  In the future,
--- 1945,1953 ----
  	   to look at, so start over.  */
  	goto top;
        else
! 	free_command_lines (&bs->commands);
      }
!   do_cleanups (old_chain);
  }
  
  /* This is the normal print function for a bpstat.  In the future,
*************** bpstat_stop_status (CORE_ADDR *pc, int n
*** 2730,2736 ****
  	    /* We will stop here */
  	    if (b->disposition == disp_disable)
  	      b->enable_state = bp_disabled;
! 	    bs->commands = b->commands;
  	    if (b->silent)
  	      bs->print = 0;
  	    if (bs->commands &&
--- 2729,2735 ----
  	    /* 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 &&
*************** delete_breakpoint (struct breakpoint *bp
*** 6787,6800 ****
      if (bs->breakpoint_at == bpt)
        {
  	bs->breakpoint_at = NULL;
- 
- 	/* we'd call bpstat_clear_actions, but that free's stuff and due
- 	   to the multiple pointers pointing to one item with no
- 	   reference counts found anywhere through out the bpstat's (how
- 	   do you spell fragile?), we don't want to free things twice --
- 	   better a memory leak than a corrupt malloc pool! */
- 	bs->commands = NULL;
  	bs->old_val = NULL;
        }
    /* On the chance that someone will soon try again to delete this same
       bp, we mark it as deleted before freeing its storage. */
--- 6786,6793 ----
      if (bs->breakpoint_at == bpt)
        {
  	bs->breakpoint_at = NULL;
  	bs->old_val = NULL;
+ 	/* bs->commands will be freed later.  */
        }
    /* On the chance that someone will soon try again to delete this same
       bp, we mark it as deleted before freeing its storage. */
Index: cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.13
diff -p -r1.13 cli-script.c
*** cli/cli-script.c	30 Jul 2002 13:45:14 -0000	1.13
--- cli/cli-script.c	27 Aug 2002 00:43:32 -0000
*************** make_cleanup_free_command_lines (struct 
*** 1012,1017 ****
--- 1012,1047 ----
  {
    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 = xstrdup (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;
+ }
  
  static void
  validate_comname (char *comname)
Index: testsuite/gdb.base/commands.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/commands.exp,v
retrieving revision 1.10
diff -p -r1.10 commands.exp
*** testsuite/gdb.base/commands.exp	13 Dec 2001 22:42:23 -0000	1.10
--- testsuite/gdb.base/commands.exp	27 Aug 2002 00:43:32 -0000
*************** proc deprecated_command_test {} {
*** 440,446 ****
--- 440,559 ----
  	    "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 "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 "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 "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.*$gdb_prompt $" {
+ 	    pass "run factorial until breakpoint"
+         }
+ 	-re ".*$gdb_prompt $" {
+ 	    fail "run factorial until breakpoint"
+ 	}
+ 	default { fail "(timeout) run factorial until breakpoint" }
+ 	timeout { fail "(timeout) run factorial until breakpoint" }
+     }
+ }
+ 
+ proc temporary_breakpoint_commands {} {
+     global gdb_prompt
+     
+     gdb_test "set args 1" "" "set args in temporary_breakpoint_commands"
+     delete_breakpoints
+ 
+     # Create a temporary breakpoint, and associate a commands list to it.
+     # This test will verify that this commands list is executed when the
+     # breakpoint is hit.
+     gdb_test "tbreak factorial" \
+ 	    "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \
+ 	    "breakpoint in temporary_breakpoint_commands"
+     
+     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 tbreak command"}
+ 	-re "$gdb_prompt $"   {fail "add silent tbreak command"}
+ 	timeout               {fail "(timeout) add silent tbreak command"}
+      }
+     send_gdb "printf \"factorial tbreak commands executed\\n\"\n"
+     gdb_expect {
+ 	-re ">"               {pass "add printf tbreak command"}
+ 	-re "$gdb_prompt $"   {fail "add printf tbreak command"}
+ 	timeout               {fail "(timeout) add printf tbreak command"}
+      }
+     send_gdb "cont\n"
+     gdb_expect {
+ 	-re ">"               {pass "add cont tbreak command"}
+ 	-re "$gdb_prompt $"   {fail "add cont tbreak command"}
+ 	timeout               {fail "(timeout) add cont tbreak command"} }
+     send_gdb "end\n"
+     gdb_expect {
+ 	-re "$gdb_prompt $"   {pass "end tbreak commands"}
+ 	timeout               {fail "(timeout) end tbreak commands"}
+      }
+ 
+     gdb_run_cmd
+     gdb_expect {
+ 	-re ".*factorial tbreak commands executed.*1.*Program exited normally.*" {
+ 	    pass "run factorial until temporary breakpoint"
+ 	}
+ 	timeout { fail "(timeout) run factorial until temporary breakpoint" }
+     }
+ }
+   
  gdbvar_simple_if_test
  gdbvar_simple_while_test
  gdbvar_complex_if_while_test
*************** user_defined_command_test
*** 454,456 ****
--- 567,571 ----
  watchpoint_command_test
  test_command_prompt_position
  deprecated_command_test
+ bp_deleted_in_command_test
+ temporary_breakpoint_commands

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]