[Lldb-commits] [PATCH] D13296: [LLDB] Fix watchpoint ignore feature for architectures with watchpoint_exceptions_received=before

[Jim Ingham via lldb-commits]

> On Oct 29, 2015, at 3:34 AM, Mohit Bhakkad <mohit.bhakkad at gmail.com> wrote:
> 
> mohit.bhakkad added a comment.
> 
> In http://reviews.llvm.org/D13296#265608, @jingham wrote:
> 
>> This change alters the timing for the handling of ignore counts for watchpoints.  The original implementation (and the way ignore counts work for breakpoints) is that the breakpoint's ignore count gets checked during the "synchronous" phase of breakpoint stop evaluation.  So it happens when the private stop event is received, and if the ignore count is not satisfied then a public event is not generated.
>> 
>> Your version will move the decision making to the async ShouldStop, i.e. when an event is getting pulled from the public event queue.  If these were breakpoints it would mean that the synchronous callbacks would still fire even if they fail their ignore count test.  Watchpoints don't currently have synchronous callbacks - and I'm not sure we would ever need to support that.  If you moved the adjustment code you do in PerformAction to the synchronous callback (Watchpoint::ShouldStop) that would keep breakpoints & watchpoints behaving symmetrically.  It's worth trying that, I am pretty sure that will be a safe place to do your little dance.  But if it isn't, then I don't mind having it in PerformAction provided you put a note to that effect somewhere obvious in Watchpoint.h.
> 
> 
> Hi @jingham, I tried to move adjustment code to ShouldStopSynchronous before we call ShouldStop, but its not getting stepped to next PC. Looks like threadplan are not active in that phase of code. Could you suggest anything wrong I may be doing or should I resubmit this patch by putting condition code before we print anything.

I think it is fine to do that latter.  The watchpoint code really needs an overhaul anyway, so taking the easy route here seems fine.  Just leave a comment somewhere explaining the behavior difference so when we get around to fixing it up we'll remember that it works this way.

Jim


> 
>> Note, if you end up going with the current patch, isn't quite right, however.  You need to move the check for the watchpoint ignore count up before printing out the old & new values.  An ignored watchpoint shouldn't print anything, it should be as if it never stopped.  Where you have the check, the old & new values will get printed even if the watchpoint is ignored.
> 
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D13296
> 
> 
>