Re: [5/9] Associate parsed condition with location

[Vladimir Prus <vladimir at codesourcery dot com>]

On Saturday 08 September 2007 15:11:40 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Sat, 8 Sep 2007 00:42:24 +0400
> > 
> > 	* breakpoint.c: Adjust.
> > 	* tui/tui-winsource.c: Adjust.
> 
> Nitpicking: these aren't, strictly speaking, valid ChangeLog entries,
> because they don't name the functions where the changes were made.

I've added the list of functions.

> 
> > --- gdb/breakpoint.h	(/work/mb_mainline/4_bpstat_owner)	(revision 4741)
> > +++ gdb/breakpoint.h	(/work/mb_mainline/5_per_loc_cond)	(revision 4741)
> > @@ -243,6 +243,13 @@ struct bp_location
> >       than reference counting.  */
> >    struct breakpoint *owner;
> >  
> > +  /* Conditional.  Break only if this expression's value is nonzero.  
> > +     Unlike string form of condition, which is associated with breakpoint,
> > +     this is associated with location, since if breakpoint has several
> > +     PC locations, the evaluation of expression can be different for
> > +     different PCs.  */
> > +  struct expression *cond;
> 
> I think we should use ``location'' instead of ``PC'', since you make
> the change for watchpoints as well, where the PC is irrelevant.

Changed.

Is the attached fine with you?

- Volodya

Attachment: mainline_5_per_loc_cond.ChangeLog
Description: Text document

