Re: [rfa] insert_hw_breakpoint

[Andrew Cagney <ac131313@ges.redhat.com>]

> On Aug 12,  4:04pm, Grace Sainsbury wrote:
> 
> 
>> -  len = strlen (shadow);
>> -  len = len ? len : 1;
>> +  BREAKPOINT_FROM_PC (&addr, &len);
>> +  
>>    if (remote_protocol_Z[Z_PACKET_HARDWARE_BP].support == PACKET_DISABLE)
>>      error ("Can't set hardware breakpoint without the '%s' (%s) packet\n",
>>  	   remote_protocol_Z[Z_PACKET_HARDWARE_BP].name,
>> @@ -5027,7 +5027,8 @@
>>    char *buf = alloca (rs->remote_packet_size);
>>    char *p = buf;
>>    
>> -  len = sizeof (shadow);
>> +  BREAKPOINT_FROM_PC (&addr, &len);
>> + 
>>    if (remote_protocol_Z[Z_PACKET_HARDWARE_BP].support == PACKET_DISABLE)
>>      error ("Can't clear hardware breakpoint without the '%s' (%s) packet\n",
>>  	   remote_protocol_Z[Z_PACKET_HARDWARE_BP].name,

Grace,

Assuming it still works with the (RedBoot?) stub you're using then yes 
ok.  (The stub must have been ignoring the length as that could be garbage).

> Do stubs actually care about this length field for the packets under
> consideration?

The length indicates the number of bytes to consider -- in theory this 
allows GDB to set h/w breakpoint across an address range.  I suspect, 
though, that all existing stubs ignore the length since GDB hasn't been 
doing anything sensible with it.

> I agree that the use of strlen() and sizeof() seem dubious if not
> outright wrong, but, unfortunately, not all architectures can define
> a sensible BREAKPOINT_FROM_PC. (*)  I would like to see gdb relying less
> on BREAKPOINT_FROM_PC, not more.

The address should definitly be run through BREAKPOINT_FROM_PC().  On 
Arm, MIPS16, sh5, ...it will strip out any least significant mode bit so 
that the stub is handed a proper address.

As for the length, the choices would be 1, instruction size or 
breakpoint size (returned by BREAKPOINT_FROM_PC).  Given breakpoint size 
is the size of the smallest instruction, it looks to be a good 
compromize. On the i386 it is one anyway, on Arm it would be 2/4 
depending on/indicating the mode of the cpu.

> (*) E.g, IA-64.  For IA-64, I did define a BREAKPOINT_FROM_PC, but I
> would have preferred not to.  The values returned by
> ia64_breakpoint_from_pc() are fairly arbitrary and are of absolutely
> no use for saving/restoring actual breakpoints.

I think that is a separate problem.

enjoy,
Andrew