This looks fine. Your description of the patch doesn't use terminology in quite the same way as lldb does. This isn't an MI event, it is just a regular lldb event. There really aren't MI events. That's not terribly important, except if you were to think that these are only used by MI...
Also, just to be clear, lldb breakpoints are either never or always pending depending on how you look at it. In lldb itself we only call a breakpoint "pending" if it has no locations (i.e. the breakpoint specification hasn't yet matched any real location in the program.) Also, breakpoints never stop resolving themselves in lldb. So for instance, suppose you set a breakpoint by function name on Foo. If it exists in your main executable, then before you run it that location will be added to the breakpoint (and an eBreakpointEventTypeLocationsAdded event will be generated.) That location will have an address, but it will be the address of Foo in the binary, which may or may not be where it will get loaded. This is an interesting point in the life of the breakpoint, since once you have a location, resolved or not, you can put conditions and commands on it, and enable or disable it. In that sense, this is a more interesting event than "resolved". When you run your program and the location gets a real address in memory, a breakpoint site will be created and all the locations will bind to it - and with your patch a eBreakpointEventTypeLocationsResolved event will be generated for each location at that real address. Then if another shared library gets loaded which also has a function Foo, another location will be created, and bound to it's physical address, and you will now get two events, eBreakpointEventTypeLocationsAdded and eBreakpointEventTypeLocationsResolved. Etc. as more libraries are loaded. And of course, if the library is unloaded, the location's physical address will become invalid and the site will get destroyed. However, the location will stay around waiting for the library to get loaded again. Locations only go away when a changed version of a binary is loaded which causes the locations to change. It doesn't look like there is a "location unresolved" event, but if you are reporting location resolution, you might want to report unresolution or whatever is a good word for that... Anyway, in itself this change looks fine to me, but I just wanted to explain how the system works in case you also want to handle location changed and/or unresolved as well in the MI. Jim > On Apr 6, 2015, at 3:25 PM, Chuck Ries <[email protected]> wrote: > > Hi abidh, jingham, > > This checkin sends an MI event when a module is loaded that causes a pending > breakpoint to bind to it's real address in the target. This allows > breakpoints to be set before the process is launched, and the target address > of the BP to be discovered when the module loads, prior to the breakpoint > being hit. > > http://reviews.llvm.org/D8847 > > Files: > source/Breakpoint/BreakpointLocation.cpp > tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp > > Index: source/Breakpoint/BreakpointLocation.cpp > =================================================================== > --- source/Breakpoint/BreakpointLocation.cpp > +++ source/Breakpoint/BreakpointLocation.cpp > @@ -536,6 +536,7 @@ > BreakpointLocation::SetBreakpointSite (BreakpointSiteSP& bp_site_sp) > { > m_bp_site_sp = bp_site_sp; > + SendBreakpointLocationChangedEvent > (eBreakpointEventTypeLocationsResolved); > return true; > } > > Index: tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp > =================================================================== > --- tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp > +++ tools/lldb-mi/MICmnLLDBDebuggerHandleEvents.cpp > @@ -243,6 +243,7 @@ > break; > case lldb::eBreakpointEventTypeLocationsResolved: > pEventType = "eBreakpointEventTypeLocationsResolved"; > + bOk = HandleEventSBBreakpointCmn(vEvent); > break; > case lldb::eBreakpointEventTypeEnabled: > pEventType = "eBreakpointEventTypeEnabled"; > > EMAIL PREFERENCES > http://reviews.llvm.org/settings/panel/emailpreferences/ > <D8847.23298.patch> _______________________________________________ lldb-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
