infrun.c:handle_inferior_event: Rework random signal checks.
authorPedro Alves <palves@redhat.com>
Thu, 14 Nov 2013 19:43:26 +0000 (19:43 +0000)
committerPedro Alves <palves@redhat.com>
Thu, 14 Nov 2013 19:50:51 +0000 (19:50 +0000)
Looking at the current random signal checks:

  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
    random_signal
      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
   GDB_SIGNAL_TRAP)
   != BPSTAT_SIGNAL_NO)
  || stopped_by_watchpoint
  || ecs->event_thread->control.trap_expected
  || (ecs->event_thread->control.step_range_end
      && (ecs->event_thread->control.step_resume_breakpoint
  == NULL)));
  else
    {
      enum bpstat_signal_value sval;

      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
     ecs->event_thread->suspend.stop_signal);
      random_signal = (sval == BPSTAT_SIGNAL_NO);

      if (sval == BPSTAT_SIGNAL_HIDE)
ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
    }

We can observe:

  - the stepping checks bit:

          ...
  || ecs->event_thread->control.trap_expected
  || (ecs->event_thread->control.step_range_end
      && (ecs->event_thread->control.step_resume_breakpoint
  == NULL)));
          ...

    is just like currently_stepping:

     static int
     currently_stepping (struct thread_info *tp)
     {
       return ((tp->control.step_range_end
                && tp->control.step_resume_breakpoint == NULL)
               || tp->control.trap_expected
               || bpstat_should_step ());
     }

    except it misses the bpstat_should_step check (***).

    It's not really necessary to check bpstat_should_step in the
    random signal tests, because software watchpoints always end up in
    the bpstat list anyway, which means bpstat_explains_signal with
    GDB_SIGNAL_TRAP always returns at least BPSSTAT_SIGNAL_HIDE, but I
    think the code is clearer if we reuse currently_stepping.

    *** - bpstat_should_step checks to see if there's any software
    watchpoint in the breakpoint list, because we need to force the
    target to single-step all the way, to evaluate the watchpoint's
    value at each step.

  - we never hide GDB_SIGNAL_TRAP, even if the bpstat returns
    BPSTAT_SIGNAL_HIDE, which is actually the default for all
    breakpoints.  If we make the default be BPSTAT_SIGNAL_PASS, then
    we can merge the two bpstat_explains_signal paths.

gdb/
2013-11-14  Pedro Alves  <palves@redhat.com>

* breakpoint.c (bpstat_explains_signal) <Moribund locations>:
Return BPSTAT_SIGNAL_PASS instead of BPSTAT_SIGNAL_HIDE.
(explains_signal_watchpoint): Return BPSTAT_SIGNAL_PASS instead of
BPSTAT_SIGNAL_HIDE.
(base_breakpoint_explains_signal): Return BPSTAT_SIGNAL_PASS
instead of BPSTAT_SIGNAL_HIDE.
* infrun.c (handle_inferior_event): Rework random signal checks.

gdb/ChangeLog
gdb/breakpoint.c
gdb/infrun.c

index 48a57d86334afb3394c83f6a23b47824c1d7f011..bb09ebe90d1904637c1cd6fb2b6714df1dd143ee 100644 (file)
@@ -1,3 +1,13 @@
+2013-11-14  Pedro Alves  <palves@redhat.com>
+
+       * breakpoint.c (bpstat_explains_signal) <Moribund locations>:
+       Return BPSTAT_SIGNAL_PASS instead of BPSTAT_SIGNAL_HIDE.
+       (explains_signal_watchpoint): Return BPSTAT_SIGNAL_PASS instead of
+       BPSTAT_SIGNAL_HIDE.
+       (base_breakpoint_explains_signal): Return BPSTAT_SIGNAL_PASS
+       instead of BPSTAT_SIGNAL_HIDE.
+       * infrun.c (handle_inferior_event): Rework random signal checks.
+
 2013-11-14  Pedro Alves  <palves@redhat.com>
 
        * infrun.c (struct execution_control_state): Remove
index 5bfed9d14ec0dd1eabfbf90a81851042593f9799..e8ea0c30a0d9d80f339a73210a0ee8ee43781bec 100644 (file)
@@ -4238,7 +4238,7 @@ bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
          /* A moribund location can never explain a signal other than
             GDB_SIGNAL_TRAP.  */
          if (sig == GDB_SIGNAL_TRAP)
-           newval = BPSTAT_SIGNAL_HIDE;
+           newval = BPSTAT_SIGNAL_PASS;
          else
            newval = BPSTAT_SIGNAL_NO;
        }
@@ -10779,7 +10779,7 @@ explains_signal_watchpoint (struct breakpoint *b, enum gdb_signal sig)
   if (b->type == bp_watchpoint && sig != GDB_SIGNAL_TRAP)
     return BPSTAT_SIGNAL_NO;
 
-  return BPSTAT_SIGNAL_HIDE;
+  return BPSTAT_SIGNAL_PASS;
 }
 
 /* The breakpoint_ops structure to be used in hardware watchpoints.  */
@@ -12890,7 +12890,7 @@ base_breakpoint_decode_linespec (struct breakpoint *b, char **s,
 static enum bpstat_signal_value
 base_breakpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
-  return BPSTAT_SIGNAL_HIDE;
+  return BPSTAT_SIGNAL_PASS;
 }
 
 /* The default "after_condition_true" method.  */
index f37d881857c6b08e8383d31f8554d656a2964f9c..8eb2ddd525101fb2b8907fba975a439504dd145b 100644 (file)
@@ -4234,7 +4234,7 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
                        "infrun: no user watchpoint explains "
                        "watchpoint SIGTRAP, ignoring\n");
 
-  /* NOTE: cagney/2003-03-29: These two checks for a random signal
+  /* NOTE: cagney/2003-03-29: These 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
      comment, that went with the test, read:
@@ -4254,27 +4254,22 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
      be necessary for call dummies on a non-executable stack on
      SPARC.  */
 
-  if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
-    random_signal
-      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-                                  GDB_SIGNAL_TRAP)
-          != BPSTAT_SIGNAL_NO)
-         || stopped_by_watchpoint
-         || ecs->event_thread->control.trap_expected
-         || (ecs->event_thread->control.step_range_end
-             && (ecs->event_thread->control.step_resume_breakpoint
-                 == NULL)));
-  else
-    {
-      enum bpstat_signal_value sval;
+  /* See if the breakpoints module can explain the signal.  */
+  sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+                                ecs->event_thread->suspend.stop_signal);
+  random_signal = (sval == BPSTAT_SIGNAL_NO);
+
+  /* If not, perhaps stepping/nexting can.  */
+  if (random_signal)
+    random_signal = !(ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+                     && currently_stepping (ecs->event_thread));
 
-      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
-                                    ecs->event_thread->suspend.stop_signal);
-      random_signal = (sval == BPSTAT_SIGNAL_NO);
+  /* No?  Perhaps we got a moribund watchpoint.  */
+  if (random_signal)
+    random_signal = !stopped_by_watchpoint;
 
-      if (sval == BPSTAT_SIGNAL_HIDE)
-       ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-    }
+  if (sval == BPSTAT_SIGNAL_HIDE)
+    ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
 
   /* For the program's own signals, act according to
      the signal handling tables.  */