llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) <details> <summary>Changes</summary> This change adds tracking of the StackFrameList that created each frame by storing a weak pointer (m_frame_list_wp) in both `StackFrame` and `ExecutionContextRef`. When resolving frames through `ExecutionContextRef::GetFrameSP`, the code now first attempts to use the remembered frame list instead of immediately calling `Thread::GetStackFrameList`. This breaks circular dependencies that can occur during frame provider initialization, where creating a frame provider might trigger `ExecutionContext` resolution, which would then call back into `Thread::GetStackFrameList()`, creating an infinite loop. The `StackFrameList` now sets m_frame_list_wp on every frame it creates, and a new virtual method `GetOriginatingStackFrameList` allows frames to expose their originating list. --- Full diff: https://github.com/llvm/llvm-project/pull/170226.diff 5 Files Affected: - (modified) lldb/include/lldb/Target/ExecutionContext.h (+9-2) - (modified) lldb/include/lldb/Target/StackFrame.h (+14) - (modified) lldb/include/lldb/lldb-forward.h (+1) - (modified) lldb/source/Target/ExecutionContext.cpp (+15-2) - (modified) lldb/source/Target/StackFrameList.cpp (+5) ``````````diff diff --git a/lldb/include/lldb/Target/ExecutionContext.h b/lldb/include/lldb/Target/ExecutionContext.h index fe8bce7f69713..361ea95a5fbfc 100644 --- a/lldb/include/lldb/Target/ExecutionContext.h +++ b/lldb/include/lldb/Target/ExecutionContext.h @@ -268,7 +268,10 @@ class ExecutionContextRef { m_tid = LLDB_INVALID_THREAD_ID; } - void ClearFrame() { m_stack_id.Clear(); } + void ClearFrame() { + m_stack_id.Clear(); + m_frame_list_wp.reset(); + } protected: // Member variables @@ -279,7 +282,11 @@ class ExecutionContextRef { ///< object refers to in case the /// backing object changes StackID m_stack_id; ///< The stack ID that this object refers to in case the - ///backing object changes + ///< backing object changes + mutable lldb::StackFrameListWP m_frame_list_wp; ///< Weak reference to the + ///< frame list that created + ///< this frame, used to avoid + ///< circular dependencies }; /// \class ExecutionContext ExecutionContext.h diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index cdbe8ae3c6779..a6eb8715a3768 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -532,6 +532,19 @@ class StackFrame : public ExecutionContextScope, lldb::RecognizedStackFrameSP GetRecognizedFrame(); + /// Get the StackFrameList that created this frame. + /// + /// Returns the StackFrameList that originally created this frame, allowing + /// frames to resolve execution contexts without calling + /// Thread::GetStackFrameList(), which can cause circular dependencies + /// during frame provider initialization. + /// + /// \return + /// The StackFrameList that created this frame, or nullptr if not set. + virtual lldb::StackFrameListSP GetOriginatingStackFrameList() const { + return m_frame_list_wp.lock(); + } + protected: friend class StackFrameList; @@ -574,6 +587,7 @@ class StackFrame : public ExecutionContextScope, /// well as any other frame with the same trait. bool m_behaves_like_zeroth_frame; lldb::VariableListSP m_variable_list_sp; + lldb::StackFrameListWP m_frame_list_wp; /// Value objects for each variable in m_variable_list_sp. ValueObjectList m_variable_list_value_objects; std::optional<lldb::RecognizedStackFrameSP> m_recognized_frame_sp; diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index 09aa036d27875..ccfe5efa19e1d 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -440,6 +440,7 @@ typedef std::unique_ptr<lldb_private::SourceManager> SourceManagerUP; typedef std::shared_ptr<lldb_private::StackFrame> StackFrameSP; typedef std::weak_ptr<lldb_private::StackFrame> StackFrameWP; typedef std::shared_ptr<lldb_private::StackFrameList> StackFrameListSP; +typedef std::weak_ptr<lldb_private::StackFrameList> StackFrameListWP; typedef std::shared_ptr<lldb_private::StackFrameRecognizer> StackFrameRecognizerSP; typedef std::unique_ptr<lldb_private::StackFrameRecognizerManager> diff --git a/lldb/source/Target/ExecutionContext.cpp b/lldb/source/Target/ExecutionContext.cpp index a795913047639..e6ce5525c6507 100644 --- a/lldb/source/Target/ExecutionContext.cpp +++ b/lldb/source/Target/ExecutionContext.cpp @@ -466,10 +466,13 @@ operator=(const ExecutionContext &exe_ctx) { else m_tid = LLDB_INVALID_THREAD_ID; lldb::StackFrameSP frame_sp(exe_ctx.GetFrameSP()); - if (frame_sp) + if (frame_sp) { m_stack_id = frame_sp->GetStackID(); - else + m_frame_list_wp = frame_sp->GetOriginatingStackFrameList(); + } else { m_stack_id.Clear(); + m_frame_list_wp.reset(); + } return *this; } @@ -511,6 +514,7 @@ void ExecutionContextRef::SetThreadSP(const lldb::ThreadSP &thread_sp) { void ExecutionContextRef::SetFrameSP(const lldb::StackFrameSP &frame_sp) { if (frame_sp) { m_stack_id = frame_sp->GetStackID(); + m_frame_list_wp = frame_sp->GetOriginatingStackFrameList(); SetThreadSP(frame_sp->GetThread()); } else { ClearFrame(); @@ -638,6 +642,15 @@ lldb::ThreadSP ExecutionContextRef::GetThreadSP() const { lldb::StackFrameSP ExecutionContextRef::GetFrameSP() const { if (m_stack_id.IsValid()) { + // Try the remembered frame list first to avoid circular dependencies + // during frame provider initialization. + if (auto frame_list_sp = m_frame_list_wp.lock()) { + if (auto frame_sp = frame_list_sp->GetFrameWithStackID(m_stack_id)) + return frame_sp; + } + + // Fallback: ask the thread, which might re-trigger the frame provider + // initialization. lldb::ThreadSP thread_sp(GetThreadSP()); if (thread_sp) return thread_sp->GetFrameWithStackID(m_stack_id); diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index ccf874fc03ebd..8412e33aaba32 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -330,6 +330,7 @@ void StackFrameList::SynthesizeTailCallFrames(StackFrame &next_frame) { m_thread.shared_from_this(), frame_idx, concrete_frame_idx, cfa, cfa_is_valid, pc, StackFrame::Kind::Regular, artificial, behaves_like_zeroth_frame, &sc); + synth_frame->m_frame_list_wp = shared_from_this(); m_frames.push_back(synth_frame); LLDB_LOG(log, "Pushed frame {0} at {1:x}", callee->GetDisplayName(), pc); } @@ -445,6 +446,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, unwind_frame_sp = std::make_shared<StackFrame>( m_thread.shared_from_this(), m_frames.size(), idx, reg_ctx_sp, cfa, pc, behaves_like_zeroth_frame, nullptr); + unwind_frame_sp->m_frame_list_wp = shared_from_this(); m_frames.push_back(unwind_frame_sp); } } else { @@ -479,6 +481,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, // although its concrete index will stay the same. SynthesizeTailCallFrames(*unwind_frame_sp.get()); + unwind_frame_sp->m_frame_list_wp = shared_from_this(); m_frames.push_back(unwind_frame_sp); } @@ -503,6 +506,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, unwind_frame_sp->GetRegisterContextSP(), cfa, next_frame_address, behaves_like_zeroth_frame, &next_frame_sc)); + frame_sp->m_frame_list_wp = shared_from_this(); m_frames.push_back(frame_sp); unwind_sc = next_frame_sc; curr_frame_address = next_frame_address; @@ -559,6 +563,7 @@ bool StackFrameList::FetchFramesUpTo(uint32_t end_idx, prev_frame->UpdatePreviousFrameFromCurrentFrame(*curr_frame); // Now copy the fixed up previous frame into the current frames so the // pointer doesn't change. + prev_frame_sp->m_frame_list_wp = shared_from_this(); m_frames[curr_frame_idx] = prev_frame_sp; #if defined(DEBUG_STACK_FRAMES) `````````` </details> https://github.com/llvm/llvm-project/pull/170226 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
