Re: Display of read/access watchpoints when HAVE_NONSTEPPABLE_WATCHPOINT

[Orjan Friberg <orjan.friberg@axis.com>]

Eli Zaretskii wrote:
>>Date: Thu, 15 Apr 2004 15:23:59 +0200
>>From: Orjan Friberg <orjan.friberg@axis.com>
>>
>>The test program (repeated below) *does* work for i386 (though I didn't 
>>say that), because it also stops when foo is written (thus updating the 
>>value of foo when watchpoint_check is called), so by the time it stops 
>>when the second read happens the value hasn't changed since the last 
>>time and GDB decides it's a valid hit.
> 
> 
> Right; sorry I forgot about that.  It's been a long time since I
> hacked that part of GDB.

Apologies; I didn't mean to imply that that was obvious.  (I certainly 
didn't know that was why it worked on the i386 when I posted the example 
in the first place.)

>>1. Add a check if the target cannot set "pure" read watchpoints to the 
>>b->type == bp_read_watchpoint check at WP_VALUE_CHANGED (my 
>>interpretation of Eli's suggestion).
> 
> 
> On balance, this is probably the best solution, although it's not
> quite clean.  The ability to set read watchpoints would be part of the
> architecture vector, right?

Yes, I think so.  Also, this seems like the least intrusive change. 
(Nevertheless, since you were kind enough to comment on the other 
suggestions, I'll continue the discussion of them.)

>>2. Somehow don't update the value in watchpoint_check when it's a false 
>>hit.  (Then the b->type == bp_read_watchpoint check at WP_VALUE_CHANGED 
>>isn't needed.)
> 
> 
> Wouldn't this reintroduce the bug that I was trying to solve back
> then?  That is, will GDB still DTRT when both rwatch and watch are set
> at the same variable?

You're right; this won't work.  I failed to account for the fact that 
the code *relies* on the i386 stopping (and updating the value) when 
writing to an rwatched variable.

In addition, I'm not sure how we'd detect a false hit for the i386 (the 
"somehow" part).  For my upcoming target, I have bits telling me if it 
was a read or write that caused the exception.

>>3. Add some distinction between "wanted watchpoint type" and "actual 
>>watchpoint type".
> 
> 
> Any specific ideas how to do that?  For that matter, what is ``wanted
> watchpoint type'', and how can we decide that?

The idea was that when inserting a watchpoint, target_insert_watchpoint 
would return the type of watchpoint that was actually inserted.  So, for 
the i386 an insertion of a read watchpoint would return an access 
watchpoint (if I've understood correctly).  The breakpoint struct would 
then have an additional field storing this returned value.

In bpstat_stop_status, the code would then be something like

   case WP_VALUE_CHANGED:
     if (b->type == bp_read_watchpoint
         && b->actual_type == bp_access_watchpoint)
       /* Don't stop.  */
       ...


I'll have a look at implementing the first suggestion.  Thanks.

-- 
Orjan Friberg
Axis Communications