* breakpoint.c (moribund_locations): New.

        (bpstat_stop_status): Process moribund locations.
        (update_global_location_list): Add removed
        locations to moribund_locations.
        (breakpoint_retire_moribund): New.
        * breakpoint.h (struct bp_location): New field
        events_till_retirement.
        (breakpoint_retire_moribund): Declare.
        * thread.c (thread_count): New.
        * infrun.c (handle_inferior_event): Call
        breakpoint_retire_moribund.
        * gdbthread.h (thread_count): Declare.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 25b8de39bac3d0d240dc09ecf3ace88037c9617c..d54679d9d7faa9fc7301d2e91eeebd5070cadd06 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,18 @@
+2008-06-28  Vladimir Prus  <vladimir@codesourcery.com>
+
+        * breakpoint.c (moribund_locations): New.
+        (bpstat_stop_status): Process moribund locations.
+        (update_global_location_list): Add removed
+        locations to moribund_locations.
+        (breakpoint_retire_moribund): New.
+        * breakpoint.h (struct bp_location): New field
+        events_till_retirement.
+        (breakpoint_retire_moribund): Declare.
+        * thread.c (thread_count): New.
+        * infrun.c (handle_inferior_event): Call
+        breakpoint_retire_moribund.
+        * gdbthread.h (thread_count): Declare.
+
 2008-06-27  Joseph Myers  <joseph@codesourcery.com>
 
        * dfp.c (decimal_convert): Call match_endianness before and after

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 26eff8dd1dd6e0115243ab4e12c58a405c784c9f..73d962cc945d097e5c44dc0d51a1483a5bf5d7d1 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -308,6 +308,11 @@ struct breakpoint *breakpoint_chain;
 
 struct bp_location *bp_location_chain;
 
+/* The locations that no longer correspond to any breakpoint,
+   unlinked from bp_location_chain, but for which a hit
+   may still be reported by a target.  */
+VEC(bp_location_p) *moribund_locations = NULL;
+
 /* Number of last breakpoint made.  */
 
 int breakpoint_count;
