[Lldb-commits] [lldb] [lldb][DataFormatter] Simplify std::map formatter (PR #97579)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 3 06:58:36 PDT 2024


https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/97579

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/https://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).

>From 9dabd3a399f37789b6a9bc7578b76e738c344f1d Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 3 Jul 2024 10:55:40 +0200
Subject: [PATCH 1/3] [lldb][DataFormatter][NFC] Factor out MapIterator logic
 into separate helper

This patch factors all the logic for advancing the `MapIterator`
out of `GetChildAtIndex`. This, in my opinion, helps readability,
and will be useful for upcoming cleanups in this area.

While here, some drive-by changes:
* added a couple of clarification comments
* fixed a variable name typo
* turned the `return lldb::ValueObjectSP()` into `return nullptr`
* added an assertion to make sure we keep the iterator cache in a valid
  state
---
 .../Plugins/Language/CPlusPlus/LibCxxMap.cpp  | 115 +++++++++++-------
 1 file changed, 72 insertions(+), 43 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 44fe294ced722..370dfa35e7703 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -17,6 +17,7 @@
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
+#include "lldb/lldb-forward.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 
   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.
+  ///
+  /// \param[in] idx The child index that we're looking to get
+  ///                the key/value pair for.
+  ///
+  /// \param[in] max_depth The maximum search depth after which
+  ///                      we stop trying to find the key/value
+  ///                      pair for.
+  ///
+  /// \returns On success, returns the ValueObjectSP corresponding
+  ///          to the __tree_node's __value_ member (which holds
+  ///          the key/value pair the formatter wants to display).
+  ///          On failure, will return nullptr.
+  ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth);
+
   ValueObject *m_tree = nullptr;
   ValueObject *m_root_node = nullptr;
   CompilerType m_element_type;
@@ -299,75 +316,88 @@ void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
   }
 }
 
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
-    uint32_t idx) {
-  static ConstString g_cc_("__cc_"), g_cc("__cc");
-  static ConstString g_nc("__nc");
-  uint32_t num_children = CalculateNumChildrenIgnoringErrors();
-  if (idx >= num_children)
-    return lldb::ValueObjectSP();
-  if (m_tree == nullptr || m_root_node == nullptr)
-    return lldb::ValueObjectSP();
-
-  MapIterator iterator(m_root_node, num_children);
+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_advancde = idx;
+  size_t actual_advance = idx;
   if (need_to_skip) {
+    // 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_advancde = 1;
+      actual_advance = 1;
     }
   }
 
-  ValueObjectSP iterated_sp(iterator.advance(actual_advancde));
-  if (!iterated_sp) {
+  ValueObjectSP iterated_sp(iterator.advance(actual_advance));
+  if (!iterated_sp)
     // this tree is garbage - stop
-    m_tree =
-        nullptr; // this will stop all future searches until an Update() happens
-    return iterated_sp;
-  }
+    return nullptr;
 
-  if (!GetDataType()) {
-    m_tree = nullptr;
-    return lldb::ValueObjectSP();
-  }
+  if (!GetDataType())
+    return nullptr;
 
   if (!need_to_skip) {
     Status error;
     iterated_sp = iterated_sp->Dereference(error);
-    if (!iterated_sp || error.Fail()) {
-      m_tree = nullptr;
-      return lldb::ValueObjectSP();
-    }
+    if (!iterated_sp || error.Fail())
+      return nullptr;
+
     GetValueOffset(iterated_sp);
     auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
-    if (child_sp)
+    if (child_sp) {
+      // Old layout (pre 089a7cc5dea)
       iterated_sp = child_sp;
-    else
+    } else {
       iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
           m_skip_size, m_element_type, true);
-    if (!iterated_sp) {
-      m_tree = nullptr;
-      return lldb::ValueObjectSP();
     }
+
+    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) {
-      m_tree = nullptr;
-      return lldb::ValueObjectSP();
-    }
+
+    if (m_skip_size == UINT32_MAX)
+      return nullptr;
+
     iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
                                                          m_element_type, true);
-    if (!iterated_sp) {
-      m_tree = nullptr;
-      return lldb::ValueObjectSP();
-    }
+    if (!iterated_sp)
+      return nullptr;
+  }
+
+  m_iterators[idx] = iterator;
+  assert(iterated_sp != nullptr &&
+         "Cached MapIterator for invalid ValueObject");
+
+  return iterated_sp;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
+    uint32_t idx) {
+  static ConstString g_cc_("__cc_"), g_cc("__cc");
+  static ConstString g_nc("__nc");
+  uint32_t num_children = CalculateNumChildrenIgnoringErrors();
+  if (idx >= num_children)
+    return nullptr;
+
+  if (m_tree == nullptr || m_root_node == nullptr)
+    return nullptr;
+
+  ValueObjectSP key_val_sp = GetKeyValuePair(idx, /*max_depth=*/num_children);
+  if (!key_val_sp) {
+    // this will stop all future searches until an Update() happens
+    m_tree = nullptr;
+    return nullptr;
   }
 
   // at this point we have a valid
@@ -375,7 +405,7 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
   // all items named __value_
   StreamString name;
   name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString()));
+  auto potential_child_sp = key_val_sp->Clone(ConstString(name.GetString()));
   if (potential_child_sp) {
     switch (potential_child_sp->GetNumChildrenIgnoringErrors()) {
     case 1: {
@@ -396,7 +426,6 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
     }
     }
   }
-  m_iterators[idx] = iterator;
   return potential_child_sp;
 }
 

