[gdb/f14/master] - Fix crash on stale bpstat (BZ 661773).

Jan Kratochvil jankratochvil at fedoraproject.org
Thu Jan 13 20:39:42 UTC 2011


commit 79184ebd8ac723c2f40ad8a228365d9e787bb7d8
Author: Jan Kratochvil <jan.kratochvil at redhat.com>
Date:   Thu Jan 13 21:39:27 2011 +0100

    - Fix crash on stale bpstat (BZ 661773).

 gdb-stale-bpstat-2of3.patch |  757 +++++++++++++++++++++++++++++++++++++++++++
 gdb-stale-bpstat-3of3.patch |   11 +
 gdb.spec                    |   11 +-
 3 files changed, 778 insertions(+), 1 deletions(-)
---
diff --git a/gdb-stale-bpstat-2of3.patch b/gdb-stale-bpstat-2of3.patch
new file mode 100644
index 0000000..bb7189b
--- /dev/null
+++ b/gdb-stale-bpstat-2of3.patch
@@ -0,0 +1,757 @@
+--- ./gdb/breakpoint.c	2011-01-13 17:41:49.000000000 +0100
++++ ./gdb/breakpoint.c	2011-01-13 21:01:03.000000000 +0100
+@@ -142,7 +142,7 @@ static void watchpoints_info (char *, in
+ 
+ static int breakpoint_1 (int, int, int (*) (const struct breakpoint *));
+ 
+-static bpstat bpstat_alloc (const struct bp_location *, bpstat);
++static bpstat bpstat_alloc (struct bp_location *, bpstat);
+ 
+ static int breakpoint_cond_eval (void *);
+ 
+@@ -209,6 +209,8 @@ static int single_step_breakpoint_insert
+ 						   CORE_ADDR pc);
+ 
+ static void free_bp_location (struct bp_location *loc);
++static void incref_bp_location (struct bp_location *loc);
++static void decref_bp_location (struct bp_location **loc);
+ 
+ static struct bp_location *allocate_bp_location (struct breakpoint *bpt);
+ 
+@@ -216,9 +218,6 @@ static void update_global_location_list 
+ 
+ static void update_global_location_list_nothrow (int);
+ 
+-static int bpstat_remove_bp_location_callback (struct thread_info *th,
+-					       void *data);
+-
+ static int is_hardware_watchpoint (const struct breakpoint *bpt);
+ 
+ static int is_watchpoint (const struct breakpoint *bpt);
+@@ -2663,7 +2662,7 @@ breakpoint_init_inferior (enum inf_conte
+ 
+   /* Get rid of the moribund locations.  */
+   for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, bpt); ++ix)
+-    free_bp_location (bpt);
++    decref_bp_location (&bpt);
+   VEC_free (bp_location_p, moribund_locations);
+ }
+ 
+@@ -2911,12 +2910,16 @@ ep_is_catchpoint (struct breakpoint *ep)
+   return (ep->type == bp_catchpoint);
+ }
+ 
+-void 
++/* Frees any storage that is part of a bpstat.  Does not walk the
++   'next' chain.  */
++
++static void
+ bpstat_free (bpstat bs)
+ {
+   if (bs->old_val != NULL)
+     value_free (bs->old_val);
+   decref_counted_command_line (&bs->commands);
++  decref_bp_location (&bs->bp_location_at);
+   xfree (bs);
+ }
+ 
+@@ -2959,6 +2962,7 @@ bpstat_copy (bpstat bs)
+       tmp = (bpstat) xmalloc (sizeof (*tmp));
+       memcpy (tmp, bs, sizeof (*tmp));
+       incref_counted_command_line (tmp->commands);
++      incref_bp_location (tmp->bp_location_at);
+       if (bs->old_val != NULL)
+ 	{
+ 	  tmp->old_val = value_copy (bs->old_val);
+@@ -2986,7 +2990,7 @@ bpstat_find_breakpoint (bpstat bsp, stru
+ 
+   for (; bsp != NULL; bsp = bsp->next)
+     {
+-      if (bsp->breakpoint_at && bsp->breakpoint_at->owner == breakpoint)
++      if (bsp->breakpoint_at == breakpoint)
+ 	return bsp;
+     }
+   return NULL;
+@@ -3013,7 +3017,7 @@ bpstat_num (bpstat *bsp, int *num)
+      correspond to a single breakpoint -- otherwise, 
+      this function might return the same number more
+      than once and this will look ugly.  */
+-  b = (*bsp)->breakpoint_at ? (*bsp)->breakpoint_at->owner : NULL;
++  b = (*bsp)->breakpoint_at;
+   *bsp = (*bsp)->next;
+   if (b == NULL)
+     return -1;			/* breakpoint that's been deleted since */
+@@ -3225,13 +3229,11 @@ print_it_typical (bpstat bs)
+      which has since been deleted.  */
+   if (bs->breakpoint_at == NULL)
+     return PRINT_UNKNOWN;
+-  bl = bs->breakpoint_at;
+ 
+-  /* bl->owner can be NULL if it was a momentary breakpoint
+-     which has since been placed into moribund_locations.  */
+-  if (bl->owner == NULL)
+-    return PRINT_UNKNOWN;
+-  b = bl->owner;
++  gdb_assert (bs->bp_location_at != NULL);
++
++  bl = bs->bp_location_at;
++  b = bs->breakpoint_at;
+ 
+   stb = ui_out_stream_new (uiout);
+   old_chain = make_cleanup_ui_out_stream_delete (stb);
+@@ -3240,7 +3242,7 @@ print_it_typical (bpstat bs)
+     {
+     case bp_breakpoint:
+     case bp_hardware_breakpoint:
+-      bp_temp = bs->breakpoint_at->owner->disposition == disp_del;
++      bp_temp = b->disposition == disp_del;
+       if (bl->address != bl->requested_address)
+ 	breakpoint_adjustment_warning (bl->requested_address,
+ 	                               bl->address,
+@@ -3431,9 +3433,8 @@ print_bp_stop_message (bpstat bs)
+ 
+     case print_it_normal:
+       {
+-	const struct bp_location *bl = bs->breakpoint_at;
+-	struct breakpoint *b = bl ? bl->owner : NULL;
+-	
++	struct breakpoint *b = bs->breakpoint_at;
++
+ 	/* Normal case.  Call the breakpoint's print_it method, or
+ 	   print_it_typical.  */
+ 	/* FIXME: how breakpoint can ever be NULL here?  */
+@@ -3512,13 +3513,15 @@ breakpoint_cond_eval (void *exp)
+ /* Allocate a new bpstat and chain it to the current one.  */
+ 
+ static bpstat
+-bpstat_alloc (const struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
++bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
+ {
+   bpstat bs;
+ 
+   bs = (bpstat) xmalloc (sizeof (*bs));
+   cbs->next = bs;
+-  bs->breakpoint_at = bl;
++  bs->breakpoint_at = bl->owner;
++  bs->bp_location_at = bl;
++  incref_bp_location (bl);
+   /* If the condition is false, etc., don't do the commands.  */
+   bs->commands = NULL;
+   bs->commands_left = NULL;
+@@ -3611,10 +3614,9 @@ watchpoint_check (void *p)
+   struct frame_info *fr;
+   int within_current_scope;
+ 
+-  /* BS is built for existing struct breakpoint.  */
++  /* BS is built from an existing struct breakpoint.  */
+   gdb_assert (bs->breakpoint_at != NULL);
+-  gdb_assert (bs->breakpoint_at->owner != NULL);
+-  b = bs->breakpoint_at->owner;
++  b = bs->breakpoint_at;
+ 
+   gdb_assert (is_watchpoint (b));
+ 
+@@ -3805,9 +3807,9 @@ bpstat_check_watchpoint (bpstat bs)
+   struct breakpoint *b;
+ 
+   /* BS is built for existing struct breakpoint.  */
+-  bl = bs->breakpoint_at;
++  bl = bs->bp_location_at;
+   gdb_assert (bl != NULL);
+-  b = bl->owner;
++  b = bs->breakpoint_at;
+   gdb_assert (b != NULL);
+ 
+   if (is_watchpoint (b))
+@@ -3956,6 +3958,7 @@ bpstat_check_watchpoint (bpstat bs)
+ /* Check conditions (condition proper, frame, thread and ignore count)
+    of breakpoint referred to by BS.  If we should not stop for this
+    breakpoint, set BS->stop to 0.  */
++
+ static void
+ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
+ {
+@@ -3964,9 +3967,9 @@ bpstat_check_breakpoint_conditions (bpst
+   struct breakpoint *b;
+ 
+   /* BS is built for existing struct breakpoint.  */
+-  bl = bs->breakpoint_at;
++  bl = bs->bp_location_at;
+   gdb_assert (bl != NULL);
+-  b = bl->owner;
++  b = bs->breakpoint_at;
+   gdb_assert (b != NULL);
+ 
+   if (frame_id_p (b->frame_id)
+@@ -3977,19 +3980,12 @@ bpstat_check_breakpoint_conditions (bpst
+       int value_is_zero = 0;
+       struct expression *cond;
+ 
+-      /* If this is a scope breakpoint, mark the associated
+-	 watchpoint as triggered so that we will handle the
+-	 out-of-scope event.  We'll get to the watchpoint next
+-	 iteration.  */
+-      if (b->type == bp_watchpoint_scope && b->related_breakpoint != b)
+-	b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
+-
+       if (is_watchpoint (b))
+ 	cond = b->cond_exp;
+       else
+ 	cond = bl->cond;
+ 
+-      if (cond && bl->owner->disposition != disp_del_at_next_stop)
++      if (cond && b->disposition != disp_del_at_next_stop)
+ 	{
+ 	  int within_current_scope = 1;
+ 
+@@ -4100,10 +4096,15 @@ bpstat_stop_status (struct address_space
+   bpstat bs = root_bs;
+   int ix;
+   int need_remove_insert;
++  int removed_any;
++  bpstat bs_prev;
+ 
+-  /* ALL_BP_LOCATIONS iteration would break across
+-     update_global_location_list possibly executed by
+-     bpstat_check_breakpoint_conditions's inferior call.  */
++  /* First, build the bpstat chain with locations that explain a
++     target stop, while being careful to not set the target running,
++     as that may invalidate locations (in particular watchpoint
++     locations are recreated).  Resuming will happen here with
++     breakpoint conditions or watchpoint expressions that include
++     inferior function calls.  */
+ 
+   ALL_BREAKPOINTS (b)
+     {
+@@ -4112,8 +4113,6 @@ bpstat_stop_status (struct address_space
+ 
+       for (bl = b->loc; bl != NULL; bl = bl->next)
+ 	{
+-	  bpstat bs_prev = bs;
+-
+ 	  /* For hardware watchpoints, we look only at the first location.
+ 	     The watchpoint_check function will work on the entire expression,
+ 	     not the individual locations.  For read watchpoints, the
+@@ -4131,27 +4130,66 @@ bpstat_stop_status (struct address_space
+ 	  /* Come here if it's a watchpoint, or if the break address matches */
+ 
+ 	  bs = bpstat_alloc (bl, bs);	/* Alloc a bpstat to explain stop */
+-	  gdb_assert (bs_prev->next == bs);
+ 
+-	  /* Assume we stop.  Should we find watchpoint that is not actually
+-	     triggered, or if condition of breakpoint is false, we'll reset
+-	     'stop' to 0.  */
++	  /* Assume we stop.  Should we find a watchpoint that is not
++	     actually triggered, or if the condition of the breakpoint
++	     evaluates as false, we'll reset 'stop' to 0.  */
+ 	  bs->stop = 1;
+ 	  bs->print = 1;
+ 
+-	  if (!bpstat_check_watchpoint (bs))
++	  /* If this is a scope breakpoint, mark the associated
++	     watchpoint as triggered so that we will handle the
++	     out-of-scope event.  We'll get to the watchpoint next
++	     iteration.  */
++	  if (b->type == bp_watchpoint_scope)
++	    b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
++	}
++    }
++
++  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
++    {
++      if (breakpoint_address_match (loc->pspace->aspace, loc->address,
++				    aspace, bp_addr))
++	{
++	  bs = bpstat_alloc (loc, bs);
++	  /* For hits of moribund locations, we should just proceed.  */
++	  bs->stop = 0;
++	  bs->print = 0;
++	  bs->print_it = print_it_noop;
++	}
++    }
++
++  /* Terminate the chain.  */
++  bs->next = NULL;
++
++  /* Now go through the locations that caused the target to stop, and
++     check whether we're interested in reporting this stop to higher
++     layers, or whether we should resume the target transparently.  */
++
++  removed_any = 0;
++
++  bs_prev = root_bs;
++  for (bs = root_bs->next; bs != NULL; bs_prev = bs, bs = bs->next)
++    {
++      if (!bs->stop)
++	continue;
++
++	  if (!bpstat_check_watchpoint (bs) && bs->print_it == print_it_noop
++	      && !bs->stop)
+ 	    {
+ 	      /* Ensure bpstat_explains_signal stays false if this BL could not be
+ 		 the cause of this trap.  */
+ 
+-	      gdb_assert (bs->print_it == print_it_noop);
+-	      gdb_assert (!bs->stop);
++//	      gdb_assert (bs->print_it == print_it_noop);
++//	      gdb_assert (!bs->stop);
++	      bs_prev->next = bs->next;
+ 	      xfree (bs);
+ 	      bs = bs_prev;
+-	      bs->next = NULL;
+ 	      continue;
+ 	    }
+ 
++      b = bs->breakpoint_at;
++
+ 	  if (b->type == bp_thread_event || b->type == bp_overlay_event
+ 	      || b->type == bp_longjmp_master
+ 	      || b->type == bp_std_terminate_master
+@@ -4160,7 +4198,7 @@ bpstat_stop_status (struct address_space
+ 	    bs->stop = 0;
+ 	  else
+ 	    bpstat_check_breakpoint_conditions (bs, ptid);
+-	
++
+ 	  if (bs->stop)
+ 	    {
+ 	      ++(b->hit_count);
+@@ -4170,7 +4208,7 @@ bpstat_stop_status (struct address_space
+ 		{
+ 		  if (b->enable_state != bp_permanent)
+ 		    b->enable_state = bp_disabled;
+-		  update_global_location_list (0);
++		  removed_any = 1;
+ 		}
+ 	      if (b->silent)
+ 		bs->print = 0;
+@@ -4191,24 +4229,8 @@ bpstat_stop_status (struct address_space
+ 	  /* Print nothing for this entry if we dont stop or dont print.  */
+ 	  if (bs->stop == 0 || bs->print == 0)
+ 	    bs->print_it = print_it_noop;
+-	}
+     }
+ 
+-  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+-    {
+-      if (breakpoint_address_match (loc->pspace->aspace, loc->address,
+-				    aspace, bp_addr))
+-	{
+-	  bs = bpstat_alloc (loc, bs);
+-	  /* For hits of moribund locations, we should just proceed.  */
+-	  bs->stop = 0;
+-	  bs->print = 0;
+-	  bs->print_it = print_it_noop;
+-	}
+-    }
+-
+-  bs->next = NULL;		/* Terminate the chain */
+-
+   /* If we aren't stopping, the value of some hardware watchpoint may
+      not have changed, but the intermediate memory locations we are
+      watching may have.  Don't bother if we're stopping; this will get
+@@ -4217,18 +4239,17 @@ bpstat_stop_status (struct address_space
+   if (! bpstat_causes_stop (root_bs->next))
+     for (bs = root_bs->next; bs != NULL; bs = bs->next)
+       if (!bs->stop
+-	  && bs->breakpoint_at->owner
+-	  && is_hardware_watchpoint (bs->breakpoint_at->owner))
++	  && bs->breakpoint_at
++	  && is_hardware_watchpoint (bs->breakpoint_at))
+ 	{
+-	  update_watchpoint (bs->breakpoint_at->owner, 0 /* don't reparse. */);
+-	  /* Updating watchpoints invalidates bs->breakpoint_at.
+-	     Prevent further code from trying to use it.  */
+-	  bs->breakpoint_at = NULL;
++	  update_watchpoint (bs->breakpoint_at, 0 /* don't reparse. */);
+ 	  need_remove_insert = 1;
+ 	}
+ 
+   if (need_remove_insert)
+     update_global_location_list (1);
++  else if (removed_any)
++    update_global_location_list (0);
+ 
+   return root_bs->next;
+ }
+@@ -4283,10 +4304,10 @@ bpstat_what (bpstat bs_head)
+ 	     breakpoint which has since been deleted.  */
+ 	  bptype = bp_none;
+ 	}
+-      else if (bs->breakpoint_at->owner == NULL)
++      else if (bs->breakpoint_at == NULL)
+ 	bptype = bp_none;
+       else
+-	bptype = bs->breakpoint_at->owner->type;
++	bptype = bs->breakpoint_at->type;
+ 
+       switch (bptype)
+ 	{
+@@ -4326,13 +4347,13 @@ bpstat_what (bpstat bs_head)
+ 	case bp_longjmp:
+ 	case bp_exception:
+ 	  this_action = BPSTAT_WHAT_SET_LONGJMP_RESUME;
+-	  retval.is_longjmp = bs->breakpoint_at->owner->type == bp_longjmp;
++	  retval.is_longjmp = bs->breakpoint_at->type == bp_longjmp;
+ 	  break;
+ 	case bp_longjmp_resume:
+ 	case bp_exception_resume:
+ 	  this_action = BPSTAT_WHAT_CLEAR_LONGJMP_RESUME;
+ 	  retval.is_longjmp
+-	    = bs->breakpoint_at->owner->type == bp_longjmp_resume;
++	    = bs->breakpoint_at->type == bp_longjmp_resume;
+ 	  break;
+ 	case bp_step_resume:
+ 	  if (bs->stop)
+@@ -4456,15 +4477,13 @@ bpstat_what (bpstat bs_head)
+     {
+       if (bs->breakpoint_at == NULL)
+ 	continue;
+-      if (bs->breakpoint_at->owner == NULL)
+-	continue;
+-      switch (bs->breakpoint_at->owner->type)
++      switch (bs->breakpoint_at->type)
+ 	{
+ 	case bp_gnu_ifunc_resolver:
+-	  gnu_ifunc_resolver_stop (bs->breakpoint_at->owner);
++	  gnu_ifunc_resolver_stop (bs->breakpoint_at);
+ 	  break;
+ 	case bp_gnu_ifunc_resolver_return:
+-	  gnu_ifunc_resolver_return_stop (bs->breakpoint_at->owner);
++	  gnu_ifunc_resolver_return_stop (bs->breakpoint_at);
+ 	  break;
+ 	}
+     }
+@@ -5500,32 +5519,41 @@ allocate_bp_location (struct breakpoint 
+       internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
+     }
+ 
++  loc->refc = 1;
+   return loc;
+ }
+ 
+-static void free_bp_location (struct bp_location *loc)
++static void
++free_bp_location (struct bp_location *loc)
+ {
+-  /* Be sure no bpstat's are pointing at it after it's been freed.  */
+-  /* FIXME, how can we find all bpstat's?
+-     We just check stop_bpstat for now.  Note that we cannot just
+-     remove bpstats pointing at bpt from the stop_bpstat list
+-     entirely, as breakpoint commands are associated with the bpstat;
+-     if we remove it here, then the later call to
+-         bpstat_do_actions (&stop_bpstat);
+-     in event-top.c won't do anything, and temporary breakpoints
+-     with commands won't work.  */
+-
+-  iterate_over_threads (bpstat_remove_bp_location_callback, loc);
+-
+   if (loc->cond)
+     xfree (loc->cond);
+ 
+   if (loc->function_name)
+     xfree (loc->function_name);
+-  
++
+   xfree (loc);
+ }
+ 
++/* Increment reference count.  */
++
++static void
++incref_bp_location (struct bp_location *bl)
++{
++  ++bl->refc;
++}
++
++/* Decrement reference count.  If the reference count reaches 0,
++   destroy the bp_location.  Sets *BLP to NULL.  */
++
++static void
++decref_bp_location (struct bp_location **blp)
++{
++  if (--(*blp)->refc == 0)
++    free_bp_location (*blp);
++  *blp = NULL;
++}
++
+ /* Helper to set_raw_breakpoint below.  Creates a breakpoint
+    that has type BPTYPE and has no locations as yet.  */
+ /* This function is used in gdbtk sources and thus can not be made static.  */
+@@ -9276,11 +9304,10 @@ breakpoint_auto_delete (bpstat bs)
+   struct breakpoint *b, *temp;
+ 
+   for (; bs; bs = bs->next)
+-    if (bs->breakpoint_at 
+-	&& bs->breakpoint_at->owner
+-	&& bs->breakpoint_at->owner->disposition == disp_del
++    if (bs->breakpoint_at
++	&& bs->breakpoint_at->disposition == disp_del
+ 	&& bs->stop)
+-      delete_breakpoint (bs->breakpoint_at->owner);
++      delete_breakpoint (bs->breakpoint_at);
+ 
+   ALL_BREAKPOINTS_SAFE (b, temp)
+   {
+@@ -9593,7 +9620,10 @@ update_global_location_list (int should_
+ 	      VEC_safe_push (bp_location_p, moribund_locations, old_loc);
+ 	    }
+ 	  else
+-	    free_bp_location (old_loc);
++	    {
++	      old_loc->owner = NULL;
++	      decref_bp_location (&old_loc);
++	    }
+ 	}
+     }
+ 
+@@ -9676,7 +9706,7 @@ breakpoint_retire_moribund (void)
+   for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+     if (--(loc->events_till_retirement) == 0)
+       {
+-	free_bp_location (loc);
++	decref_bp_location (&loc);
+ 	VEC_unordered_remove (bp_location_p, moribund_locations, ix);
+ 	--ix;
+       }
+@@ -9691,14 +9721,15 @@ update_global_location_list_nothrow (int
+     update_global_location_list (inserting);
+ }
+ 
+-/* Clear LOC from a BPS.  */
++/* Clear BKP from a BPS.  */
++
+ static void
+-bpstat_remove_bp_location (bpstat bps, struct bp_location *loc)
++bpstat_remove_bp_location (bpstat bps, struct breakpoint *bpt)
+ {
+   bpstat bs;
+ 
+   for (bs = bps; bs; bs = bs->next)
+-    if (bs->breakpoint_at == loc)
++    if (bs->breakpoint_at == bpt)
+       {
+ 	bs->breakpoint_at = NULL;
+ 	bs->old_val = NULL;
+@@ -9708,16 +9739,16 @@ bpstat_remove_bp_location (bpstat bps, s
+ 
+ /* Callback for iterate_over_threads.  */
+ static int
+-bpstat_remove_bp_location_callback (struct thread_info *th, void *data)
++bpstat_remove_breakpoint_callback (struct thread_info *th, void *data)
+ {
+-  struct bp_location *loc = data;
++  struct breakpoint *bpt = data;
+ 
+-  bpstat_remove_bp_location (th->stop_bpstat, loc);
++  bpstat_remove_bp_location (th->stop_bpstat, bpt);
+   return 0;
+ }
+ 
+ /* Delete a breakpoint and clean up all traces of it in the data
+-   structures. */
++   structures.  */
+ 
+ void
+ delete_breakpoint (struct breakpoint *bpt)
+@@ -9785,6 +9816,19 @@ delete_breakpoint (struct breakpoint *bp
+   xfree (bpt->exec_pathname);
+   clean_up_filters (&bpt->syscalls_to_be_caught);
+ 
++
++  /* Be sure no bpstat's are pointing at the breakpoint after it's
++     been freed.  */
++  /* FIXME, how can we find all bpstat's?  We just check stop_bpstat
++     in all threeds for now.  Note that we cannot just remove bpstats
++     pointing at bpt from the stop_bpstat list entirely, as breakpoint
++     commands are associated with the bpstat; if we remove it here,
++     then the later call to bpstat_do_actions (&stop_bpstat); in
++     event-top.c won't do anything, and temporary breakpoints with
++     commands won't work.  */
++
++  iterate_over_threads (bpstat_remove_breakpoint_callback, bpt);
++
+   /* Now that breakpoint is removed from breakpoint
+      list, update the global location list.  This
+      will remove locations that used to belong to
+--- ./gdb/breakpoint.h	2011-01-13 17:41:49.000000000 +0100
++++ ./gdb/breakpoint.h	2011-01-13 18:13:40.000000000 +0100
+@@ -253,13 +253,18 @@ struct bp_location
+      the same parent breakpoint.  */
+   struct bp_location *next;
+ 
++  /* The reference count.  */
++  int refc;
++
+   /* Type of this breakpoint location.  */
+   enum bp_loc_type loc_type;
+ 
+   /* Each breakpoint location must belong to exactly one higher-level
+-     breakpoint.  This and the DUPLICATE flag are more straightforward
+-     than reference counting.  This pointer is NULL iff this bp_location is in
+-     (and therefore only in) moribund_locations.  */
++     breakpoint.  This pointer is NULL iff this bp_location is no
++     longer attached to a breakpoint.  For example, when a breakpoint
++     is deleted, its locations may still be found in the
++     moribund_locations list, or if we had stopped for it, in
++     bpstats.  */
+   struct breakpoint *owner;
+ 
+   /* Conditional.  Break only if this expression's value is nonzero.
+@@ -573,10 +578,6 @@ DEF_VEC_P(breakpoint_p);
+ 
+ typedef struct bpstats *bpstat;
+ 
+-/* Frees any storage that is part of a bpstat.
+-   Does not walk the 'next' chain.  */
+-extern void bpstat_free (bpstat);
+-
+ /* Clears a chain of bpstat, freeing storage
+    of each.  */
+ extern void bpstat_clear (bpstat *);
+@@ -745,16 +746,41 @@ enum bp_print_how
+ 
+ struct bpstats
+   {
+-    /* Linked list because there can be two breakpoints at the same
+-       place, and a bpstat reflects the fact that both have been hit.  */
++    /* Linked list because there can be more than one breakpoint at
++       the same place, and a bpstat reflects the fact that all have
++       been hit.  */
+     bpstat next;
+-    /* Breakpoint that we are at.  */
+-    const struct bp_location *breakpoint_at;
++
++    /* Location that caused the stop.  Locations are refcounted, so
++       this will never be NULL.  Note that this location may end up
++       detached from a breakpoint, but that does not necessary mean
++       that the struct breakpoint is gone.  E.g., consider a
++       watchpoint with a condition that involves an inferior function
++       call.  Watchpoint locations are recreated often (on resumes,
++       hence on infcalls too).  Between creating the bpstat and after
++       evaluating the watchpoint condition, this location may hence
++       end up detached from its original owner watchpoint, even though
++       the watchpoint is still listed.  If it's condition evaluates as
++       true, we still want this location to cause a stop, and we will
++       still need to know which watchpoint it was originally attached.
++       What this means is that we should not (in most cases) follow
++       the `bpstat->bp_location->owner' link, but instead use the
++       `breakpoint_at' field below.  */
++    struct bp_location *bp_location_at;
++
++    /* Breakpoint that caused the stop.  This is nullified if the
++       breakpoint ends up being deleted.  See comments on
++       `bp_location_at' above for why do we need this field instead of
++       following the location's owner.  */
++    struct breakpoint *breakpoint_at;
++
+     /* The associated command list.  */
+     struct counted_command_line *commands;
++
+     /* Commands left to be done.  This points somewhere in
+        base_command.  */
+     struct command_line *commands_left;
++
+     /* Old value associated with a watchpoint.  */
+     struct value *old_val;
+ 
+--- ./gdb/testsuite/gdb.base/watch-cond-infcall.c	1970-01-01 01:00:00.000000000 +0100
++++ ./gdb/testsuite/gdb.base/watch-cond-infcall.c	2011-01-13 18:13:40.000000000 +0100
+@@ -0,0 +1,33 @@
++/* This testcase is part of GDB, the GNU debugger.
++
++   Copyright 2010 Free Software Foundation, Inc.
++
++   This program is free software; you can redistribute it and/or modify
++   it under the terms of the GNU General Public License as published by
++   the Free Software Foundation; either version 3 of the License, or
++   (at your option) any later version.
++
++   This program is distributed in the hope that it will be useful,
++   but WITHOUT ANY WARRANTY; without even the implied warranty of
++   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++   GNU General Public License for more details.
++
++   You should have received a copy of the GNU General Public License
++   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
++
++volatile int var;
++
++int
++return_1 (void)
++{
++  return 1;
++}
++
++int
++main(void)
++{
++  var++;
++  var++;	/* watchpoint-stop */
++
++  return 0;     /* break-at-exit */
++}
+--- ./gdb/testsuite/gdb.base/watch-cond-infcall.exp	1970-01-01 01:00:00.000000000 +0100
++++ ./gdb/testsuite/gdb.base/watch-cond-infcall.exp	2011-01-13 18:13:40.000000000 +0100
+@@ -0,0 +1,61 @@
++# Copyright 2010 Free Software Foundation, Inc.
++
++# This program is free software; you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation; either version 3 of the License, or
++# (at your option) any later version.
++#
++# This program is distributed in the hope that it will be useful,
++# but WITHOUT ANY WARRANTY; without even the implied warranty of
++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++# GNU General Public License for more details.
++#
++# You should have received a copy of the GNU General Public License
++# along with this program.  If not, see <http://www.gnu.org/licenses/>.
++
++# Test for watchpoints with conditions that involve inferior function
++# calls.
++
++set testfile "watch-cond-infcall"
++set srcfile ${testfile}.c
++set binfile ${objdir}/${subdir}/${testfile}
++
++if { [build_executable ${testfile}.exp ${testfile} ${testfile}.c {debug}] } {
++    untested ${testfile}.exp
++    return -1
++}
++
++proc test_watchpoint { hw teststr } {
++    global testfile
++    global pf_prefix
++
++    set old_pf_prefix $pf_prefix
++    lappend pf_prefix "$teststr:"
++
++    clean_restart ${testfile}
++
++    if { ![runto main] } then {
++	fail "run to main"
++	return
++    }
++
++    if { ! $hw } {
++	gdb_test_no_output "set can-use-hw-watchpoints 0" ""
++    }
++
++    gdb_test "watch var if return_1 ()" "atchpoint .*: var"
++
++    gdb_breakpoint [gdb_get_line_number "break-at-exit"]
++
++    gdb_test "continue" \
++	"atchpoint \[0-9\]+: var\r\n\r\nOld value = 0\r\nNew value = 1\r\n.*watchpoint-stop.*" \
++	"continue"
++
++    set pf_prefix $old_pf_prefix
++}
++
++if { ![target_info exists gdb,no_hardware_watchpoints] } {
++    test_watchpoint 1 "hw"
++}
++
++test_watchpoint 0 "sw"
diff --git a/gdb-stale-bpstat-3of3.patch b/gdb-stale-bpstat-3of3.patch
new file mode 100644
index 0000000..e7e10bb
--- /dev/null
+++ b/gdb-stale-bpstat-3of3.patch
@@ -0,0 +1,11 @@
+--- ./gdb/testsuite/gdb.cp/infcall-dlopen.exp.orig	2011-01-13 21:16:47.000000000 +0100
++++ ./gdb/testsuite/gdb.cp/infcall-dlopen.exp	2011-01-13 21:17:19.000000000 +0100
+@@ -40,6 +40,8 @@ if { ![runto_main] } {
+ 
+ for {set i 0} {$i < 10} {incr i} {
+     gdb_test "p openlib (\"${libfile}\")" " = 1" "test $i"
++    # https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=660197
++    gdb_test "info program" "\r\nProgram stopped at .*\r\nIt stopped at breakpoint 1\\." "test $i info program"
+     # Try to exploit the GDB trashed memory.
+     gdb_test "b openlib" {Breakpoint [0-9]+ at .*} "test $i stub 1"
+     gdb_test_no_output {delete $bpnum} "test $i stub 2"
diff --git a/gdb.spec b/gdb.spec
index 2573326..ec60d2a 100644
--- a/gdb.spec
+++ b/gdb.spec
@@ -27,7 +27,7 @@ Version: 7.2
 
 # The release always contains a leading reserved number, start it at 1.
 # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
-Release: 29%{?_with_upstream:.upstream}%{dist}
+Release: 30%{?_with_upstream:.upstream}%{dist}
 
 License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and GFDL and BSD and Public Domain
 Group: Development/Debuggers
@@ -634,6 +634,10 @@ Patch520: gdb-bz642879-elfread-sigint-stale.patch
 #=drop
 Patch527: gdb-bz653644-gdbindex-double-free.patch
 
+# Fix crash on stale bpstat (BZ 661773).
+Patch529: gdb-stale-bpstat-2of3.patch
+Patch530: gdb-stale-bpstat-3of3.patch
+
 # Backport gdb.base/break-interp.exp test (+prelink fix) on PPC (BZ 663449).
 #=drop
 Patch533: gdb-ppc-test-break-interp-1of6.patch
@@ -956,6 +960,8 @@ rm -f gdb/jv-exp.c gdb/m2-exp.c gdb/objc-exp.c gdb/p-exp.c
 %patch538 -p1
 %patch539 -p1
 %patch540 -p1
+%patch529 -p1
+%patch530 -p1
 %patch541 -p1
 %patch542 -p1
 %patch543 -p1
@@ -1332,6 +1338,9 @@ fi
 %endif
 
 %changelog
+* Thu Jan 13 2011 Jan Kratochvil <jan.kratochvil at redhat.com> - 7.2-30.fc14
+- Fix crash on stale bpstat (BZ 661773).
+
 * Mon Jan  3 2011 Jan Kratochvil <jan.kratochvil at redhat.com> - 7.2-29.fc14
 - Backport support of template parameters (Tom Tromey, BZ 562758).
 - New test gdb.base/gnu-ifunc.exp:"static gnu_ifunc" (BZ 632259).


More information about the scm-commits mailing list