[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Mon Jul 8 03:08:19 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/97579
>From 223606fa569d4d9ced91461ec10e63ef414ad15e Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 3 Jul 2024 11:43:47 +0200
Subject: [PATCH] [lldb][DataFormatter] Simplify std::map formatter
Depends on:
* https://github.com/llvm/llvm-project/pull/97544
* https://github.com/llvm/llvm-project/pull/97549
* https://github.com/llvm/llvm-project/pull/97551
This patch tries to simplify the way in which the
`std::map` formatter goes from the root `__tree`
pointer to a specific key/value pair.
Previously we would synthesize a structure that
mimicked what `__iter_pointer` looked like
in memory, then called `GetChildCompilerTypeAtIndex`
on it to find the byte offset into that structure
at which the pair was located at, and finally
use that offset through a call to `GetSyntheticChildAtOffset`
to retrieve that pair. Not only was this logic hard to follow,
and encoded the libc++ layout in non-obvious ways, it was also
fragile to alignment miscalculations
(https://github.com/llvm/llvm-project/pull/97443); this would
break after the new layout of std::map landed as part of
inhttps://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to
the `__node_pointer` and uses a straightforward
`GetChildMemberWithName("__value_")` to get to the key/value
we care about. This allows us to get rid of some support
infrastructure/class state.
Ideally we would fix the underlying alignment issue, but
this unblocks the libc++ refactor in the interim,
while also benefitting the formatter in terms of readability (in my opinion).
---
.../Plugins/Language/CPlusPlus/LibCxxMap.cpp | 172 +++++-------------
1 file changed, 48 insertions(+), 124 deletions(-)
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index c2bb3555908be..093e182df1188 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -18,11 +18,31 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-forward.h"
+#include <cstdint>
+#include <locale>
+#include <optional>
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::formatters;
+// The flattened layout of the std::__tree_iterator::__ptr_ looks
+// as follows:
+//
+// The following shows the contiguous block of memory:
+//
+// +-----------------------------+ class __tree_end_node
+// __ptr_ | pointer __left_; |
+// +-----------------------------+ class __tree_node_base
+// | pointer __right_; |
+// | __parent_pointer __parent_; |
+// | bool __is_black_; |
+// +-----------------------------+ class __tree_node
+// | __node_value_type __value_; | <<< our key/value pair
+// +-----------------------------+
+//
+// where __ptr_ has type __iter_pointer.
+
class MapEntry {
public:
MapEntry() = default;
@@ -181,10 +201,6 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
size_t GetIndexOfChildWithName(ConstString name) override;
private:
- bool GetDataType();
-
- void GetValueOffset(const lldb::ValueObjectSP &node);
-
/// Returns the ValueObject for the __tree_node type that
/// holds the key/value pair of the node at index \ref idx.
///
@@ -203,8 +219,7 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
ValueObject *m_tree = nullptr;
ValueObject *m_root_node = nullptr;
- CompilerType m_element_type;
- uint32_t m_skip_size = UINT32_MAX;
+ CompilerType m_node_ptr_type;
size_t m_count = UINT32_MAX;
std::map<size_t, MapIterator> m_iterators;
};
@@ -234,7 +249,7 @@ class LibCxxMapIteratorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
LibcxxStdMapSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
- : SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_iterators() {
+ : SyntheticChildrenFrontEnd(*valobj_sp) {
if (valobj_sp)
Update();
}
@@ -260,146 +275,52 @@ llvm::Expected<uint32_t> lldb_private::formatters::
return m_count;
}
-bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
- if (m_element_type.IsValid())
- return true;
- m_element_type.Clear();
- ValueObjectSP deref;
- Status error;
- deref = m_root_node->Dereference(error);
- if (!deref || error.Fail())
- return false;
- deref = deref->GetChildMemberWithName("__value_");
- if (deref) {
- m_element_type = deref->GetCompilerType();
- return true;
- }
- deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
- if (!deref)
- return false;
- m_element_type = deref->GetCompilerType()
- .GetTypeTemplateArgument(1)
- .GetTypeTemplateArgument(1);
- if (m_element_type) {
- std::string name;
- uint64_t bit_offset_ptr;
- uint32_t bitfield_bit_size_ptr;
- bool is_bitfield_ptr;
- m_element_type = m_element_type.GetFieldAtIndex(
- 0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
- m_element_type = m_element_type.GetTypedefedType();
- return m_element_type.IsValid();
- } else {
- m_element_type = m_backend.GetCompilerType().GetTypeTemplateArgument(0);
- return m_element_type.IsValid();
- }
-}
-
-void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
- const lldb::ValueObjectSP &node) {
- if (m_skip_size != UINT32_MAX)
- return;
- if (!node)
- return;
- CompilerType node_type(node->GetCompilerType());
- uint64_t bit_offset;
- if (node_type.GetIndexOfFieldWithName("__value_", nullptr, &bit_offset) !=
- UINT32_MAX) {
- // Old layout (pre d05b10ab4fc65)
- m_skip_size = bit_offset / 8u;
- } else {
- auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
- if (!ast_ctx)
- return;
- CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
- llvm::StringRef(),
- {{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
- {"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
- {"payload", (m_element_type.GetCompleteType(), m_element_type)}});
- std::string child_name;
- uint32_t child_byte_size;
- int32_t child_byte_offset = 0;
- uint32_t child_bitfield_bit_size;
- uint32_t child_bitfield_bit_offset;
- bool child_is_base_class;
- bool child_is_deref_of_parent;
- uint64_t language_flags;
- auto child_type =
- llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
- nullptr, 4, true, true, true, child_name, child_byte_size,
- child_byte_offset, child_bitfield_bit_size,
- child_bitfield_bit_offset, child_is_base_class,
- child_is_deref_of_parent, nullptr, language_flags));
- if (child_type && child_type->IsValid())
- m_skip_size = (uint32_t)child_byte_offset;
- }
-}
-
ValueObjectSP
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
size_t idx, size_t max_depth) {
MapIterator iterator(m_root_node, max_depth);
- const bool need_to_skip = (idx > 0);
- size_t actual_advance = idx;
- if (need_to_skip) {
+ size_t advance_by = idx;
+ if (idx > 0) {
// If we have already created the iterator for the previous
// index, we can start from there and advance by 1.
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
iterator = cached_iterator->second;
- actual_advance = 1;
+ advance_by = 1;
}
}
- ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+ ValueObjectSP iterated_sp(iterator.advance(advance_by));
if (!iterated_sp)
// this tree is garbage - stop
return nullptr;
- if (!GetDataType())
+ if (!m_node_ptr_type.IsValid())
return nullptr;
- if (!need_to_skip) {
- Status error;
- iterated_sp = iterated_sp->Dereference(error);
- if (!iterated_sp || error.Fail())
- return nullptr;
-
- GetValueOffset(iterated_sp);
- auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
- if (child_sp) {
- // Old layout (pre 089a7cc5dea)
- iterated_sp = child_sp;
- } else {
- iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
- m_skip_size, m_element_type, true);
- }
+ // iterated_sp is a __iter_pointer at this point.
+ // We can cast it to a __node_pointer (which is what libc++ does).
+ auto value_type_sp = iterated_sp->Cast(m_node_ptr_type);
+ if (!value_type_sp)
+ return nullptr;
- if (!iterated_sp)
- return nullptr;
- } else {
- // because of the way our debug info is made, we need to read item 0
- // first so that we can cache information used to generate other elements
- if (m_skip_size == UINT32_MAX)
- GetChildAtIndex(0);
-
- if (m_skip_size == UINT32_MAX)
- return nullptr;
-
- iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
- m_element_type, true);
- if (!iterated_sp)
- return nullptr;
- }
+ // After dereferencing the __node_pointer, we will have a handle to
+ // a std::__1::__tree_node<void *>, which has the __value_ member
+ // we are looking for.
+ Status s;
+ value_type_sp = value_type_sp->Dereference(s);
+ if (!value_type_sp || s.Fail())
+ return nullptr;
+
+ // Finally, get the key/value pair.
+ value_type_sp = value_type_sp->GetChildMemberWithName("__value_");
+ if (!value_type_sp)
+ return nullptr;
m_iterators[idx] = iterator;
- assert(iterated_sp != nullptr &&
- "Cached MapIterator for invalid ValueObject");
- return iterated_sp;
+ return value_type_sp;
}
lldb::ValueObjectSP
@@ -459,6 +380,9 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
if (!m_tree)
return lldb::ChildCacheState::eRefetch;
m_root_node = m_tree->GetChildMemberWithName("__begin_node_").get();
+ m_node_ptr_type =
+ m_tree->GetCompilerType().GetDirectNestedTypeWithName("__node_pointer");
+
return lldb::ChildCacheState::eRefetch;
}
More information about the lldb-commits
mailing list