Re: STEP_SKIPS_DELAY question, sort of

[Orjan Friberg <orjan.friberg@axis.com>]

Andrew Cagney wrote:
> 
> Can a simple, separate, more explicit logic like:
>     if (we just did a step and STEP_SKIPS_DELAY (pc))
>       set up for another step
>       return;
> work?  The [handle_inferior_event patch snipped] was nested within other 
> logic and that's not good from a readability / maintainability point of 
> view.

Now that I've done a lot more testing, I'm picking this up again.  (Below is 
just the infrun.c part; gdbarch.sh obviously needs a patch, and mips-tdep.c 
needs to have its current STEP_SKIPS_DELAY implementation converted - I'll post 
a complete patch if this part is approved of.)

A few things:
* I'm not sure how to reliably detect the situation "stepping off a breakpoint" 
in handle_inferior_event.  I used stop_signal == TARGET_SIGNAL_TRAP && 
trap_expected && currently_stepping (ecs)); could that be too inclusive?.
* Distinguishing between "step" and "continue" (using step_range_end) is not 
necessary for it to work, but explicitly returning in the "continue" case is 
making things a bit clearer.
* As the comment suggests, in the "step" case we don't want to preclude the 
stop_after_trap check - I assume that whatever is in the delay slot could 
potentially correspond to a single line of code (if nothing else, then at least 
an asm("...") construct.)

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.177
diff -u -r1.177 infrun.c
--- infrun.c    13 Sep 2004 18:26:28 -0000      1.177
+++ infrun.c    1 Oct 2004 11:22:42 -0000
@@ -721,17 +721,12 @@

        if (read_pc () == stop_pc && breakpoint_here_p (read_pc ()))
         oneproc = 1;
-
-#ifndef STEP_SKIPS_DELAY
-#define STEP_SKIPS_DELAY(pc) (0)
-#define STEP_SKIPS_DELAY_P (0)
-#endif
-      /* Check breakpoint_here_p first, because breakpoint_here_p is fast
-         (it just checks internal GDB data structures) and STEP_SKIPS_DELAY
-         is slow (it needs to read memory from the target).  */
-      if (STEP_SKIPS_DELAY_P
-         && breakpoint_here_p (read_pc () + 4)
-         && STEP_SKIPS_DELAY (read_pc ()))
+
+      /* If we stepped into something that needs to be stepped again before
+        before re-inserting the breakpoint, then do so.  */
+      else if (gdbarch_single_step_through_delay_p (current_gdbarch)
+              && gdbarch_single_step_through_delay (current_gdbarch,
+                                                    get_current_frame ()))
         oneproc = 1;
      }
    else
@@ -1793,6 +1788,35 @@
    stopped_by_random_signal = 0;
    breakpoints_failed = 0;

+  if (gdbarch_single_step_through_delay_p (current_gdbarch)
+      && stop_signal == TARGET_SIGNAL_TRAP && trap_expected
+      && currently_stepping (ecs))
+    {
+      /* We are in the process of stepping off a breakpoint.  If we stepped
+        into something that needs to be stepped again before re-inserting
+        the breakpoint, then do so.  */
+      int step_through_delay
+       = gdbarch_single_step_through_delay (current_gdbarch,
+                                            get_current_frame ());
+      if (step_range_end == 0 && step_through_delay)
+       {
+         /* The user issued a continue when stopped at a breakpoint.
+            Set up for another trap and get out of here.  */
+         ecs->another_trap = 1;
+         keep_going (ecs);
+         return;
+       }
+      else if (step_through_delay)
+       {
+         /* The user issued a step when stopped at a breakpoint.
+            Maybe we should stop, maybe we should not - the delay slot
+            *might* correspond to a line of source.  In any case, don't decide
+            that here, just set ecs->another_trap, making sure we
+            single-step again before breakpoints are re-inserted.  */
+         ecs->another_trap = 1;
+       }
+    }
+
    /* Look at the cause of the stop, and decide what to do.
       The alternatives are:
       1) break; to really stop and return to the debugger,

-- 
Orjan Friberg
Axis Communications