From 4f40123fbab7efd3958f3507c7e95854bdb065cb Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Fri, 13 Sep 2024 09:04:28 -0700 Subject: [PATCH 1/6] [lldb] Set the stop reason when receiving swbreak/hwbreak (#108518) xusheng added support for swbreak/hwbreak a month ago, and no special support was needed in ProcessGDBRemote when they're received because lldb already marks a thread as having hit a breakpoint when it stops at a breakpoint site. However, with changes I am working on, we need to know the real stop reason a thread stopped or the breakpoint hit will not be recognized. This is similar to how lldb processes the "watch/rwatch/awatch" keys in a thread stop packet -- we set the `reason` to `watchpoint`, and these set it to `breakpoint` so we set the stop reason correctly later in these methods. (cherry picked from commit 65a4d11b1e67429d53df1fcee0f93492aa95c448) --- 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 ba0384be51c70..dc0dc5f43088e 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -2273,6 +2273,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 acceb776f6099693c146ad16ea3e8076af9f09bd Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Fri, 13 Sep 2024 09:02:31 -0700 Subject: [PATCH 2/6] [lldb] Add pc check for thread-step-by-bp algorithms (#108504) lldb-server built with NativeProcessLinux.cpp and NativeProcessFreeBSD.cpp can use breakpoints to implement instruction stepping on cores where there is no native instruction-step primitive. Currently these set a breakpoint, continue, and if we hit the breakpoint with the original thread, set the stop reason to be "trace". I am wrapping up a change to lldb's breakpoint algorithm where I change its current behavior of "if a thread stops at a breakpoint site, we set the thread's stop reason to breakpoint-hit, even if the breakpoint hasn't been executed" + "when resuming any thread at a breakpoint site, instruction-step past the breakpoint before resuming" to a behavior of "when a thread executes a breakpoint, set the stop reason to breakpoint-hit" + "when a thread has hit a breakpoint, when the thread resumes, we silently step past the breakpoint and then resume the thread". For these lldb-server targets doing breakpoint stepping, this means that if we are sitting on a breakpoint that has not yet executed, and instruction-step the thread, we will execute the breakpoint instruction at $pc (instead of $next-pc where it meant to go), and stop again -- at the same pc value. Then we will rewrite the stop reason to 'trace'. The higher level logic will see that we haven't hit the breakpoint instruction again, so it will try to instruction step again, hitting the breakpoint again forever. To fix this, I'm checking that the thread matches the one we are instruction-stepping-by-breakpoint AND that we've stopped at the breakpoint address we are stepping to. Only in that case will the stop reason be rewritten to "trace" hiding the implementation detail that the step was done by breakpoints. (cherry picked from commit 213c59ddd2a702ddd3d849cea250440b1ed718e0) --- .../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 97fff4b9f65a8..80b27571f43d5 100644 --- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp @@ -319,9 +319,12 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) { info.pl_siginfo.si_addr); if (thread) { + auto ®ctx = static_cast( + 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 573f4185ac651..68c94425aa662 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 fd6515bae117e879052ff3b50a580706ee6ee9e9 Mon Sep 17 00:00:00 2001 From: Kelvin Lee Date: Mon, 23 Sep 2024 20:44:58 +1000 Subject: [PATCH 3/6] [lldb][FreeBSD] Fix a typo in NativeProcessFreeBSD::MonitorSIGTRAP() (#109643) Apparently a typo is causing compile error, added by https://github.com/llvm/llvm-project/pull/108504. (cherry picked from commit 85220a0c651e565ab576c5be17c1ec5c9eb2a350) --- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp index 80b27571f43d5..bf552e19742c4 100644 --- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp @@ -324,7 +324,7 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) { 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()) { + thread_info->second == regctx.GetPC()) { thread->SetStoppedByTrace(); Status brkpt_error = RemoveBreakpoint(thread_info->second); if (brkpt_error.Fail()) From e0e0e21437959f2135b0ac80f3ae8a0e600f3b90 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Wed, 11 Sep 2024 16:09:48 -0700 Subject: [PATCH 4/6] [Dexter] Adapt to upcoming lldb stepping behavior (#108127) lldb will change how it reports stop reasons around breakpoints in the near future. I landed an earlier version of this change and noticed debuginfo test failures on the CI bots due to the changes. I'm addressing the issues found by CI at https://github.com/llvm/llvm-project/pull/105594 and will re-land once I've done all of them. Currently, when lldb stops at a breakpoint instruction -- but has not yet executed the instruction -- it will overwrite the thread's Stop Reason with "breakpoint-hit". This caused bugs when the original stop reason was important to the user - for instance, a watchpoint on an AArch64 system where we have to instruction-step past the watchpoint to find the new value. Normally we would instruction step, fetch the new value, then report the user that a watchpoint has been hit with the old and new values. But if the instruction after this access is a breakpoint site, we overwrite the "watchpoint hit" stop reason (and related actions) with "breakpoint hit". dexter sets breakpoints on all source lines, then steps line-to-line, hitting the breakpoints. But with this new behavior, we see two steps per source line: The first step gets us to the start of the next line, with a "step completed" stop reason. Then we step again and we execute the breakpoint instruction, stop with the pc the same, and report "breakpoint hit". Now we can step a second time and move past the breakpoint. I've changed the `step` method in LLDB.py to check if we step to a breakpoint site but have a "step completed" stop reason -- in which case we have this new breakpoint behavior, and we need to step a second time to actually hit the breakpoint like the debuginfo tests expect. (cherry picked from commit 93e45a69dde16e6a3ac0ddbcc596ac3843d59c43) --- .../dexter/dex/debugger/lldb/LLDB.py | 27 +++++++++++++++++++ 1 file changed, 27 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 3944c1c4b009d..2307550aca047 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,33 @@ def launch(self, cmdline): def step(self): self._thread.StepInto() + stop_reason = self._thread.GetStopReason() + # If we (1) completed a step and (2) are sitting at a breakpoint, + # but (3) the breakpoint is not reported as the stop reason, then + # we'll need to step once more to hit the breakpoint. + # + # dexter sets breakpoints on every source line, then steps + # each source line. Older lldb's would overwrite the stop + # reason with "breakpoint hit" when we stopped at a breakpoint, + # even if the breakpoint hadn't been exectued yet. One + # step per source line, hitting a breakpoint each time. + # + # But a more accurate behavior is that the step completes + # with step-completed stop reason, then when we step again, + # we execute the breakpoint and stop (with the pc the same) and + # a breakpoint-hit stop reason. So we need to step twice per line. + 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.IsEnabled() + and 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 288e3238a175404b848f7ebf409763c9ff1d9d59 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 13 Feb 2025 11:30:10 -0800 Subject: [PATCH 5/6] [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 (cherry picked from commit b666ac3b63e01bfa602318c877ea2395fea53f89) --- lldb/include/lldb/Target/Thread.h | 24 ++ .../Process/Utility/StopInfoMachException.cpp | 315 ++++++++---------- .../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, 263 insertions(+), 261 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index b03e8115c7c52..78686d267d82e 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -129,6 +129,7 @@ class Thread : public std::enable_shared_from_this, 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 @@ -381,6 +382,26 @@ class Thread : public std::enable_shared_from_this, 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 @@ -1367,6 +1388,9 @@ class Thread : public std::enable_shared_from_this, 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 698ba0f0f720f..29a64a2a03bf0 100644 --- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp +++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp @@ -491,38 +491,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) { @@ -610,6 +578,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 @@ -636,166 +623,154 @@ 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, ] Same as software breakpoint! + // + // Software breakpoint: + // x86 [6, 2, 0] + // Intel KDP [6, 2, ] + // arm64 [6, 1, ] + // armv7 [6, 0x102, ] Same as instruction step! + // + // Hardware breakpoint: + // x86 [6, 1, , 0] + // x86/Rosetta not implemented, see software breakpoint + // arm64 [6, 1, ] + // armv7 not implemented, see software breakpoint + // + // Hardware watchpoint: + // x86 [6, 1, , 0] (both Intel hw and Rosetta) + // arm64 [6, 0x102, , 0] + // armv7 [6, 0x102, , 0] + // + // arm64 BRK instruction (imm arg not reflected in the ME) + // [ 6, 1, ] + // + // In order of codes mach exceptions: + // [6, 1, 0] - instruction step + // [6, 1, ] - hardware breakpoint or watchpoint + // + // [6, 2, 0] - software breakpoint + // [6, 2, ] - software breakpoint + // + // [6, 3] - instruction step + // + // [6, 0x102, ] armv7 instruction step + // [6, 0x102, ] armv7 software breakpoint + // [6, 0x102, , 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 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 (bp_site_sp->ValidForThisThread(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 (bp_site_sp->ValidForThisThread(thread)) { + // Update the PC if we were asked to do so, but only do so if we find + // a breakpoint that we know about because 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 b25068dda53ff..8f8d4e2162c78 100644 --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp @@ -378,24 +378,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( stop_thread->GetRegisterContext().get()); uint32_t slot_id = reg_ctx->GetTriggeredHardwareBreakpointSlotId(); @@ -420,8 +413,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: @@ -444,9 +435,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 " @@ -454,6 +445,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 dc0dc5f43088e..710bf1531ba33 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1600,29 +1600,8 @@ bool ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) { // If we have "jstopinfo" then we have stop descriptions for all threads // that have stop reasons, and if there is no entry for a thread, then it // has no stop reason. - 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; } @@ -1727,6 +1706,12 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo( if (!thread_sp->StopInfoIsUpToDate()) { thread_sp->SetStopInfo(StopInfoSP()); + 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) { const size_t exc_data_size = exc_data.size(); @@ -1743,26 +1728,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. @@ -1878,39 +1847,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); @@ -1924,8 +1895,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 88a4ca3b0389f..d0d1508e85172 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 bd086550f4d82..31fbaf88803b2 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -1426,6 +1426,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 662831866bc66..baee2c530c54e 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -222,6 +222,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(), @@ -548,6 +549,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; } @@ -583,6 +585,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 { @@ -659,7 +662,15 @@ bool 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. @@ -750,6 +761,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); } @@ -1977,6 +1989,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 73de4a294388b..ecea28c6e1f6d 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 3a7440a31677a..1315288b35c55 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 1aad387c11e064357b62a33cc9b779ffbf9287f0 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Wed, 12 Feb 2025 14:00:41 -0800 Subject: [PATCH 6/6] [lldb] inserted a typeo when checking in a suggested fix (cherry picked from commit fa71238da800f3a818ec0e0649462389dc577890) --- lldb/source/Target/ThreadPlanStepOut.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 4a633467ee164..0b90146da0f4b 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -398,8 +398,11 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) { } if (!done) { - StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); - done = !IsYounger(frame_zero_id, m_step_out_to_id); + StopInfoSP stop_info_sp = GetPrivateStopInfo(); + if (stop_info_sp && stop_info_sp->GetStopReason() == eStopReasonBreakpoint) { + StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); + done = !IsYounger(frame_zero_id, m_step_out_to_id); + } } // The normal step out computations think we are done, so all we need to do