Re: Save the length of inserted breakpoints

[Daniel Jacobowitz <drow@false.org>]

On Wed, Apr 12, 2006 at 08:37:22PM +0200, Mark Kettenis wrote:
> > Would a new "struct bp_target_info", defined and allocated centrally
> > for convenience, allay this concern?  [Conveniently I can do the bulk
> > of the changes for that with sed :-)]
> 
> Why hide things away if all you're going to need is a buffer and a
> length?  I've tried really hard to see why one would need more than
> that, but failed completely.
> 
> So I think we should have:
> 
> int target_insert_breakpoint(CORE_ADDR addr, gdb_byte *buf, int *size);
> int target_remove_breakpoint(CORE_ADDR addr, gdb_byte *buf, int size);

And then if you come up with a reason, you're going to need to hand
edit every one of these targets again.  It's not a bundle of fun.  Is
that really necessary?

You need an address because the address at which the breakpoint is
inserted may not match the requested address.  This happens in several
different places in the breakpoint infrastructure (I believe I counted
three disjoint hooks for it), but I am particularly looking at
BREAKPOINT_FROM_PC, which takes the PC by reference.  In the ARM case,
given 0x4001, it strips the low bit off and returns a two byte
breakpoint.  If we don't allow the target to save the
actually-inserted-at address, then it has to call BREAKPOINT_FROM_PC
again.  It feels much more robust to me to save this address when we
initially adjust it.  Here's where we inserted the breakpoint, so
that's where we should remove it from.

I can think of plenty of other places where another constant might
be useful.  You might want to record which hardware breakpoint
registers were used, for instance, instead of digging around
to figure out which ones to clear.  Adding a new member to
"struct bp_target" for that would be easy.

-- 
Daniel Jacobowitz
CodeSourcery