>From 78fa7f2b0382a187311954bdc40bb371841855e0 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 2/3] [lldb][DataFormatter] Clean up
 LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair

This patch cleans up the core of the `std::map` libc++ formatter.

Depends on https://github.com/llvm/llvm-project/pull/97544 and
https://github.com/llvm/llvm-project/pull/97549.

Changes:
* Renamed `m_skip_size` to `m_value_type_offset` to better
  describe what it's actually for.
* Made updating `m_skip_size` (now `m_value_type_offset`)
  isolated to `GetKeyValuePair` (instead of doing so in the
  `GetValueOffset` helper).
* We don't need special logic for the 0th index, so I merged the
  two `need_to_skip` branches.
---
 .../Plugins/Language/CPlusPlus/LibCxxMap.cpp  | 133 ++++++++----------
 1 file changed, 55 insertions(+), 78 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index 370dfa35e7703..e12704ce28443 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -18,6 +18,8 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/lldb-forward.h"
+#include <cstdint>
+#include <optional>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -183,7 +185,7 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 private:
   bool GetDataType();
 
-  void GetValueOffset(const lldb::ValueObjectSP &node);
+  std::optional<uint32_t> GetValueOffset();
 
   /// Returns the ValueObject for the __tree_node type that
   /// holds the key/value pair of the node at index \ref idx.
@@ -204,7 +206,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;
+  uint32_t m_value_type_offset = UINT32_MAX;
   size_t m_count = UINT32_MAX;
   std::map<size_t, MapIterator> m_iterators;
 };
@@ -274,46 +276,43 @@ bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
   }
 }
 
-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 089a7cc5dea)
-    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;
-  }
+std::optional<uint32_t>
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset() {
+  if (!m_tree)
+    return std::nullopt;
+
+  auto ast_ctx = m_tree->GetCompilerType()
+                     .GetTypeSystem()
+                     .dyn_cast_or_null<TypeSystemClang>();
+  if (!ast_ctx)
+    return std::nullopt;
+
+  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())
+    return std::nullopt;
+
+  return child_byte_offset;
 }
 
 ValueObjectSP
@@ -321,19 +320,18 @@ 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;
@@ -341,42 +339,21 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
   if (!GetDataType())
     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);
-    }
-
-    if (!iterated_sp)
+  if (m_value_type_offset == UINT32_MAX) {
+    if (auto offset = GetValueOffset())
+      m_value_type_offset = *offset;
+    else
       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;
+  assert(m_value_type_offset != UINT32_MAX);
 
-    iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
-                                                         m_element_type, true);
-    if (!iterated_sp)
-      return nullptr;
-  }
+  iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_value_type_offset,
+                                                       m_element_type, true);
+  if (!iterated_sp)
+    return nullptr;
 
   m_iterators[idx] = iterator;
-  assert(iterated_sp != nullptr &&
-         "Cached MapIterator for invalid ValueObject");
 
   return iterated_sp;
 }

>From 6e76c18cfe4f34bc7c691d2bf2155098aa8580cc Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 3 Jul 2024 15:45:10 +0200
Subject: [PATCH 3/3] [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  | 133 ++++++------------
 1 file changed, 40 insertions(+), 93 deletions(-)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index e12704ce28443..120f3ac131b34 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -19,12 +19,30 @@
 #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;
@@ -183,10 +201,6 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  bool GetDataType();
-
-  std::optional<uint32_t> GetValueOffset();
-
   /// Returns the ValueObject for the __tree_node type that
   /// holds the key/value pair of the node at index \ref idx.
   ///
@@ -205,8 +219,7 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 
   ValueObject *m_tree = nullptr;
   ValueObject *m_root_node = nullptr;
-  CompilerType m_element_type;
-  uint32_t m_value_type_offset = UINT32_MAX;
+  CompilerType m_node_ptr_type;
   size_t m_count = UINT32_MAX;
   std::map<size_t, MapIterator> m_iterators;
 };
@@ -215,7 +228,7 @@ class LibcxxStdMapSyntheticFrontEnd : 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();
 }
@@ -241,80 +254,6 @@ 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();
-  }
-}
-
-std::optional<uint32_t>
-lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset() {
-  if (!m_tree)
-    return std::nullopt;
-
-  auto ast_ctx = m_tree->GetCompilerType()
-                     .GetTypeSystem()
-                     .dyn_cast_or_null<TypeSystemClang>();
-  if (!ast_ctx)
-    return std::nullopt;
-
-  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())
-    return std::nullopt;
-
-  return child_byte_offset;
-}
-
 ValueObjectSP
 lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
     size_t idx, size_t max_depth) {
@@ -336,26 +275,31 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
     // this tree is garbage - stop
     return nullptr;
 
-  if (!GetDataType())
+  if (!m_node_ptr_type.IsValid())
     return nullptr;
 
-  if (m_value_type_offset == UINT32_MAX) {
-    if (auto offset = GetValueOffset())
-      m_value_type_offset = *offset;
-    else
-      return nullptr;
-  }
+  // 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;
 
-  assert(m_value_type_offset != UINT32_MAX);
+  // 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;
 
-  iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_value_type_offset,
-                                                       m_element_type, true);
-  if (!iterated_sp)
+  // Finally, get the key/value pair.
+  value_type_sp = value_type_sp->GetChildMemberWithName("__value_");
+  if (!value_type_sp)
     return nullptr;
 
   m_iterators[idx] = iterator;
 
-  return iterated_sp;
+  return value_type_sp;
 }
 
 lldb::ValueObjectSP
@@ -415,6 +359,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