[Lldb-commits] [lldb] 44bb442 - [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-05-24T13:01:11+01:00
New Revision: 44bb442fd5be3860e7819cb216621b5ea59970c3

URL: https://github.com/llvm/llvm-project/commit/44bb442fd5be3860e7819cb216621b5ea59970c3
DIFF: https://github.com/llvm/llvm-project/commit/44bb442fd5be3860e7819cb216621b5ea59970c3.diff

LOG: [lldb][DataFormatter] Add dereference support to libstdcpp std::shared_ptr formatter

This mimicks the implementation of the libstdcpp std::unique_ptr
formatter.

This has been attempted several years ago in
`0789722d85cf1f1fdbe2ffb2245ea0ba034a9f94` but was reverted in
`e7dd3972094c2f2fb42dc9d4d5344e54a431e2ce`.

The difference to the original patch is that we now maintain
a `$$dereference$$` member and we only store weak pointers
to the other children inside the synthetic frontend. This is
what the libc++ formatters do to prevent the recursion mentioned
in the revert commit.

Added: 
    

Modified: 
    lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
    lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index c7f1c79422246..bd129d2f64060 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -73,6 +73,15 @@ class LibStdcppSharedPtrSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
   bool MightHaveChildren() override;
 
   size_t GetIndexOfChildWithName(ConstString name) override;
+private:
+
+  // The lifetime of a ValueObject and all its derivative ValueObjects
+  // (children, clones, etc.) is managed by a ClusterManager. These
+  // objects are only destroyed when every shared pointer to any of them
+  // is destroyed, so we must not store a shared pointer to any ValueObject
+  // derived from our backend ValueObject (since we're in the same cluster).
+  ValueObject* m_ptr_obj = nullptr; // Underlying pointer (held, not owned)
+  ValueObject* m_obj_obj = nullptr; // Underlying object (held, not owned)
 };
 
 } // end of anonymous namespace
@@ -367,24 +376,48 @@ size_t LibStdcppSharedPtrSyntheticFrontEnd::CalculateNumChildren() { return 1; }
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
-  ValueObjectSP valobj_sp = m_backend.GetSP();
-  if (!valobj_sp)
-    return lldb::ValueObjectSP();
-
   if (idx == 0)
-    return valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
-  else
-    return lldb::ValueObjectSP();
+    return m_ptr_obj->GetSP();
+  if (idx == 1)
+    return m_obj_obj->GetSP();
+
+  return lldb::ValueObjectSP();
 }
 
-bool LibStdcppSharedPtrSyntheticFrontEnd::Update() { return false; }
+bool LibStdcppSharedPtrSyntheticFrontEnd::Update() {
+  auto backend = m_backend.GetSP();
+  if (!backend)
+    return false;
+
+  auto valobj_sp = backend->GetNonSyntheticValue();
+  if (!valobj_sp)
+    return false;
+
+  auto ptr_obj_sp = valobj_sp->GetChildMemberWithName(ConstString("_M_ptr"), true);
+  if (!ptr_obj_sp)
+    return false;
+
+  m_ptr_obj = ptr_obj_sp->Clone(ConstString("pointer")).get();
+
+  if (m_ptr_obj) {
+    Status error;
+    ValueObjectSP obj_obj = m_ptr_obj->Dereference(error);
+    if (error.Success()) {
+      m_obj_obj = obj_obj->Clone(ConstString("object")).get();
+    }
+  }
+
+  return false;
+}
 
 bool LibStdcppSharedPtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
     ConstString name) {
-  if (name == "_M_ptr")
+  if (name == "pointer")
     return 0;
+  if (name == "object" || name == "$$dereference$$")
+    return 1;
   return UINT32_MAX;
 }
 

diff  --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
index 3d37af31d66c5..13bcb68cfd1fd 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/TestDataFormatterStdSmartPtr.py
@@ -33,6 +33,13 @@ def test_with_run_command(self):
         self.expect("frame variable iwp", substrs=['iwp = 123'])
         self.expect("frame variable swp", substrs=['swp = "foobar"'])
 
+        self.expect("frame variable *nsp", substrs=['*nsp = <parent is NULL>'])
+        self.expect("frame variable *isp", substrs=['*isp = 123'])
+        self.expect("frame variable *ssp", substrs=['*ssp = "foobar"'])
+        self.expect("frame variable *fsp", substrs=['*fsp = (mem = 5)'])
+
+        self.expect("frame variable fsp->mem", substrs=['(int) fsp->mem = 5'])
+
         self.runCmd("continue")
 
         self.expect("frame variable nsp", substrs=['nsp = nullptr'])

diff  --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
index 7ba50875a6db9..6e4b869e1c8e0 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/smart_ptr/main.cpp
@@ -1,12 +1,17 @@
 #include <memory>
 #include <string>
 
+struct Foo {
+  int mem = 5;
+};
+
 int
 main()
 {
     std::shared_ptr<char> nsp;
     std::shared_ptr<int> isp(new int{123});
     std::shared_ptr<std::string> ssp = std::make_shared<std::string>("foobar");
+    std::shared_ptr<Foo> fsp = std::make_shared<Foo>();
 
     std::weak_ptr<char> nwp;
     std::weak_ptr<int> iwp = isp;
@@ -15,6 +20,7 @@ main()
     nsp.reset(); // Set break point at this line.
     isp.reset();
     ssp.reset();
+    fsp.reset();
 
     return 0; // Set break point at this line.
 }