This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: STEP_SKIPS_DELAY question, sort of


Andrew Cagney wrote:


Can this new mechanism somehow superseed STEP_SKIPS_DELAY - it seems to be the exact oposite but there could be common ground here.


[proceed patch snipped]

They both seem to be asking the question: "given PC and a list of breakpoints, should the inferior be h/w single-stepped?".  That would mean pushing the alternative:
  breakpoint_here_p (read_pc () - 2)
  breakpoint_here_p (read_pc () + 4)
calls into that architecture method.


Agreed. (STEP_SKIPS_IN_DELAY was just to have something to put in the patch.)

What about using the name STEP_SKIPS_DELAY for both, and introducing a DELAY_SIZE which would return a positive value (meaning the diff from the current pc to the delay slot) or a negative (meaning the diff from the delay slot to the instruction preceding it)? Or does the word "size" imply an absolute value?

If the:
>> breakpoint_here_p (read_pc () - 2)
and
>> breakpoint_here_p (read_pc () + 4)
logic is moved to the per-architecture STEP_SKIPS_DELAY I don't think DELAY_SIZE is needed.


I also think this needs a new macro name that better reflects what the test is doing. But I've no good ideas :-/ (SINGLE_STEP_THROUGH_DELAY (pc)?)

[handle_inferior_event patch snipped]

I'm just not sure how this bit of logic should fit in. I'm guessing its the second half of the state m/c sequence:

1. step off breakpoint at `PC'
2. step through delay


Unless I missed something on the way, the procedure when doing a continue from a
breakpoint that sits on the branch instruction is this:

1. proceed decides it needs to step once before continuing (since read_pc () == stop_pc && breakpoint_here_p (read_pc ()))
2. resume is called, with step = 1
3. target is single-stepped
4. handle_inferior_event is called (at which point we're stopped in the delay slot)

yes (step off breakpoint at `PC')


It is at this point we need to single-step again (before inserting breakpoints again), so I set ecs->another_trap. Then:

5. keep_going is called, and since ecs->anther_trap is set, it doesn't call insert_breakpoints.
6. resume is called again, with step = 1
7. target is single-stepped
8. handle_inferior_event is called again (but doesn't set ecs->another_trap this time)
9. keep_going is called, and inserts the breakpoints again

ok (step through delay)


I can't say where would be a better place to put the decision of whether to single-step again. Any suggestions?

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.


Andrew



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]