Fix libc++ vector<bool> data formatter (bug #32553)

[labath]

Summary

The iteration list through the available data formatters was undefined,
which meant that the vector<bool> formatter kicked in only in cases
where it happened to be queried before the general vector formatter. To
fix this, I merge the two data formatter entries into one, and select
which implementation to use in the factory function.

[labath]

Remove the vector<bool> summary provider as well (picked up from Tamas's version of the patch).

[tberghammer]

The previous version of the data formatter was triggering for std::vector<std::allocator<bool>> as well. Jason, do you know why was it the case? Do we need that functionality because of a broken compiler version or can it be removed?

Note: I added a few comments about the data formatter itself but feel free to ignore them if you want to keep this change small.

source/Plugins/Language/CPlusPlus/LibCxxVector.cpp

52–53 ↗

(On Diff #94667)

Not needed

201–228 ↗

(On Diff #94667)

This switch seems silly. You can just replace it with "mask = (1 << bit_index)"

233–234 ↗

(On Diff #94667)

I think the formatting here makes the code pretty hard to read

301–304 ↗

(On Diff #94667)

Is there a reason you are not using ValueObject::GetCompilerType()?

[labath]

source/Plugins/Language/CPlusPlus/LibCxxVector.cpp

301–304 ↗

(On Diff #94667)

Because I did not know it exists. :)

[labath]

Address review comments.

[jasonmolenda]

This all looks good to me, thanks for doing this Pavel. Tamas asked in an earlier comment, "The previous version of the data formatter was triggering for std::vector<std::allocator<bool>> as well. Jason, do you know why was it the case? Do we need that functionality because of a broken compiler version or can it be removed?"

I have no idea why that was there. I could dig around in the svn commit history to try to figure it out, but it may have been unnecessary to begin with. If in doubt, we can drop it and put it back in if we hear of any problems.

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py

class LibcxxVBoolDataFormatterTestCase(TestBase):
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break at.
         self.line = line_number('main.cpp', '// Set break point at this line.')
 
     @add_test_categories(["libc++"])
-    @expectedFailureAll(bugnumber="llvm.org/pr32553")
     def test_with_run_command(self):
         """Test that that file and class static variables display correctly."""
         self.build()
         self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
 
         lldbutil.run_break_set_by_file_and_line(
             self, "main.cpp", self.line, num_expected_locations=-1)
 

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

cpp_category_sp->GetTypeSummariesContainer()->Add(
       std_wstring_summary_sp);
 
   SyntheticChildren::Flags stl_synth_flags;
   stl_synth_flags.SetCascades(true).SetSkipPointers(false).SetSkipReferences(
       false);
 
   AddCXXSynthetic(
       cpp_category_sp,
-      lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEndCreator,
-      "libc++ std::vector<bool> synthetic children",
-      ConstString(
-          "^std::__(ndk)?1::vector<bool, std::__(ndk)?1::allocator<bool> >$"),
-      stl_synth_flags, true);
-  AddCXXSynthetic(
-      cpp_category_sp,
       lldb_private::formatters::LibcxxStdVectorSyntheticFrontEndCreator,
       "libc++ std::vector synthetic children",
       ConstString("^std::__(ndk)?1::vector<.+>(( )?&)?$"), stl_synth_flags,
       true);
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibcxxStdListSyntheticFrontEndCreator,
       "libc++ std::list synthetic children",
       ConstString("^std::__(ndk)?1::list<.+>(( )?&)?$"), stl_synth_flags, true);
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibcxxStdMapSyntheticFrontEndCreator,
       "libc++ std::map synthetic children",
       ConstString("^std::__(ndk)?1::map<.+> >(( )?&)?$"), stl_synth_flags,
       true);
   AddCXXSynthetic(
       cpp_category_sp,
-      lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEndCreator,
-      "libc++ std::vector<bool> synthetic children",
-      ConstString("std::__(ndk)?1::vector<std::__(ndk)?1::allocator<bool> >"),
-      stl_synth_flags, true);
-  AddCXXSynthetic(
-      cpp_category_sp,
-      lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEndCreator,
-      "libc++ std::vector<bool> synthetic children",
-      ConstString(
-          "std::__(ndk)?1::vector<bool, std::__(ndk)?1::allocator<bool> >"),
-      stl_synth_flags, true);
-  AddCXXSynthetic(
-      cpp_category_sp,
       lldb_private::formatters::LibcxxStdMapSyntheticFrontEndCreator,
       "libc++ std::set synthetic children",
       ConstString("^std::__(ndk)?1::set<.+> >(( )?&)?$"), stl_synth_flags,
       true);
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibcxxStdMapSyntheticFrontEndCreator,
       "libc++ std::multiset synthetic children",
AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEndCreator,
       "weak_ptr synthetic children",
       ConstString("^(std::__(ndk)?1::)weak_ptr<.+>(( )?&)?$"), stl_synth_flags,
       true);
 
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
-  AddCXXSummary(
-      cpp_category_sp, lldb_private::formatters::LibcxxContainerSummaryProvider,
-      "libc++ std::vector<bool> summary provider",
-      ConstString(
-          "std::__(ndk)?1::vector<bool, std::__(ndk)?1::allocator<bool> >"),
-      stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibcxxContainerSummaryProvider,
                 "libc++ std::vector summary provider",
                 ConstString("^std::__(ndk)?1::vector<.+>(( )?&)?$"),
                 stl_summary_flags, true);
   AddCXXSummary(cpp_category_sp,
                 lldb_private::formatters::LibcxxContainerSummaryProvider,
                 "libc++ std::list summary provider",
AddCXXSummary(cpp_category_sp,
                 stl_summary_flags, true);
 
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibCxxVectorIteratorSyntheticFrontEndCreator,
       "std::vector iterator synthetic children",
       ConstString("^std::__(ndk)?1::__wrap_iter<.+>$"), stl_synth_flags, true);
 
-  AddCXXSummary(
-      cpp_category_sp, lldb_private::formatters::LibcxxContainerSummaryProvider,
-      "libc++ std::vector<bool> summary provider",
-      ConstString(
-          "std::__(ndk)?1::vector<bool, std::__(ndk)?1::allocator<bool> >"),
-      stl_summary_flags, true);
   AddCXXSynthetic(
       cpp_category_sp,
       lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEndCreator,
       "std::map iterator synthetic children",
       ConstString("^std::__(ndk)?1::__map_iterator<.+>$"), stl_synth_flags,
       true);
 
   AddCXXSynthetic(

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

bool LibcxxWStringSummaryProvider(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &options); // libc++ std::wstring
 
 bool LibcxxSmartPointerSummaryProvider(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions
         &options); // libc++ std::shared_ptr<> and std::weak_ptr<>
 
-class LibcxxVectorBoolSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
-public:
-  LibcxxVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
-
-  size_t CalculateNumChildren() override;
-
-  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
-
-  bool Update() override;
-
-  bool MightHaveChildren() override;
-
-  size_t GetIndexOfChildWithName(const ConstString &name) override;
-
-  ~LibcxxVectorBoolSyntheticFrontEnd() override;
-
-private:
-  CompilerType m_bool_type;
-  ExecutionContextRef m_exe_ctx_ref;
-  uint64_t m_count;
-  lldb::addr_t m_base_data_address;
-  std::map<size_t, lldb::ValueObjectSP> m_children;
-};
-
 SyntheticChildrenFrontEnd *
 LibcxxVectorBoolSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                          lldb::ValueObjectSP);
 
 bool LibcxxContainerSummaryProvider(ValueObject &valobj, Stream &stream,
                                     const TypeSummaryOptions &options);
 
 class LibCxxMapIteratorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {

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

if (count_sp)
     stream.Printf(" strong=%" PRIu64, 1 + count_sp->GetValueAsUnsigned(0));
 
   if (weakcount_sp)
     stream.Printf(" weak=%" PRIu64, 1 + weakcount_sp->GetValueAsUnsigned(0));
 
   return true;
 }
 
-lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
-    LibcxxVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
-    : SyntheticChildrenFrontEnd(*valobj_sp), m_bool_type(), m_exe_ctx_ref(),
-      m_count(0), m_base_data_address(0), m_children() {
-  if (valobj_sp) {
-    Update();
-    m_bool_type =
-        valobj_sp->GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeBool);
-  }
-}
-
-size_t lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
-    CalculateNumChildren() {
-  return m_count;
-}
-
-lldb::ValueObjectSP
-lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::GetChildAtIndex(
-    size_t idx) {
-  auto iter = m_children.find(idx), end = m_children.end();
-  if (iter != end)
-    return iter->second;
-  if (idx >= m_count)
-    return ValueObjectSP();
-  if (m_base_data_address == 0 || m_count == 0)
-    return ValueObjectSP();
-  if (!m_bool_type)
-    return ValueObjectSP();
-  size_t byte_idx = (idx >> 3); // divide by 8 to get byte index
-  size_t bit_index = (idx & 7); // efficient idx % 8 for bit index
-  lldb::addr_t byte_location = m_base_data_address + byte_idx;
-  ProcessSP process_sp(m_exe_ctx_ref.GetProcessSP());
-  if (!process_sp)
-    return ValueObjectSP();
-  uint8_t byte = 0;
-  uint8_t mask = 0;
-  Error err;
-  size_t bytes_read = process_sp->ReadMemory(byte_location, &byte, 1, err);
-  if (err.Fail() || bytes_read == 0)
-    return ValueObjectSP();
-  switch (bit_index) {
-  case 0:
-    mask = 1;
-    break;
-  case 1:
-    mask = 2;
-    break;
-  case 2:
-    mask = 4;
-    break;
-  case 3:
-    mask = 8;
-    break;
-  case 4:
-    mask = 16;
-    break;
-  case 5:
-    mask = 32;
-    break;
-  case 6:
-    mask = 64;
-    break;
-  case 7:
-    mask = 128;
-    break;
-  default:
-    return ValueObjectSP();
-  }
-  bool bit_set = ((byte & mask) != 0);
-  DataBufferSP buffer_sp(
-      new DataBufferHeap(m_bool_type.GetByteSize(nullptr), 0));
-  if (bit_set && buffer_sp && buffer_sp->GetBytes())
-    *(buffer_sp->GetBytes()) =
-        1; // regardless of endianness, anything non-zero is true
-  StreamString name;
-  name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  ValueObjectSP retval_sp(CreateValueObjectFromData(
-      name.GetString(), DataExtractor(buffer_sp, process_sp->GetByteOrder(),
-                                      process_sp->GetAddressByteSize()),
-      m_exe_ctx_ref, m_bool_type));
-  if (retval_sp)
-    m_children[idx] = retval_sp;
-  return retval_sp;
-}
-
-/*(std::__1::vector<std::__1::allocator<bool> >) vBool = {
- __begin_ = 0x00000001001000e0
- __size_ = 56
- __cap_alloc_ = {
- std::__1::__libcpp_compressed_pair_imp<unsigned long,
- std::__1::allocator<unsigned long> > = {
- __first_ = 1
- }
- }
- }*/
-
-bool lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::Update() {
-  m_children.clear();
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-    return false;
-  m_exe_ctx_ref = valobj_sp->GetExecutionContextRef();
-  ValueObjectSP size_sp(
-      valobj_sp->GetChildMemberWithName(ConstString("__size_"), true));
-  if (!size_sp)
-    return false;
-  m_count = size_sp->GetValueAsUnsigned(0);
-  if (!m_count)
-    return true;
-  ValueObjectSP begin_sp(
-      valobj_sp->GetChildMemberWithName(ConstString("__begin_"), true));
-  if (!begin_sp) {
-    m_count = 0;
-    return false;
-  }
-  m_base_data_address = begin_sp->GetValueAsUnsigned(0);
-  if (!m_base_data_address) {
-    m_count = 0;
-    return false;
-  }
-  return false;
-}
-
-bool lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
-    MightHaveChildren() {
-  return true;
-}
-
-size_t lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
-    GetIndexOfChildWithName(const ConstString &name) {
-  if (!m_count || !m_base_data_address)
-    return UINT32_MAX;
-  const char *item_name = name.GetCString();
-  uint32_t idx = ExtractIndexFromString(item_name);
-  if (idx < UINT32_MAX && idx >= CalculateNumChildren())
-    return UINT32_MAX;
-  return idx;
-}
-
-lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
-    ~LibcxxVectorBoolSyntheticFrontEnd() = default;
-
-SyntheticChildrenFrontEnd *
-lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEndCreator(
-    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
-  return (valobj_sp ? new LibcxxVectorBoolSyntheticFrontEnd(valobj_sp)
-                    : nullptr);
-}
-
 /*
  (lldb) fr var ibeg --raw --ptr-depth 1
  (std::__1::__map_iterator<std::__1::__tree_iterator<std::__1::pair<int,
  std::__1::basic_string<char, std::__1::char_traits<char>,
  std::__1::allocator<char> > >, std::__1::__tree_node<std::__1::pair<int,
  std::__1::basic_string<char, std::__1::char_traits<char>,
  std::__1::allocator<char> > >, void *> *, long> >) ibeg = {
  __i_ = {

lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp

public:
   size_t GetIndexOfChildWithName(const ConstString &name) override;
 
 private:
   ValueObject *m_start;
   ValueObject *m_finish;
   CompilerType m_element_type;
   uint32_t m_element_size;
 };
+
+class LibcxxVectorBoolSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  LibcxxVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+  size_t CalculateNumChildren() override;
+
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+  bool Update() override;
+
+  bool MightHaveChildren() override { return true; }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override;
+
+private:
+  CompilerType m_bool_type;
+  ExecutionContextRef m_exe_ctx_ref;
+  uint64_t m_count;
+  lldb::addr_t m_base_data_address;
+  std::map<size_t, lldb::ValueObjectSP> m_children;
+};
+
 } // namespace formatters
 } // namespace lldb_private
 
 lldb_private::formatters::LibcxxStdVectorSyntheticFrontEnd::
     LibcxxStdVectorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
     : SyntheticChildrenFrontEnd(*valobj_sp), m_start(nullptr),
       m_finish(nullptr), m_element_type(), m_element_size(0) {
   if (valobj_sp)

 
 size_t lldb_private::formatters::LibcxxStdVectorSyntheticFrontEnd::
     GetIndexOfChildWithName(const ConstString &name) {
   if (!m_start || !m_finish)
     return UINT32_MAX;
   return ExtractIndexFromString(name.GetCString());
 }
 
+lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
+    LibcxxVectorBoolSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
+    : SyntheticChildrenFrontEnd(*valobj_sp), m_bool_type(), m_exe_ctx_ref(),
+      m_count(0), m_base_data_address(0), m_children() {
+  if (valobj_sp) {
+    Update();
+    m_bool_type =
+        valobj_sp->GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeBool);
+  }
+}
+
+size_t lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
+    CalculateNumChildren() {
+  return m_count;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::GetChildAtIndex(
+    size_t idx) {
+  auto iter = m_children.find(idx), end = m_children.end();
+  if (iter != end)
+    return iter->second;
+  if (idx >= m_count)
+    return ValueObjectSP();
+  if (m_base_data_address == 0 || m_count == 0)
+    return ValueObjectSP();
+  if (!m_bool_type)
+    return ValueObjectSP();
+  size_t byte_idx = (idx >> 3); // divide by 8 to get byte index
+  size_t bit_index = (idx & 7); // efficient idx % 8 for bit index
+  lldb::addr_t byte_location = m_base_data_address + byte_idx;
+  ProcessSP process_sp(m_exe_ctx_ref.GetProcessSP());
+  if (!process_sp)
+    return ValueObjectSP();
+  uint8_t byte = 0;
+  uint8_t mask = 0;
+  Error err;
+  size_t bytes_read = process_sp->ReadMemory(byte_location, &byte, 1, err);
+  if (err.Fail() || bytes_read == 0)
+    return ValueObjectSP();
+  mask = 1 << bit_index;
+  bool bit_set = ((byte & mask) != 0);
+  DataBufferSP buffer_sp(
+      new DataBufferHeap(m_bool_type.GetByteSize(nullptr), 0));
+  if (bit_set && buffer_sp && buffer_sp->GetBytes()) {
+    // regardless of endianness, anything non-zero is true
+    *(buffer_sp->GetBytes()) = 1;
+  }
+  StreamString name;
+  name.Printf("[%" PRIu64 "]", (uint64_t)idx);
+  ValueObjectSP retval_sp(CreateValueObjectFromData(
+      name.GetString(),
+      DataExtractor(buffer_sp, process_sp->GetByteOrder(),
+                    process_sp->GetAddressByteSize()),
+      m_exe_ctx_ref, m_bool_type));
+  if (retval_sp)
+    m_children[idx] = retval_sp;
+  return retval_sp;
+}
+
+/*(std::__1::vector<std::__1::allocator<bool> >) vBool = {
+ __begin_ = 0x00000001001000e0
+ __size_ = 56
+ __cap_alloc_ = {
+ std::__1::__libcpp_compressed_pair_imp<unsigned long,
+ std::__1::allocator<unsigned long> > = {
+ __first_ = 1
+ }
+ }
+ }*/
+
+bool lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::Update() {
+  m_children.clear();
+  ValueObjectSP valobj_sp = m_backend.GetSP();
+  if (!valobj_sp)
+    return false;
+  m_exe_ctx_ref = valobj_sp->GetExecutionContextRef();
+  ValueObjectSP size_sp(
+      valobj_sp->GetChildMemberWithName(ConstString("__size_"), true));
+  if (!size_sp)
+    return false;
+  m_count = size_sp->GetValueAsUnsigned(0);
+  if (!m_count)
+    return true;
+  ValueObjectSP begin_sp(
+      valobj_sp->GetChildMemberWithName(ConstString("__begin_"), true));
+  if (!begin_sp) {
+    m_count = 0;
+    return false;
+  }
+  m_base_data_address = begin_sp->GetValueAsUnsigned(0);
+  if (!m_base_data_address) {
+    m_count = 0;
+    return false;
+  }
+  return false;
+}
+
+size_t lldb_private::formatters::LibcxxVectorBoolSyntheticFrontEnd::
+    GetIndexOfChildWithName(const ConstString &name) {
+  if (!m_count || !m_base_data_address)
+    return UINT32_MAX;
+  const char *item_name = name.GetCString();
+  uint32_t idx = ExtractIndexFromString(item_name);
+  if (idx < UINT32_MAX && idx >= CalculateNumChildren())
+    return UINT32_MAX;
+  return idx;
+}
+
 lldb_private::SyntheticChildrenFrontEnd *
 lldb_private::formatters::LibcxxStdVectorSyntheticFrontEndCreator(
     CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
-  return (valobj_sp ? new LibcxxStdVectorSyntheticFrontEnd(valobj_sp)
-                    : nullptr);
+  if (!valobj_sp)
+    return nullptr;
+  CompilerType type = valobj_sp->GetCompilerType();
+  if (!type.IsValid() || type.GetNumTemplateArguments() == 0)
+    return nullptr;
+  TemplateArgumentKind kind;
+  CompilerType arg_type = type.GetTemplateArgument(0, kind);
+  if (arg_type.GetTypeName() == ConstString("bool"))
+    return new LibcxxVectorBoolSyntheticFrontEnd(valobj_sp);
+  return new LibcxxStdVectorSyntheticFrontEnd(valobj_sp);
 }