Page MenuHomePhabricator

Parametrize scripted ThreadPlans using SBStructuredData

Authored by jingham on Oct 2 2019, 5:56 PM.



It is trivial to write a ScriptedThreadPlan to do "step until the variable 'foo' changes value". It is equally easy to write one that does "step until the variable 'bar' changes value". But you would have to write a different Python class for each of these tasks, since there's no way to pass parameters into the ThreadPlan on creation.

This patch adds that ability. You can say:

(lldb) thread step-scripted -C Module.ThreadPlan -k variable_name -v foo

lldb will package the key and value into an SBStructuredData dictionary, and pass that to the init method of the ThreadPlan class. Then the ThreadPlan can fetch the value for that key from the dictionary and watch that variable.

I also added an SB API that allows you to pass any SBStructuredData to SBThread.StepUsingScriptedThreadPlan, if you need to pass in something more complex than a simple key/value dictionary.

This revision depends on:

Not sure who to put on to review this, most of the folks who worked actively on the PythonObjects aren't really active anymore... I added folks who touched some of the relevant files recently...

Diff Detail


Event Timeline

jingham created this revision.Oct 2 2019, 5:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 2 2019, 5:56 PM
jingham updated this revision to Diff 222952.Oct 2 2019, 7:02 PM

Missed a bit that removed m_class_name from the old command options (and didn't use the new option.)

aprantl accepted this revision.Oct 3 2019, 12:50 PM

From a purely mechanical point of view, this lgtm.

This revision is now accepted and ready to land.Oct 3 2019, 12:50 PM
JDevlieghere added inline comments.Oct 3 2019, 1:14 PM
35 ↗(On Diff #222952)

Why do we need the StructuredDataImpl and not the StructuredData?

985 ↗(On Diff #222952)

It appears this is missing a corresponding LLDB_REGISTER macro.

78 ↗(On Diff #222952)

Same here

404 ↗(On Diff #222952)

This one is fine it seems.

409 ↗(On Diff #222952)

Swap this and have an early return?

jingham updated this revision to Diff 223096.Oct 3 2019, 2:26 PM

Added the missing REGISTER macros

jingham marked 4 inline comments as done.Oct 3 2019, 2:32 PM
jingham added inline comments.
35 ↗(On Diff #222952)

I figured out that I had to do it this way when I implemented the same feature as part of the ScriptedBreakpointResolvers. This change is largely copied from that. But I don't remember why now.

I'll go back and figure that out again if you are really curious, but since this is following a successful precedent, I'd rather not do that right now...

409 ↗(On Diff #222952)

Every other function in this file is written this way. I don't want to fix them piecemeal, and doing them all here would make this commit too hard to read.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2019, 3:48 PM