diff --git a/lldb/include/lldb/Target/StackID.h b/lldb/include/lldb/Target/StackID.h index 11f4e9af44207..a965c3f6c7225 100644 --- a/lldb/include/lldb/Target/StackID.h +++ b/lldb/include/lldb/Target/StackID.h @@ -36,7 +36,7 @@ class StackID { void Clear() { m_pc = LLDB_INVALID_ADDRESS; m_cfa = LLDB_INVALID_ADDRESS; - m_cfa_on_stack = true; + m_cfa_on_stack = eLazyBoolCalculate; m_symbol_scope = nullptr; } @@ -57,15 +57,18 @@ class StackID { return *this; } - bool IsCFAOnStack() const { return m_cfa_on_stack; } + /// Check if the CFA is on the stack, or elsewhere in the process, such as on + /// the heap. + bool IsCFAOnStack(Process &process) const; + + /// Determine if the first StackID is "younger" than the second. + static bool IsYounger(const StackID &lhs, const StackID &rhs, + Process &process); protected: friend class StackFrame; - explicit StackID(lldb::addr_t pc, lldb::addr_t cfa, lldb::ThreadSP thread_sp) - : m_pc(pc), m_cfa(cfa), m_cfa_on_stack(IsStackAddress(cfa, thread_sp)) {} - - bool IsStackAddress(lldb::addr_t addr, lldb::ThreadSP thread_sp) const; + explicit StackID(lldb::addr_t pc, lldb::addr_t cfa) : m_pc(pc), m_cfa(cfa) {} void SetPC(lldb::addr_t pc) { m_pc = pc; } @@ -84,7 +87,7 @@ class StackID { // below) // True if the CFA is an address on the stack, false if it's an address // elsewhere (ie heap). - bool m_cfa_on_stack = true; + mutable LazyBool m_cfa_on_stack = eLazyBoolCalculate; SymbolContextScope *m_symbol_scope = nullptr; // If nullptr, there is no block or symbol for this frame. // If not nullptr, this will either be the scope for the @@ -98,9 +101,6 @@ class StackID { bool operator==(const StackID &lhs, const StackID &rhs); bool operator!=(const StackID &lhs, const StackID &rhs); -// frame_id_1 < frame_id_2 means "frame_id_1 is YOUNGER than frame_id_2" -bool operator<(const StackID &lhs, const StackID &rhs); - } // namespace lldb_private #endif // LLDB_TARGET_STACKID_H diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h index cd5d4a4ecc1f8..28b53a14ad746 100644 --- a/lldb/include/lldb/Target/ThreadPlan.h +++ b/lldb/include/lldb/Target/ThreadPlan.h @@ -555,6 +555,28 @@ class ThreadPlan : public std::enable_shared_from_this, bool IsUsuallyUnexplainedStopReason(lldb::StopReason); + /// Determine if the first StackID is younger than the second. + /// + /// A callee is considered younger than its caller, and is also younger than + /// all ancestor frames leading up to the caller. Consider this stack: + /// + /// +------------------+ + /// | Fa | + /// +------------------+ + /// | Fb | + /// +------------------+ + /// | ... | + /// +------------------+ + /// | Fy | + /// +------------------+ + /// | Fz | + /// +------------------+ + /// + /// In this case Fz is younger than each of Fy, ..., Fb, and Fa. Fy is not + /// younger than Fz, but is younger than all frames above it in the stack, + /// including Fa and Fb. + bool IsYounger(const StackID &lhs, const StackID &rhs) const; + Status m_status; Process &m_process; lldb::tid_t m_tid; diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 1a81e3ff4b527..f27fab360db31 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -57,8 +57,8 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, const SymbolContext *sc_ptr) : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(), - m_id(pc, cfa, thread_sp), m_frame_code_addr(pc), m_sc(), m_flags(), - m_frame_base(), m_frame_base_error(), m_cfa_is_valid(cfa_is_valid), + m_id(pc, cfa), m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(), + m_frame_base_error(), m_cfa_is_valid(cfa_is_valid), m_stack_frame_kind(kind), m_behaves_like_zeroth_frame(behaves_like_zeroth_frame), m_variable_list_sp(), m_variable_list_value_objects(), @@ -83,10 +83,9 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, const SymbolContext *sc_ptr) : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), - m_reg_context_sp(reg_context_sp), m_id(pc, cfa, thread_sp), - m_frame_code_addr(pc), m_sc(), m_flags(), m_frame_base(), - m_frame_base_error(), m_cfa_is_valid(true), - m_stack_frame_kind(StackFrame::Kind::Regular), + m_reg_context_sp(reg_context_sp), m_id(pc, cfa), m_frame_code_addr(pc), + m_sc(), m_flags(), m_frame_base(), m_frame_base_error(), + m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular), m_behaves_like_zeroth_frame(behaves_like_zeroth_frame), m_variable_list_sp(), m_variable_list_value_objects(), m_recognized_frame_sp(), m_disassembly(), m_mutex() { @@ -110,8 +109,7 @@ StackFrame::StackFrame(const ThreadSP &thread_sp, user_id_t frame_idx, : m_thread_wp(thread_sp), m_frame_index(frame_idx), m_concrete_frame_index(unwind_frame_index), m_reg_context_sp(reg_context_sp), - m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa, - thread_sp), + m_id(pc_addr.GetLoadAddress(thread_sp->CalculateTarget().get()), cfa), m_frame_code_addr(pc_addr), m_sc(), m_flags(), m_frame_base(), m_frame_base_error(), m_cfa_is_valid(true), m_stack_frame_kind(StackFrame::Kind::Regular), diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index 2196f048ebc47..8cf2d1068ca0d 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -17,18 +17,17 @@ using namespace lldb_private; -bool StackID::IsStackAddress(lldb::addr_t addr, - lldb::ThreadSP thread_sp) const { - if (addr == LLDB_INVALID_ADDRESS) - return false; - - if (thread_sp) - if (auto process_sp = thread_sp->GetProcess()) { +bool StackID::IsCFAOnStack(Process &process) const { + if (m_cfa_on_stack == eLazyBoolCalculate) { + m_cfa_on_stack = eLazyBoolNo; + if (m_cfa != LLDB_INVALID_ADDRESS) { MemoryRegionInfo mem_info; - if (process_sp->GetMemoryRegionInfo(addr, mem_info).Success()) - return mem_info.IsStackMemory() == MemoryRegionInfo::eYes; + if (process.GetMemoryRegionInfo(m_cfa, mem_info).Success()) + if (mem_info.IsStackMemory() == MemoryRegionInfo::eYes) + m_cfa_on_stack = eLazyBoolYes; } - return true; // assumed + } + return m_cfa_on_stack == eLazyBoolYes; } void StackID::Dump(Stream *s) { @@ -74,19 +73,20 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { return lhs_scope != rhs_scope; } -bool lldb_private::operator<(const StackID &lhs, const StackID &rhs) { - const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress(); - const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress(); - +bool StackID::IsYounger(const StackID &lhs, const StackID &rhs, + Process &process) { // FIXME: rdar://76119439 - // Heuristic: At the boundary between an async parent frame calling a regular - // child frame, the CFA of the parent async function is a heap addresses, and - // the CFA of concrete child function is a stack addresses. Therefore, if lhs - // is on stack, and rhs is not, lhs is considered less than rhs (regardless of - // address values). - if (lhs.IsCFAOnStack() && !rhs.IsCFAOnStack()) + // At the boundary between an async parent frame calling a regular child + // frame, the CFA of the parent async function is a heap addresses, and the + // CFA of concrete child function is a stack address. Therefore, if lhs is + // on stack, and rhs is not, lhs is considered less than rhs, independent of + // address values. + if (lhs.IsCFAOnStack(process) && !rhs.IsCFAOnStack(process)) return true; + const lldb::addr_t lhs_cfa = lhs.GetCallFrameAddress(); + const lldb::addr_t rhs_cfa = rhs.GetCallFrameAddress(); + // FIXME: We are assuming that the stacks grow downward in memory. That's not // necessary, but true on // all the machines we care about at present. If this changes, we'll have to diff --git a/lldb/source/Target/ThreadPlan.cpp b/lldb/source/Target/ThreadPlan.cpp index 7927fc3140145..9f7ada5844f82 100644 --- a/lldb/source/Target/ThreadPlan.cpp +++ b/lldb/source/Target/ThreadPlan.cpp @@ -180,6 +180,10 @@ bool ThreadPlan::IsUsuallyUnexplainedStopReason(lldb::StopReason reason) { } } +bool ThreadPlan::IsYounger(const StackID &lhs, const StackID &rhs) const { + return StackID::IsYounger(lhs, rhs, m_process); +} + // ThreadPlanNull ThreadPlanNull::ThreadPlanNull(Thread &thread) diff --git a/lldb/source/Target/ThreadPlanStepInstruction.cpp b/lldb/source/Target/ThreadPlanStepInstruction.cpp index 42bee79c42bbd..ba64a47c6f2fe 100644 --- a/lldb/source/Target/ThreadPlanStepInstruction.cpp +++ b/lldb/source/Target/ThreadPlanStepInstruction.cpp @@ -110,7 +110,7 @@ bool ThreadPlanStepInstruction::IsPlanStale() { SetPlanComplete(); } return (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr); - } else if (cur_frame_id < m_stack_id) { + } else if (IsYounger(cur_frame_id, m_stack_id)) { // If the current frame is younger than the start frame and we are stepping // over, then we need to continue, but if we are doing just one step, we're // done. @@ -140,7 +140,8 @@ bool ThreadPlanStepInstruction::ShouldStop(Event *event_ptr) { StackID cur_frame_zero_id = cur_frame_sp->GetStackID(); - if (cur_frame_zero_id == m_stack_id || m_stack_id < cur_frame_zero_id) { + if (cur_frame_zero_id == m_stack_id || + IsYounger(m_stack_id, cur_frame_zero_id)) { if (thread.GetRegisterContext()->GetPC(0) != m_instruction_addr) { if (--m_iteration_count <= 0) { SetPlanComplete(); diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 0ae92f29cb4ba..b4cef55d15dc3 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -334,12 +334,12 @@ bool ThreadPlanStepOut::DoPlanExplainsStop(Event *event_ptr) { if (m_step_out_to_id == frame_zero_id) done = true; - else if (m_step_out_to_id < frame_zero_id) { + else if (IsYounger(m_step_out_to_id, frame_zero_id)) { // Either we stepped past the breakpoint, or the stack ID calculation // was incorrect and we should probably stop. done = true; } else { - done = (m_immediate_step_from_id < frame_zero_id); + done = IsYounger(m_immediate_step_from_id, frame_zero_id); } if (done) { @@ -398,7 +398,7 @@ bool ThreadPlanStepOut::ShouldStop(Event *event_ptr) { if (!done) { StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); - done = !(frame_zero_id < m_step_out_to_id); + 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 @@ -608,5 +608,5 @@ bool ThreadPlanStepOut::IsPlanStale() { // then there's something for us to do. Otherwise, we're stale. StackID frame_zero_id = GetThread().GetStackFrameAtIndex(0)->GetStackID(); - return !(frame_zero_id < m_step_out_to_id); + return !IsYounger(frame_zero_id, m_step_out_to_id); } diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp index ca51705eda695..91015a4a35255 100644 --- a/lldb/source/Target/ThreadPlanStepRange.cpp +++ b/lldb/source/Target/ThreadPlanStepRange.cpp @@ -220,7 +220,7 @@ lldb::FrameComparison ThreadPlanStepRange::CompareCurrentFrameToStartFrame() { if (cur_frame_id == m_stack_id) { frame_order = eFrameCompareEqual; - } else if (cur_frame_id < m_stack_id) { + } else if (IsYounger(cur_frame_id, m_stack_id)) { frame_order = eFrameCompareYounger; } else { StackFrameSP cur_parent_frame = thread.GetStackFrameAtIndex(1); diff --git a/lldb/source/Target/ThreadPlanStepUntil.cpp b/lldb/source/Target/ThreadPlanStepUntil.cpp index ee0f803510e68..1388ea1e41fcb 100644 --- a/lldb/source/Target/ThreadPlanStepUntil.cpp +++ b/lldb/source/Target/ThreadPlanStepUntil.cpp @@ -174,7 +174,7 @@ void ThreadPlanStepUntil::AnalyzeStop() { bool done; StackID cur_frame_zero_id; - done = (m_stack_id < cur_frame_zero_id); + done = IsYounger(m_stack_id, cur_frame_zero_id); if (done) { m_stepped_out = true; @@ -200,7 +200,7 @@ void ThreadPlanStepUntil::AnalyzeStop() { if (frame_zero_id == m_stack_id) done = true; - else if (frame_zero_id < m_stack_id) + else if (IsYounger(frame_zero_id, m_stack_id)) done = false; else { StackFrameSP older_frame_sp = thread.GetStackFrameAtIndex(1);