Skip to content

Conversation

@medismailben
Copy link
Member

This patch allows threads to have multiple SyntheticFrameProviderSP instances that chain together sequentially. Each provider receives the output of the previous provider as input, creating a transformation pipeline.

It changes Thread::m_frame_provider_sp to a vector, adds provider parameter to SyntheticStackFrameList to avoid calling back into Thread::GetFrameProvider() during frame fetching, updated LoadScriptedFrameProvider() to chain providers by wrapping each previous provider's output in a SyntheticStackFrameList for the next provider and finally, loads ALL matching providers in priority order instead of just the first one.

The chaining works as follows:

  Real Unwinder Frames
      ↓
  Provider 1 (priority 10) → adds/transforms frames
      ↓
  Provider 2 (priority 20) → transforms Provider 1's output
      ↓
  Provider 3 (priority 30) → transforms Provider 2's output
      ↓
  Final frame list shown to user

This patch also adds a test for this (test_chained_frame_providers) to verify that 3 providers chain correctly: AddFooFrameProvider, AddBarFrameProvider, AddBazFrameProvider.

@llvmbot
Copy link
Member

llvmbot commented Dec 18, 2025

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

This patch allows threads to have multiple SyntheticFrameProviderSP instances that chain together sequentially. Each provider receives the output of the previous provider as input, creating a transformation pipeline.

It changes Thread::m_frame_provider_sp to a vector, adds provider parameter to SyntheticStackFrameList to avoid calling back into Thread::GetFrameProvider() during frame fetching, updated LoadScriptedFrameProvider() to chain providers by wrapping each previous provider's output in a SyntheticStackFrameList for the next provider and finally, loads ALL matching providers in priority order instead of just the first one.

The chaining works as follows:

  Real Unwinder Frames
      ↓
  Provider 1 (priority 10) → adds/transforms frames
      ↓
  Provider 2 (priority 20) → transforms Provider 1's output
      ↓
  Provider 3 (priority 30) → transforms Provider 2's output
      ↓
  Final frame list shown to user

This patch also adds a test for this (test_chained_frame_providers) to verify that 3 providers chain correctly: AddFooFrameProvider, AddBarFrameProvider, AddBazFrameProvider.


Patch is 26.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/172849.diff

