diff --git a/lldb/include/lldb/API/SBThreadPlan.h b/lldb/include/lldb/API/SBThreadPlan.h index d02ab80f76a76..9b7ac03d8265c 100644 --- a/lldb/include/lldb/API/SBThreadPlan.h +++ b/lldb/include/lldb/API/SBThreadPlan.h @@ -100,6 +100,7 @@ class LLDB_API SBThreadPlan { lldb::addr_t range_size, SBError &error); + // Note: first_insn is not used. SBThreadPlan QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to, bool first_insn = false); SBThreadPlan QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to, diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 6ede7fa301a82..1454f2ff4d18a 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -843,18 +843,6 @@ class Thread : public std::enable_shared_from_this, /// this one. /// Otherwise this plan will go on the end of the plan stack. /// - /// \param[in] addr_context - /// When dealing with stepping through inlined functions the current PC is - /// not enough information to know - /// what "step" means. For instance a series of nested inline functions - /// might start at the same address. - // The \a addr_context provides the current symbol context the step - /// is supposed to be out of. - // FIXME: Currently unused. - /// - /// \param[in] first_insn - /// \b true if this is the first instruction of a function. - /// /// \param[in] stop_other_threads /// \b true if we will stop other threads while we single step this one. /// @@ -876,9 +864,8 @@ class Thread : public std::enable_shared_from_this, /// A shared pointer to the newly queued thread plan, or nullptr if the /// plan could not be queued. virtual lldb::ThreadPlanSP QueueThreadPlanForStepOut( - bool abort_other_plans, SymbolContext *addr_context, bool first_insn, - bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, + bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, Status &status, LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate); /// Queue the plan used to step out of the function at the current PC of @@ -892,18 +879,6 @@ class Thread : public std::enable_shared_from_this, /// this one. /// Otherwise this plan will go on the end of the plan stack. /// - /// \param[in] addr_context - /// When dealing with stepping through inlined functions the current PC is - /// not enough information to know - /// what "step" means. For instance a series of nested inline functions - /// might start at the same address. - // The \a addr_context provides the current symbol context the step - /// is supposed to be out of. - // FIXME: Currently unused. - /// - /// \param[in] first_insn - /// \b true if this is the first instruction of a function. - /// /// \param[in] stop_other_threads /// \b true if we will stop other threads while we single step this one. /// @@ -940,9 +915,9 @@ class Thread : public std::enable_shared_from_this, /// A shared pointer to the newly queued thread plan, or nullptr if the /// plan could not be queued. virtual lldb::ThreadPlanSP QueueThreadPlanForStepOutNoShouldStop( - bool abort_other_plans, SymbolContext *addr_context, bool first_insn, - bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, bool continue_to_next_branch = false); + bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, Status &status, + bool continue_to_next_branch = false); /// Gets the plan used to step through the code that steps from a function /// call site at the current PC into the actual function call. diff --git a/lldb/include/lldb/Target/ThreadPlanStepOut.h b/lldb/include/lldb/Target/ThreadPlanStepOut.h index 013c675afc33d..f37d09467dda3 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOut.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOut.h @@ -17,8 +17,7 @@ namespace lldb_private { class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere { public: - ThreadPlanStepOut(Thread &thread, SymbolContext *addr_context, - bool first_insn, bool stop_others, Vote report_stop_vote, + ThreadPlanStepOut(Thread &thread, bool stop_others, Vote report_stop_vote, Vote report_run_vote, uint32_t frame_idx, LazyBool step_out_avoids_code_without_debug_info, bool continue_to_next_branch = false, @@ -76,12 +75,15 @@ class ThreadPlanStepOut : public ThreadPlan, public ThreadPlanShouldStopHere { StreamString m_constructor_errors; friend lldb::ThreadPlanSP Thread::QueueThreadPlanForStepOut( - bool abort_other_plans, SymbolContext *addr_context, bool first_insn, - bool stop_others, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, + bool abort_other_plans, bool stop_others, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, Status &status, LazyBool step_out_avoids_code_without_debug_info); void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info); + + void SetupReturnAddress(lldb::StackFrameSP return_frame_sp, + lldb::StackFrameSP immediate_return_from_sp, + uint32_t frame_idx, bool continue_to_next_branch); // Need an appropriate marker for the current stack so we can tell step out // from step in. diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp index d9469fc1390d6..315ca201830a9 100644 --- a/lldb/source/API/SBThread.cpp +++ b/lldb/source/API/SBThread.cpp @@ -659,8 +659,8 @@ void SBThread::StepOut(SBError &error) { const LazyBool avoid_no_debug = eLazyBoolCalculate; Status new_plan_status; ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut( - abort_other_plans, nullptr, false, stop_other_threads, eVoteYes, - eVoteNoOpinion, 0, new_plan_status, avoid_no_debug)); + abort_other_plans, stop_other_threads, eVoteYes, eVoteNoOpinion, 0, + new_plan_status, avoid_no_debug)); if (new_plan_status.Success()) error = ResumeNewPlan(exe_ctx, new_plan_sp.get()); @@ -703,8 +703,8 @@ void SBThread::StepOutOfFrame(SBFrame &sb_frame, SBError &error) { Status new_plan_status; ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut( - abort_other_plans, nullptr, false, stop_other_threads, eVoteYes, - eVoteNoOpinion, frame_sp->GetFrameIndex(), new_plan_status)); + abort_other_plans, stop_other_threads, eVoteYes, eVoteNoOpinion, + frame_sp->GetFrameIndex(), new_plan_status)); if (new_plan_status.Success()) error = ResumeNewPlan(exe_ctx, new_plan_sp.get()); diff --git a/lldb/source/API/SBThreadPlan.cpp b/lldb/source/API/SBThreadPlan.cpp index 083a050de8a38..135ba77a38ace 100644 --- a/lldb/source/API/SBThreadPlan.cpp +++ b/lldb/source/API/SBThreadPlan.cpp @@ -312,8 +312,8 @@ SBThreadPlan::QueueThreadPlanForStepOut(uint32_t frame_idx_to_step_to, Status plan_status; SBThreadPlan plan = SBThreadPlan(thread_plan_sp->GetThread().QueueThreadPlanForStepOut( - false, &sc, first_insn, false, eVoteYes, eVoteNoOpinion, - frame_idx_to_step_to, plan_status)); + false, false, eVoteYes, eVoteNoOpinion, frame_idx_to_step_to, + plan_status)); if (plan_status.Fail()) error.SetErrorString(plan_status.AsCString()); diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 224c523e69c5c..e8224b8fe0f1e 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -558,8 +558,7 @@ class CommandObjectThreadStepWithTypeAndScope : public CommandObjectParsed { true, abort_other_plans, bool_stop_other_threads, new_plan_status); } else if (m_step_type == eStepTypeOut) { new_plan_sp = thread->QueueThreadPlanForStepOut( - abort_other_plans, nullptr, false, bool_stop_other_threads, eVoteYes, - eVoteNoOpinion, + abort_other_plans, bool_stop_other_threads, eVoteYes, eVoteNoOpinion, thread->GetSelectedFrameIndex(DoNoSelectMostRelevantFrame), new_plan_status, m_options.m_step_out_avoid_no_debug); } else if (m_step_type == eStepTypeScripted) { diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp index 4093cbdd955f7..a24240fc7cf31 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp @@ -161,11 +161,10 @@ bool AppleThreadPlanStepThroughObjCTrampoline::ShouldStop(Event *event_ptr) { eSymbolContextEverything); Status status; const bool abort_other_plans = false; - const bool first_insn = true; const uint32_t frame_idx = 0; m_run_to_sp = GetThread().QueueThreadPlanForStepOutNoShouldStop( - abort_other_plans, &sc, first_insn, false, eVoteNoOpinion, - eVoteNoOpinion, frame_idx, status); + abort_other_plans, false, eVoteNoOpinion, eVoteNoOpinion, frame_idx, + status); if (m_run_to_sp && status.Success()) m_run_to_sp->SetPrivate(true); return false; @@ -242,8 +241,7 @@ AppleThreadPlanStepThroughDirectDispatch :: AppleThreadPlanStepThroughDirectDispatch( Thread &thread, AppleObjCTrampolineHandler &handler, llvm::StringRef dispatch_func_name) - : ThreadPlanStepOut(thread, nullptr, true /* first instruction */, false, - eVoteNoOpinion, eVoteNoOpinion, + : ThreadPlanStepOut(thread, false, eVoteNoOpinion, eVoteNoOpinion, 0 /* Step out of zeroth frame */, eLazyBoolNo /* Our parent plan will decide this when we are done */ diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index accc4708c24e1..ddd4315b6cc9e 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -1341,28 +1341,26 @@ ThreadPlanSP Thread::QueueThreadPlanForStepInRange( } ThreadPlanSP Thread::QueueThreadPlanForStepOut( - bool abort_other_plans, SymbolContext *addr_context, bool first_insn, - bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, + bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, Status &status, LazyBool step_out_avoids_code_without_debug_info) { ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut( - *this, addr_context, first_insn, stop_other_threads, report_stop_vote, - report_run_vote, frame_idx, step_out_avoids_code_without_debug_info)); + *this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx, + step_out_avoids_code_without_debug_info)); status = QueueThreadPlan(thread_plan_sp, abort_other_plans); return thread_plan_sp; } ThreadPlanSP Thread::QueueThreadPlanForStepOutNoShouldStop( - bool abort_other_plans, SymbolContext *addr_context, bool first_insn, - bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote, - uint32_t frame_idx, Status &status, bool continue_to_next_branch) { + bool abort_other_plans, bool stop_other_threads, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, Status &status, + bool continue_to_next_branch) { const bool calculate_return_value = false; // No need to calculate the return value here. ThreadPlanSP thread_plan_sp(new ThreadPlanStepOut( - *this, addr_context, first_insn, stop_other_threads, report_stop_vote, - report_run_vote, frame_idx, eLazyBoolNo, continue_to_next_branch, - calculate_return_value)); + *this, stop_other_threads, report_stop_vote, report_run_vote, frame_idx, + eLazyBoolNo, continue_to_next_branch, calculate_return_value)); ThreadPlanStepOut *new_plan = static_cast(thread_plan_sp.get()); @@ -2035,13 +2033,12 @@ Status Thread::StepOut(uint32_t frame_idx) { Status error; Process *process = GetProcess().get(); if (StateIsStoppedState(process->GetState(), true)) { - const bool first_instruction = false; const bool stop_other_threads = false; const bool abort_other_plans = false; - ThreadPlanSP new_plan_sp(QueueThreadPlanForStepOut( - abort_other_plans, nullptr, first_instruction, stop_other_threads, - eVoteYes, eVoteNoOpinion, frame_idx, error)); + ThreadPlanSP new_plan_sp = + QueueThreadPlanForStepOut(abort_other_plans, stop_other_threads, + eVoteYes, eVoteNoOpinion, frame_idx, error); new_plan_sp->SetIsControllingPlan(true); new_plan_sp->SetOkayToDiscard(false); diff --git a/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp b/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp index 4bccf96d721b8..c8783b59fd12f 100644 --- a/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp +++ b/lldb/source/Target/ThreadPlanCallOnFunctionExit.cpp @@ -29,8 +29,6 @@ void ThreadPlanCallOnFunctionExit::DidPush() { Status status; m_step_out_threadplan_sp = GetThread().QueueThreadPlanForStepOut( false, // abort other plans - nullptr, // addr_context - true, // first instruction true, // stop other threads eVoteNo, // do not say "we're stopping" eVoteNoOpinion, // don't care about run state broadcasting diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp index d2cca49987f0f..153de77ebf596 100644 --- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp +++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp @@ -176,8 +176,8 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback( if (!return_plan_sp) return_plan_sp = current_plan->GetThread().QueueThreadPlanForStepOutNoShouldStop( - false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, - frame_index, status, true); + false, stop_others, eVoteNo, eVoteNoOpinion, frame_index, status, + true); return return_plan_sp; } diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp index 42bee79c42bbd..ff6a11703570c 100644 --- a/lldb/source/Target/ThreadPlanStepInstruction.cpp +++ b/lldb/source/Target/ThreadPlanStepInstruction.cpp @@ -198,8 +198,7 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { // for now it is safer to run others. const bool stop_others = false; thread.QueueThreadPlanForStepOutNoShouldStop( - false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0, - m_status); + false, stop_others, eVoteNo, eVoteNoOpinion, 0, m_status); return false; } else { if (log) { diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index a05c46db6b8ca..546405c267601 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -31,10 +31,36 @@ using namespace lldb_private; uint32_t ThreadPlanStepOut::s_default_flag_values = 0; +/// Computes the target frame this plan should step out to. +static StackFrameSP +ComputeTargetFrame(Thread &thread, uint32_t start_frame_idx, + std::vector &skipped_frames) { + uint32_t frame_idx = start_frame_idx + 1; + StackFrameSP return_frame_sp = thread.GetStackFrameAtIndex(frame_idx); + if (!return_frame_sp) + return nullptr; + + while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) { + skipped_frames.push_back(return_frame_sp); + + frame_idx++; + return_frame_sp = thread.GetStackFrameAtIndex(frame_idx); + + // We never expect to see an artificial frame without a regular ancestor. + // Defensively refuse to step out. + if (!return_frame_sp) { + LLDB_LOG(GetLog(LLDBLog::Step), + "Can't step out of frame with artificial ancestors"); + return nullptr; + } + } + return return_frame_sp; +} + // ThreadPlanStepOut: Step out of the current frame ThreadPlanStepOut::ThreadPlanStepOut( - Thread &thread, SymbolContext *context, bool first_insn, bool stop_others, - Vote report_stop_vote, Vote report_run_vote, uint32_t frame_idx, + Thread &thread, bool stop_others, Vote report_stop_vote, + Vote report_run_vote, uint32_t frame_idx, LazyBool step_out_avoids_code_without_debug_info, bool continue_to_next_branch, bool gather_return_value) : ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote, @@ -44,34 +70,25 @@ ThreadPlanStepOut::ThreadPlanStepOut( m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others), m_immediate_step_from_function(nullptr), m_calculate_return_value(gather_return_value) { - Log *log = GetLog(LLDBLog::Step); SetFlagsToDefault(); SetupAvoidNoDebug(step_out_avoids_code_without_debug_info); m_step_from_insn = thread.GetRegisterContext()->GetPC(0); - uint32_t return_frame_index = frame_idx + 1; - StackFrameSP return_frame_sp(thread.GetStackFrameAtIndex(return_frame_index)); + StackFrameSP return_frame_sp = + ComputeTargetFrame(thread, frame_idx, m_stepped_past_frames); StackFrameSP immediate_return_from_sp(thread.GetStackFrameAtIndex(frame_idx)); + SetupReturnAddress(return_frame_sp, immediate_return_from_sp, + frame_idx, continue_to_next_branch); +} + +void ThreadPlanStepOut::SetupReturnAddress( + StackFrameSP return_frame_sp, StackFrameSP immediate_return_from_sp, + uint32_t frame_idx, bool continue_to_next_branch) { if (!return_frame_sp || !immediate_return_from_sp) return; // we can't do anything here. ValidatePlan() will return false. - // While stepping out, behave as-if artificial frames are not present. - while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) { - m_stepped_past_frames.push_back(return_frame_sp); - - ++return_frame_index; - return_frame_sp = thread.GetStackFrameAtIndex(return_frame_index); - - // We never expect to see an artificial frame without a regular ancestor. - // If this happens, log the issue and defensively refuse to step out. - if (!return_frame_sp) { - LLDB_LOG(log, "Can't step out of frame with artificial ancestors"); - return; - } - } - m_step_out_to_id = return_frame_sp->GetStackID(); m_immediate_step_from_id = immediate_return_from_sp->GetStackID(); @@ -84,7 +101,7 @@ ThreadPlanStepOut::ThreadPlanStepOut( // First queue a plan that gets us to this inlined frame, and when we get // there we'll queue a second plan that walks us out of this frame. m_step_out_to_inline_plan_sp = std::make_shared( - thread, nullptr, false, stop_others, eVoteNoOpinion, eVoteNoOpinion, + GetThread(), m_stop_others, eVoteNoOpinion, eVoteNoOpinion, frame_idx - 1, eLazyBoolNo, continue_to_next_branch); static_cast(m_step_out_to_inline_plan_sp.get()) ->SetShouldStopHereCallbacks(nullptr, nullptr); @@ -96,7 +113,6 @@ ThreadPlanStepOut::ThreadPlanStepOut( } } else { // Find the return address and set a breakpoint there: - // FIXME - can we do this more securely if we know first_insn? Address return_address(return_frame_sp->GetFrameCodeAddress()); if (continue_to_next_branch) { @@ -125,6 +141,7 @@ ThreadPlanStepOut::ThreadPlanStepOut( // Perform some additional validation on the return address. uint32_t permissions = 0; + Log *log = GetLog(LLDBLog::Step); if (!m_process.GetLoadAddressPermissions(m_return_addr, permissions)) { LLDB_LOGF(log, "ThreadPlanStepOut(%p): Return address (0x%" PRIx64 ") permissions not found.", static_cast(this), diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp index 643ee827c865c..60a623e603e76 100644 --- a/lldb/source/Target/ThreadPlanStepOverRange.cpp +++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp @@ -192,8 +192,7 @@ bool ThreadPlanStepOverRange::ShouldStop(Event *event_ptr) { if (m_next_branch_bp_sp) return false; new_plan_sp = thread.QueueThreadPlanForStepOutNoShouldStop( - false, nullptr, true, stop_others, eVoteNo, eVoteNoOpinion, 0, - m_status, true); + false, stop_others, eVoteNo, eVoteNoOpinion, 0, m_status, true); break; } else { new_plan_sp = thread.QueueThreadPlanForStepThrough(