Re: [PATCH 1/3] Implement "to_stopped_by_hw_breakpoint" for x86 debug registers.

[John Baldwin <jhb at FreeBSD dot org>]

On 3/2/18 12:19 PM, Pedro Alves wrote:
> Hi John,
> 
> This LGTM, with the nits below addressed.
> 
> On 02/28/2018 01:46 AM, John Baldwin wrote:
>> This change should be a no-op since a target
>> still needs to implement the "to_supports_stopped_by_hw_breakpoint"
>> method before this function is used.
> 
> I think it'd be good to dd something like this as a comment somewhere.
> Maybe next to where t->to_stopped_by_hw_breakpoint is set?
> 
> Because while looking at the patch, I didn't notice that comment in
> the git log, and it took me a bit to realize that this does not affect
> all x86 ports as is.

Ok, will do.  The comment is also useful to help clarify the difference
with 'to_stopped_by_watchpoint' which doesn't have a matching
'to_supports_stopped_by_watchpoint'.  (I found this difference a bit
confusing myself.)

>> +/* Return non-zero if the inferior has some breakpoint that triggered.
>> +   Otherwise return zero.  */
>> +
>> +int
>> +x86_dr_stopped_by_breakpoint (struct x86_debug_reg_state *state)
>> +{
> 
> I was also slightly confused by this until I realized that you
> meant _hardware_ breakpoint.  Can you rename this to
> x86_dr_stopped_by_hw_breakpoint, and update the comment too?

Yes.  I had originally modeled this on the 'stopped_by_watchpoint' naming
and had just assumed 'hw_' since x86 debug registers aren't used for
software breakpoints, but I agree that the more explicit name is clearer.

-- 
John Baldwin