[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior (PR #105594)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

The details of the bug that Martin found were actually bit a interesting.

The core of the issue is that the thread has a ThreadPlanStepOut, and before we resume execution, because we're at a BreakpointSite that has been hit, we push a ThreadPlanStepOverBreakpoint plan.  It sees that the thread's state is eStateRunning (not eStateStepping -- so this is a thread doing a "continue" style resume).  StepOverBreakpoint marks itself as "AutoResume == true" so that when it completes, it will run the thread as it was intended to be doing originally.

We do our instruction step.  StepOverBreakpoint has completed, and it says we should resume execution.  StepOut says it has completed execution because the StackID == the StackID it needed to execute to.  It pops itself off the stack and removes its breakpoint.  Thread::ShouldResume uses the StepOverBreakpoint's "auto resume == true" state to resume execution, and we lose control.

Jim points out that StepOverBreakpoint is unique because it was not tied to any higher level/controlling thread plan.  It was injected on the thread just before we resume execution, and it tries to guess is the thread wants to stop when it's done or resume execution based on the thread's running state when it's added.

He thinks StepOverBreakpoint should maybe be owned by the first thread plan on the stack, so when StepOut removes itself from the stack, StepOverBreakpoint's opinion about resuming is not relevant.

The other part I did not dig in to was what exactly Thread::ShouldStop is doing when the current thread plan (StepOverBreakpoint) says 'auto resume' and the next one up the stack (StepOut) says "I'm complete, stop", and it resumed.  I wanted to avoid touching something as central as Thread::ShouldStop's logic here, but I don't quite understand why the result of these two inputs was, "resume".

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