This is the mail archive of the gdb-patches@sourceware.org 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: mips-tdep.c: Adjust breakpoints in branch delay slots


[Sorry about the mess -- the bloody MIPS mail server keeps failing today.]

On Thu, 24 May 2007, Daniel Jacobowitz wrote:

> > +/* Return non-zero if the ADDR instruction has a branch delay slot
> > +   (i.e. it is a jump or branch instruction).  This function was based
> > +   on mips32_next_pc().  */
> > +
> > +static int
> > +mips32_instruction_has_delay_slot (CORE_ADDR addr)
> > +{
> > +  gdb_byte buf[MIPS_INSN32_SIZE];
> > +  int status;
> > +
> > +  status = read_memory_nobpt (addr, buf, MIPS_INSN32_SIZE);
> > +  return !status && mips32_next_pc (addr) != addr + 4;
> > +}
> 
> That's not what you want.  mips32_next_pc calls read_register to
> figure out if the branch condition is true, and we don't have any idea
> yet (and might not be connected to a target).

 Oh yes, indeed -- I have not looked too deeply into this bit of the 
patch, sorry.

> > +  /* If a breakpoint is set on the instruction in a branch delay slot,
> > +     GDB gets confused.  When the breakpoint is hit, the PC isn't on
> > +     the instruction in the branch delay slot, the PC will point to
> > +     the branch instruction.  Since the PC doesn't match any known
> > +     breakpoints, GDB reports a trap exception.
> 
> Just checking, but does this happen in a userspace environment like
> Linux too?

 Oh absolutely.  Actually the very complaint I quoted was taken from 
Linux.

> > +     There are two possible fixes for this problem.
> > +
> > +     1) When the breakpoint gets hit, see if the BD bit is set in the
> > +     Cause register (which indicates the last exception occurred in a
> > +     branch delay slot).  If the BD bit is set, fix the PC to point to
> > +     the instruction in the branch delay slot.
> > +
> > +     2) When the user sets the breakpoint, don't allow him to set the
> > +     breakpoint on the instruction in the branch delay slot.  Instead
> > +     move the breakpoint to the branch instruction (which will have
> > +     the same result).
> > +
> > +     The problem with the first solution is that if the user then
> > +     single-steps the processor, the branch instruction will get
> > +     skipped (since GDB thinks the PC is on the instruction in the
> > +     branch delay slot).
> > +
> > +     So, we'll use the second solution.  To do this we need to know if
> > +     the instruction we're trying to set the breakpoint on is in the
> > +     branch delay slot. */
> 
> Abstractly, which of these solutions would you say was better?  I bet
> we could make the first one work if we tried hard enough.  One way
> would be to fake the PC, but that's pretty gross.  Still, it might be
> possible if it was worthwhile.

 In principle on bare iron it should be doable, but the problem is, IIRC, 
the state of cp0.cause.bd is not propagated under Linux to userland in any 
way.  And at the moment I'm not sure how this would affect EJTAG debugging 
either.

 I do agree the first solution would be a bit better, because in reality 
(yes I did write such code myself!) you can have control flow which jumps 
into a branch delay slot under certain circumstances (and obviously at 
times executes the preceding branch normally as well).

> BTW, see mips_single_step_through_delay, which could probably be
> improved.  Doesn't its comment about MIPS16 having no delay slots
> disagree with this patch?  Which is right?

 There are both kinds of jumps/branches in MIPS16e:

- jal, jalr, jalx and jr do have a delay slot,

- branches and jalrc and jrc have no delay slot (the two latters are 
  MIPS16e additions over "traditional" MIPS16).

> > +      /* Make sure we don't scan back before the beginning of the
> > +         current function, since we may fetch constant data or MIPS32
> > +         insns that looks like a MIPS16 jump.  Of course we might do
> > +         that anyway if the compiler has moved constants inline. :-(  */
> 
> This can happen for MIPS32 too at the start of a function.  All in all
> I'm a little bit nervous about this adjustment, since it will fail
> very confusingly if you get unlucky with constant pool data.  That's
> why I was asking about the alternative solution above.

 Well, if Linux and EJTAG can be done at all, I might have a look into it.

> Oh, and I wholeheartedly agree with Mark about testcases.  It
> shouldn't be hard to add this to gdb.arch/.

 I have thought a general test case would be tough, but a MIPS-specific 
one should be easy indeed.

> Thanks for the patch!

 Credit goes to Chris!

  Maciej


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