PATCH for Review: Fix Rendezvous breakpoint to only be set once, resolve addr in BreakpointLocationList::FindByAddress

This should fix the issue where multiple Rendezvous breakpoints could get set, get confused about the Rendezvous state, and then things head south.

Also resolve the address in BreakpointLocationList FindByAddress (thanks Jim for that pointer).

All the Linux tests run fine with this patch. Please let me know if it’s ok to commit. Thanks.
-Mike

http://llvm-reviews.chandlerc.com/D1145

Files:
source/Breakpoint/BreakpointLocationList.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h

Index: source/Breakpoint/BreakpointLocationList.cpp

D1145.1.patch (3.16 KB)

You might think about storing the breakpoint ID instead of a shared pointer to the breakpoint.

Can I ask why? There should only be one of these I believe.

I think ThreadPlanStepRange.cpp does something similar with
m_next_branch_bp_sp. Does that make sense to switch to IDs as well?

Thanks Greg.

You might think about storing the breakpoint ID instead of a shared pointer to the breakpoint.

Can I ask why? There should only be one of these I believe.

It really shouldn't matter really. Just to make sure no strong reference to a breakpoint stays around longer than required.

I think ThreadPlanStepRange.cpp does something similar with m_next_branch_bp_sp. Does that make sense to switch to IDs as well?

I can see the thread plans doing this because they will be accessing the breakpoint as soon as you stop. For the dynamic loaders, we tend to create the breakpoint and then the callback will be called automatically, so we aren't digging up the breakpoint each time we hit it.

This doesn't need to be done, this is just out the MacOSX dynamic loader does it.

Greg

Got it - I just wanted to understand the reasoning. I've switched over,
re-ran the tests, and posted the new patch here. Please let me know if this
looks ok when you get a chance and I'll commit.

http://llvm-reviews.chandlerc.com/D1145

Thanks for info Greg.
-Mike

Looks good.