[Lldb-commits] [PATCH] D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

The fix seems good.

The fact that a breakpoint hit while evaluating an expression doesn't check the condition is a known limitation.

You've got to protect against artificial recursions in handling breakpoints.  For instance, you could have a breakpoint condition in a function where the condition calls back into the same function, hitting the breakpoint again, calling the condition again, etc...  Sounds a bit far-fetched, but it actually happened in the field, which is why I added this protection.  The code that does this is in StopInfoBreakpoint.cpp::PerformAction.

We could be smarter about condition evaluation, since nesting expression evaluation does work.  Maybe tracking how deeply nested the condition evaluation is and having a cutoff at some (settable?) depth.  Or have a "I promise I'm not going to do anything stupid" setting - though that seems hacky...  I don't think there is a bug about this, but it would be great to file one.

Note, we also prohibit breakpoint command execution in expressions.  We have to do that because the current implementation of the command evaluator isn't re-entrant.  We really should fix that some day, but that's a decent chunk of work.  Anyway, so we have to be draconian about command evaluation, but with some care we could relax the condition evaluation prohibition.


https://reviews.llvm.org/D33283