[Lldb-commits] [PATCH] D18692: Fix a cornercase in breakpoint reporting

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 5 09:50:30 PDT 2016


labath added a comment.

In http://reviews.llvm.org/D18692#389529, @clayborg wrote:

> Looks fine. We should probably put the code that checks for a breakpoint at the thread PC into a function since it is done about 5 times in this function.


I'll prepare a follow-up to deduplicate the code here.

In http://reviews.llvm.org/D18692#389651, @jingham wrote:

> The only worry I have about this is
>
> - thread A is stopped at a breakpoint, and then we stop on thread B instead, then we report a breakpoint, great!
> - The user steps thread B, which we do by suspending thread B and stepping A.
> - Then A stops, and we report ANOTHER hit on thread B.
>
>   Your fix looks better than what was there previously.  If it is easy to check "last stop reason was this breakpoint hit, and this thread's temporary resume state is suspended, then don't report the hit, that would be more accurate.


I tried to do the following:

- have two threads: A and B
- hit a breakpoint on thread A
- switch to thread B and resume it
- hit a breakpoint on thread B

After this, thread A still reports as being stopped at a breakpoint, but:

- the breakpoint is not counted as hit twice
- the code I am changing is not even executed, as we back out of this function early due to `thread_sp->StopInfoIsUpToDate()` being true

So, while I could easily add the check for eStateSuspended, it seems that this situation is already handled elsewhere (probably by Thread::IsStillAtLastBreakpointHit, although I'm not that familiar with this code). Which makes sense as this situation could happen even for normal breakpoint hits, not just the ones I "simulate" here.

Does that make sense?


http://reviews.llvm.org/D18692





More information about the lldb-commits mailing list