[lldb-dev] Question about breakpoint hit counts

[jingham at apple.com]

If a thread hits a thread specific breakpoint that is not for that thread, you want that thread to say it stopped for no reason, not that it stopped for a reason that isn't actually going to cause it to stop.  For instance, suppose I had a thread specific breakpoint set on thread A, and both Thread A and Thread B hit that breakpoint.  It would be confusing if the stop reason for B was that breakpoint, since the breakpoint was not for that thread. The fact that it happens to be sitting on the breakpoint is irrelevant to the user. 

And it would similarly be confusing to say that because the breakpoint was for that thread, you knew that the breakpoint was actually going to stop.  Many other things could cause it not to stop.

The right thing to do is: if the breakpoint is for this thread, create a breakpoint stop info with no opinion about should_stop, and if not, create an empty stop info.

Both ProcessGDBRemote.cpp and StopInfoMachException.cpp do it this way.  

For some reason, PosixThreads.cpp does the right thing if the breakpoint is for that thread - creates one with no opinion about the should_stop if it IS for the thread, but if it isn't for that thread it creates a stop info with should stop set to false.  That latter bit seems wrong to me for the reason given in the first PP.  The commit that added the should_stop = false breakpoint for a not-for-this-breakpoint thread doesn't say why it was done that way, and Matt isn't working on lldb anymore so he probably doesn't remember...  

Somebody adventurous on the Linux side might want to try getting PosixThreads.cpp to do it the way the other code does, and see if that causes any fallout.

On the Windows side, you should do it the way ProcessGDBRemote.cpp does it.

Jim


> On Jan 14, 2015, at 1:28 PM, Zachary Turner <zturner at google.com> wrote:
> 
> When I hit a breakpoint on Windows, I'm doing something like this:
> 
>         BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
>         lldb::break_id_t break_id = LLDB_INVALID_BREAK_ID;
>         bool should_stop = true;
>         if (site)
>         {
>             should_stop = site->ValidForThisThread(stop_thread.get());
>             break_id = site->GetID();
>         }
> 
>         stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*stop_thread, break_id, should_stop);
>         stop_thread->SetStopInfo(stop_info);
> 
> When should_stop is true (which for now is basically always), this results in the breakpoint's hit count not increasing.  It seems this is because specifying a value for should_stop leads to m_should_stop_is_valid being initialized to true.  But in StopInfoBreakpoint::ShouldStopSynchronous(), we only bump the hit count if m_should_stop_is_valid is false.
> 
> I can fix this bug by using 
> 
> StopInfo::CreateStopReasonWithBreakpointSiteID(*stop_thread, break_id);
> 
> instead, and since m_stop_info_is_valid is false, and it bumps the hit count later.  But I'm not sure if this is the right thing to do, or if the behavior I'm seeing with specifying a value for should_stop on creation and the hit count not going up is a bug.
> 
> Can anyone explain this?
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev