This is the mail archive of the gdb-patches@sourceware.cygnus.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]

Re: [breakpoint.c] Does ``break overloaded if x'' leak memory?


Andrew Cagney wrote:
> 
> Hello,
> 
> Now that I understand how breapoint.c:break_command_1() can end up with
> several breakpoints :-) I'm wondering if the code that parses the ``if''
> condition leaks memory:
> 
> [....]
>
> As far as I can tell the if statement is re-parsed each time round this
> loop but, unfortunatly, each time, the old result is thrown away :-(.
> At the end the code ends up with just one expression and that is stored
> in all the created breakpoints.
> 
>         Andrew
> 

I've attatched a patch that fixes this problem.  It goes through and
ensures that each breakpoint has an individual copy of everything (and
simplifying things a tiny bit I think :-).

	Andrew
Tue Dec 14 23:47:15 1999  Andrew Cagney  <cagney@b1.cygnus.com>

	* breakpoint.c (break_command_1): Pre-allocate each breakpoints
 	addr_string.  Allocate a separate cond and cond_string for each
 	breakpoint.

Index: breakpoint.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/breakpoint.c,v
retrieving revision 1.278
diff -p -r1.278 breakpoint.c
*** breakpoint.c	1999/12/13 21:27:12	1.278
--- breakpoint.c	1999/12/14 12:58:43
*************** break_command_1 (arg, flag, from_tty)
*** 4729,4749 ****
    int tempflag, hardwareflag;
    struct symtabs_and_lines sals;
    struct symtab_and_line sal;
!   register struct expression *cond = 0;
    register struct breakpoint *b;
! 
!   /* Pointers in arg to the start, and one past the end, of the condition.  */
    char *cond_start = NULL;
    char *cond_end = NULL;
!   /* Pointers in arg to the start, and one past the end,
!      of the address part.  */
    char *addr_start = NULL;
    char *addr_end = NULL;
    struct cleanup *old_chain;
!   struct cleanup *canonical_strings_chain = NULL;
!   char **canonical = (char **) NULL;
    int i;
!   int thread;
  
    hardwareflag = flag & BP_HARDWAREFLAG;
    tempflag = flag & BP_TEMPFLAG;
--- 4748,4770 ----
    int tempflag, hardwareflag;
    struct symtabs_and_lines sals;
    struct symtab_and_line sal;
!   register struct expression **cond = 0;
    register struct breakpoint *b;
!   /* Pointers in arg to the start, and one past the end, of the
!      condition.  */
    char *cond_start = NULL;
    char *cond_end = NULL;
!   char **cond_string = (char **) NULL;
!   /* Pointers in arg to the start, and one past the end, of the
!      address part.  */
    char *addr_start = NULL;
    char *addr_end = NULL;
+   char **addr_string = (char **) NULL;
    struct cleanup *old_chain;
!   struct cleanup *breakpoint_chain = NULL;
    int i;
!   int thread = -1;
!   int ignore_count = 0;
  
    hardwareflag = flag & BP_HARDWAREFLAG;
    tempflag = flag & BP_TEMPFLAG;
*************** break_command_1 (arg, flag, from_tty)
*** 4753,4759 ****
  
    INIT_SAL (&sal);		/* initialize to zeroes */
  
!   /* If no arg given, or if first arg is 'if ', use the default breakpoint. */
  
    if (!arg || (arg[0] == 'i' && arg[1] == 'f'
  	       && (arg[2] == ' ' || arg[2] == '\t')))
--- 4774,4781 ----
  
    INIT_SAL (&sal);		/* initialize to zeroes */
  
!   /* If no arg given, or if first arg is 'if ', use the default
!      breakpoint. */
  
    if (!arg || (arg[0] == 'i' && arg[1] == 'f'
  	       && (arg[2] == ' ' || arg[2] == '\t')))
*************** break_command_1 (arg, flag, from_tty)
*** 4784,4792 ****
  	  && (!current_source_symtab
  	      || (arg && (*arg == '+' || *arg == '-'))))
  	sals = decode_line_1 (&arg, 1, default_breakpoint_symtab,
! 			      default_breakpoint_line, &canonical);
        else
! 	sals = decode_line_1 (&arg, 1, (struct symtab *) NULL, 0, &canonical);
  
        addr_end = arg;
      }
--- 4806,4814 ----
  	  && (!current_source_symtab
  	      || (arg && (*arg == '+' || *arg == '-'))))
  	sals = decode_line_1 (&arg, 1, default_breakpoint_symtab,
! 			      default_breakpoint_line, &addr_string);
        else
! 	sals = decode_line_1 (&arg, 1, (struct symtab *) NULL, 0, &addr_string);
  
        addr_end = arg;
      }
*************** break_command_1 (arg, flag, from_tty)
*** 4794,4811 ****
    if (!sals.nelts)
      return;
  
