[Lldb-commits] [PATCH] D91238: Recognize __builtin_debugtrap on arm64, advance pc past it so users can continue easily

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

The attraction of having stub fix up the PC in the stop after hitting the trap for breakpoints (in this case by moving the PC before the trap on architectures that execute the trap) was that this decision could be made simply in the stub, but if lldb had to check DECR_PC_AFTER_BREAK to understand a breakpoint stop, that would need to happen in a bunch of different places (as it did in gdb) and would be easier to get wrong.

But the only job we would need to do to proceed past unrecognized traps is to check, when we are about to continue, whether the next instruction is a trap.  If it is a trap we would push a "step over trap" plan before whatever else we were going to do.  And in fact, we already do that for breakpoints because the user might have put a breakpoint on the next instruction before hitting "continue" and we really don't want to stop at that breakpoint.  We always check this last thing before the continue.  So implementing proceed past unrecognized trap in lldb would be very contained.  We'd just have to expand the "is there a breakpoint at the current PC" to "is there a breakpoint or any other architecture defined 'trap' instruction at the current PC."

That weakens the argument for doing it in the stub rather than in lldb.

Doing it in lldb is a little nicer too, since on systems that execute their traps, you would just never see a trap instruction at the PC when you were about to continue, so the same solution would work.  And as Pavel says, then we wouldn't have to change lldb-server as well.

So provided lldb has an easy way to answer the question "is this a trap I see before me" I'm mildly in favor of extending lldb's "should I push a step-over-breakpoint plan now" logic to check both "is there a breakpoint here" and "is there any other architecture defined trap here".  But not enough  to say you shouldn't do it this way...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91238/new/

https://reviews.llvm.org/D91238