[Lldb-commits] [lldb] [lldb/Target] Unify frame provider descriptor and chain IDs (PR #190712)

Med Ismail Bennani via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 7 12:40:28 PDT 2026


https://github.com/medismailben updated https://github.com/llvm/llvm-project/pull/190712

>From 15d16a51513caef2786ccaab91e5692748582439 Mon Sep 17 00:00:00 2001
From: Med Ismail Bennani <ismail at bennani.ma>
Date: Mon, 6 Apr 2026 16:15:08 -0700
Subject: [PATCH] [lldb/Target] Unify frame provider descriptor and chain IDs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace the two separate ID systems for frame providers — hash-based
descriptor IDs in Target and sequential chain IDs in Thread — with a
single monotonic counter in Target.

Provider IDs are now assigned by Target::AddScriptedFrameProviderDescriptor
and used directly as the chain ID in Thread, so RegisterScriptedFrameProvider
returns the same ID used by 'bt --provider'. Also add duplicate detection to
reject registering a provider with the same class name and arguments twice.

Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
---
 .../lldb/Target/SyntheticFrameProvider.h      | 19 +++++++++++----
 lldb/include/lldb/Target/Target.h             |  1 +
 lldb/include/lldb/Target/Thread.h             |  4 ----
 lldb/source/Target/SyntheticFrameProvider.cpp | 12 +++++++++-
 lldb/source/Target/Target.cpp                 | 24 ++++++++++++++-----
 lldb/source/Target/Thread.cpp                 |  7 +-----
 .../TestFrameProviderPassThroughPrefix.py     |  4 ++--
 7 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/Target/SyntheticFrameProvider.h b/lldb/include/lldb/Target/SyntheticFrameProvider.h
index bbd52b144412d..249a9bb714ff6 100644
--- a/lldb/include/lldb/Target/SyntheticFrameProvider.h
+++ b/lldb/include/lldb/Target/SyntheticFrameProvider.h
@@ -36,6 +36,10 @@ struct ScriptedFrameProviderDescriptor {
   /// satisfies ANY of the specs in this vector (OR logic).
   std::vector<ThreadSpec> thread_specs;
 
+  /// Monotonic ID assigned by Target when this descriptor is registered.
+  /// Zero means no ID has been assigned yet.
+  uint32_t m_id = 0;
+
   ScriptedFrameProviderDescriptor() = default;
 
   ScriptedFrameProviderDescriptor(lldb::ScriptedMetadataSP metadata_sp)
@@ -83,12 +87,19 @@ struct ScriptedFrameProviderDescriptor {
   /// Check if this descriptor has valid metadata for script-based providers.
   bool IsValid() const { return scripted_metadata_sp != nullptr; }
 
-  /// Get a unique identifier for this descriptor based on its contents.
-  /// The ID is computed from the class name and arguments dictionary,
-  /// not from the pointer address, so two descriptors with the same
-  /// contents will have the same ID.
+  /// Get a unique identifier for this descriptor.
+  /// Returns the monotonic ID assigned by Target if set (non-zero),
+  /// otherwise falls back to the content-based hash from ScriptedMetadata.
   uint32_t GetID() const;
 
+  /// Set the monotonic ID for this descriptor. Called by Target when
+  /// the descriptor is registered.
+  void SetID(uint32_t id) { m_id = id; }
+
+  /// Get the content-based hash from ScriptedMetadata.
+  /// Used for duplicate detection (same class name + args).
+  uint32_t GetArgumentsHash() const;
+
   /// Dump a description of this descriptor to the given stream.
   void Dump(Stream *s) const;
 };
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 25ab90e906aeb..67f373aa5a325 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1828,6 +1828,7 @@ class Target : public std::enable_shared_from_this<Target>,
   llvm::MapVector<uint32_t, ScriptedFrameProviderDescriptor>
       m_frame_provider_descriptors;
   mutable std::recursive_mutex m_frame_provider_descriptors_mutex;
+  uint32_t m_next_frame_provider_id = 1;
 
   typedef std::map<lldb::LanguageType, lldb::REPLSP> REPLMap;
   REPLMap m_repl_map;
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 8b69e83d94ca4..4353725ca47f6 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1477,10 +1477,6 @@ class Thread : public std::enable_shared_from_this<Thread>,
   mutable llvm::DenseMap<lldb::frame_list_id_t, lldb::StackFrameListWP>
       m_frame_lists_by_id;
 
-  /// Counter for assigning unique provider IDs. Starts at 1 since 0 is
-  /// reserved for normal unwinder frames. Persists across ClearStackFrames.
-  lldb::frame_list_id_t m_next_provider_id = 1;
-
 private:
   bool m_extended_info_fetched; // Have we tried to retrieve the m_extended_info
                                 // for this thread?
diff --git a/lldb/source/Target/SyntheticFrameProvider.cpp b/lldb/source/Target/SyntheticFrameProvider.cpp
index 0e4408eaad480..df59e62ee7bbf 100644
--- a/lldb/source/Target/SyntheticFrameProvider.cpp
+++ b/lldb/source/Target/SyntheticFrameProvider.cpp
@@ -27,7 +27,7 @@ void ScriptedFrameProviderDescriptor::Dump(Stream *s) const {
   if (!s)
     return;
 
-  s->Format("  ID: {0:x}\n", GetID());
+  s->Format("  ID: {0}\n", GetID());
   s->Printf("  Name: %s\n", GetName().str().c_str());
 
   std::string description = GetDescription();
@@ -56,6 +56,16 @@ void ScriptedFrameProviderDescriptor::Dump(Stream *s) const {
 }
 
 uint32_t ScriptedFrameProviderDescriptor::GetID() const {
+  if (m_id != 0)
+    return m_id;
+
+  if (!scripted_metadata_sp)
+    return 0;
+
+  return scripted_metadata_sp->GetID();
+}
+
+uint32_t ScriptedFrameProviderDescriptor::GetArgumentsHash() const {
   if (!scripted_metadata_sp)
     return 0;
 
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index cba04a67a00cc..6f325ef39b022 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3721,8 +3721,6 @@ llvm::Expected<uint32_t> Target::AddScriptedFrameProviderDescriptor(
   if (!descriptor.IsValid())
     return llvm::createStringError("invalid frame provider descriptor");
 
-  uint32_t descriptor_id = descriptor.GetID();
-
   llvm::StringRef name = descriptor.GetName();
   if (name.empty())
     return llvm::createStringError(
@@ -3731,12 +3729,25 @@ llvm::Expected<uint32_t> Target::AddScriptedFrameProviderDescriptor(
   {
     std::unique_lock<std::recursive_mutex> guard(
         m_frame_provider_descriptors_mutex);
-    m_frame_provider_descriptors[descriptor_id] = descriptor;
-  }
 
-  InvalidateThreadFrameProviders();
+    // Check for duplicate: same class name and args (content hash).
+    uint32_t args_hash = descriptor.GetArgumentsHash();
+    for (const auto &entry : m_frame_provider_descriptors) {
+      if (entry.second.GetArgumentsHash() == args_hash)
+        return llvm::createStringError(
+            "a frame provider with the same class name and arguments is "
+            "already registered");
+    }
 
-  return descriptor_id;
+    uint32_t descriptor_id = m_next_frame_provider_id++;
+    ScriptedFrameProviderDescriptor new_descriptor = descriptor;
+    new_descriptor.SetID(descriptor_id);
+    m_frame_provider_descriptors[descriptor_id] = new_descriptor;
+
+    InvalidateThreadFrameProviders();
+
+    return descriptor_id;
+  }
 }
 
 bool Target::RemoveScriptedFrameProviderDescriptor(uint32_t id) {
@@ -3757,6 +3768,7 @@ void Target::ClearScriptedFrameProviderDescriptors() {
     std::lock_guard<std::recursive_mutex> guard(
         m_frame_provider_descriptors_mutex);
     m_frame_provider_descriptors.clear();
+    m_next_frame_provider_id = 1;
   }
 
   InvalidateThreadFrameProviders();
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index fab9d2f3eada5..c199fd236f5cd 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1658,10 +1658,7 @@ llvm::Error Thread::LoadScriptedFrameProvider(
   if (!provider_or_err)
     return provider_or_err.takeError();
 
-  lldb::frame_list_id_t provider_id = m_next_provider_id++;
-  if (m_next_provider_id ==
-      0) // Wrapped past max; skip 0 (reserved for unwinder).
-    m_next_provider_id = 1;
+  lldb::frame_list_id_t provider_id = descriptor.GetID();
 
   m_frame_providers.insert({provider_id, *provider_or_err});
 
@@ -1696,7 +1693,6 @@ void Thread::ClearScriptedFrameProvider() {
   m_frame_providers.clear();
   m_provider_chain_ids.clear();
   m_frame_lists_by_id.clear();
-  m_next_provider_id = 1; // Reset counter.
   m_unwinder_frames_sp.reset();
   m_curr_frames_sp.reset();
   m_prev_frames_sp.reset();
@@ -1736,7 +1732,6 @@ void Thread::ClearStackFrames() {
   // chain configuration (m_provider_chain_ids) so providers are re-loaded
   // with consistent IDs on the next GetStackFrameList() call.
   m_frame_providers.clear();
-  m_next_provider_id = 1;
   m_frame_lists_by_id.clear();
   m_extended_info.reset();
   m_extended_info_fetched = false;
diff --git a/lldb/test/API/functionalities/scripted_frame_provider/pass_through_prefix/TestFrameProviderPassThroughPrefix.py b/lldb/test/API/functionalities/scripted_frame_provider/pass_through_prefix/TestFrameProviderPassThroughPrefix.py
index 071a7382dcab9..a6128b91ca6a2 100644
--- a/lldb/test/API/functionalities/scripted_frame_provider/pass_through_prefix/TestFrameProviderPassThroughPrefix.py
+++ b/lldb/test/API/functionalities/scripted_frame_provider/pass_through_prefix/TestFrameProviderPassThroughPrefix.py
@@ -13,9 +13,9 @@
 class FrameProviderPassThroughPrefixTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
-    # The frame list IDs used by 'bt --provider' are internal sequential IDs:
+    # The frame list IDs used by 'bt --provider' match the descriptor IDs
+    # returned by RegisterScriptedFrameProvider:
     # 0 = base unwinder, 1 = first provider, 2 = second provider, etc.
-    # These are NOT the descriptor IDs returned by RegisterScriptedFrameProvider.
     UNWINDER_FRAME_LIST_ID = 0
     FIRST_PROVIDER_FRAME_LIST_ID = 1
     SECOND_PROVIDER_FRAME_LIST_ID = 2



More information about the lldb-commits mailing list