[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior [WIP] (PR #105594)
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Fri Sep 13 01:59:19 PDT 2024
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/105594
>From 56ca564185bdceea25162a1ce3b1e8c8fa2641e2 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Fri, 19 Jul 2024 17:26:13 -0700
Subject: [PATCH 1/7] [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
---
lldb/include/lldb/Target/Thread.h | 24 ++
.../Process/Utility/StopInfoMachException.cpp | 322 ++++++++----------
.../Process/Windows/Common/ProcessWindows.cpp | 32 +-
.../Process/gdb-remote/ProcessGDBRemote.cpp | 93 ++---
.../Process/scripted/ScriptedThread.cpp | 11 +
lldb/source/Target/StopInfo.cpp | 2 +
lldb/source/Target/Thread.cpp | 15 +-
.../TestConsecutiveBreakpoints.py | 26 +-
.../TestStepOverBreakpoint.py | 6 +-
9 files changed, 266 insertions(+), 265 deletions(-)
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 38b65b2bc58490..22d452c7b4b8a3 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -131,6 +131,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
register_backup_sp; // You need to restore the registers, of course...
uint32_t current_inlined_depth;
lldb::addr_t current_inlined_pc;
+ lldb::addr_t stopped_at_unexecuted_bp;
};
/// Constructor
@@ -380,6 +381,26 @@ class Thread : public std::enable_shared_from_this<Thread>,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) {}
+ /// When a thread stops at an enabled BreakpointSite that has not executed,
+ /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+ /// If that BreakpointSite was actually triggered (the instruction was
+ /// executed, for a software breakpoint), regardless of whether the
+ /// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
+ /// should be called to record that fact.
+ ///
+ /// Depending on the structure of the Process plugin, it may be easiest
+ /// to call SetThreadStoppedAtUnexecutedBP(pc) unconditionally when at
+ /// a BreakpointSite, and later when it is known that it was triggered,
+ /// SetThreadHitBreakpointSite() can be called. These two methods
+ /// overwrite the same piece of state in the Thread, the last one
+ /// called on a Thread wins.
+ void SetThreadStoppedAtUnexecutedBP(lldb::addr_t pc) {
+ m_stopped_at_unexecuted_bp = pc;
+ }
+ void SetThreadHitBreakpointSite() {
+ m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
+ }
+
/// Whether this Thread already has all the Queue information cached or not
///
/// A Thread may be associated with a libdispatch work Queue at a given
@@ -1314,6 +1335,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
bool m_should_run_before_public_stop; // If this thread has "stop others"
// private work to do, then it will
// set this.
+ lldb::addr_t m_stopped_at_unexecuted_bp; // Set to the address of a breakpoint
+ // instruction that we have not yet
+ // hit, but will hit when we resume.
const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
/// for easy UI/command line access.
lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index 25cee369d7ee3d..f1853be12638ea 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -488,38 +488,6 @@ const char *StopInfoMachException::GetDescription() {
return m_description.c_str();
}
-static StopInfoSP GetStopInfoForHardwareBP(Thread &thread, Target *target,
- uint32_t exc_data_count,
- uint64_t exc_sub_code,
- uint64_t exc_sub_sub_code) {
- // Try hardware watchpoint.
- if (target) {
- // The exc_sub_code indicates the data break address.
- WatchpointResourceSP wp_rsrc_sp =
- target->GetProcessSP()->GetWatchpointResourceList().FindByAddress(
- (addr_t)exc_sub_code);
- if (wp_rsrc_sp && wp_rsrc_sp->GetNumberOfConstituents() > 0) {
- return StopInfo::CreateStopReasonWithWatchpointID(
- thread, wp_rsrc_sp->GetConstituentAtIndex(0)->GetID());
- }
- }
-
- // Try hardware breakpoint.
- ProcessSP process_sp(thread.GetProcess());
- if (process_sp) {
- // The exc_sub_code indicates the data break address.
- lldb::BreakpointSiteSP bp_sp =
- process_sp->GetBreakpointSiteList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
- if (bp_sp && bp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithBreakpointSiteID(thread,
- bp_sp->GetID());
- }
- }
-
- return nullptr;
-}
-
#if defined(__APPLE__)
const char *
StopInfoMachException::MachException::Name(exception_type_t exc_type) {
@@ -607,6 +575,25 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
target ? target->GetArchitecture().GetMachine()
: llvm::Triple::UnknownArch;
+ ProcessSP process_sp(thread.GetProcess());
+ RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
+ // Caveat: with x86 KDP if we've hit a breakpoint, the pc we
+ // receive is past the breakpoint instruction.
+ // If we have a breakpoints at 0x100 and 0x101, we hit the
+ // 0x100 breakpoint and the pc is reported at 0x101.
+ // We will initially mark this thread as being stopped at an
+ // unexecuted breakpoint at 0x101. Later when we see that
+ // we stopped for a Breakpoint reason, we will decrement the
+ // pc, and update the thread to record that we hit the
+ // breakpoint at 0x100.
+ // The fact that the pc may be off by one at this point
+ // (for an x86 KDP breakpoint hit) is not a problem.
+ addr_t pc = reg_ctx_sp->GetPC();
+ BreakpointSiteSP bp_site_sp =
+ process_sp->GetBreakpointSiteList().FindByAddress(pc);
+ if (bp_site_sp && bp_site_sp->IsEnabled())
+ thread.SetThreadStoppedAtUnexecutedBP(pc);
+
switch (exc_type) {
case 1: // EXC_BAD_ACCESS
case 2: // EXC_BAD_INSTRUCTION
@@ -633,171 +620,158 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // A mach exception comes with 2-4 pieces of data.
+ // The sub-codes are only provided for certain types
+ // of mach exceptions.
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Here are all of the EXC_BREAKPOINT, exc_type==6,
+ // exceptions we can receive.
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, <stop-pc>] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, <bp-addr + 1>]
+ // arm64 [6, 1, <bp-addr>]
+ // armv7 [6, 0x102, <bp-addr>] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, <bp-addr>, 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, <bp-addr>]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, <accessed-addr>, 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, <accessed-addr>, 0]
+ // armv7 [6, 0x102, <accessed-addr>, 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, <addr-of-BRK-insn>]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, <bp-addr>] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, <bp-addr + 1>] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, <stop-pc>] armv7 instruction step
+ // [6, 0x102, <bp-addr>] armv7 software breakpoint
+ // [6, 0x102, <accessed-addr>, 0] arm64/armv7 watchpoint
+
case 6: // EXC_BREAKPOINT
{
- bool is_actual_breakpoint = false;
- bool is_trace_if_actual_breakpoint_missing = false;
- switch (cpu) {
- case llvm::Triple::x86:
- case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
- if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
- } else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
- return stop_info;
- }
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
- // KDP returns EXC_I386_BPTFLT for trace breakpoints
- if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
- is_actual_breakpoint = true;
+ bool stopped_by_hitting_breakpoint = false;
+ bool stopped_by_completing_stepi = false;
+ bool stopped_watchpoint = false;
+ std::optional<addr_t> address;
+
+ // exc_code 1
+ if (exc_code == 1) {
+ if (exc_sub_code == 0) {
+ stopped_by_completing_stepi = true;
+ } else {
+ // Ambiguous: could be signalling a
+ // breakpoint or watchpoint hit.
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ address = exc_sub_code;
+ }
+ }
+
+ // exc_code 2
+ if (exc_code == 2) {
+ if (exc_sub_code == 0)
+ stopped_by_hitting_breakpoint = true;
+ else {
+ stopped_by_hitting_breakpoint = true;
+ // Intel KDP software breakpoint
if (!pc_already_adjusted)
pc_decrement = 1;
}
- break;
+ }
- case llvm::Triple::arm:
- case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
- // LWP_TODO: We need to find the WatchpointResource that matches
- // the address, and evaluate its Watchpoints.
-
- // It's a watchpoint, then, if the exc_sub_code indicates a
- // known/enabled data break address from our watchpoint list.
- lldb::WatchpointSP wp_sp;
- if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
- if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
- wp_sp->GetID());
- } else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
- }
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
- // is currently returning this so accept it
- // as indicating a breakpoint until the
- // kernel is fixed
- {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+ // exc_code 3
+ if (exc_code == 3)
+ stopped_by_completing_stepi = true;
- case llvm::Triple::aarch64_32:
- case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
- // Instruction step has completed.
- //
- // exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
- // Software breakpoint instruction executed.
-
- if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
- {
- // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
- // is set
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
- if (thread.GetTemporaryResumeState() != eStateStepping)
- not_stepping_but_got_singlestep_exception = true;
- }
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
- // LWP_TODO: We need to find the WatchpointResource that matches
- // the address, and evaluate its Watchpoints.
-
- // It's a watchpoint, then, if the exc_sub_code indicates a
- // known/enabled data break address from our watchpoint list.
- lldb::WatchpointSP wp_sp;
- if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
- if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
- wp_sp->GetID());
- }
- // EXC_ARM_DA_DEBUG seems to be reused for EXC_BREAKPOINT as well as
- // EXC_BAD_ACCESS
- if (thread.GetTemporaryResumeState() == eStateStepping)
- return StopInfo::CreateStopReasonToTrace(thread);
+ // exc_code 0x102
+ if (exc_code == 0x102 && exc_sub_code != 0) {
+ if (cpu == llvm::Triple::arm || cpu == llvm::Triple::thumb) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_by_completing_stepi = true;
}
- // It looks like exc_sub_code has the 4 bytes of the instruction that
- // triggered the exception, i.e. our breakpoint opcode
- is_actual_breakpoint = exc_code == 1;
- break;
+ stopped_watchpoint = true;
+ address = exc_sub_code;
}
- default:
- break;
- }
+ // The Mach Exception may have been ambiguous --
+ // e.g. we stopped either because of a breakpoint
+ // or a watchpoint. We'll disambiguate which it
+ // really was.
- if (is_actual_breakpoint) {
- RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
+ if (stopped_by_hitting_breakpoint) {
addr_t pc = reg_ctx_sp->GetPC() - pc_decrement;
- ProcessSP process_sp(thread.CalculateProcess());
-
- lldb::BreakpointSiteSP bp_site_sp;
- if (process_sp)
+ if (address)
+ bp_site_sp =
+ process_sp->GetBreakpointSiteList().FindByAddress(*address);
+ if (!bp_site_sp && reg_ctx_sp) {
bp_site_sp = process_sp->GetBreakpointSiteList().FindByAddress(pc);
+ }
if (bp_site_sp && bp_site_sp->IsEnabled()) {
- // Update the PC if we were asked to do so, but only do so if we find
- // a breakpoint that we know about cause this could be a trap
- // instruction in the code
- if (pc_decrement > 0 && adjust_pc_if_needed)
- reg_ctx_sp->SetPC(pc);
-
- // If the breakpoint is for this thread, then we'll report the hit,
- // but if it is for another thread, we can just report no reason. We
- // don't need to worry about stepping over the breakpoint here, that
- // will be taken care of when the thread resumes and notices that
- // there's a breakpoint under the pc. If we have an operating system
- // plug-in, we might have set a thread specific breakpoint using the
- // operating system thread ID, so we can't make any assumptions about
- // the thread ID so we must always report the breakpoint regardless
- // of the thread.
+ // We've hit this breakpoint, whether it was intended for this thread
+ // or not. Clear this in the Tread object so we step past it on resume.
+ thread.SetThreadHitBreakpointSite();
+
+ // If we have an operating system plug-in, we might have set a thread
+ // specific breakpoint using the operating system thread ID, so we
+ // can't make any assumptions about the thread ID so we must always
+ // report the breakpoint regardless of the thread.
if (bp_site_sp->ValidForThisThread(thread) ||
- thread.GetProcess()->GetOperatingSystem() != nullptr)
+ thread.GetProcess()->GetOperatingSystem() != nullptr) {
+ // Update the PC if we were asked to do so, but only do so if we find
+ // a breakpoint that we know about cause this could be a trap
+ // instruction in the code
+ if (pc_decrement > 0 && adjust_pc_if_needed && reg_ctx_sp)
+ reg_ctx_sp->SetPC(pc);
return StopInfo::CreateStopReasonWithBreakpointSiteID(
thread, bp_site_sp->GetID());
- else if (is_trace_if_actual_breakpoint_missing)
- return StopInfo::CreateStopReasonToTrace(thread);
- else
+ } else {
return StopInfoSP();
+ }
}
+ }
- // Don't call this a trace if we weren't single stepping this thread.
- if (is_trace_if_actual_breakpoint_missing &&
- thread.GetTemporaryResumeState() == eStateStepping) {
- return StopInfo::CreateStopReasonToTrace(thread);
+ // Breakpoint-hit events are handled.
+ // Now handle watchpoints.
+
+ if (stopped_watchpoint && address) {
+ WatchpointResourceSP wp_rsrc_sp =
+ target->GetProcessSP()->GetWatchpointResourceList().FindByAddress(
+ *address);
+ if (wp_rsrc_sp && wp_rsrc_sp->GetNumberOfConstituents() > 0) {
+ return StopInfo::CreateStopReasonWithWatchpointID(
+ thread, wp_rsrc_sp->GetConstituentAtIndex(0)->GetID());
}
}
+
+ // Finally, handle instruction step.
+
+ if (stopped_by_completing_stepi) {
+ if (thread.GetTemporaryResumeState() != eStateStepping)
+ not_stepping_but_got_singlestep_exception = true;
+ else
+ return StopInfo::CreateStopReasonToTrace(thread);
+ }
+
} break;
case 7: // EXC_SYSCALL
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index f383b3d40a4f3a..8b12b87eacbc61 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -377,24 +377,17 @@ void ProcessWindows::RefreshStateAfterStop() {
if (!stop_thread)
return;
- switch (active_exception->GetExceptionCode()) {
- case EXCEPTION_SINGLE_STEP: {
- RegisterContextSP register_context = stop_thread->GetRegisterContext();
- const uint64_t pc = register_context->GetPC();
- BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
- if (site && site->ValidForThisThread(*stop_thread)) {
- LLDB_LOG(log,
- "Single-stepped onto a breakpoint in process {0} at "
- "address {1:x} with breakpoint site {2}",
- m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
- site->GetID());
- stop_info = StopInfo::CreateStopReasonWithBreakpointSiteID(*stop_thread,
- site->GetID());
- stop_thread->SetStopInfo(stop_info);
+ RegisterContextSP register_context = stop_thread->GetRegisterContext();
+ uint64_t pc = register_context->GetPC();
- return;
- }
+ // If we're at a BreakpointSite, mark this as an Unexecuted Breakpoint.
+ // We'll clear that state if we've actually executed the breakpoint.
+ BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
+ if (site && site->IsEnabled())
+ stop_thread->SetThreadStoppedAtUnexecutedBP(pc);
+ switch (active_exception->GetExceptionCode()) {
+ case EXCEPTION_SINGLE_STEP: {
auto *reg_ctx = static_cast<RegisterContextWindows *>(
stop_thread->GetRegisterContext().get());
uint32_t slot_id = reg_ctx->GetTriggeredHardwareBreakpointSlotId();
@@ -419,8 +412,6 @@ void ProcessWindows::RefreshStateAfterStop() {
}
case EXCEPTION_BREAKPOINT: {
- RegisterContextSP register_context = stop_thread->GetRegisterContext();
-
int breakpoint_size = 1;
switch (GetTarget().GetArchitecture().GetMachine()) {
case llvm::Triple::aarch64:
@@ -443,9 +434,9 @@ void ProcessWindows::RefreshStateAfterStop() {
}
// The current PC is AFTER the BP opcode, on all architectures.
- uint64_t pc = register_context->GetPC() - breakpoint_size;
+ pc = register_context->GetPC() - breakpoint_size;
- BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
+ site = GetBreakpointSiteList().FindByAddress(pc);
if (site) {
LLDB_LOG(log,
"detected breakpoint in process {0} at address {1:x} with "
@@ -453,6 +444,7 @@ void ProcessWindows::RefreshStateAfterStop() {
m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
site->GetID());
+ stop_thread->SetThreadHitBreakpointSite();
if (site->ValidForThisThread(*stop_thread)) {
LLDB_LOG(log,
"Breakpoint site {0} is valid for this thread ({1:x}), "
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 0efe8fb238491e..159d08417be9ed 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1598,29 +1598,8 @@ bool ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) {
// that have stop reasons, and if there is no entry for a thread, then it
// has no stop reason.
thread->GetRegisterContext()->InvalidateIfNeeded(true);
- if (!GetThreadStopInfoFromJSON(thread, m_jstopinfo_sp)) {
- // If a thread is stopped at a breakpoint site, set that as the stop
- // reason even if it hasn't executed the breakpoint instruction yet.
- // We will silently step over the breakpoint when we resume execution
- // and miss the fact that this thread hit the breakpoint.
- const size_t num_thread_ids = m_thread_ids.size();
- for (size_t i = 0; i < num_thread_ids; i++) {
- if (m_thread_ids[i] == thread->GetID() && m_thread_pcs.size() > i) {
- addr_t pc = m_thread_pcs[i];
- lldb::BreakpointSiteSP bp_site_sp =
- thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
- if (bp_site_sp) {
- if (bp_site_sp->ValidForThisThread(*thread)) {
- thread->SetStopInfo(
- StopInfo::CreateStopReasonWithBreakpointSiteID(
- *thread, bp_site_sp->GetID()));
- return true;
- }
- }
- }
- }
+ if (!GetThreadStopInfoFromJSON(thread, m_jstopinfo_sp))
thread->SetStopInfo(StopInfoSP());
- }
return true;
}
@@ -1729,6 +1708,12 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
if (ThreadSP memory_thread_sp = m_thread_list.GetBackingThread(thread_sp))
thread_sp = memory_thread_sp;
+ addr_t pc = thread_sp->GetRegisterContext()->GetPC();
+ BreakpointSiteSP bp_site_sp =
+ thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+ if (bp_site_sp && bp_site_sp->IsEnabled())
+ thread_sp->SetThreadStoppedAtUnexecutedBP(pc);
+
if (exc_type != 0) {
// For thread plan async interrupt, creating stop info on the
// original async interrupt request thread instead. If interrupt thread
@@ -1755,26 +1740,10 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
// to no reason.
if (!reason.empty() && reason != "none") {
if (reason == "trace") {
- addr_t pc = thread_sp->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp =
- thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(
- pc);
-
- // If the current pc is a breakpoint site then the StopInfo should be
- // set to Breakpoint Otherwise, it will be set to Trace.
- if (bp_site_sp && bp_site_sp->ValidForThisThread(*thread_sp)) {
- thread_sp->SetStopInfo(
- StopInfo::CreateStopReasonWithBreakpointSiteID(
- *thread_sp, bp_site_sp->GetID()));
- } else
- thread_sp->SetStopInfo(
- StopInfo::CreateStopReasonToTrace(*thread_sp));
+ thread_sp->SetStopInfo(StopInfo::CreateStopReasonToTrace(*thread_sp));
handled = true;
} else if (reason == "breakpoint") {
- addr_t pc = thread_sp->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp =
- thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(
- pc);
+ thread_sp->SetThreadHitBreakpointSite();
if (bp_site_sp) {
// If the breakpoint is for this thread, then we'll report the hit,
// but if it is for another thread, we can just report no reason.
@@ -1890,39 +1859,41 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
StopInfo::CreateStopReasonVForkDone(*thread_sp));
handled = true;
}
- } else if (!signo) {
- addr_t pc = thread_sp->GetRegisterContext()->GetPC();
- lldb::BreakpointSiteSP bp_site_sp =
- thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-
- // If a thread is stopped at a breakpoint site, set that as the stop
- // reason even if it hasn't executed the breakpoint instruction yet.
- // We will silently step over the breakpoint when we resume execution
- // and miss the fact that this thread hit the breakpoint.
- if (bp_site_sp && bp_site_sp->ValidForThisThread(*thread_sp)) {
- thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(
- *thread_sp, bp_site_sp->GetID()));
- handled = true;
- }
}
if (!handled && signo && !did_exec) {
if (signo == SIGTRAP) {
// Currently we are going to assume SIGTRAP means we are either
// hitting a breakpoint or hardware single stepping.
+
+ // We can't disambiguate between stepping-to-a-breakpointsite and
+ // hitting-a-breakpointsite.
+ //
+ // A user can instruction-step, and be stopped at a BreakpointSite.
+ // Or a user can be sitting at a BreakpointSite,
+ // instruction-step which hits the breakpoint and the pc does not
+ // advance.
+ //
+ // In both cases, we're at a BreakpointSite when stopped, and
+ // the resume state was eStateStepping.
+
+ // Assume if we're at a BreakpointSite, we hit it.
handled = true;
addr_t pc =
thread_sp->GetRegisterContext()->GetPC() + m_breakpoint_pc_offset;
- lldb::BreakpointSiteSP bp_site_sp =
+ BreakpointSiteSP bp_site_sp =
thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(
pc);
+ // We can't know if we hit it or not. So if we are stopped at
+ // a BreakpointSite, assume we hit it, and should step past the
+ // breakpoint when we resume. This is contrary to how we handle
+ // BreakpointSites in any other location, but we can't know for
+ // sure what happened so it's a reasonable default.
if (bp_site_sp) {
- // If the breakpoint is for this thread, then we'll report the hit,
- // but if it is for another thread, we can just report no reason.
- // We don't need to worry about stepping over the breakpoint here,
- // that will be taken care of when the thread resumes and notices
- // that there's a breakpoint under the pc.
+ if (bp_site_sp->IsEnabled())
+ thread_sp->SetThreadHitBreakpointSite();
+
if (bp_site_sp->ValidForThisThread(*thread_sp)) {
if (m_breakpoint_pc_offset != 0)
thread_sp->GetRegisterContext()->SetPC(pc);
@@ -1936,8 +1907,6 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
} else {
// If we were stepping then assume the stop was the result of the
// trace. If we were not stepping then report the SIGTRAP.
- // FIXME: We are still missing the case where we single step over a
- // trap instruction.
if (thread_sp->GetTemporaryResumeState() == eStateStepping)
thread_sp->SetStopInfo(
StopInfo::CreateStopReasonToTrace(*thread_sp));
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index 88a4ca3b0389fb..d0d1508e851728 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;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index bd7032b803df90..27983531b3f8eb 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -1388,6 +1388,8 @@ class StopInfoVForkDone : public StopInfo {
StopInfoSP StopInfo::CreateStopReasonWithBreakpointSiteID(Thread &thread,
break_id_t break_id) {
+ thread.SetThreadHitBreakpointSite();
+
return StopInfoSP(new StopInfoBreakpoint(thread, break_id));
}
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index fcf0f4e2519085..38297ae2a0d76f 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -223,6 +223,7 @@ Thread::Thread(Process &process, lldb::tid_t tid, bool use_invalid_index_id)
m_process_wp(process.shared_from_this()), m_stop_info_sp(),
m_stop_info_stop_id(0), m_stop_info_override_stop_id(0),
m_should_run_before_public_stop(false),
+ m_stopped_at_unexecuted_bp(LLDB_INVALID_ADDRESS),
m_index_id(use_invalid_index_id ? LLDB_INVALID_INDEX32
: process.GetNextThreadIndexID(tid)),
m_reg_context_sp(), m_state(eStateUnloaded), m_state_mutex(),
@@ -525,6 +526,7 @@ bool Thread::CheckpointThreadState(ThreadStateCheckpoint &saved_state) {
saved_state.current_inlined_depth = GetCurrentInlinedDepth();
saved_state.m_completed_plan_checkpoint =
GetPlans().CheckpointCompletedPlans();
+ saved_state.stopped_at_unexecuted_bp = m_stopped_at_unexecuted_bp;
return true;
}
@@ -560,6 +562,7 @@ void Thread::RestoreThreadStateFromCheckpoint(
saved_state.current_inlined_depth);
GetPlans().RestoreCompletedPlanCheckpoint(
saved_state.m_completed_plan_checkpoint);
+ m_stopped_at_unexecuted_bp = saved_state.stopped_at_unexecuted_bp;
}
StateType Thread::GetState() const {
@@ -628,7 +631,15 @@ void Thread::SetupForResume() {
const addr_t thread_pc = reg_ctx_sp->GetPC();
BreakpointSiteSP bp_site_sp =
GetProcess()->GetBreakpointSiteList().FindByAddress(thread_pc);
- if (bp_site_sp) {
+ // If we're at a BreakpointSite which we have either
+ // 1. already triggered/hit, or
+ // 2. the Breakpoint was added while stopped, or the pc was moved
+ // to this BreakpointSite
+ // Step past the breakpoint before resuming.
+ // If we stopped at a breakpoint instruction/BreakpointSite location
+ // without hitting it, and we're still at that same address on
+ // resuming, then we want to hit the BreakpointSite when we resume.
+ if (bp_site_sp && m_stopped_at_unexecuted_bp != thread_pc) {
// Note, don't assume there's a ThreadPlanStepOverBreakpoint, the
// target may not require anything special to step over a breakpoint.
@@ -717,6 +728,7 @@ bool Thread::ShouldResume(StateType resume_state) {
if (need_to_resume) {
ClearStackFrames();
+ m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
// Let Thread subclasses do any special work they need to prior to resuming
WillResume(resume_state);
}
@@ -1909,6 +1921,7 @@ Unwind &Thread::GetUnwinder() {
void Thread::Flush() {
ClearStackFrames();
m_reg_context_sp.reset();
+ m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
}
bool Thread::IsStillAtLastBreakpointHit() {
diff --git a/lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py b/lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py
index 73de4a294388b7..ecea28c6e1f6d0 100644
--- a/lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py
+++ b/lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py
@@ -2,7 +2,6 @@
Test that we handle breakpoints on consecutive instructions correctly.
"""
-
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -64,20 +63,30 @@ def test_single_step(self):
"""Test that single step stops at the second breakpoint."""
self.prepare_test()
+ # Instruction step to the next instruction
+ # We haven't executed breakpoint2 yet, we're sitting at it now.
+ step_over = False
+ self.thread.StepInstruction(step_over)
+
step_over = False
self.thread.StepInstruction(step_over)
+ # We've now hit the breakpoint and this StepInstruction has
+ # been interrupted, it is still sitting on the thread plan stack.
+
self.assertState(self.process.GetState(), lldb.eStateStopped)
self.assertEqual(
self.thread.GetFrameAtIndex(0).GetPCAddress().GetLoadAddress(self.target),
self.bkpt_address.GetLoadAddress(self.target),
)
- self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(
- self.process, self.breakpoint2
- )
- self.assertIsNotNone(
- self.thread, "Expected one thread to be stopped at breakpoint 2"
- )
+
+ # One more instruction to complete the Step that was interrupted
+ # earlier.
+ self.thread.StepInstruction(step_over)
+ strm = lldb.SBStream()
+ self.thread.GetDescription(strm)
+ self.assertIn("instruction step into", strm.GetData())
+ self.assertIsNotNone(self.thread, "Expected to see that step-in had completed")
self.finish_test()
@@ -106,4 +115,7 @@ def test_single_step_thread_specific(self):
"Stop reason should be 'plan complete'",
)
+ # Hit our second breakpoint
+ self.process.Continue()
+
self.finish_test()
diff --git a/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py b/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
index 3a7440a31677ac..1315288b35c551 100644
--- a/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
+++ b/lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py
@@ -5,7 +5,6 @@
and eStopReasonPlanComplete when breakpoint's condition fails.
"""
-
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
@@ -90,6 +89,11 @@ def test_step_instruction(self):
self.assertGreaterEqual(step_count, steps_expected)
break
+ # We did a `stepi` when we hit our last breakpoint, and the stepi was not
+ # completed yet, so when we resume it will complete (running process.Continue()
+ # would have the same result - we step one instruction and stop again when
+ # our interrupted stepi completes).
+ self.thread.StepInstruction(True)
# Run the process until termination
self.process.Continue()
self.assertState(self.process.GetState(), lldb.eStateExited)
>From 3408ce164ffc6857e968c4fff4d40ccd1a7014a5 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 28 Aug 2024 16:53:06 -0700
Subject: [PATCH 2/7] Recognize `swbreak:`/`hwbreak:` in stop packet
mark it as a `reason:breakpoint` so the correct StopInfo is created.
---
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 159d08417be9ed..3cd33d73d74d64 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2283,6 +2283,8 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
StreamString ostr;
ostr.Printf("%" PRIu64, wp_addr);
description = std::string(ostr.GetString());
+ } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+ reason = "breakpoint";
} else if (key.compare("library") == 0) {
auto error = LoadModules();
if (error) {
>From 48a0fc1ade6705fd53610cd6504e38ac98194e39 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 28 Aug 2024 18:33:17 -0700
Subject: [PATCH 3/7] Add pc check for instruction stepping by breakpoints
NativeProcessFreeBSD::MonitorSIGTRAP (MIPS) and
NativeProcessLinux::MonitorBreakpoint (MIPS, armv7, RISCV, LoongArch)
do instruction stepping by setting a breakpoint on the next instruction
that will be executed, and resuming the thread. They check that there
was one of these "instruction step" breakpoints set for this thread,
and rewrite the stop reason from "breakpoint hit" to "trace".
However, if a user is sitting at a BreakpointSite (break instruction),
and does `stepi`, then a breakpoint instruction will be put on the
next instruction, the thread resumes, and we stop at the same
location having hit our *original* breakpoint, not the one inserted
on the next instruction for stepping. The stop reason is rewritten to
"trace" but the pc hasn't advanced. The stepping logic will try to
instruction step again and it infinite loops.
I changed NativeProcessLinux and NativeProcessFreeBSD to only rewrite
the stop reason to "trace" when the pc is the breakpoint we added
for the insn-step. I don't have any of the devices that use this
behavior, but I did at least compile test NativeProcessLinux in an
Ubuntu AArch64 VM (but my core doesn't support aarch32 so I can't
test it).
---
.../Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp | 5 ++++-
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp | 7 +++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 064bddd3705205..d2eb2448b911af 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -318,9 +318,12 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
info.pl_siginfo.si_addr);
if (thread) {
+ auto ®ctx = static_cast<NativeRegisterContextFreeBSD &>(
+ thread->GetRegisterContext());
auto thread_info =
m_threads_stepping_with_breakpoint.find(thread->GetID());
- if (thread_info != m_threads_stepping_with_breakpoint.end()) {
+ if (thread_info != m_threads_stepping_with_breakpoint.end() &&
+ threads_info->second == regctx.GetPC()) {
thread->SetStoppedByTrace();
Status brkpt_error = RemoveBreakpoint(thread_info->second);
if (brkpt_error.Fail())
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 59fc8726b76739..033caa91cfd18c 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -829,8 +829,11 @@ void NativeProcessLinux::MonitorBreakpoint(NativeThreadLinux &thread) {
thread.SetStoppedByBreakpoint();
FixupBreakpointPCAsNeeded(thread);
- if (m_threads_stepping_with_breakpoint.find(thread.GetID()) !=
- m_threads_stepping_with_breakpoint.end())
+ NativeRegisterContextLinux ®_ctx = thread.GetRegisterContext();
+ auto stepping_with_bp_it =
+ m_threads_stepping_with_breakpoint.find(thread.GetID());
+ if (stepping_with_bp_it != m_threads_stepping_with_breakpoint.end() &&
+ stepping_with_bp_it->second == reg_ctx.GetPC())
thread.SetStoppedByTrace();
StopRunningThreads(thread.GetID());
>From a82695c5aa1f6cb9746f51b44c4499d59b58f921 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Thu, 5 Sep 2024 17:39:59 -0700
Subject: [PATCH 4/7] Update debuginfo dexter stepping for new lldb behavior
dexter sets breakpoints on every source line in the file.
Then it step-in's to every source line, and tests are written
to expect that one step will move between lines, hitting the
breakpoints that are set there.
With this new algorithm, lldb will now step to the next source
line, and be stopped at a breakpoint, but it has not yet hit the
breakpoint. The stop reason will be "step-in completed". When
you do "step-in" again, we will hit the breakpoint instruction,
stop with a "breakpoint-hit" stop reason -- but the pc has not
advanced. Then when you "step-in" again, we will move to the
next source line.
In short, lldb needs to step twice in a situation with breakpoints
on every source line and stepping across them.
This patch detects when lldb is behaving this way, and preserves
the behavior the tests expect. It will step-in, and if we stop
with a step-in stop reason and we are stopped at a breakpoint site,
we will step once again to hit the breakpoint.
---
.../dexter/dex/debugger/lldb/LLDB.py | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
index 3944c1c4b009db..aec7eeb35817ff 100644
--- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
+++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
@@ -206,6 +206,28 @@ def launch(self, cmdline):
def step(self):
self._thread.StepInto()
+ stop_reason = self._thread.GetStopReason()
+ # If lldb has stepped to a breakpoint, but not
+ # executed it yet, lldb used to report eStopReasonBreakpoint
+ # even though the breakpoint hadn't been hit.
+ # Now lldb will report eStopReasonPlanComplete for
+ # the step completing, and when you step again, you will
+ # hit the breakpoint at this location -- because you have
+ # now actually executed the breakpoint instruction. This
+ # means that stepping past a source line with a breakpoint
+ # on the first instruction requires stepping twice. All of
+ # the debuginfo tests assume that it will succed with a single
+ # step. So we silently step again to hit the breakpoint, when
+ # we've stopped at a breakpoint location but haven't hit it yet.
+ if stop_reason == self._interface.eStopReasonPlanComplete:
+ stepped_to_breakpoint = False
+ pc = self._thread.GetFrameAtIndex(0).GetPC()
+ for bp in self._target.breakpoints:
+ for bploc in bp.locations:
+ if bploc.GetAddress().GetLoadAddress(self._target) == pc:
+ stepped_to_breakpoint = True
+ if stepped_to_breakpoint:
+ self._thread.StepInto()
def go(self) -> ReturnCode:
self._process.Continue()
>From 4468b354c4a7522560915a23ceeee5e88997b576 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Wed, 11 Sep 2024 15:13:17 -0700
Subject: [PATCH 5/7] Revert "Update debuginfo dexter stepping for new lldb
behavior"
This reverts commit a82695c5aa1f6cb9746f51b44c4499d59b58f921.
---
.../dexter/dex/debugger/lldb/LLDB.py | 22 -------------------
1 file changed, 22 deletions(-)
diff --git a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
index aec7eeb35817ff..3944c1c4b009db 100644
--- a/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
+++ b/cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py
@@ -206,28 +206,6 @@ def launch(self, cmdline):
def step(self):
self._thread.StepInto()
- stop_reason = self._thread.GetStopReason()
- # If lldb has stepped to a breakpoint, but not
- # executed it yet, lldb used to report eStopReasonBreakpoint
- # even though the breakpoint hadn't been hit.
- # Now lldb will report eStopReasonPlanComplete for
- # the step completing, and when you step again, you will
- # hit the breakpoint at this location -- because you have
- # now actually executed the breakpoint instruction. This
- # means that stepping past a source line with a breakpoint
- # on the first instruction requires stepping twice. All of
- # the debuginfo tests assume that it will succed with a single
- # step. So we silently step again to hit the breakpoint, when
- # we've stopped at a breakpoint location but haven't hit it yet.
- if stop_reason == self._interface.eStopReasonPlanComplete:
- stepped_to_breakpoint = False
- pc = self._thread.GetFrameAtIndex(0).GetPC()
- for bp in self._target.breakpoints:
- for bploc in bp.locations:
- if bploc.GetAddress().GetLoadAddress(self._target) == pc:
- stepped_to_breakpoint = True
- if stepped_to_breakpoint:
- self._thread.StepInto()
def go(self) -> ReturnCode:
self._process.Continue()
>From 5b5a5b6555740d6d9e2cc2c0da2cb8a965377990 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Fri, 13 Sep 2024 01:50:06 -0700
Subject: [PATCH 6/7] Revert "Add pc check for instruction stepping by
breakpoints"
Landing this in a separate PR
https://github.com/llvm/llvm-project/pull/108504
This reverts commit 48a0fc1ade6705fd53610cd6504e38ac98194e39.
---
.../Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp | 5 +----
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp | 7 ++-----
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 80b27571f43d55..97fff4b9f65a82 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -319,12 +319,9 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
info.pl_siginfo.si_addr);
if (thread) {
- auto ®ctx = static_cast<NativeRegisterContextFreeBSD &>(
- thread->GetRegisterContext());
auto thread_info =
m_threads_stepping_with_breakpoint.find(thread->GetID());
- if (thread_info != m_threads_stepping_with_breakpoint.end() &&
- threads_info->second == regctx.GetPC()) {
+ if (thread_info != m_threads_stepping_with_breakpoint.end()) {
thread->SetStoppedByTrace();
Status brkpt_error = RemoveBreakpoint(thread_info->second);
if (brkpt_error.Fail())
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 38b7092682873b..5c262db8db7fde 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -829,11 +829,8 @@ void NativeProcessLinux::MonitorBreakpoint(NativeThreadLinux &thread) {
thread.SetStoppedByBreakpoint();
FixupBreakpointPCAsNeeded(thread);
- NativeRegisterContextLinux ®_ctx = thread.GetRegisterContext();
- auto stepping_with_bp_it =
- m_threads_stepping_with_breakpoint.find(thread.GetID());
- if (stepping_with_bp_it != m_threads_stepping_with_breakpoint.end() &&
- stepping_with_bp_it->second == reg_ctx.GetPC())
+ if (m_threads_stepping_with_breakpoint.find(thread.GetID()) !=
+ m_threads_stepping_with_breakpoint.end())
thread.SetStoppedByTrace();
StopRunningThreads(thread.GetID());
>From 4e38a621287f3bd730bd3e0689e063b2bb7fa844 Mon Sep 17 00:00:00 2001
From: Jason Molenda <jmolenda at apple.com>
Date: Fri, 13 Sep 2024 01:58:48 -0700
Subject: [PATCH 7/7] Revert "Recognize `swbreak:`/`hwbreak:` in stop packet"
Breaking this patch out into a separate PR
https://github.com/llvm/llvm-project/pull/108518
This reverts commit 3408ce164ffc6857e968c4fff4d40ccd1a7014a5.
---
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b3badc88460212..f9e37a5f36d9f3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2286,8 +2286,6 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
StreamString ostr;
ostr.Printf("%" PRIu64, wp_addr);
description = std::string(ostr.GetString());
- } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
- reason = "breakpoint";
} else if (key.compare("library") == 0) {
auto error = LoadModules();
if (error) {
More information about the lldb-commits
mailing list