Re: STEP_SKIPS_DELAY question, sort of

[Andrew Cagney <cagney@gnu.org>]

Sorry, picking up old threads.

Orjan Friberg wrote:
> 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?.

I think this is sufficient.  Can you add something mentioning "stepping 
off a breakpoint" to the comment - that makes the intent clear.

> * 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.

Always a good idea.

Andrew


> * 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,
>