diff options
| author | Jason Molenda <jmolenda@apple.com> | 2025-02-13 11:30:10 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-02-13 11:30:10 -0800 |
| commit | b666ac3b63e01bfa602318c877ea2395fea53f89 (patch) | |
| tree | f2dbc6e8527596ac71e08d3c90f3546a240fce8c /lldb/source/Plugins/Process/scripted/ScriptedThread.cpp | |
| parent | 72f4e656b8feb3665bd074fadf1db1e72b385030 (diff) | |
[lldb] Change lldb's breakpoint handling behavior, reland (#126988)
lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.
In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.
Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.
One non-intuitive UX from this change, butt is necessary: If you're
stopped at a BreakpointSite that has not yet executed, you `stepi`, you
will hit the breakpoint and the pc will not yet advance. This thread has
not completed its stepi, and the ThreadPlanStepInstruction is still on
the stack. If you then `continue` the thread, lldb will now stop and
say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution.
The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.
Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.
The change to each Process plugin that does execution control is that
1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record that.
When the thread resumes, if the pc is still at the same site, we will
continue, hit the breakpoint, and stop again.
2. When we've actually hit a breakpoint (enabled for this thread or
not), the Process plugin should call
Thread::SetThreadHitBreakpointSite(). When we go to resume the thread,
we will push a step-over-breakpoint ThreadPlan before resuming.
The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.
I first landed this patch in July 2024 via
https://github.com/llvm/llvm-project/pull/96260
but the CI bots and wider testing found a number of test case failures
that needed to be updated, I reverted it. I've fixed all of those issues
in separate PRs and this change should run cleanly on all the CI bots
now.
rdar://123942164
Diffstat (limited to 'lldb/source/Plugins/Process/scripted/ScriptedThread.cpp')
| -rw-r--r-- | lldb/source/Plugins/Process/scripted/ScriptedThread.cpp | 11 |
1 files changed, 11 insertions, 0 deletions
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp index 88a4ca3b0389..d0d1508e8517 100644 --- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp +++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp @@ -229,6 +229,17 @@ bool ScriptedThread::CalculateStopInfo() { LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.", error, LLDBLog::Thread); + // If we're at a BreakpointSite, mark that we stopped there and + // need to hit the breakpoint when we resume. This will be cleared + // if we CreateStopReasonWithBreakpointSiteID. + if (RegisterContextSP reg_ctx_sp = GetRegisterContext()) { + addr_t pc = reg_ctx_sp->GetPC(); + if (BreakpointSiteSP bp_site_sp = + GetProcess()->GetBreakpointSiteList().FindByAddress(pc)) + if (bp_site_sp->IsEnabled()) + SetThreadStoppedAtUnexecutedBP(pc); + } + lldb::StopInfoSP stop_info_sp; lldb::StopReason stop_reason_type; |
