diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 2ff1f50d497e2..d3e429e9256df 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 @@ -378,6 +379,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 @@ -1312,6 +1333,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 25cee369d7ee3..f1853be12638e 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, ] 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 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 f383b3d40a4f3..8b12b87eacbc6 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( 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 604c92369e9a2..9ab03c1fb8ff3 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) { const size_t exc_data_size = exc_data.size(); @@ -1745,26 +1730,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. @@ -1880,39 +1849,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); @@ -1926,8 +1897,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 95f78056b1644..895306b5ef0d4 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -1365,6 +1365,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 e75f5a356cec2..2105efe9305fe 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -217,6 +217,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(), @@ -519,6 +520,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; } @@ -554,6 +556,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 { @@ -622,7 +625,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. @@ -711,6 +722,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); } @@ -1894,6 +1906,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)