tberghammer created this revision.
Herald added a reviewer: EricWF.

Fix the libc++ std::vector<bool> data formatter

Previously we randomly triggered either the std::vector<...> or the
std::vector<bool> data formatter causing an issue when the former got
triggered for an std::vector<bool>.

This change moves the logic to decide which one to trigger from the
regular expression used to register the formatter into the formatter
creation logic as the former had no way to specify precedence.

Bug: llvm.org/pr32553


https://reviews.llvm.org/D31882

Files:
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxVector.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -23,6 +23,7 @@
 
 namespace lldb_private {
 namespace formatters {
+namespace {
 class LibcxxStdVectorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 public:
   LibcxxStdVectorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
@@ -45,6 +46,29 @@
   CompilerType m_element_type;
   uint32_t m_element_size;
 };
+
+class LibcxxStdVectorBoolSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  LibcxxStdVectorBoolSyntheticFrontEnd(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;
+
+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;
+};
+} // anonymous namespace
 } // namespace formatters
 } // namespace lldb_private
 
@@ -133,9 +157,129 @@
   return ExtractIndexFromString(name.GetCString());
 }
 
+lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::
+    LibcxxStdVectorBoolSyntheticFrontEnd(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::LibcxxStdVectorBoolSyntheticFrontEnd::
+    CalculateNumChildren() {
+  return m_count;
+}
+
+lldb::ValueObjectSP
+lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::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;
+  Error err;
+  size_t bytes_read = process_sp->ReadMemory(byte_location, &byte, 1, err);
+  if (err.Fail() || bytes_read == 0)
+    return ValueObjectSP();
+  bool bit_set = ((byte & (1 << bit_index)) != 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::LibcxxStdVectorBoolSyntheticFrontEnd::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::LibcxxStdVectorBoolSyntheticFrontEnd::
+    MightHaveChildren() {
+  return true;
+}
+
+size_t lldb_private::formatters::LibcxxStdVectorBoolSyntheticFrontEnd::
+    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);
+  static RegularExpression vector_bool_regex("^std::__(ndk)?1::vector<bool(,( "
+                                             ")?std::__(ndk)?1::allocator<bool>"
+                                             "( )?)?>(( )?&)?$");
+  if (!valobj_sp)
+    return nullptr;
+
+  ConstString type_name = valobj_sp->GetCompilerType().GetTypeName();
+  if (vector_bool_regex.Execute(type_name.GetStringRef()))
+    return new LibcxxStdVectorBoolSyntheticFrontEnd(valobj_sp);
+
+  return new LibcxxStdVectorSyntheticFrontEnd(valobj_sp);
 }
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -32,34 +32,6 @@
     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);
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -76,155 +76,6 @@
   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,
Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -560,13 +560,6 @@
 
   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,
@@ -584,19 +577,6 @@
       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,
@@ -653,12 +633,6 @@
 
   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",
@@ -723,12 +697,6 @@
       "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,
Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
+++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vbool/TestDataFormatterLibcxxVBool.py
@@ -24,9 +24,8 @@
         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."""
+        """Test the libc++ std::vector<bool> data formatter."""
         self.build()
         self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D... Tamas Berghammer via Phabricator via lldb-commits

Reply via email to