* infrun.c (handle_inferior_event): Hardware hatchpoint traps are

	never random signals.
	* breakpoint.c (update_global_location_list): Always delete
	immediately delete hardware watchpoint locations and other
	locations whose target address isn't meaningful.  Update comment
	explaining the hazard of moribund locations.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ea55fb184205217757f2476dc6bae221a7d8d6da..e2e2e1e274581fa9c5db61b41c924a536f16550f 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2009-11-20  Pedro Alves  <pedro@codesourcery.com>
+
+       * infrun.c (handle_inferior_event): Hardware hatchpoint traps are
+       never random signals.
+       * breakpoint.c (update_global_location_list): Always delete
+       immediately delete hardware watchpoint locations and other
+       locations whose target address isn't meaningful.  Update comment
+       explaining the hazard of moribund locations.
+
 2009-11-19  Joel Brobecker  <brobecker@adacore.com>
 
        * ada-lang.c (discrete_type_p): TYPE_CODE_BOOL is also a discrete type.

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7968068a4afa0dda4e95e9abcc6a02671eaec505..493b1f4843fc1129b9c8b0fc104e856318928247 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8300,20 +8300,55 @@ update_global_location_list (int should_insert)
 
       if (!found_object)
        {
-         if (removed && non_stop)
+         if (removed && non_stop
+             && breakpoint_address_is_meaningful (old_loc->owner)
+             && !is_hardware_watchpoint (old_loc->owner))
            {
-             /* 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.  */
+             /* This location was removed from the target.  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.
+                We apply an heuristic to be able to distinguish such
+                SIGTRAPs from other random SIGTRAPs: we keep this
+                breakpoint location for a bit, and will retire it
+                after we see some number of events.  The theory here
+                is that reporting of events should, "on the average",
+                be fair, so after a while we'll see events from all
+                threads that have anything of interest, and no longer
+                need to keep this breakpoint location around.  We
+                don't hold locations forever so to reduce chances of
+                mistaking a non-breakpoint SIGTRAP for a breakpoint
+                SIGTRAP.
+
+                The heuristic failing can be disastrous on
+                decr_pc_after_break targets.
+
+                On decr_pc_after_break targets, like e.g., x86-linux,
+                if we fail to recognize a late breakpoint SIGTRAP,
+                because events_till_retirement has reached 0 too
+                soon, we'll fail to do the PC adjustment, and report
+                a random SIGTRAP to the user.  When the user resumes
+                the inferior, it will most likely immediately crash
+                with SIGILL/SIGBUS/SEGSEGV, or worse, get silently
+                corrupted, because of being resumed e.g., in the
+                middle of a multi-byte instruction, or skipped a
+                one-byte instruction.  This was actually seen happen
+                on native x86-linux, and should be less rare on
+                targets that do not support new thread events, like
+                remote, due to the heuristic depending on
+                thread_count.
+
+                Mistaking a random SIGTRAP for a breakpoint trap
+                causes similar symptoms (PC adjustment applied when
+                it shouldn't), but then again, playing with SIGTRAPs
+                behind the debugger's back is asking for trouble.
+
+                Since hardware watchpoint traps are always
+                distinguishable from other traps, so we don't need to
+                apply keep hardware watchpoint moribund locations
+                around.  We simply always ignore hardware watchpoint
+                traps we can no longer explain.  */
+
              old_loc->events_till_retirement = 3 * (thread_count () + 1);
              old_loc->owner = NULL;
 

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3c1716733525e9bc1418d8fdbc3ca03d56809a92..cb5278c107929138d2f6753f701b0dbaf20aeed3 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3593,6 +3593,21 @@ targets should add new threads to the thread list themselves in non-stop mode.")
         function.  */
       stop_print_frame = 1;
 
+      /* This is where we handle "moribund" watchpoints.  Unlike
+        software breakpoints traps, hardware watchpoint traps are
+        always distinguishable from random traps.  If no high-level
+        watchpoint is associated with the reported stop data address
+        anymore, then the bpstat does not explain the signal ---
+        simply make sure to ignore it if `stopped_by_watchpoint' is
+        set.  */
+
+      if (debug_infrun
+         && ecs->event_thread->stop_signal == TARGET_SIGNAL_TRAP
+         && !bpstat_explains_signal (ecs->event_thread->stop_bpstat)
+         && stopped_by_watchpoint)
+       fprintf_unfiltered (gdb_stdlog, "\
+infrun: no user watchpoint explains watchpoint SIGTRAP, ignoring\n");
+
       /* NOTE: cagney/2003-03-29: These two checks for a random signal
          at one stage in the past included checks for an inferior
          function call's call dummy's return breakpoint.  The original
@@ -3616,6 +3631,7 @@ targets should add new threads to the thread list themselves in non-stop mode.")
       if (ecs->event_thread->stop_signal == TARGET_SIGNAL_TRAP)
        ecs->random_signal
          = !(bpstat_explains_signal (ecs->event_thread->stop_bpstat)
+             || stopped_by_watchpoint
              || ecs->event_thread->trap_expected
              || (ecs->event_thread->step_range_end
                  && ecs->event_thread->step_resume_breakpoint == NULL));