[Lldb-commits] [lldb] [lldb] Step over non-lldb breakpoints (PR #174348)

[Duncan McBain via lldb-commits]

DuncanMcBain wrote:

Thanks for reverting, and sorry for the failures!

I think I've worked out some of the failures. Specifically, it seems like using `self.platformIsDarwin` is a safer check and will work regardless of what flavour of MacOS it is. I think I have also fixed the Arm 32 failure, as I'd just made mistakes in the masks inside the IsValidTrapInstruction implementation. I don't really have a good way to test that other than to say that the updated patterns are more correct.

Looking at the other two failures (TestExprWithSideEffect.cpp and its ConvenienceVar friend), I don't really know for sure but my best guess is that, because both the original fix in debugserver _and_ the updated code from this PR coexist, combined with having two or more __builtin_debugtrap() calls in a row, first debugserver will increment the PC, then the StopInfoMachException::PerformAction code I added will _also_ see a trap instruction at the PC. This is a different trap to the one that we saw originally, but there's not really any way to know that, so we step over that one as well, and the result is that two instructions are stepped over for the price of one. This isn't a problem in other cases where there's only one trap, because then (most likely) the original fix from 5 years ago will handle it first, and the new code will see no trap to step over, so will do nothing, handing control back to the user.

I don't know if I'm right with that hypothesis, though it seems plausible. If I am able to test it, and that's the bug, how would you like to proceed? Should we try to remove the debugserver code at the same time?

I haven't pushed updates to the branch yet though I can likely do that later on today, and I can open a new PR when I do.

Thanks again for your time!

https://github.com/llvm/llvm-project/pull/174348