Skip to content

Commit b666ac3

Browse files
authored
[lldb] Change lldb's breakpoint handling behavior, reland (llvm#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 llvm#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
1 parent 72f4e65 commit b666ac3

File tree

9 files changed

+263
-261
lines changed

9 files changed

+263
-261
lines changed

Diff for: lldb/include/lldb/Target/Thread.h

+24
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
131131
register_backup_sp; // You need to restore the registers, of course...
132132
uint32_t current_inlined_depth;
133133
lldb::addr_t current_inlined_pc;
134+
lldb::addr_t stopped_at_unexecuted_bp;
134135
};
135136

136137
/// Constructor
@@ -383,6 +384,26 @@ class Thread : public std::enable_shared_from_this<Thread>,
383384

384385
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) {}
385386

387+
/// When a thread stops at an enabled BreakpointSite that has not executed,
388+
/// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
389+
/// If that BreakpointSite was actually triggered (the instruction was
390+
/// executed, for a software breakpoint), regardless of whether the
391+
/// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
392+
/// should be called to record that fact.
393+
///
394+
/// Depending on the structure of the Process plugin, it may be easiest
395+
/// to call SetThreadStoppedAtUnexecutedBP(pc) unconditionally when at
396+
/// a BreakpointSite, and later when it is known that it was triggered,
397+
/// SetThreadHitBreakpointSite() can be called. These two methods
398+
/// overwrite the same piece of state in the Thread, the last one
399+
/// called on a Thread wins.
400+
void SetThreadStoppedAtUnexecutedBP(lldb::addr_t pc) {
401+
m_stopped_at_unexecuted_bp = pc;
402+
}
403+
void SetThreadHitBreakpointSite() {
404+
m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
405+
}
406+
386407
/// Whether this Thread already has all the Queue information cached or not
387408
///
388409
/// A Thread may be associated with a libdispatch work Queue at a given
@@ -1337,6 +1358,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
13371358
bool m_should_run_before_public_stop; // If this thread has "stop others"
13381359
// private work to do, then it will
13391360
// set this.
1361+
lldb::addr_t m_stopped_at_unexecuted_bp; // Set to the address of a breakpoint
1362+
// instruction that we have not yet
1363+
// hit, but will hit when we resume.
13401364
const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
13411365
/// for easy UI/command line access.
13421366
lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this

0 commit comments

Comments
 (0)