Page MenuHomePhabricator

WWW docs for scripted breakpoint resolvers
ClosedPublic

Authored by jingham on Sep 13 2018, 5:35 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

jingham created this revision.Sep 13 2018, 5:35 PM

See inlined comments.

www/python-reference.html
324 ↗(On Diff #165404)

Maybe specify the SVN revision this went in with just in case? If they try this with older LLDBs it won't work.

340 ↗(On Diff #165404)

Maybe mention if no modules are specified, it will search all?

365 ↗(On Diff #165404)

Might be good to specify the module and compile unit filter are handled outside of the python class? Not sure anyone will know. Might be good to specify that the only thing we need to write in python is the resolver, with the caveat of addresses getting rejected due to searcher.

368–369 ↗(On Diff #165404)

Maybe have some create example commands and detail what options are for the searcher which is auto handled for us and what part is for the resolver?

jingham updated this revision to Diff 165614.Sep 14 2018, 5:09 PM

I think this should address your concerns, though I did so more by reworking the presentation, so you won't find inserts exactly at the places you pointed out.

But I tried to make it clear that you only write the Resolver, and also gave more examples of how the SearchFilter gets provided.

clayborg added inline comments.Sep 17 2018, 7:31 AM
www/python-reference.html
325 ↗(On Diff #165614)

Is that SVN revision high enough? Seems to be missing a digit?

334–335 ↗(On Diff #165614)

Feel free to ignore this suggestion, but maybe:

In lldb, a breakpoint is composed of three parts: the Searcher, the Resolver, and the Options. The Searcher and Resolver cooperate to determine how breakpoint locations are set and differ between each breakpoint type. Options determine what happens when a location triggers and includes the commands, conditions, ignore counts, etc. Options are common between all breakpoint types.
348 ↗(On Diff #165614)

Remove good from above?

s/specifies a list of good Modules/specifies a list of Modules/
384 ↗(On Diff #165614)
At present, when adding a scripted Breakpoint type, you only need to provide a custom Resolver as the Searcher is handled automatically by LLDB using breakpoint options from the command line or SBTarget::BreakpointCreateXXX() function.

Is it the Resolvers job to provide the search depth? Or is this a Searcher option masquerading as Resolver callback?

jingham marked 3 inline comments as done.Sep 17 2018, 11:37 AM

I used your suggestion and fixed the few other bits.

www/python-reference.html
325 ↗(On Diff #165614)

Yeah, dunno how that got into my paste buffer...

334–335 ↗(On Diff #165614)

Nice. I changed Options -> Stop Options because on the command you also use options to specify the resolver & filter to make it a little clearer.

348 ↗(On Diff #165614)

Sure. We don't have black list filters at this point, only white list ones. So "good" is redundant except if you didn't know that. But I agree, from context this is pretty clear.

384 ↗(On Diff #165614)

It is currently the Resolver's job to provide the search depth.

The Searcher's all have the ability to support any search depth, that's all in base class code. And that's not functionality that's overridden in the actual implementation of search filters. So the depth is a fairly inessential part of the searchers. OTOH, the Resolver cares about what depth it gets called back at, the strategy it uses to resolve locations depends on that. So it made sense to me for the Resolver to be the one that states the search depth, not the Searcher.

However, the Resolver doesn't create the search filter, they are independently made and assembled. It also seemed awkward to have the assembler (the Target in this case) have to ask the Resolver "what depth do you want" and then tell that to the Search Filter. That's not something the assembler needs to know about at all.

So it made more sense to me to have this conveyed in a communication between the Resolver & Search Filter.

When I was originally thinking about this, I didn't want to rule out the ability for the Resolver to have a state dependent control over the depth. For instance, you might want to run over modules, but then in some particular module, ask to be fed Compile Units. That's why I left it as a callback, not some "Resolver, configure your searcher" mechanism. We don't use that anywhere - and I'd need to add some communication to make it possible. But that would be pretty straightforward to do, and given that we don't pay anything substantial to retain the possibility, I'm think the way it is done now is good.

jingham updated this revision to Diff 165796.Sep 17 2018, 11:38 AM
jingham marked 3 inline comments as done.

Addressed review comments from Greg.

clayborg accepted this revision.Sep 17 2018, 1:01 PM

Thanks for doing all the updates. Pretty clear and concise now.

This revision is now accepted and ready to land.Sep 17 2018, 1:01 PM
This revision was automatically updated to reflect the committed changes.