Re: Save the length of inserted breakpoints

[Michael Snyder <msnyder@redhat.com>]

Daniel Jacobowitz wrote:
> 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.

I haven't followed this discussion closely, so forgive me
if I'm recapitulating something that's already been said.

What about something like "void *target_data" in the breakpoint struct?
The target can add whatever it likes, and the core breakpoint code
doesn't need to know what it is.  If it's non-null when the bp is
freed, then the target should be given an opportunity to delete it.