[lldb/Target] Track containing StackFrameList to avoid circular dependencies

[medismailben]

This change adds tracking of the StackFrameList that produced 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.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

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 --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)

[github-actions]

🐧 Linux x64 Test Results

• 33206 tests passed
• 507 tests skipped

✅ The build succeeded and all tests passed.

[jimingham]

This LGTM except for a suggested change in the comment for m_frame_list_wp. You make it sound like this is some optional thing that you could choose to use or not. But if it is resolveable it IS the stack frame list in which you are supposed to look up m_stack_id. Your comment needs to say that.
Approved provided the comment is fixed.