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 <ztur...@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@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
_______________________________________________
lldb-dev mailing list
lldb-dev@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