14 Files Affected:

  • (modified) lldb/examples/python/templates/scripted_frame_provider.py (+28)
  • (modified) lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h (+16)
  • (modified) lldb/include/lldb/Target/StackFrameList.h (+5-1)
  • (modified) lldb/include/lldb/Target/SyntheticFrameProvider.h (+21)
  • (modified) lldb/include/lldb/Target/Thread.h (+4-4)
  • (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp (+18)
  • (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.h (+2)
  • (modified) lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp (+7)
  • (modified) lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.h (+2)
  • (modified) lldb/source/Target/StackFrameList.cpp (+7-6)
  • (modified) lldb/source/Target/SyntheticFrameProvider.cpp (+14)
  • (modified) lldb/source/Target/Thread.cpp (+56-28)
  • (modified) lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py (+92)
  • (modified) lldb/test/API/functionalities/scripted_frame_provider/test_frame_providers.py (+78)
diff --git a/lldb/examples/python/templates/scripted_frame_provider.py b/lldb/examples/python/templates/scripted_frame_provider.py
index 7a72f1a24c9da..a45ef9427a532 100644
--- a/lldb/examples/python/templates/scripted_frame_provider.py
+++ b/lldb/examples/python/templates/scripted_frame_provider.py
@@ -79,6 +79,34 @@ def get_description(self):
         """
         pass
 
+    @staticmethod
+    def get_priority():
+        """Get the priority of this frame provider.
+
+        This static method is called to determine the evaluation order when
+        multiple frame providers could apply to the same thread. Lower numbers
+        indicate higher priority (like Unix nice values).
+
+        Returns:
+            int or None: Priority value where 0 is highest priority.
+                Return None for default priority (UINT32_MAX - lowest priority).
+
+        Example:
+
+        .. code-block:: python
+
+            @staticmethod
+            def get_priority():
+                # High priority - runs before most providers
+                return 10
+
+            @staticmethod
+            def get_priority():
+                # Default priority - runs last
+                return None
+        """
+        return None  # Default/lowest priority
+
     def __init__(self, input_frames, args):
         """Construct a scripted frame provider.
 
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h
index 49b60131399d5..b04af0c817b6e 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedFrameProviderInterface.h
@@ -39,6 +39,22 @@ class ScriptedFrameProviderInterface : public ScriptedInterface {
   ///         empty string if no description is available.
   virtual std::string GetDescription(llvm::StringRef class_name) { return {}; }
 
+  /// Get the priority of this frame provider.
+  ///
+  /// This is called by the descriptor to fetch the priority from the
+  /// scripted implementation. Implementations should call a static method
+  /// on the scripting class to retrieve the priority. Lower numbers indicate
+  /// higher priority (like Unix nice values).
+  ///
+  /// \param class_name The name of the scripting class implementing the
+  /// provider.
+  ///
+  /// \return Priority value where 0 is highest priority, or std::nullopt for
+  ///         default priority (UINT32_MAX - lowest priority).
+  virtual std::optional<uint32_t> GetPriority(llvm::StringRef class_name) {
+    return std::nullopt;
+  }
+
   virtual StructuredData::ObjectSP GetFrameAtIndex(uint32_t index) {
     return {};
   }
diff --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h
index 539c070ff0f4b..42a0be9b196b9 100644
--- a/lldb/include/lldb/Target/StackFrameList.h
+++ b/lldb/include/lldb/Target/StackFrameList.h
@@ -243,7 +243,8 @@ class SyntheticStackFrameList : public StackFrameList {
 public:
   SyntheticStackFrameList(Thread &thread, lldb::StackFrameListSP input_frames,
                           const lldb::StackFrameListSP &prev_frames_sp,
-                          bool show_inline_frames);
+                          bool show_inline_frames,
+                          lldb::SyntheticFrameProviderSP provider);
 
 protected:
   /// Override FetchFramesUpTo to lazily return frames from the provider
@@ -255,6 +256,9 @@ class SyntheticStackFrameList : public StackFrameList {
   /// The input stack frame list that the provider transforms.
   /// This could be a real StackFrameList or another SyntheticStackFrameList.
   lldb::StackFrameListSP m_input_frames;
+
+  /// The provider that transforms the input frames.
+  lldb::SyntheticFrameProviderSP m_provider;
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Target/SyntheticFrameProvider.h b/lldb/include/lldb/Target/SyntheticFrameProvider.h
index 2d5330cb03105..bbd52b144412d 100644
--- a/lldb/include/lldb/Target/SyntheticFrameProvider.h
+++ b/lldb/include/lldb/Target/SyntheticFrameProvider.h
@@ -56,6 +56,16 @@ struct ScriptedFrameProviderDescriptor {
   ///         empty string if no description is available.
   std::string GetDescription() const;
 
+  /// Get the priority of this frame provider.
+  ///
+  /// Priority determines the order in which providers are evaluated when
+  /// multiple providers could apply to the same thread. Lower numbers indicate
+  /// higher priority (like Unix nice values).
+  ///
+  /// \return Priority value where 0 is highest priority, or std::nullopt for
+  ///         default priority (UINT32_MAX - lowest priority).
+  std::optional<uint32_t> GetPriority() const;
+
   /// Check if this descriptor applies to the given thread.
   bool AppliesToThread(Thread &thread) const {
     // If no thread specs specified, applies to all threads.
@@ -143,6 +153,17 @@ class SyntheticFrameProvider : public PluginInterface {
 
   virtual std::string GetDescription() const = 0;
 
+  /// Get the priority of this frame provider.
+  ///
+  /// Priority determines the order in which providers are evaluated when
+  /// multiple providers could apply to the same thread. Lower numbers indicate
+  /// higher priority (like Unix nice values).
+  ///
+  /// \return
+  ///     Priority value where 0 is highest priority, or std::nullopt for
+  ///     default priority (UINT32_MAX - lowest priority).
+  virtual std::optional<uint32_t> GetPriority() const { return std::nullopt; }
+
   /// Get a single stack frame at the specified index.
   ///
   /// This method is called lazily - frames are only created when requested.
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 46ce192556756..808bb024d4d64 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1302,8 +1302,8 @@ class Thread : public std::enable_shared_from_this<Thread>,
 
   void ClearScriptedFrameProvider();
 
-  lldb::SyntheticFrameProviderSP GetFrameProvider() const {
-    return m_frame_provider_sp;
+  const std::vector<lldb::SyntheticFrameProviderSP> &GetFrameProviders() const {
+    return m_frame_providers;
   }
 
 protected:
@@ -1409,8 +1409,8 @@ class Thread : public std::enable_shared_from_this<Thread>,
   /// The Thread backed by this thread, if any.
   lldb::ThreadWP m_backed_thread;
 
-  /// The Scripted Frame Provider, if any.
-  lldb::SyntheticFrameProviderSP m_frame_provider_sp;
+  /// The Scripted Frame Providers for this thread.
+  std::vector<lldb::SyntheticFrameProviderSP> m_frame_providers;
 
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the m_extended_info
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp
index 3dde5036453f4..2d87c1b10bfec 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.cpp
@@ -70,6 +70,24 @@ std::string ScriptedFrameProviderPythonInterface::GetDescription(
   return obj->GetStringValue().str();
 }
 
+std::optional<uint32_t>
+ScriptedFrameProviderPythonInterface::GetPriority(llvm::StringRef class_name) {
+  Status error;
+  StructuredData::ObjectSP obj =
+      CallStaticMethod(class_name, "get_priority", error);
+
+  if (!ScriptedInterface::CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj,
+                                                    error))
+    return std::nullopt;
+
+  // Try to extract as unsigned integer. Return nullopt if Python returned None
+  // or if extraction fails.
+  if (StructuredData::UnsignedInteger *int_obj = obj->GetAsUnsignedInteger())
+    return static_cast<uint32_t>(int_obj->GetValue());
+
+  return std::nullopt;
+}
+
 StructuredData::ObjectSP
 ScriptedFrameProviderPythonInterface::GetFrameAtIndex(uint32_t index) {
   Status error;
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.h
index 97a5cc7c669ea..884b0355a659e 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedFrameProviderPythonInterface.h
@@ -43,6 +43,8 @@ class ScriptedFrameProviderPythonInterface
 
   std::string GetDescription(llvm::StringRef class_name) override;
 
+  std::optional<uint32_t> GetPriority(llvm::StringRef class_name) override;
+
   StructuredData::ObjectSP GetFrameAtIndex(uint32_t index) override;
 
   static void Initialize();
diff --git a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp
index 739963e6f0c2f..4aad8f2cb628f 100644
--- a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp
+++ b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.cpp
@@ -106,6 +106,13 @@ std::string ScriptedFrameProvider::GetDescription() const {
   return m_interface_sp->GetDescription(m_descriptor.GetName());
 }
 
+std::optional<uint32_t> ScriptedFrameProvider::GetPriority() const {
+  if (!m_interface_sp)
+    return std::nullopt;
+
+  return m_interface_sp->GetPriority(m_descriptor.GetName());
+}
+
 llvm::Expected<StackFrameSP>
 ScriptedFrameProvider::GetFrameAtIndex(uint32_t idx) {
   if (!m_interface_sp)
diff --git a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.h b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.h
index 3434bf26ade24..6937f9acbc9a9 100644
--- a/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.h
+++ b/lldb/source/Plugins/SyntheticFrameProvider/ScriptedFrameProvider/ScriptedFrameProvider.h
@@ -40,6 +40,8 @@ class ScriptedFrameProvider : public SyntheticFrameProvider {
 
   std::string GetDescription() const override;
 
+  std::optional<uint32_t> GetPriority() const override;
+
   /// Get a single stack frame at the specified index.
   llvm::Expected<lldb::StackFrameSP> GetFrameAtIndex(uint32_t idx) override;
 
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 896a760f61d26..b1af2bb65494a 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -58,23 +58,24 @@ StackFrameList::~StackFrameList() {
 
 SyntheticStackFrameList::SyntheticStackFrameList(
     Thread &thread, lldb::StackFrameListSP input_frames,
-    const lldb::StackFrameListSP &prev_frames_sp, bool show_inline_frames)
+    const lldb::StackFrameListSP &prev_frames_sp, bool show_inline_frames,
+    lldb::SyntheticFrameProviderSP provider)
     : StackFrameList(thread, prev_frames_sp, show_inline_frames),
-      m_input_frames(std::move(input_frames)) {}
+      m_input_frames(std::move(input_frames)), m_provider(std::move(provider)) {
+}
 
 bool SyntheticStackFrameList::FetchFramesUpTo(
     uint32_t end_idx, InterruptionControl allow_interrupt) {
 
   size_t num_synthetic_frames = 0;
-  // Check if the thread has a synthetic frame provider.
-  if (auto provider_sp = m_thread.GetFrameProvider()) {
-    // Use the synthetic frame provider to generate frames lazily.
+  // Use the provider to generate frames lazily.
+  if (m_provider) {
     // Keep fetching until we reach end_idx or the provider returns an error.
     for (uint32_t idx = m_frames.size(); idx <= end_idx; idx++) {
       if (allow_interrupt &&
           m_thread.GetProcess()->GetTarget().GetDebugger().InterruptRequested())
         return true;
-      auto frame_or_err = provider_sp->GetFrameAtIndex(idx);
+      auto frame_or_err = m_provider->GetFrameAtIndex(idx);
       if (!frame_or_err) {
         // Provider returned error - we've reached the end.
         LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), frame_or_err.takeError(),
diff --git a/lldb/source/Target/SyntheticFrameProvider.cpp b/lldb/source/Target/SyntheticFrameProvider.cpp
index 97ff42d1ed53e..e799ad23b7512 100644
--- a/lldb/source/Target/SyntheticFrameProvider.cpp
+++ b/lldb/source/Target/SyntheticFrameProvider.cpp
@@ -34,6 +34,13 @@ void ScriptedFrameProviderDescriptor::Dump(Stream *s) const {
   if (!description.empty())
     s->Printf("  Description: %s\n", description.c_str());
 
+  // Show priority information.
+  std::optional<uint32_t> priority = GetPriority();
+  if (priority.has_value())
+    s->Printf("  Priority: %u\n", *priority);
+  else
+    s->PutCString("  Priority: Default (no priority specified)\n");
+
   // Show thread filter information.
   if (thread_specs.empty()) {
     s->PutCString("  Thread Filter: (applies to all threads)\n");
@@ -62,6 +69,13 @@ std::string ScriptedFrameProviderDescriptor::GetDescription() const {
   return {};
 }
 
+std::optional<uint32_t> ScriptedFrameProviderDescriptor::GetPriority() const {
+  // If we have an interface, call get_priority() to fetch it.
+  if (interface_sp && scripted_metadata_sp)
+    return interface_sp->GetPriority(scripted_metadata_sp->GetClassName());
+  return std::nullopt;
+}
+
 llvm::Expected<SyntheticFrameProviderSP> SyntheticFrameProvider::CreateInstance(
     StackFrameListSP input_frames,
     const ScriptedFrameProviderDescriptor &descriptor) {
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index b40e753aca1e9..53ce687fdace3 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -262,7 +262,7 @@ void Thread::DestroyThread() {
   std::lock_guard<std::recursive_mutex> guard(m_frame_mutex);
   m_curr_frames_sp.reset();
   m_prev_frames_sp.reset();
-  m_frame_provider_sp.reset();
+  m_frame_providers.clear();
   m_prev_framezero_pc.reset();
 }
 
@@ -1448,33 +1448,51 @@ StackFrameListSP Thread::GetStackFrameList() {
   if (m_curr_frames_sp)
     return m_curr_frames_sp;
 
-  // First, try to load a frame provider if we don't have one yet.
-  if (!m_frame_provider_sp) {
+  // First, try to load frame providers if we don't have any yet.
+  if (m_frame_providers.empty()) {
     ProcessSP process_sp = GetProcess();
     if (process_sp) {
       Target &target = process_sp->GetTarget();
       const auto &descriptors = target.GetScriptedFrameProviderDescriptors();
 
-      // Find first descriptor that applies to this thread.
+      // Collect all descriptors that apply to this thread.
+      std::vector<const ScriptedFrameProviderDescriptor *>
+          applicable_descriptors;
       for (const auto &entry : descriptors) {
         const ScriptedFrameProviderDescriptor &descriptor = entry.second;
         if (descriptor.IsValid() && descriptor.AppliesToThread(*this)) {
-          if (llvm::Error error = LoadScriptedFrameProvider(descriptor)) {
-            LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(error),
-                           "Failed to load scripted frame provider: {0}");
-          }
-          break; // Use first matching descriptor (success or failure).
+          applicable_descriptors.push_back(&descriptor);
+        }
+      }
+
+      // Sort by priority (lower number = higher priority).
+      std::sort(applicable_descriptors.begin(), applicable_descriptors.end(),
+                [](const ScriptedFrameProviderDescriptor *a,
+                   const ScriptedFrameProviderDescriptor *b) {
+                  // nullopt (no priority) sorts last (UINT32_MAX).
+                  uint32_t priority_a = a->GetPriority().value_or(UINT32_MAX);
+                  uint32_t priority_b = b->GetPriority().value_or(UINT32_MAX);
+                  return priority_a < priority_b;
+                });
+
+      // Load ALL matching providers in priority order.
+      for (const auto *descriptor : applicable_descriptors) {
+        if (llvm::Error error = LoadScriptedFrameProvider(*descriptor)) {
+          LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(error),
+                         "Failed to load scripted frame provider: {0}");
+          continue; // Try next provider if this one fails.
         }
       }
     }
   }
 
-  // Create the frame list based on whether we have a provider.
-  if (m_frame_provider_sp) {
-    // We have a provider - create synthetic frame list.
-    StackFrameListSP input_frames = m_frame_provider_sp->GetInputFrames();
+  // Create the frame list based on whether we have providers.
+  if (!m_frame_providers.empty()) {
+    // We have providers - use the last one in the chain.
+    // The last provider has already been chained with all previous providers.
+    StackFrameListSP input_frames = m_frame_providers.back()->GetInputFrames();
     m_curr_frames_sp = std::make_shared<SyntheticStackFrameList>(
-        *this, input_frames, m_prev_frames_sp, true);
+        *this, input_frames, m_prev_frames_sp, true, m_frame_providers.back());
   } else {
     // No provider - use normal unwinder frames.
     m_curr_frames_sp =
@@ -1488,29 +1506,39 @@ llvm::Error Thread::LoadScriptedFrameProvider(
     const ScriptedFrameProviderDescriptor &descriptor) {
   std::lock_guard<std::recursive_mutex> guard(m_frame_mutex);
 
-  // Note: We don't create input_frames here - it will be created lazily
-  // by SyntheticStackFrameList when frames are first fetched.
-  // Creating them too early can cause crashes during thread initialization.
-
-  // Create a temporary StackFrameList just to get the thread reference for the
-  // provider. The provider won't actually use this - it will get real input
-  // frames from SyntheticStackFrameList later.
-  StackFrameListSP temp_frames =
-      std::make_shared<StackFrameList>(*this, m_prev_frames_sp, true);
+  // Create input frames for this provider:
+  // - If no providers exist yet, use real unwinder frames
+  // - If providers exist, wrap the previous provider in a
+  // SyntheticStackFrameList
+  //   This creates the chain: each provider's OUTPUT becomes the next
+  //   provider's INPUT
+  StackFrameListSP input_frames;
+  if (m_frame_providers.empty()) {
+    // First provider gets real unwinder frames
+    input_frames =
+        std::make_shared<StackFrameList>(*this, m_prev_frames_sp, true);
+  } else {
+    // Subsequent providers get the previous provider's OUTPUT
+    // We create a SyntheticStackFrameList that wraps the previous provider
+    SyntheticFrameProviderSP prev_provider = m_frame_providers.back();
+    StackFrameListSP prev_input = prev_provider->GetInputFrames();
+    input_frames = std::make_shared<SyntheticStackFrameList>(
+        *this, prev_input, m_prev_frames_sp, true, prev_provider);
+  }
 
   auto provider_or_err =
-      SyntheticFrameProvider::CreateInstance(temp_frames, descriptor);
+      SyntheticFrameProvider::CreateInstance(input_frames, descriptor);
   if (!provider_or_err)
     return provider_or_err.takeError();
 
-  ClearScriptedFrameProvider();
-  m_frame_provider_sp = *provider_or_err;
+  // Append to the chain
+  m_frame_providers.push_back(*provider_or_err);
   return llvm::Error::success();
 }
 
 void Thread::ClearScriptedFrameProvider() {
   std::lock_guard<std::recursive_mutex> guard(m_frame_mutex);
-  m_frame_provider_sp.reset();
+  m_frame_providers.clear();
   m_curr_frames_sp.reset();
   m_prev_frames_sp.reset();
 }
@@ -1535,7 +1563,7 @@ void Thread::ClearStackFrames() {
     m_prev_frames_sp.swap(m_curr_frames_sp);
   m_curr_frames_sp.reset();
 
-  m_frame_provider_sp.reset();
+  m_frame_providers.clear();
   m_extended_info.reset();
   m_extended_info_fetched = false;
 }
diff --git a/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py b/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py
index e2b0f0f9cd50d..08991d42cef47 100644
--- a/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py
+++ b/lldb/test/API/functionalities/scripted_frame_provider/TestScriptedFrameProvider.py
@@ -638,3 +638,95 @@ def test_valid_pc_no_module_frames(self):
         frame2 = thread.GetFrameAtIndex(2)
         self.assertIsNotNone(frame2)
         self.assertIn("thread_func", frame2.GetFunctionName())
+
+    def te...
[truncated]

@medismailben medismailben force-pushed the frame-provider-chaining branch from ea7df91 to b6d22b4 Compare December 18, 2025 13:14
@medismailben
Copy link
Member Author

Depends on #172848.

Copy link
Member

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I follow what this PR is supposed to do, but I had some trouble following the implementation. I added some comments inline to explain where I was confused.

As an overall suggestion, I think you should add a test case for providers with the same priority level. I'm not sure what the intended order is in that case. If any order is possible, you should write a test that checks all scenarios.

// Subsequent providers get the previous provider's OUTPUT
// We create a SyntheticStackFrameList that wraps the previous provider
SyntheticFrameProviderSP prev_provider = m_frame_providers.back();
StackFrameListSP prev_input = prev_provider->GetInputFrames();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something here feels strange. The comment saying you're getting the previous provider's output but then you call this variable prev_input. It sounds like prev_input represents the frame list that the previous provider received? Or is it supposed to be the input from the previous provider?

I think I understand what's supposed to happen but I'm having trouble following the implementation logic here.

/// The Scripted Frame Provider, if any.
lldb::SyntheticFrameProviderSP m_frame_provider_sp;
/// The Scripted Frame Providers for this thread.
std::vector<lldb::SyntheticFrameProviderSP> m_frame_providers;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: name this something like "m_executed_frame_providers" or "m_loaded_frame_providers". From the implementation, I think that this is the list of providers that have already run. But I initially guessed/assumed that this might already be populated when the SyntheticStackFrameList was created.

This patch allows threads to have multiple SyntheticFrameProviderSP instances
that chain together sequentially. Each provider receives the output of the
previous provider as input, creating a transformation pipeline.

It changes `Thread::m_frame_provider_sp` to a vector, adds provider parameter
to SyntheticStackFrameList to avoid calling back into
`Thread::GetFrameProvider()` during frame fetching, updated
`LoadScriptedFrameProvider()` to chain providers by wrapping each previous
provider's output in a `SyntheticStackFrameList` for the next provider and
finally, loads ALL matching providers in priority order instead of just the
first one.

The chaining works as follows:
```
  Real Unwinder Frames
      ↓
  Provider 1 (priority 10) → adds/transforms frames
      ↓
  Provider 2 (priority 20) → transforms Provider 1's output
      ↓
  Provider 3 (priority 30) → transforms Provider 2's output
      ↓
  Final frame list shown to user
  ```

This patch also adds a test for this (test_chained_frame_providers) to verify
that 3 providers chain correctly: `AddFooFrameProvider`, `AddBarFrameProvider`,
`AddBazFrameProvider`.

Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
@medismailben medismailben force-pushed the frame-provider-chaining branch from b6d22b4 to 53791c7 Compare December 19, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants