[LLDB][DataFormatter] Add data formatter for libcxx std::unordered_map iterator

[Michael137]

Summary

This patch adds a formatter for libcxx's std::unordered_map iterators.
The implementation follows a similar appraoch to the std::map iterator
formatter. I was hesistant about coupling the two into a common
implementation since the libcxx layouts might change for one of the
the containers but not the other.

All std::unordered_map iterators are covered with this patch:

1. const/non-const key/value iterators
2. const/non-const bucket iterators

Note that, we currently don't have a formatter for std::unordered_map.
This patch doesn't change that, we merely add support for its iterators,
because that's what Xcode users requested. One can still see contents
of std::unordered_map, whereas with iterators it's less ergonomic.

Testing

• Added API test

[Michael137]

• Fixed class documentation

[Michael137]

• Cleaned up documentation further

[Michael137]

• Fixed test case for const bucket iterator

[aprantl]

Nice! Few minor stylistic comments inside.

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

404

Nit: This... and . at the end.

412

Does this get more readable if we assign this subexpression to an auto helper variable?

449

maybe invert the condition and convert to early exit?

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py

14

just curious: what does the _with_run_command suffix mean? Or was this copied from another test?

[Michael137]

• Address stylistic comments
• Change test name

[Michael137]

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

412

Definitely :)

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py

14

Ah good catch. This was from at the map formatter test. Presumably the name refers to its usage of self.runCmd. I'll change it to something more meaningful

lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

AddCXXSynthetic(
       true);
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEndCreator,
       "std::map iterator synthetic children",
       ConstString("^std::__[[:alnum:]]+::__map_iterator<.+>$"), stl_synth_flags,
       true);
+
+  AddCXXSynthetic(
+      cpp_category_sp,
+      lldb_private::formatters::
+          LibCxxUnorderedMapIteratorSyntheticFrontEndCreator,
+      "std::unordered_map iterator synthetic children",
+      ConstString("^std::__[[:alnum:]]+::__hash_map_(const_)?iterator<.+>$"),
+      stl_synth_flags, true);
 }
 
 static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
     return;
 
   TypeSummaryImpl::Flags stl_summary_flags;
   stl_summary_flags.SetCascades(true)

lldb/source/Plugins/Language/CPlusPlus/LibCxx.h

private:
   ValueObject *m_pair_ptr;
   lldb::ValueObjectSP m_pair_sp;
 };
 
 SyntheticChildrenFrontEnd *
 LibCxxMapIteratorSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                           lldb::ValueObjectSP);
 
+/// Formats libcxx's std::unordered_map iterators
+///
+/// In raw form a std::unordered_map 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;
+
+  size_t CalculateNumChildren() override;
+
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+  bool 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);
 
 class LibcxxSharedPtrSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 public:
   LibcxxSharedPtrSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
 

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 
 #include "Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h"
 #include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+#include "lldb/lldb-enumerations.h"
 #include <tuple>
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
 bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
if (m_pair_ptr) {
 
       auto addr(m_pair_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
       m_pair_ptr = nullptr;
       if (addr && addr != LLDB_INVALID_ADDRESS) {
         TypeSystemClang *ast_ctx =
             llvm::dyn_cast_or_null<TypeSystemClang>(pair_type.GetTypeSystem());
         if (!ast_ctx)
           return false;
+
+        // Mimick layout of std::__tree_iterator::__ptr_ and read it in
+        // from process memory.
+        //
+        // In memory this looks like:
+        //   <-- __tree_end_node -->
+        //   pointer __left_;
+        //
+        //   <-- __tree_node_base -->
+        //   pointer __right_;
+        //   __parent_pointer __parent_;
+        //   bool __is_black_;
+        //
+        //   <-- __tree_node -->
+        //   __node_value_type __value_; <<< This is the key/value pair we're
+        //   interested in
+        //
         CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
             ConstString(),
             {{"ptr0",
               ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
              {"ptr1",
               ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
              {"ptr2",
               ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},

 
 SyntheticChildrenFrontEnd *
 lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEndCreator(
     CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
   return (valobj_sp ? new LibCxxMapIteratorSyntheticFrontEnd(valobj_sp)
                     : nullptr);
 }
 
+lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+    : SyntheticChildrenFrontEnd(*valobj_sp) {
+  if (valobj_sp)
+    Update();
+}
+
+bool lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    Update() {
+  m_pair_sp.reset();
+  m_iter_ptr = nullptr;
+
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+    return false;
+
+  TargetSP target_sp(valobj_sp->GetTargetSP());
+
+  if (!target_sp)
+    return false;
+
+  if (!valobj_sp)
+    return false;
+
+  // 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,
+                       ValueObject::GetValueForExpressionPathOptions()
+                           .DontCheckDotVsArrowSyntax()
+                           .SetSyntheticChildrenTraversal(
+                               ValueObject::GetValueForExpressionPathOptions::
+                                   SyntheticChildrenTraversal::None),
+                       nullptr)
+                   .get();
+
+  if (m_iter_ptr) {
+    auto iter_child(
+        valobj_sp->GetChildMemberWithName(ConstString("__i_"), true));
+    if (!iter_child) {
+      m_iter_ptr = nullptr;
+      return false;
+    }
+
+    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 false;
+    }
+
+    auto addr(m_iter_ptr->GetValueAsUnsigned(LLDB_INVALID_ADDRESS));
+    m_iter_ptr = nullptr;
+
+    if (addr && addr != LLDB_INVALID_ADDRESS) {
+      TypeSystemClang *ast_ctx =
+          llvm::dyn_cast_or_null<TypeSystemClang>(pair_type.GetTypeSystem());
+      if (!ast_ctx)
+        return false;
+
+      // Mimick layout of std::__hash_iterator::__node_ and read it in
+      // from process memory.
+      //
+      // In memory this looks like:
+      //   <-- __hash_node_base -->
+      //   __next_pointer __next_;
+      //
+      //   <-- __hash_node -->
+      //   size_t            __hash_;
+      //   __node_value_type __value_; <<< This is the key/value pair we're
+      //   interested in
+      //
+      CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
+          ConstString(),
+          {{"__next_",
+            ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
+           {"__hash_", ast_ctx->GetBasicType(lldb::eBasicTypeUnsignedLongLong)},
+           {"__value_", pair_type}});
+      llvm::Optional<uint64_t> size = tree_node_type.GetByteSize(nullptr);
+      if (!size)
+        return false;
+      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 false;
+      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, true);
+    }
+  }
+
+  return false;
+}
+
+size_t lldb_private::formatters::LibCxxUnorderedMapIteratorSyntheticFrontEnd::
+    CalculateNumChildren() {
+  return 2;
+}
+
+lldb::ValueObjectSP lldb_private::formatters::
+    LibCxxUnorderedMapIteratorSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
+  if (m_pair_sp)
+    return m_pair_sp->GetChildAtIndex(idx, true);
+  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 = {
  (std::__1::__wrap_iter<int *>::iterator_type) __i = 0x00000001001037a0 {
  (int) *__i = 1
  }
  }
 */

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/Makefile

+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/TestDataFormatterLibccUnorderedMap.py

+"""
+Test formatting of std::unordered_map related structures.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxUnorderedMapDataFormatterTestCase(TestBase):
+
+    @add_test_categories(['libc++'])
+    def test_with_run_command(self):
+        """Test that std::unordered_map related structures are formatted correctly when printed.
+           Currently only tests format of std::unordered_map iterators.
+        """
+        self.build()
+        (self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, 'Break here', lldb.SBFileSpec('main.cpp', False))
+
+        # Test empty iterators
+        self.expect_expr('empty_iter', '')
+        self.expect_expr('const_empty_iter', '')
+
+        lldbutil.continue_to_breakpoint(process, bkpt)
+
+        # Check that key/value is correctly formatted
+        self.expect_expr('foo', result_children=[
+                 ValueCheck(name='first', summary='"Foo"'),
+                 ValueCheck(name='second', summary='"Bar"')
+            ])
+
+        # Check invalid iterator is empty
+        self.expect_expr('invalid', '')
+
+        # Const key/val iterator
+        self.expect_expr('const_baz', result_children=[
+                 ValueCheck(name='first', summary='"Baz"'),
+                 ValueCheck(name='second', summary='"Qux"')
+            ])
+
+        # Bucket iterators
+        # I.e., std::__hash_map_const_iterator<const_local_iterator<...>>
+        # and std::__hash_map_iterator<local_iterator<...>>
+        self.expect_expr('bucket_it', result_children=[
+                 ValueCheck(name='first', summary='"Baz"'),
+                 ValueCheck(name='second', summary='"Qux"')
+            ])
+
+        self.expect_expr('bucket_it', result_children=[
+                 ValueCheck(name='first', summary='"Baz"'),
+                 ValueCheck(name='second', summary='"Qux"')
+            ])

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered_map/main.cpp

+#include <cstdio>
+#include <string>
+#include <unordered_map>
+
+using StringMapT = std::unordered_map<std::string, std::string>;
+
+int main() {
+  StringMapT string_map;
+  {
+    auto empty_iter = string_map.begin();
+    auto const_empty_iter = string_map.cbegin();
+    std::printf("Break here");
+  }
+  string_map["Foo"] = "Bar";
+  string_map["Baz"] = "Qux";
+
+  {
+    auto foo = string_map.find("Foo");
+    auto invalid = string_map.find("Invalid");
+
+    StringMapT::const_iterator const_baz = string_map.find("Baz");
+    auto bucket_it = string_map.begin(string_map.bucket("Baz"));
+    auto const_bucket_it = string_map.cbegin(string_map.bucket("Baz"));
+    std::printf("Break here");
+  }
+
+  return 0;
+}