[Lldb-commits] [lldb] [lldb][DataFormatter] Surface CalculateNumChildren errors in std::vector summary (PR #135944)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

<details>
<summary>Changes</summary>

When the data-formatters happen to break (e.g., due to layout changes in libc++), there's no clear indicator of them failing from a user's perspective. E.g., for `std::vector`s we would just show:
```
(std::vector<int>) v = size=0 {}
```
which is highly misleading, especially if `v.size()` returns a non-zero size.

This patch surfaces the various errors that could occur when calculating the number of children of a vector.

rdar://146964266

---
Full diff: https://github.com/llvm/llvm-project/pull/135944.diff


5 Files Affected:

- (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp (+16-5) 
- (modified) lldb/source/ValueObject/ValueObject.cpp (+9-3) 
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/Makefile (+3) 
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py (+35) 
- (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/main.cpp (+26) 


``````````diff
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
index d538cac9f9134..ce2261b6f03c3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -83,19 +83,30 @@ lldb_private::formatters::LibcxxStdVectorSyntheticFrontEnd::
 llvm::Expected<uint32_t> lldb_private::formatters::
     LibcxxStdVectorSyntheticFrontEnd::CalculateNumChildren() {
   if (!m_start || !m_finish)
-    return 0;
+    return llvm::createStringError(
+        "Failed to determine start/end of vector data.");
+
   uint64_t start_val = m_start->GetValueAsUnsigned(0);
   uint64_t finish_val = m_finish->GetValueAsUnsigned(0);
 
-  if (start_val == 0 || finish_val == 0)
+  // A default-initialized empty vector.
+  if (start_val == 0 && finish_val == 0)
     return 0;
 
-  if (start_val >= finish_val)
-    return 0;
+  if (start_val == 0)
+    return llvm::createStringError("Invalid value for start of vector.");
+
+  if (finish_val == 0)
+    return llvm::createStringError("Invalid value for end of vector.");
+
+  if (start_val > finish_val)
+    return llvm::createStringError(
+        "Start of vector data begins after end pointer.");
 
   size_t num_children = (finish_val - start_val);
   if (num_children % m_element_size)
-    return 0;
+    return llvm::createStringError("Size not multiple of element size.");
+
   return num_children / m_element_size;
 }
 
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index eac24353de90b..8741cb7343166 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -1521,10 +1521,16 @@ bool ValueObject::DumpPrintableRepresentation(
       str = GetLocationAsCString();
       break;
 
-    case eValueObjectRepresentationStyleChildrenCount:
-      strm.Printf("%" PRIu64 "", (uint64_t)GetNumChildrenIgnoringErrors());
-      str = strm.GetString();
+    case eValueObjectRepresentationStyleChildrenCount: {
+      if (auto err = GetNumChildren()) {
+        strm.Printf("%" PRIu32, *err);
+        str = strm.GetString();
+      } else {
+        strm << "error: " << toString(err.takeError());
+        str = strm.GetString();
+      }
       break;
+    }
 
     case eValueObjectRepresentationStyleType:
       str = GetTypeName().GetStringRef();
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/Makefile
new file mode 100644
index 0000000000000..38cfa81053488
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+override CXXFLAGS_EXTRAS += -std=c++14
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
new file mode 100644
index 0000000000000..6986d71c37a8b
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/TestDataFormatterLibcxxInvalidVectorSimulator.py
@@ -0,0 +1,35 @@
+"""
+Test we can understand various layouts of the libc++'s std::string
+"""
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import functools
+
+
+class LibcxxInvalidVectorDataFormatterSimulatorTestCase(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, "return 0", lldb.SBFileSpec("main.cpp"))
+
+        self.expect(
+            "frame variable v1",
+            substrs=["size=error: Invalid value for end of vector."],
+        )
+        self.expect(
+            "frame variable v2",
+            substrs=["size=error: Invalid value for start of vector."],
+        )
+        self.expect(
+            "frame variable v3",
+            substrs=["size=error: Start of vector data begins after end pointer."],
+        )
+        self.expect(
+            "frame variable v4",
+            substrs=["size=error: Failed to determine start/end of vector data."],
+        )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/main.cpp
new file mode 100644
index 0000000000000..c1b9b6724787b
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/main.cpp
@@ -0,0 +1,26 @@
+#define COMPRESSED_PAIR_REV 2
+#include <libcxx-simulators-common/compressed_pair.h>
+
+namespace std {
+namespace __1 {
+template <typename T> struct vector {
+  T *__begin_;
+  T *__end_;
+  _LLDB_COMPRESSED_PAIR(T *, __cap_ = nullptr, void *, __alloc_);
+};
+} // namespace __1
+
+namespace __2 {
+template <typename T> struct vector {};
+} // namespace __2
+} // namespace std
+
+int main() {
+  int arr[] = {1, 2, 3};
+  std::__1::vector<int> v1{.__begin_ = arr, .__end_ = nullptr};
+  std::__1::vector<int> v2{.__begin_ = nullptr, .__end_ = arr};
+  std::__1::vector<int> v3{.__begin_ = &arr[3], .__end_ = arr};
+  std::__2::vector<int> v4;
+
+  return 0;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/135944