Re: [PATCH] Program Breakpoints

[Ross Morley <ross@tensilica.com>]

Pedro Alves wrote:

>Before looking at the code, I'd like to see the interaction of
>program breakpoints with decr_pc_after_break adjustment formalized.
>I did a quick skim and couldn't find it handled.
>  
>
>E.g, if you were to implement support for this on x86 gdbserver,
>assuming int3 traps, it appears to me that the only option is
>for the target to always rewind the pc before reporting to GDB, and
>for GDB to never adjust it itself, even for regular breakpoint hits,
>otherwise, e.g., consecutive breakpoints will be mishandled.  This
>also suggests that there has to be prior negotiation (qSupported) to
>enable the support.  Has this been considered?
>  
>
I have thought about this. I understand decr_pc_after_break is for archs
that have already incremented the PC after hitting a trap, so GDB needs
to decrement it back to the trap in order to replace it with the original
code to step over it.

No special handling is needed for program breakpoints because:

- decr_pc_after_break only applies to breakpoints inserted by GDB
  and therefore known to GDB (adjust_pc_after_break in infrun.c
  appears to only adjust the PC if it hit a software break known
  to GDB). Program breakpoints are by definition not those, and
  their special handling is not applied to those. If a target were
  to adjust the PC for a program breakpoint, it would then have to
  report a non-zero size in STOPPED_BY_TRAP_INSTRUCTION (see next
  bullet), but that would not be the normal handling for such archs.

- decr_pc_after_break archs report size==0 in their implementation
  of STOPPED_BY_TRAP_INSTRUCTION(&size). That means the PC does not
  need adjustment to step over a program breakpoint. GDB will report
  it stopped for a program breakpoint (if it wasn't in the breakpoint
  table) but will not do anything special on resume. Perhaps I should
  make that clearer in the comment in target.h on that macro, by
  explicit reference to decr_pc_after_break.

>(A small request: please include the -p switch in your `cvs diff'
>commands, or add it to .cvsrc.  I use -Nurp myself.)
>
>  
>
OK.

Thanks,
Ross