@@ -3003,10 +3008,12 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
 {
   struct breakpoint *b = NULL;
   const struct bp_location *bl;
+  struct bp_location *loc;
   /* Root of the chain of bpstat's */
   struct bpstats root_bs[1];
   /* Pointer to the last thing in the chain currently.  */
   bpstat bs = root_bs;
+  int ix;
 
   ALL_BP_LOCATIONS (bl)
   {
@@ -3075,6 +3082,18 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
       bs->print_it = print_it_noop;
   }
 
+  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+    {
+      if (loc->address == 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 */
   bs = root_bs->next;          /* Re-grab the head of the chain */
 
@@ -3089,6 +3108,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, ptid_t ptid)
   if (bs == NULL)
     for (bs = root_bs->next; bs != NULL; bs = bs->next)
       if (!bs->stop
+         && bs->breakpoint_at->owner
          && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint
              || bs->breakpoint_at->owner->type == bp_read_watchpoint
              || bs->breakpoint_at->owner->type == bp_access_watchpoint))
@@ -3253,6 +3273,9 @@ bpstat_what (bpstat bs)
        /* I suspect this can happen if it was a momentary breakpoint
           which has since been deleted.  */
        continue;
+      if (bs->breakpoint_at->owner == NULL)
+       bs_class = bp_nostop;
+      else
       switch (bs->breakpoint_at->owner->type)
        {
        case bp_none:
@@ -6956,7 +6979,9 @@ breakpoint_auto_delete (bpstat bs)
   struct breakpoint *b, *temp;
 
   for (; bs; bs = bs->next)
-    if (bs->breakpoint_at && bs->breakpoint_at->owner->disposition == disp_del
+    if (bs->breakpoint_at 
+       && bs->breakpoint_at->owner
+       && bs->breakpoint_at->owner->disposition == disp_del
        && bs->stop)
       delete_breakpoint (bs->breakpoint_at->owner);
 
@@ -7004,6 +7029,9 @@ update_global_location_list (void)
       /* Tells if 'loc' is found amoung the new locations.  If not, we
         have to free it.  */
       int found_object = 0;
+      /* Tells if the location should remain inserted in the target.  */
+      int keep_in_target = 0;
+      int removed = 0;
       for (loc2 = bp_location_chain; loc2; loc2 = loc2->global_next)
        if (loc2 == loc)
          {
@@ -7020,13 +7048,12 @@ update_global_location_list (void)
       if (loc->inserted)
        {
          /* If the location is inserted now, we might have to remove it.  */
-         int keep = 0;
 
          if (found_object && should_be_inserted (loc))
            {
              /* The location is still present in the location list, and still
                 should be inserted.  Don't do anything.  */
-             keep = 1;
+             keep_in_target = 1;
            }
          else
            {
@@ -7044,30 +7071,53 @@ update_global_location_list (void)
                      {           
                        loc2->inserted = 1;
                        loc2->target_info = loc->target_info;
-                       keep = 1;
+                       keep_in_target = 1;
                        break;
                      }
                  }
            }
 
-         if (!keep)
-           if (remove_breakpoint (loc, mark_uninserted))
-             {
-               /* This is just about all we can do.  We could keep this
-                  location on the global list, and try to remove it next
-                  time, but there's no particular reason why we will
-                  succeed next time.  
-
-                  Note that at this point, loc->owner is still valid,
-                  as delete_breakpoint frees the breakpoint only
-                  after calling us.  */
-               printf_filtered (_("warning: Error removing breakpoint %d\n"), 
-                                loc->owner->number);
-             }
+         if (!keep_in_target)
+           {
+             if (remove_breakpoint (loc, mark_uninserted))
+               {
+                 /* This is just about all we can do.  We could keep this
+                    location on the global list, and try to remove it next
+                    time, but there's no particular reason why we will
+                    succeed next time.  
+                    
+                    Note that at this point, loc->owner is still valid,
+                    as delete_breakpoint frees the breakpoint only
+                    after calling us.  */
+                 printf_filtered (_("warning: Error removing breakpoint %d\n"), 
+                                  loc->owner->number);
+               }
+             removed = 1;
+           }
        }
 
       if (!found_object)
-       free_bp_location (loc);
+       {             
+         if (removed)
+           {
+             /* This location was removed from the targets.  In non-stop mode,
+                a race condition is possible where we've removed a breakpoint,
+                but stop events for that breakpoint are already queued and will
+                arrive later.  To suppress spurious SIGTRAPs reported to user,
+                we keep this breakpoint location for a bit, and will retire it
+                after we see 3 * thread_count events.
+                The theory here is that reporting of events should, 
+                "on the average", be fair, so after that many event we'll see
+                events from all threads that have anything of interest, and no
+                longer need to keep this breakpoint.  This is just a 
+                heuristic, but if it's wrong, we'll report unexpected SIGTRAP,
+                which is usability issue, but not a correctness problem.  */     
+             loc->events_till_retirement = 3 * (thread_count () + 1);
+             loc->owner = NULL;
+           }
+
+         free_bp_location (loc);
+       }
     }
     
   ALL_BREAKPOINTS (b)
@@ -7079,6 +7129,21 @@ update_global_location_list (void)
     insert_breakpoint_locations ();
 }
 
+void
+breakpoint_retire_moribund (void)
+{
+  struct bp_location *loc;
+  int ix;
+
+  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+    if (--(loc->events_till_retirement) == 0)
+      {
+       free_bp_location (loc);
+       VEC_unordered_remove (bp_location_p, moribund_locations, ix);
+       --ix;
+      }
+}
+
 static void
 update_global_location_list_nothrow (void)
 {

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 80544e4dedd698afc4a19cade6d5425f82851879..2c98d64e8bc6d01902ee3e949d3d2dbbfb3a8cf3 100644 (file)
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -298,6 +298,17 @@ struct bp_location
 
   /* Similarly, for the breakpoint at an overlay's LMA, if necessary.  */
   struct bp_target_info overlay_target_info;
+
+  /* In a non-stop mode, it's possible that we delete a breakpoint,
+     but as we do that, some still running thread hits that breakpoint.
+     For that reason, we need to keep locations belonging to deleted
+     breakpoints for a bit, so that don't report unexpected SIGTRAP.
+     We can't keep such locations forever, so we use a heuristic --
+     after we process certain number of inferior events since
+     breakpoint was deleted, we retire all locations of that breakpoint.
+     This variable keeps a number of events still to go, when
+     it becomes 0 this location is retired.  */
+  int events_till_retirement;
 };
 
 /* This structure is a collection of function pointers that, if available,
@@ -865,4 +876,9 @@ void breakpoint_restore_shadows (gdb_byte *buf, ULONGEST memaddr,
 
 extern int breakpoints_always_inserted_mode (void);
 
+/* Called each time new event from target is processed.
+   Retires previously deleted breakpoint locations that
+   in our opinion won't ever trigger.  */
+extern void breakpoint_retire_moribund (void);
+
 #endif /* !defined (BREAKPOINT_H) */

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 4440417674dbd4177307aa4ec2ba30a8ef05c4ec..9ce27538fb7a634abc522c9d6b00524545970901 100644 (file)
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -118,6 +118,8 @@ extern struct thread_info *find_thread_pid (ptid_t ptid);
 typedef int (*thread_callback_func) (struct thread_info *, void *);
 extern struct thread_info *iterate_over_threads (thread_callback_func, void *);
 
+extern int thread_count (void);
+
 /* infrun context switch: save the debugger state for the given thread.  */
 extern void save_infrun_state (ptid_t ptid,
                               CORE_ADDR prev_pc,

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 73b92be5ae248fb645c4036d734034cf0c10f598..2c4ebc8fc4403d0f5e99cd31dc624e37da0ad983 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1705,6 +1705,8 @@ handle_inferior_event (struct execution_control_state *ecs)
   int stopped_by_watchpoint;
   int stepped_after_stopped_by_watchpoint = 0;
 
+  breakpoint_retire_moribund ();
+
   /* Cache the last pid/waitstatus. */
   target_last_wait_ptid = ecs->ptid;
   target_last_waitstatus = *ecs->wp;

diff --git a/gdb/thread.c b/gdb/thread.c
index 80d745dc306ad9d5a1eb03e65627a16d1bb1bfbe..5b1b563fa14247a4618282e43fec0601ccf8b5e6 100644 (file)
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -229,6 +229,18 @@ iterate_over_threads (int (*callback) (struct thread_info *, void *),
   return NULL;
 }
 
+int
+thread_count (void)
+{
+  int result = 0;
+  struct thread_info *tp;
+
+  for (tp = thread_list; tp; tp = tp->next)
+    ++result;
+
+  return result;  
+}
+
 int
 valid_thread_id (int num)
 {