https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/170226
>From 02aea7717721169be358820a7e60844f86267656 Mon Sep 17 00:00:00 2001 From: Med Ismail Bennani <[email protected]> Date: Mon, 1 Dec 2025 16:47:13 -0800 Subject: [PATCH] [lldb/Target] Track containing StackFrameList to avoid circular dependencies 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 `GetContainingStackFrameList` method that allows frames to expose their source list. Signed-off-by: Med Ismail Bennani <[email protected]> --- lldb/include/lldb/Target/ExecutionContext.h | 11 +++++++++-- lldb/include/lldb/Target/StackFrame.h | 14 ++++++++++++++ lldb/include/lldb/Target/StackFrameList.h | 2 +- lldb/include/lldb/lldb-forward.h | 1 + lldb/source/Target/ExecutionContext.cpp | 17 +++++++++++++++-- lldb/source/Target/StackFrameList.cpp | 5 +++++ 6 files changed, 45 insertions(+), 5 deletions(-) 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..135bd81e4e8d4 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 contains this frame. + /// + /// Returns the StackFrameList that contains 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 contains this frame, or nullptr if not set. + virtual lldb::StackFrameListSP GetContainingStackFrameList() 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/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h index 5b0df0ddb3e29..8c14e92a41a4e 100644 --- a/lldb/include/lldb/Target/StackFrameList.h +++ b/lldb/include/lldb/Target/StackFrameList.h @@ -20,7 +20,7 @@ namespace lldb_private { class ScriptedThread; -class StackFrameList { +class StackFrameList : public std::enable_shared_from_this<StackFrameList> { public: // Constructors and Destructors StackFrameList(Thread &thread, const lldb::StackFrameListSP &prev_frames_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..b16ff26266c53 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->GetContainingStackFrameList(); + } 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->GetContainingStackFrameList(); 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) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
