Skip to content

Commit 05f0e86

Browse files
authored
[lldb] Change lldb's breakpoint handling behavior (#96260)
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, but I'm convinced it 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 thread plan 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. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. 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. rdar://123942164
1 parent be7f182 commit 05f0e86

File tree

9 files changed

+266
-265
lines changed

9 files changed

+266
-265
lines changed

lldb/include/lldb/Target/Thread.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
129129
register_backup_sp; // You need to restore the registers, of course...
130130
uint32_t current_inlined_depth;
131131
lldb::addr_t current_inlined_pc;
132+
lldb::addr_t stopped_at_unexecuted_bp;
132133
};
133134

134135
/// Constructor
@@ -378,6 +379,26 @@ class Thread : public std::enable_shared_from_this<Thread>,
378379

379380
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) {}
380381

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

0 commit comments

Comments
 (0)