[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::unordered_map::iterator formatter (PR #97754)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 8 06:30:54 PDT 2024


https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/97754

>From b10f76bd6d02106e80315a70a7b72461cb6f2a99 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 4 Jul 2024 13:35:21 +0200
Subject: [PATCH 1/3] [lldb][DataFormatter] Move std::unordered_map::iterator
 formatter into LibCxxUnorderedMap.cpp

---
 .../Plugins/Language/CPlusPlus/LibCxx.cpp     | 149 -------------
 .../Plugins/Language/CPlusPlus/LibCxx.h       |  54 +----
 .../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 196 ++++++++++++++++++
 3 files changed, 200 insertions(+), 199 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 05cfa0568c25d..feaa51a96843a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -202,155 +202,6 @@ bool lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
   return true;
 }
 
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-    LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-    : SyntheticChildrenFrontEnd(*valobj_sp) {
-  if (valobj_sp)
-    Update();
-}
-
-lldb::ChildCacheState lldb_private::formatters::
-    LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
-  m_pair_sp.reset();
-  m_iter_ptr = nullptr;
-
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-    return lldb::ChildCacheState::eRefetch;
-
-  TargetSP target_sp(valobj_sp->GetTargetSP());
-
-  if (!target_sp)
-    return lldb::ChildCacheState::eRefetch;
-
-  if (!valobj_sp)
-    return lldb::ChildCacheState::eRefetch;
-
-  auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
-                             .DontCheckDotVsArrowSyntax()
-                             .SetSyntheticChildrenTraversal(
-                                 ValueObject::GetValueForExpressionPathOptions::
-                                     SyntheticChildrenTraversal::None);
-
-  // This must be a ValueObject* because it is a child of the ValueObject we
-  // are producing children for it if were a ValueObjectSP, we would end up
-  // with a loop (iterator -> synthetic -> child -> parent == iterator) and
-  // that would in turn leak memory by never allowing the ValueObjects to die
-  // and free their memory.
-  m_iter_ptr =
-      valobj_sp
-          ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
-                                      exprPathOptions, nullptr)
-          .get();
-
-  if (m_iter_ptr) {
-    auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-    if (!iter_child) {
-      m_iter_ptr = nullptr;
-      return lldb::ChildCacheState::eRefetch;
-    }
-
-    CompilerType node_type(iter_child->GetCompilerType()
-                               .GetTypeTemplateArgument(0)
-                               .GetPointeeType());
-
-    CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-    std::string name;
-    uint64_t bit_offset_ptr;
-    uint32_t bitfield_bit_size_ptr;
-    bool is_bitfield_ptr;
-
-    pair_type = pair_type.GetFieldAtIndex(
-        0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
-    if (!pair_type) {
-      m_iter_ptr = nullptr;
-      return lldb::ChildCacheState::eRefetch;
-    }
-
-    uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-    m_iter_ptr = nullptr;
-
-    if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
-      return lldb::ChildCacheState::eRefetch;
-
-    auto ts = pair_type.GetTypeSystem();
-    auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>();
-    if (!ast_ctx)
-      return lldb::ChildCacheState::eRefetch;
-
-    // Mimick layout of std::__hash_iterator::__node_ and read it in
-    // from process memory.
-    //
-    // The following shows the contiguous block of memory:
-    //
-    //         +-----------------------------+ class __hash_node_base
-    // __node_ | __next_pointer __next_;     |
-    //         +-----------------------------+ class __hash_node
-    //         | size_t __hash_;             |
-    //         | __node_value_type __value_; | <<< our key/value pair
-    //         +-----------------------------+
-    //
-    CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-        llvm::StringRef(),
-        {{"__next_",
-          ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
-         {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
-         {"__value_", pair_type}});
-    std::optional<uint64_t> size = tree_node_type.GetByteSize(nullptr);
-    if (!size)
-      return lldb::ChildCacheState::eRefetch;
-    WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-    ProcessSP process_sp(target_sp->GetProcessSP());
-    Status error;
-    process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
-                           buffer_sp->GetByteSize(), error);
-    if (error.Fail())
-      return lldb::ChildCacheState::eRefetch;
-    DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
-                            process_sp->GetAddressByteSize());
-    auto pair_sp = CreateValueObjectFromData(
-        "pair", extractor, valobj_sp->GetExecutionContextRef(), tree_node_type);
-    if (pair_sp)
-      m_pair_sp = pair_sp->GetChildAtIndex(2);
-  }
-
-  return lldb::ChildCacheState::eRefetch;
-}
-
-llvm::Expected<uint32_t> lldb_private::formatters::
-    LibCxxUnorderedMapIteratorSyntheticFrontEnd::CalculateNumChildren() {
-  return 2;
-}
-
-lldb::ValueObjectSP lldb_private::formatters::
-    LibCxxUnorderedMapIteratorSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
-  if (m_pair_sp)
-    return m_pair_sp->GetChildAtIndex(idx);
-  return lldb::ValueObjectSP();
-}
-
-bool lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-    MightHaveChildren() {
-  return true;
-}
-
-size_t lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
-    GetIndexOfChildWithName(ConstString name) {
-  if (name == "first")
-    return 0;
-  if (name == "second")
-    return 1;
-  return UINT32_MAX;
-}
-
-SyntheticChildrenFrontEnd *
-lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEndCreator(
-    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
-  return (valobj_sp ? new LibCxxUnorderedMapIteratorSyntheticFrontEnd(valobj_sp)
-                    : nullptr);
-}
-
 /*
  (lldb) fr var ibeg --raw --ptr-depth 1 -T
  (std::__1::__wrap_iter<int *>) ibeg = {
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
index 21dba015d1ba1..5307b5251ca84 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -87,56 +87,6 @@ bool LibcxxContainerSummaryProvider(ValueObject &valobj, Stream &stream,
 bool LibcxxSpanSummaryProvider(ValueObject &valobj, Stream &stream,
                                const TypeSummaryOptions &options);
 
-/// Formats libcxx's std::unordered_map iterators
-///
-/// In raw form a std::unordered_map::iterator is represented as follows:
-///
-/// (lldb) var it --raw --ptr-depth 1
-/// (std::__1::__hash_map_iterator<
-///    std::__1::__hash_iterator<
-///      std::__1::__hash_node<
-///        std::__1::__hash_value_type<
-///            std::__1::basic_string<char, std::__1::char_traits<char>,
-///            std::__1::allocator<char> >, std::__1::basic_string<char,
-///            std::__1::char_traits<char>, std::__1::allocator<char> > >,
-///        void *> *> >)
-///  it = {
-///   __i_ = {
-///     __node_ = 0x0000600001700040 {
-///       __next_ = 0x0000600001704000
-///     }
-///   }
-/// }
-class LibCxxUnorderedMapIteratorSyntheticFrontEnd
-    : public SyntheticChildrenFrontEnd {
-public:
-  LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
-
-  ~LibCxxUnorderedMapIteratorSyntheticFrontEnd() override = default;
-
-  llvm::Expected<uint32_t> CalculateNumChildren() override;
-
-  lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override;
-
-  lldb::ChildCacheState Update() override;
-
-  bool MightHaveChildren() override;
-
-  size_t GetIndexOfChildWithName(ConstString name) override;
-
-private:
-  ValueObject *m_iter_ptr = nullptr; ///< Held, not owned. Child of iterator
-                                     ///< ValueObject supplied at construction.
-
-  lldb::ValueObjectSP m_pair_sp; ///< ValueObject for the key/value pair
-                                 ///< that the iterator currently points
-                                 ///< to.
-};
-
-SyntheticChildrenFrontEnd *
-LibCxxUnorderedMapIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
-                                                   lldb::ValueObjectSP);
-
 SyntheticChildrenFrontEnd *
 LibCxxVectorIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                              lldb::ValueObjectSP);
@@ -230,6 +180,10 @@ SyntheticChildrenFrontEnd *
 LibcxxStdUnorderedMapSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                               lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibCxxUnorderedMapIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
+                                                   lldb::ValueObjectSP);
+
 SyntheticChildrenFrontEnd *
 LibcxxInitializerListSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                               lldb::ValueObjectSP);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index af29fdb6d0010..f5be2f5a5c317 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -51,6 +51,53 @@ class LibcxxStdUnorderedMapSyntheticFrontEnd
   ValueObject *m_next_element = nullptr;
   std::vector<std::pair<ValueObject *, uint64_t>> m_elements_cache;
 };
+
+/// Formats libcxx's std::unordered_map iterators
+///
+/// In raw form a std::unordered_map::iterator is represented as follows:
+///
+/// (lldb) var it --raw --ptr-depth 1
+/// (std::__1::__hash_map_iterator<
+///    std::__1::__hash_iterator<
+///      std::__1::__hash_node<
+///        std::__1::__hash_value_type<
+///            std::__1::basic_string<char, std::__1::char_traits<char>,
+///            std::__1::allocator<char> >, std::__1::basic_string<char,
+///            std::__1::char_traits<char>, std::__1::allocator<char> > >,
+///        void *> *> >)
+///  it = {
+///   __i_ = {
+///     __node_ = 0x0000600001700040 {
+///       __next_ = 0x0000600001704000
+///     }
+///   }
+/// }
+class LibCxxUnorderedMapIteratorSyntheticFrontEnd
+    : public SyntheticChildrenFrontEnd {
+public:
+  LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+  ~LibCxxUnorderedMapIteratorSyntheticFrontEnd() override = default;
+
+  llvm::Expected<uint32_t> CalculateNumChildren() override;
+
+  lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override;
+
+  lldb::ChildCacheState Update() override;
+
+  bool MightHaveChildren() override;
+
+  size_t GetIndexOfChildWithName(ConstString name) override;
+
+private:
+  ValueObject *m_iter_ptr = nullptr; ///< Held, not owned. Child of iterator
+                                     ///< ValueObject supplied at construction.
+
+  lldb::ValueObjectSP m_pair_sp; ///< ValueObject for the key/value pair
+                                 ///< that the iterator currently points
+                                 ///< to.
+};
+
 } // namespace formatters
 } // namespace lldb_private
 
@@ -246,3 +293,152 @@ lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEndCreator(
   return (valobj_sp ? new LibcxxStdUnorderedMapSyntheticFrontEnd(valobj_sp)
                     : nullptr);
 }
+
+lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+    : SyntheticChildrenFrontEnd(*valobj_sp) {
+  if (valobj_sp)
+    Update();
+}
+
+lldb::ChildCacheState lldb_private::formatters::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
+  m_pair_sp.reset();
+  m_iter_ptr = nullptr;
+
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+    return lldb::ChildCacheState::eRefetch;
+
+  TargetSP target_sp(valobj_sp->GetTargetSP());
+
+  if (!target_sp)
+    return lldb::ChildCacheState::eRefetch;
+
+  if (!valobj_sp)
+    return lldb::ChildCacheState::eRefetch;
+
+  auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
+                             .DontCheckDotVsArrowSyntax()
+                             .SetSyntheticChildrenTraversal(
+                                 ValueObject::GetValueForExpressionPathOptions::
+                                     SyntheticChildrenTraversal::None);
+
+  // This must be a ValueObject* because it is a child of the ValueObject we
+  // are producing children for it if were a ValueObjectSP, we would end up
+  // with a loop (iterator -> synthetic -> child -> parent == iterator) and
+  // that would in turn leak memory by never allowing the ValueObjects to die
+  // and free their memory.
+  m_iter_ptr =
+      valobj_sp
+          ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
+                                      exprPathOptions, nullptr)
+          .get();
+
+  if (m_iter_ptr) {
+    auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
+    if (!iter_child) {
+      m_iter_ptr = nullptr;
+      return lldb::ChildCacheState::eRefetch;
+    }
+
+    CompilerType node_type(iter_child->GetCompilerType()
+                               .GetTypeTemplateArgument(0)
+                               .GetPointeeType());
+
+    CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
+
+    std::string name;
+    uint64_t bit_offset_ptr;
+    uint32_t bitfield_bit_size_ptr;
+    bool is_bitfield_ptr;
+
+    pair_type = pair_type.GetFieldAtIndex(
+        0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
+    if (!pair_type) {
+      m_iter_ptr = nullptr;
+      return lldb::ChildCacheState::eRefetch;
+    }
+
+    uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
+    m_iter_ptr = nullptr;
+
+    if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
+      return lldb::ChildCacheState::eRefetch;
+
+    auto ts = pair_type.GetTypeSystem();
+    auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>();
+    if (!ast_ctx)
+      return lldb::ChildCacheState::eRefetch;
+
+    // Mimick layout of std::__hash_iterator::__node_ and read it in
+    // from process memory.
+    //
+    // The following shows the contiguous block of memory:
+    //
+    //         +-----------------------------+ class __hash_node_base
+    // __node_ | __next_pointer __next_;     |
+    //         +-----------------------------+ class __hash_node
+    //         | size_t __hash_;             |
+    //         | __node_value_type __value_; | <<< our key/value pair
+    //         +-----------------------------+
+    //
+    CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+        llvm::StringRef(),
+        {{"__next_",
+          ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+         {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
+         {"__value_", pair_type}});
+    std::optional<uint64_t> size = tree_node_type.GetByteSize(nullptr);
+    if (!size)
+      return lldb::ChildCacheState::eRefetch;
+    WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
+    ProcessSP process_sp(target_sp->GetProcessSP());
+    Status error;
+    process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
+                           buffer_sp->GetByteSize(), error);
+    if (error.Fail())
+      return lldb::ChildCacheState::eRefetch;
+    DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
+                            process_sp->GetAddressByteSize());
+    auto pair_sp = CreateValueObjectFromData(
+        "pair", extractor, valobj_sp->GetExecutionContextRef(), tree_node_type);
+    if (pair_sp)
+      m_pair_sp = pair_sp->GetChildAtIndex(2);
+  }
+
+  return lldb::ChildCacheState::eRefetch;
+}
+
+llvm::Expected<uint32_t> lldb_private::formatters::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd::CalculateNumChildren() {
+  return 2;
+}
+
+lldb::ValueObjectSP lldb_private::formatters::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
+  if (m_pair_sp)
+    return m_pair_sp->GetChildAtIndex(idx);
+  return lldb::ValueObjectSP();
+}
+
+bool lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    MightHaveChildren() {
+  return true;
+}
+
+size_t lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    GetIndexOfChildWithName(ConstString name) {
+  if (name == "first")
+    return 0;
+  if (name == "second")
+    return 1;
+  return UINT32_MAX;
+}
+
+SyntheticChildrenFrontEnd *
+lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEndCreator(
+    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+  return (valobj_sp ? new LibCxxUnorderedMapIteratorSyntheticFrontEnd(valobj_sp)
+                    : nullptr);
+}

>From 840655d932ae263569c098e93d58a999c1ece644 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 4 Jul 2024 14:35:22 +0200
Subject: [PATCH 2/3] [lldb][DataFormatter] Simplify
 std::unordered_map::iterator formatter

---
 .../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 156 ++++++------------
 .../TestDataFormatterLibccIterator.py         |  16 ++
 .../libcxx/iterator/main.cpp                  |  48 ++++--
 3 files changed, 96 insertions(+), 124 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index f5be2f5a5c317..794f1ff8906de 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -52,26 +52,6 @@ class LibcxxStdUnorderedMapSyntheticFrontEnd
   std::vector<std::pair<ValueObject *, uint64_t>> m_elements_cache;
 };
 
-/// Formats libcxx's std::unordered_map iterators
-///
-/// In raw form a std::unordered_map::iterator is represented as follows:
-///
-/// (lldb) var it --raw --ptr-depth 1
-/// (std::__1::__hash_map_iterator<
-///    std::__1::__hash_iterator<
-///      std::__1::__hash_node<
-///        std::__1::__hash_value_type<
-///            std::__1::basic_string<char, std::__1::char_traits<char>,
-///            std::__1::allocator<char> >, std::__1::basic_string<char,
-///            std::__1::char_traits<char>, std::__1::allocator<char> > >,
-///        void *> *> >)
-///  it = {
-///   __i_ = {
-///     __node_ = 0x0000600001700040 {
-///       __next_ = 0x0000600001704000
-///     }
-///   }
-/// }
 class LibCxxUnorderedMapIteratorSyntheticFrontEnd
     : public SyntheticChildrenFrontEnd {
 public:
@@ -90,9 +70,6 @@ class LibCxxUnorderedMapIteratorSyntheticFrontEnd
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  ValueObject *m_iter_ptr = nullptr; ///< Held, not owned. Child of iterator
-                                     ///< ValueObject supplied at construction.
-
   lldb::ValueObjectSP m_pair_sp; ///< ValueObject for the key/value pair
                                  ///< that the iterator currently points
                                  ///< to.
@@ -304,7 +281,6 @@ lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
 lldb::ChildCacheState lldb_private::formatters::
     LibCxxUnorderedMapIteratorSyntheticFrontEnd::Update() {
   m_pair_sp.reset();
-  m_iter_ptr = nullptr;
 
   ValueObjectSP valobj_sp = m_backend.GetSP();
   if (!valobj_sp)
@@ -315,98 +291,66 @@ lldb::ChildCacheState lldb_private::formatters::
   if (!target_sp)
     return lldb::ChildCacheState::eRefetch;
 
-  if (!valobj_sp)
+  // Get the unordered_map::iterator
+  // m_backend is an 'unordered_map::iterator', aka a
+  // '__hash_map_iterator<__hash_table::iterator>'
+  //
+  // __hash_map_iterator::__i_ is a __hash_table::iterator (aka
+  // __hash_iterator<__node_pointer>)
+  auto hash_iter_sp = valobj_sp->GetChildMemberWithName("__i_");
+  if (!hash_iter_sp)
     return lldb::ChildCacheState::eRefetch;
 
-  auto exprPathOptions = ValueObject::GetValueForExpressionPathOptions()
-                             .DontCheckDotVsArrowSyntax()
-                             .SetSyntheticChildrenTraversal(
-                                 ValueObject::GetValueForExpressionPathOptions::
-                                     SyntheticChildrenTraversal::None);
-
-  // This must be a ValueObject* because it is a child of the ValueObject we
-  // are producing children for it if were a ValueObjectSP, we would end up
-  // with a loop (iterator -> synthetic -> child -> parent == iterator) and
-  // that would in turn leak memory by never allowing the ValueObjects to die
-  // and free their memory.
-  m_iter_ptr =
-      valobj_sp
-          ->GetValueForExpressionPath(".__i_.__node_", nullptr, nullptr,
-                                      exprPathOptions, nullptr)
-          .get();
-
-  if (m_iter_ptr) {
-    auto iter_child(valobj_sp->GetChildMemberWithName("__i_"));
-    if (!iter_child) {
-      m_iter_ptr = nullptr;
-      return lldb::ChildCacheState::eRefetch;
-    }
-
-    CompilerType node_type(iter_child->GetCompilerType()
-                               .GetTypeTemplateArgument(0)
-                               .GetPointeeType());
-
-    CompilerType pair_type(node_type.GetTypeTemplateArgument(0));
-
-    std::string name;
-    uint64_t bit_offset_ptr;
-    uint32_t bitfield_bit_size_ptr;
-    bool is_bitfield_ptr;
-
-    pair_type = pair_type.GetFieldAtIndex(
-        0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
-    if (!pair_type) {
-      m_iter_ptr = nullptr;
-      return lldb::ChildCacheState::eRefetch;
-    }
+  // Type is '__hash_iterator<__node_pointer>'
+  auto hash_iter_type = hash_iter_sp->GetCompilerType();
+  if (!hash_iter_type.IsValid())
+    return lldb::ChildCacheState::eRefetch;
 
-    uint64_t addr = m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
-    m_iter_ptr = nullptr;
+  // Type is '__node_pointer'
+  auto node_pointer_type = hash_iter_type.GetTypeTemplateArgument(0);
+  if (!node_pointer_type.IsValid())
+    return lldb::ChildCacheState::eRefetch;
 
-    if (addr == 0 || addr == LLDB_INVALID_ADDRESS)
-      return lldb::ChildCacheState::eRefetch;
+  // Cast the __hash_iterator to a __node_pointer (which stores our key/value
+  // pair)
+  auto hash_node_sp = hash_iter_sp->Cast(node_pointer_type);
+  if (!hash_node_sp)
+    return lldb::ChildCacheState::eRefetch;
 
-    auto ts = pair_type.GetTypeSystem();
-    auto ast_ctx = ts.dyn_cast_or_null<TypeSystemClang>();
-    if (!ast_ctx)
+  auto key_value_sp = hash_node_sp->GetChildMemberWithName("__value_");
+  if (!key_value_sp) {
+    // clang-format off
+    // Since D101206 (ba79fb2e1f), libc++ wraps the `__value_` in an
+    // anonymous union.
+    // Child 0: __hash_node_base base class
+    // Child 1: __hash_
+    // Child 2: anonymous union
+    // clang-format on
+    auto anon_union_sp = hash_node_sp->GetChildAtIndex(2);
+    if (!anon_union_sp)
       return lldb::ChildCacheState::eRefetch;
 
-    // Mimick layout of std::__hash_iterator::__node_ and read it in
-    // from process memory.
-    //
-    // The following shows the contiguous block of memory:
-    //
-    //         +-----------------------------+ class __hash_node_base
-    // __node_ | __next_pointer __next_;     |
-    //         +-----------------------------+ class __hash_node
-    //         | size_t __hash_;             |
-    //         | __node_value_type __value_; | <<< our key/value pair
-    //         +-----------------------------+
-    //
-    CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
-        llvm::StringRef(),
-        {{"__next_",
-          ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
-         {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
-         {"__value_", pair_type}});
-    std::optional<uint64_t> size = tree_node_type.GetByteSize(nullptr);
-    if (!size)
-      return lldb::ChildCacheState::eRefetch;
-    WritableDataBufferSP buffer_sp(new DataBufferHeap(*size, 0));
-    ProcessSP process_sp(target_sp->GetProcessSP());
-    Status error;
-    process_sp->ReadMemory(addr, buffer_sp->GetBytes(),
-                           buffer_sp->GetByteSize(), error);
-    if (error.Fail())
+    key_value_sp = anon_union_sp->GetChildMemberWithName("__value_");
+    if (!key_value_sp)
       return lldb::ChildCacheState::eRefetch;
-    DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
-                            process_sp->GetAddressByteSize());
-    auto pair_sp = CreateValueObjectFromData(
-        "pair", extractor, valobj_sp->GetExecutionContextRef(), tree_node_type);
-    if (pair_sp)
-      m_pair_sp = pair_sp->GetChildAtIndex(2);
   }
 
+  // Create the synthetic child, which is a pair where the key and value can be
+  // retrieved // by querying the synthetic frontend for
+  // GetIndexOfChildWithName("first") and GetIndexOfChildWithName("second")
+  // respectively.
+  //
+  // std::unordered_map stores the actual key/value pair in
+  // __hash_value_type::__cc_ (or previously __cc).
+  auto potential_child_sp = key_value_sp->Clone(ConstString("pair"));
+  if (potential_child_sp)
+    if (potential_child_sp->GetNumChildrenIgnoringErrors() == 1)
+      if (auto child0_sp = potential_child_sp->GetChildAtIndex(0);
+          child0_sp->GetName() == "__cc_" || child0_sp->GetName() == "__cc")
+        potential_child_sp = child0_sp->Clone(ConstString("pair"));
+
+  m_pair_sp = potential_child_sp;
+
   return lldb::ChildCacheState::eRefetch;
 }
 
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
index d9e316b9b8f4e..efd7128cd6ac7 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
@@ -59,3 +59,19 @@ def cleanup():
 
         self.expect("frame variable svI", substrs=['item = "hello"'])
         self.expect("expr svI", substrs=['item = "hello"'])
+
+        self.expect("frame variable iiumI", substrs=["first = 61453", "second = 51966"])
+        self.expect("expr iiumI", substrs=["first = 61453", "second = 51966"])
+
+        self.expect("frame variable siumI", substrs=['first = "hello"', "second = 137"])
+        self.expect("expr siumI", substrs=['first = "hello"', "second = 137"])
+
+        self.expect("frame variable iiumI.first", substrs=["first = 61453"])
+        self.expect("frame variable iiumI.first", substrs=["second"], matching=False)
+        self.expect("frame variable iiumI.second", substrs=["second = 51966"])
+        self.expect("frame variable iiumI.second", substrs=["first"], matching=False)
+
+        self.expect("frame variable siumI.first", substrs=['first = "hello"'])
+        self.expect("frame variable siumI.first", substrs=["second"], matching=False)
+        self.expect("frame variable siumI.second", substrs=["second = 137"])
+        self.expect("frame variable siumI.second", substrs=["first"], matching=False)
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/main.cpp
index 9d1cbfd912868..e53c0f167c325 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/main.cpp
@@ -1,38 +1,50 @@
-#include <string>
 #include <map>
+#include <string>
 #include <vector>
 
 typedef std::map<int, int> intint_map;
 typedef std::map<std::string, int> strint_map;
 
+typedef std::unordered_map<int, int> intint_umap;
+typedef std::unordered_map<std::string, int> strint_umap;
+
 typedef std::vector<int> int_vector;
 typedef std::vector<std::string> string_vector;
 
-typedef intint_map::iterator iimter;
-typedef strint_map::iterator simter;
+typedef intint_map::iterator ii_map_iter;
+typedef strint_map::iterator si_map_iter;
+typedef intint_umap::iterator ii_umap_iter;
+typedef strint_umap::iterator si_umap_iter;
 
 typedef int_vector::iterator ivter;
 typedef string_vector::iterator svter;
 
-int main()
-{
-	intint_map iim;
-	iim[0xABCD] = 0xF0F1;
+int main() {
+  intint_map iim;
+  iim[0xABCD] = 0xF0F1;
+
+  strint_map sim;
+  sim["world"] = 42;
+
+  intint_umap iium;
+  iium[0xF00D] = 0xCAFE;
 
-	strint_map sim;
-	sim["world"] = 42;
+  strint_umap sium;
+  sium["hello"] = 137;
 
-	int_vector iv;
-	iv.push_back(3);
+  int_vector iv;
+  iv.push_back(3);
 
-	string_vector sv;
-	sv.push_back("hello");
+  string_vector sv;
+  sv.push_back("hello");
 
-	iimter iimI = iim.begin();
-	simter simI = sim.begin();
+  ii_map_iter iimI = iim.begin();
+  si_map_iter simI = sim.begin();
+  ii_umap_iter iiumI = iium.begin();
+  si_umap_iter siumI = sium.begin();
 
-	ivter ivI = iv.begin();
-	svter svI = sv.begin();
+  ivter ivI = iv.begin();
+  svter svI = sv.begin();
 
-	return 0; // Set break point at this line.
+  return 0; // Set break point at this line.
 }

>From 4866450a9bf0ca1a15e0e96600b6a7fdf7704d6b Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 8 Jul 2024 14:30:40 +0100
Subject: [PATCH 3/3] fixup! fix comment

---
 lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
index 794f1ff8906de..93e7f4f4fd86c 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp
@@ -336,7 +336,7 @@ lldb::ChildCacheState lldb_private::formatters::
   }
 
   // Create the synthetic child, which is a pair where the key and value can be
-  // retrieved // by querying the synthetic frontend for
+  // retrieved by querying the synthetic frontend for
   // GetIndexOfChildWithName("first") and GetIndexOfChildWithName("second")
   // respectively.
   //



More information about the lldb-commits mailing list