--- gdb/breakpoint.c	(/work/mb_mainline/4_bpstat_owner)	(revision 4828)
+++ gdb/breakpoint.c	(/work/mb_mainline/5_per_loc_cond)	(revision 4828)
@@ -569,17 +569,17 @@ condition_command (char *arg, int from_t
   ALL_BREAKPOINTS (b)
     if (b->number == bnum)
     {
-      if (b->cond)
+      struct bp_location *loc = b->loc;
+      if (loc->cond)
 	{
-	  xfree (b->cond);
-	  b->cond = 0;
+	  xfree (loc->cond);
+	  loc->cond = 0;
 	}
       if (b->cond_string != NULL)
 	xfree (b->cond_string);
 
       if (*p == 0)
 	{
-	  b->cond = 0;
 	  b->cond_string = NULL;
 	  if (from_tty)
 	    printf_filtered (_("Breakpoint %d now unconditional.\n"), bnum);
@@ -592,7 +592,8 @@ condition_command (char *arg, int from_t
 	  b->cond_string = savestring (arg, strlen (arg));
 	  if (!b->pending)
 	    {
-	      b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+	      b->loc->cond = parse_exp_1 (&arg, 
+					  block_for_pc (b->loc->address), 0);
 	      if (*arg)
 		error (_("Junk at end of expression"));
 	    }
@@ -2923,19 +2924,19 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
       {
 	int value_is_zero = 0;
 
-	if (b->cond)
+	if (b->loc->cond)
 	  {
 	    /* Need to select the frame, with all that implies
 	       so that the conditions will have the right context.  */
 	    select_frame (get_current_frame ());
 	    value_is_zero
-	      = catch_errors (breakpoint_cond_eval, (b->cond),
+	      = catch_errors (breakpoint_cond_eval, (b->loc->cond),
 			      "Error in testing breakpoint condition:\n",
 			      RETURN_MASK_ALL);
 	    /* FIXME-someday, should give breakpoint # */
 	    free_all_values ();
 	  }
-	if (b->cond && value_is_zero)
+	if (b->loc->cond && value_is_zero)
 	  {
 	    bs->stop = 0;
 	    /* Don't consider this a hit.  */
@@ -3606,14 +3607,14 @@ print_one_breakpoint (struct breakpoint 
       ui_out_text (uiout, "\n");
     }
   
-  if (b->cond && !ada_exception_catchpoint_p (b))
+  if (b->loc->cond && !ada_exception_catchpoint_p (b))
     {
       /* We do not print the condition for Ada exception catchpoints
          because the condition is an internal implementation detail
          that we do not want to expose to the user.  */
       annotate_field (7);
       ui_out_text (uiout, "\tstop only if ");
-      print_expression (b->cond, stb->stream);
+      print_expression (b->loc->cond, stb->stream);
       ui_out_field_stream (uiout, "cond", stb);
       ui_out_text (uiout, "\n");
     }
@@ -4085,6 +4086,7 @@ allocate_bp_location (struct breakpoint 
   memset (loc, 0, sizeof (*loc));
 
   loc->owner = bpt;
+  loc->cond = NULL;
 
   switch (bp_type)
     {
@@ -4604,7 +4606,6 @@ solib_load_unload_1 (char *hookname, int
   b = set_raw_breakpoint (sals.sals[0], bp_kind);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = NULL;
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
   b->thread = thread;
@@ -4661,7 +4662,6 @@ create_fork_vfork_event_catchpoint (int 
   b = set_raw_breakpoint (sal, bp_kind);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = NULL;
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
   b->thread = thread;
@@ -4700,7 +4700,6 @@ create_exec_event_catchpoint (int tempfl
   b = set_raw_breakpoint (sal, bp_catch_exec);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = NULL;
   b->cond_string = (cond_string == NULL) ?
     NULL : savestring (cond_string, strlen (cond_string));
   b->thread = thread;
@@ -5033,7 +5032,7 @@ create_breakpoints (struct symtabs_and_l
 	b = set_raw_breakpoint (sal, type);
 	set_breakpoint_count (breakpoint_count + 1);
 	b->number = breakpoint_count;
-	b->cond = cond[i];
+	b->loc->cond = cond[i];
 	b->thread = thread;
 	if (addr_string[i])
 	  b->addr_string = addr_string[i];
@@ -5056,7 +5055,7 @@ create_breakpoints (struct symtabs_and_l
 	      {
 		arg = pending_bp->cond_string;
 		b->cond_string = savestring (arg, strlen (arg));
-		b->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
+		b->loc->cond = parse_exp_1 (&arg, block_for_pc (b->loc->address), 0);
 		if (*arg)
 		  error (_("Junk at end of pending breakpoint condition expression"));
 	      }
@@ -5399,7 +5398,7 @@ break_command_1 (char *arg, int flag, in
 		              : bp_breakpoint);
       set_breakpoint_count (breakpoint_count + 1);
       b->number = breakpoint_count;
-      b->cond = *cond;
+      b->loc->cond = *cond;
       b->thread = thread;
       b->addr_string = *addr_string;
       b->cond_string = *cond_string;
@@ -5792,7 +5791,7 @@ watch_command_1 (char *arg, int accessfl
   b->exp_valid_block = exp_valid_block;
   b->exp_string = savestring (exp_start, exp_end - exp_start);
   b->val = val;
-  b->cond = cond;
+  b->loc->cond = cond;
   if (cond_start)
     b->cond_string = savestring (cond_start, cond_end - cond_start);
   else
@@ -6352,7 +6351,6 @@ create_exception_catchpoint (int tempfla
   b = set_raw_breakpoint (*sal, bptype);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = NULL;
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
   b->thread = thread;
@@ -6432,7 +6430,6 @@ handle_gnu_v3_exceptions (int tempflag, 
   b = set_raw_breakpoint (sals.sals[0], bp_breakpoint);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = NULL;
   b->cond_string = (cond_string == NULL) ? 
     NULL : savestring (cond_string, strlen (cond_string));
   b->thread = -1;
@@ -6519,7 +6516,7 @@ create_ada_exception_breakpoint (struct 
   b->disposition = tempflag ? disp_del : disp_donttouch;
   b->number = breakpoint_count;
   b->ignore_count = 0;
-  b->cond = cond;
+  b->loc->cond = cond;
   b->addr_string = addr_string;
   b->language = language_ada;
   b->cond_string = cond_string;
@@ -6701,7 +6698,6 @@ set_breakpoint_sal (struct symtab_and_li
   b = set_raw_breakpoint (sal, bp_breakpoint);
   set_breakpoint_count (breakpoint_count + 1);
   b->number = breakpoint_count;
-  b->cond = 0;
   b->thread = -1;
   return b;
 }
@@ -7021,8 +7017,6 @@ delete_breakpoint (struct breakpoint *bp
     }
 
   free_command_lines (&bpt->commands);
-  if (bpt->cond)
-    xfree (bpt->cond);
   if (bpt->cond_string != NULL)
     xfree (bpt->cond_string);
   if (bpt->addr_string != NULL)
@@ -7062,6 +7056,8 @@ delete_breakpoint (struct breakpoint *bp
      bp, we mark it as deleted before freeing its storage. */
   bpt->type = bp_none;
 
+  if (bpt->loc->cond)
+    xfree (bpt->loc->cond);
   xfree (bpt->loc);
   xfree (bpt);
 }
@@ -7215,14 +7211,14 @@ breakpoint_re_set_one (void *bint)
 	  if (b->cond_string != NULL)
 	    {
 	      s = b->cond_string;
-	      if (b->cond)
+	      if (b->loc->cond)
 		{
-		  xfree (b->cond);
+		  xfree (b->loc->cond);
 		  /* Avoid re-freeing b->exp if an error during the call
 		     to parse_exp_1.  */
-		  b->cond = NULL;
+		  b->loc->cond = NULL;
 		}
-	      b->cond = parse_exp_1 (&s, block_for_pc (sals.sals[i].pc), 0);
+	      b->loc->cond = parse_exp_1 (&s, block_for_pc (sals.sals[i].pc), 0);
 	    }
 
 	  /* We need to re-set the breakpoint if the address changes... */
@@ -7314,14 +7310,14 @@ breakpoint_re_set_one (void *bint)
       if (b->cond_string != NULL)
 	{
 	  s = b->cond_string;
-	  if (b->cond)
+	  if (b->loc->cond)
 	    {
-	      xfree (b->cond);
+	      xfree (b->loc->cond);
 	      /* Avoid re-freeing b->exp if an error during the call
 		 to parse_exp_1.  */
-	      b->cond = NULL;
+	      b->loc->cond = NULL;
 	    }
-	  b->cond = parse_exp_1 (&s, (struct block *) 0, 0);
+	  b->loc->cond = parse_exp_1 (&s, (struct block *) 0, 0);
 	}
       if (breakpoint_enabled (b))
 	mention (b);
--- gdb/breakpoint.h	(/work/mb_mainline/4_bpstat_owner)	(revision 4828)
+++ gdb/breakpoint.h	(/work/mb_mainline/5_per_loc_cond)	(revision 4828)
@@ -243,6 +243,13 @@ struct bp_location
      than reference counting.  */
   struct breakpoint *owner;
 
+  /* Conditional.  Break only if this expression's value is nonzero.  
+     Unlike string form of condition, which is associated with breakpoint,
+     this is associated with location, since if breakpoint has several
+     locations,  the evaluation of expression can be different for
+     different locations.  */
+  struct expression *cond;
+  
   /* Nonzero if this breakpoint is now inserted.  */
   char inserted;
 
@@ -341,8 +348,6 @@ struct breakpoint
     /* Stack depth (address of frame).  If nonzero, break only if fp
        equals this.  */
     struct frame_id frame_id;
-    /* Conditional.  Break only if this expression's value is nonzero.  */
-    struct expression *cond;
 
     /* String we used to set the breakpoint (malloc'd).  */
     char *addr_string;
--- gdb/tui/tui-winsource.c	(/work/mb_mainline/4_bpstat_owner)	(revision 4828)
+++ gdb/tui/tui-winsource.c	(/work/mb_mainline/5_per_loc_cond)	(revision 4828)
@@ -449,7 +449,7 @@ tui_update_breakpoint_info (struct tui_w
                 mode |= TUI_BP_ENABLED;
               if (bp->hit_count)
                 mode |= TUI_BP_HIT;
-              if (bp->cond)
+              if (bp->loc->cond)
                 mode |= TUI_BP_CONDITIONAL;
               if (bp->type == bp_hardware_breakpoint)
                 mode |= TUI_BP_HARDWARE;

Property changes on: 
___________________________________________________________________
Name: svk:merge
 +d48a11ec-ee1c-0410-b3f5-c20844f99675:/work/mb_mainline/4_bpstat_owner:4823
  e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:182811