[Lldb-commits] [lldb] 8d106eb - [lldb/Target] Unify frame provider descriptor and chain IDs (#190712)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 8 14:20:30 PDT 2026
Author: Med Ismail Bennani
Date: 2026-04-08T14:20:25-07:00
New Revision: 8d106eb976533e522d30b0b6da78445a8f5ec697
URL: https://github.com/llvm/llvm-project/commit/8d106eb976533e522d30b0b6da78445a8f5ec697
DIFF: https://github.com/llvm/llvm-project/commit/8d106eb976533e522d30b0b6da78445a8f5ec697.diff
LOG: [lldb/Target] Unify frame provider descriptor and chain IDs (#190712)
Replace the two separate ID systems for frame providers — hash-based
descriptor IDs in Target and sequential chain IDs in Thread — with a
single monotonically increasing 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
emit a warning when registering a provider with the same class name and
arguments twice.
Signed-off-by: Med Ismail Bennani <ismail at bennani.ma>
Added:
Modified:
lldb/include/lldb/Target/SyntheticFrameProvider.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Utility/ScriptedMetadata.h
lldb/include/lldb/lldb-defines.h
lldb/source/Target/SyntheticFrameProvider.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/Thread.cpp
lldb/test/API/functionalities/scripted_frame_provider/pass_through_prefix/TestFrameProviderPassThroughPrefix.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/SyntheticFrameProvider.h b/lldb/include/lldb/Target/SyntheticFrameProvider.h
index bbd52b144412d..bff340b3bf87e 100644
--- a/lldb/include/lldb/Target/SyntheticFrameProvider.h
+++ b/lldb/include/lldb/Target/SyntheticFrameProvider.h
@@ -36,6 +36,11 @@ struct ScriptedFrameProviderDescriptor {
/// satisfies ANY of the specs in this vector (OR logic).
std::vector<ThreadSpec> thread_specs;
+ /// Monotonically increasing ID assigned by Target when this descriptor is
+ /// registered. LLDB_INVALID_FRAME_PROVIDER_ID (UINT32_MAX) means no ID has
+ /// been assigned yet.
+ uint32_t m_id = LLDB_INVALID_FRAME_PROVIDER_ID;
+
ScriptedFrameProviderDescriptor() = default;
ScriptedFrameProviderDescriptor(lldb::ScriptedMetadataSP metadata_sp)
@@ -83,11 +88,18 @@ 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.
- uint32_t GetID() const;
+ /// Get a unique identifier for this descriptor.
+ /// Returns the monotonically increasing ID assigned by Target if set,
+ /// otherwise returns LLDB_INVALID_FRAME_PROVIDER_ID (UINT32_MAX).
+ uint32_t GetID() const { return m_id; }
+
+ /// Set the monotonically increasing 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 GetHash() 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/include/lldb/Utility/ScriptedMetadata.h b/lldb/include/lldb/Utility/ScriptedMetadata.h
index ba9462e65151e..c6b660830e158 100644
--- a/lldb/include/lldb/Utility/ScriptedMetadata.h
+++ b/lldb/include/lldb/Utility/ScriptedMetadata.h
@@ -41,7 +41,7 @@ class ScriptedMetadata {
/// The ID is computed from the class name and arguments dictionary,
/// not from the pointer address, so two metadata objects with the same
/// contents will have the same ID.
- uint32_t GetID() const {
+ uint32_t GetHash() const {
if (m_class_name.empty())
return 0;
diff --git a/lldb/include/lldb/lldb-defines.h b/lldb/include/lldb/lldb-defines.h
index 515f7cff58d8a..e3f88c4681a53 100644
--- a/lldb/include/lldb/lldb-defines.h
+++ b/lldb/include/lldb/lldb-defines.h
@@ -91,6 +91,7 @@
#define LLDB_INVALID_THREAD_ID 0
#define LLDB_INVALID_FRAME_ID UINT32_MAX
#define LLDB_UNWINDER_FRAME_LIST_ID 0
+#define LLDB_INVALID_FRAME_PROVIDER_ID UINT32_MAX
#define LLDB_INVALID_SIGNAL_NUMBER INT32_MAX
#define LLDB_INVALID_SYMBOL_ID UINT32_MAX
#define LLDB_INVALID_OFFSET UINT64_MAX // Must match max of lldb::offset_t
diff --git a/lldb/source/Target/SyntheticFrameProvider.cpp b/lldb/source/Target/SyntheticFrameProvider.cpp
index 0e4408eaad480..09f2cfae0ff63 100644
--- a/lldb/source/Target/SyntheticFrameProvider.cpp
+++ b/lldb/source/Target/SyntheticFrameProvider.cpp
@@ -27,17 +27,17 @@ void ScriptedFrameProviderDescriptor::Dump(Stream *s) const {
if (!s)
return;
- s->Format(" ID: {0:x}\n", GetID());
- s->Printf(" Name: %s\n", GetName().str().c_str());
+ s->Format(" ID: {0}\n", GetID());
+ s->Format(" Name: {0}\n", GetName());
std::string description = GetDescription();
if (!description.empty())
- s->Printf(" Description: %s\n", description.c_str());
+ s->Format(" Description: {0}\n", description);
// Show priority information.
std::optional<uint32_t> priority = GetPriority();
if (priority.has_value())
- s->Printf(" Priority: %u\n", *priority);
+ s->Format(" Priority: {0}\n", *priority);
else
s->PutCString(" Priority: Default (no priority specified)\n");
@@ -45,21 +45,21 @@ void ScriptedFrameProviderDescriptor::Dump(Stream *s) const {
if (thread_specs.empty()) {
s->PutCString(" Thread Filter: (applies to all threads)\n");
} else {
- s->Printf(" Thread Filter: %zu specification(s)\n", thread_specs.size());
+ s->Format(" Thread Filter: {0} specification(s)\n", thread_specs.size());
for (size_t i = 0; i < thread_specs.size(); ++i) {
const ThreadSpec &spec = thread_specs[i];
- s->Printf(" [%zu] ", i);
+ s->Format(" [{0}] ", i);
spec.GetDescription(s, lldb::eDescriptionLevelVerbose);
- s->PutChar('\n');
+ s->EOL();
}
}
}
-uint32_t ScriptedFrameProviderDescriptor::GetID() const {
+uint32_t ScriptedFrameProviderDescriptor::GetHash() const {
if (!scripted_metadata_sp)
return 0;
- return scripted_metadata_sp->GetID();
+ return scripted_metadata_sp->GetHash();
}
std::string ScriptedFrameProviderDescriptor::GetDescription() const {
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index cba04a67a00cc..cad86bf956e38 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,27 @@ 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 descriptor_hash = descriptor.GetHash();
+ for (const auto &entry : m_frame_provider_descriptors) {
+ if (entry.second.GetHash() == descriptor_hash)
+ GetDebugger().ReportWarning(
+ llvm::formatv("frame provider idx={0} with the same class name and "
+ "arguments is already registered",
+ entry.second.GetID())
+ .str());
+ }
- 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 +3770,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 5504a919c2c5f..9eb52494163ec 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