!   /* Make sure that all storage allocated in decode_line_1 gets freed
!      in case the following `for' loop errors out.  */
!   old_chain = make_cleanup (free, sals.sals);
!   if (canonical != (char **) NULL)
!     {
!       make_cleanup (free, canonical);
!       canonical_strings_chain = make_cleanup (null_cleanup, 0);
!       for (i = 0; i < sals.nelts; i++)
! 	{
! 	  if (canonical[i] != NULL)
! 	    make_cleanup (free, canonical[i]);
! 	}
      }
  
    thread = -1;			/* No specific thread yet */
--- 4816,4856 ----
    if (!sals.nelts)
      return;
  
!   /* Create a chain of things at always need to be cleaned up. */
!   old_chain = make_cleanup (null_cleanup, 0);
! 
!   /* Make sure that all storage allocated to SALS gets freed.  */
!   make_cleanup (free, sals.sals);
! 
!   /* Allocate space for all the cond expressions. */
!   cond = xcalloc (sals.nelts, sizeof (struct expression *));
!   make_cleanup (free, cond);
! 
!   /* Allocate space for all the cond strings. */
!   cond_string = xcalloc (sals.nelts, sizeof (char **));
!   make_cleanup (free, cond_string);
! 
!   /* Always have a addr_string array, even if it is empty. */
!   if (addr_string == NULL)
!     addr_string =  xcalloc (sals.nelts, sizeof (char **));
!   make_cleanup (free, addr_string);
! 
!   /* ----------------------------- SNIP -----------------------------
!      Anything added to the cleanup chain beyond this point is assumed
!      to be part of a breakpoint.  If the breakpoint create goes
!      through then that memory is not cleaned up. */
!   breakpoint_chain = make_cleanup (null_cleanup, 0);
! 
!   /* Go through the addr_string SAL's and fill in the gaps with the
!      addr_start/end string found above. */
!   for (i = 0; i < sals.nelts; i++)
!     {
!       /* Add the string if not present. */
!       if (addr_string[i] == NULL && addr_start != NULL)
! 	addr_string[i] = savestring (addr_start, addr_end - addr_start);
!       /* Mark this as recoverable. */
!       if (addr_string[i] != NULL)
! 	make_cleanup (free, addr_string[i]);
      }
  
    thread = -1;			/* No specific thread yet */
*************** break_command_1 (arg, flag, from_tty)
*** 4855,4862 ****
  	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
  	    {
  	      tok = cond_start = end_tok + 1;
! 	      cond = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
  	      cond_end = tok;
  	    }
  	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
  	    {
--- 4900,4910 ----
  	  if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
  	    {
  	      tok = cond_start = end_tok + 1;
! 	      cond[i] = parse_exp_1 (&tok, block_for_pc (sals.sals[i].pc), 0);
! 	      make_cleanup (free, cond[i]);
  	      cond_end = tok;
+ 	      cond_string[i] = savestring (cond_start, cond_end - cond_start);
+ 	      make_cleanup (free, cond_string[i]);
  	    }
  	  else if (toklen >= 1 && strncmp (tok, "thread", toklen) == 0)
  	    {
*************** break_command_1 (arg, flag, from_tty)
*** 4888,4896 ****
  	error ("Hardware breakpoints used exceeds limit.");
      }
  
!   /* Remove the canonical strings from the cleanup, they are needed below.  */
!   if (canonical != (char **) NULL)
!     discard_cleanups (canonical_strings_chain);
  
    /* Now set all the breakpoints.  */
    for (i = 0; i < sals.nelts; i++)
--- 4936,4944 ----
  	error ("Hardware breakpoints used exceeds limit.");
      }
  
!   /* This is it: Discard all the cleanups for data inserted into the
!      breakpoint. */
!   discard_cleanups (breakpoint_chain);
  
    /* Now set all the breakpoints.  */
    for (i = 0; i < sals.nelts; i++)
*************** break_command_1 (arg, flag, from_tty)
*** 4904,4921 ****
        set_breakpoint_count (breakpoint_count + 1);
        b->number = breakpoint_count;
        b->type = hardwareflag ? bp_hardware_breakpoint : bp_breakpoint;
!       b->cond = cond;
        b->thread = thread;
! 
!       /* If a canonical line spec is needed use that instead of the
!          command string.  */
!       if (canonical != (char **) NULL && canonical[i] != NULL)
! 	b->addr_string = canonical[i];
!       else if (addr_start)
! 	b->addr_string = savestring (addr_start, addr_end - addr_start);
!       if (cond_start)
! 	b->cond_string = savestring (cond_start, cond_end - cond_start);
! 
        b->enable = enabled;
        b->disposition = tempflag ? del : donttouch;
        mention (b);
--- 4952,4962 ----
        set_breakpoint_count (breakpoint_count + 1);
        b->number = breakpoint_count;
        b->type = hardwareflag ? bp_hardware_breakpoint : bp_breakpoint;
!       b->cond = cond[i];
        b->thread = thread;
!       b->addr_string = addr_string[i];
!       b->cond_string = cond_string[i];
!       b->ignore_count = ignore_count;
        b->enable = enabled;
        b->disposition = tempflag ? del : donttouch;
        mention (b);

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