[Lldb-commits] [lldb] r374195 - [LLDB] Fix for synthetic children memory leak

Cameron Desrochers via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 9 11:27:34 PDT 2019


Author: cameron314
Date: Wed Oct  9 11:27:33 2019
New Revision: 374195

URL: http://llvm.org/viewvc/llvm-project?rev=374195&view=rev
Log:
[LLDB] Fix for synthetic children memory leak

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 the managed objects in the cluster is destroyed. This means that no object in the cluster can store a shared pointer to another object in the cluster without creating a memory leak of the entire cluster. However, some of the synthetic children front-end implementations do exactly this; this patch fixes that.

Differential Revision: https://reviews.llvm.org/D68641

Modified:
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp Wed Oct  9 11:27:33 2019
@@ -30,8 +30,15 @@ public:
   ValueObjectSP GetChildAtIndex(size_t idx) 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).
+  // Value objects created from raw data (i.e. in a different cluster) must
+  // be referenced via shared pointer to keep them alive, however.
   std::vector<ValueObjectSP> m_elements;
-  ValueObjectSP m_first;
+  ValueObject* m_first = nullptr;
   CompilerType m_bool_type;
   ByteOrder m_byte_order = eByteOrderInvalid;
   uint8_t m_byte_size = 0;
@@ -50,7 +57,7 @@ BitsetFrontEnd::BitsetFrontEnd(ValueObje
 
 bool BitsetFrontEnd::Update() {
   m_elements.clear();
-  m_first.reset();
+  m_first = nullptr;
 
   TargetSP target_sp = m_backend.GetTargetSP();
   if (!target_sp)
@@ -63,7 +70,7 @@ bool BitsetFrontEnd::Update() {
 
   m_elements.assign(size, ValueObjectSP());
 
-  m_first = m_backend.GetChildMemberWithName(ConstString("__first_"), true);
+  m_first = m_backend.GetChildMemberWithName(ConstString("__first_"), true).get();
   return false;
 }
 
@@ -86,7 +93,7 @@ ValueObjectSP BitsetFrontEnd::GetChildAt
     chunk = m_first->GetChildAtIndex(idx / *bit_size, true);
   } else {
     type = m_first->GetCompilerType();
-    chunk = m_first;
+    chunk = m_first->GetSP();
   }
   if (!type || !chunk)
     return {};

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp Wed Oct  9 11:27:33 2019
@@ -31,7 +31,6 @@ public:
 
 private:
   size_t m_size = 0;
-  ValueObjectSP m_base_sp;
 };
 } // namespace
 

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp Wed Oct  9 11:27:33 2019
@@ -38,16 +38,21 @@ public:
   }
 
 private:
-  ValueObjectSP m_container_sp;
+  // 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_container_sp = nullptr;
 };
 } // namespace
 
 bool QueueFrontEnd::Update() {
-  m_container_sp.reset();
+  m_container_sp = nullptr;
   ValueObjectSP c_sp = m_backend.GetChildMemberWithName(ConstString("c"), true);
   if (!c_sp)
     return false;
-  m_container_sp = c_sp->GetSyntheticValue();
+  m_container_sp = c_sp->GetSyntheticValue().get();
   return false;
 }
 

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp Wed Oct  9 11:27:33 2019
@@ -30,47 +30,56 @@ public:
   ValueObjectSP GetChildAtIndex(size_t idx) override;
 
 private:
-  std::vector<ValueObjectSP> m_elements;
-  ValueObjectSP m_base_sp;
+  // 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).
+  std::vector<ValueObject*> m_elements;
+  ValueObject* m_base = nullptr;
 };
 }
 
 bool TupleFrontEnd::Update() {
   m_elements.clear();
-  m_base_sp = m_backend.GetChildMemberWithName(ConstString("__base_"), true);
-  if (! m_base_sp) {
+  m_base = nullptr;
+
+  ValueObjectSP base_sp;
+  base_sp = m_backend.GetChildMemberWithName(ConstString("__base_"), true);
+  if (!base_sp) {
     // Pre r304382 name of the base element.
-    m_base_sp = m_backend.GetChildMemberWithName(ConstString("base_"), true);
+    base_sp = m_backend.GetChildMemberWithName(ConstString("base_"), true);
   }
-  if (! m_base_sp)
+  if (!base_sp)
     return false;
-  m_elements.assign(m_base_sp->GetCompilerType().GetNumDirectBaseClasses(),
-                    ValueObjectSP());
+  m_base = base_sp.get();
+  m_elements.assign(base_sp->GetCompilerType().GetNumDirectBaseClasses(),
+                    nullptr);
   return false;
 }
 
 ValueObjectSP TupleFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx >= m_elements.size())
     return ValueObjectSP();
-  if (!m_base_sp)
+  if (!m_base)
     return ValueObjectSP();
   if (m_elements[idx])
-    return m_elements[idx];
+    return m_elements[idx]->GetSP();
 
   CompilerType holder_type =
-      m_base_sp->GetCompilerType().GetDirectBaseClassAtIndex(idx, nullptr);
+      m_base->GetCompilerType().GetDirectBaseClassAtIndex(idx, nullptr);
   if (!holder_type)
     return ValueObjectSP();
-  ValueObjectSP holder_sp = m_base_sp->GetChildAtIndex(idx, true);
+  ValueObjectSP holder_sp = m_base->GetChildAtIndex(idx, true);
   if (!holder_sp)
     return ValueObjectSP();
 
   ValueObjectSP elem_sp = holder_sp->GetChildAtIndex(0, true);
   if (elem_sp)
     m_elements[idx] =
-        elem_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str()));
+        elem_sp->Clone(ConstString(llvm::formatv("[{0}]", idx).str())).get();
 
-  return m_elements[idx];
+  return m_elements[idx]->GetSP();
 }
 
 SyntheticChildrenFrontEnd *

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp Wed Oct  9 11:27:33 2019
@@ -184,7 +184,6 @@ public:
 
 private:
   size_t m_size = 0;
-  ValueObjectSP m_base_sp;
 };
 } // namespace
 

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp Wed Oct  9 11:27:33 2019
@@ -37,7 +37,12 @@ public:
   size_t GetIndexOfChildWithName(ConstString name) override;
 
 private:
-  std::vector<ValueObjectSP> m_members;
+  // 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).
+  std::vector<ValueObject*> m_members;
 };
 
 } // end of anonymous namespace
@@ -72,7 +77,7 @@ bool LibStdcppTupleSyntheticFrontEnd::Up
         if (value_sp) {
           StreamString name;
           name.Printf("[%zd]", m_members.size());
-          m_members.push_back(value_sp->Clone(ConstString(name.GetString())));
+          m_members.push_back(value_sp->Clone(ConstString(name.GetString())).get());
         }
       }
     }
@@ -86,7 +91,7 @@ bool LibStdcppTupleSyntheticFrontEnd::Mi
 lldb::ValueObjectSP
 LibStdcppTupleSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx < m_members.size())
-    return m_members[idx];
+    return m_members[idx]->GetSP();
   return lldb::ValueObjectSP();
 }
 

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp?rev=374195&r1=374194&r2=374195&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp Wed Oct  9 11:27:33 2019
@@ -39,9 +39,14 @@ public:
   bool GetSummary(Stream &stream, const TypeSummaryOptions &options);
 
 private:
-  ValueObjectSP m_ptr_obj;
-  ValueObjectSP m_obj_obj;
-  ValueObjectSP m_del_obj;
+  // 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;
+  ValueObject* m_obj_obj = nullptr;
+  ValueObject* m_del_obj = nullptr;
 
   ValueObjectSP GetTuple();
 };
@@ -92,17 +97,17 @@ bool LibStdcppUniquePtrSyntheticFrontEnd
 
   ValueObjectSP ptr_obj = tuple_frontend->GetChildAtIndex(0);
   if (ptr_obj)
-    m_ptr_obj = ptr_obj->Clone(ConstString("pointer"));
+    m_ptr_obj = ptr_obj->Clone(ConstString("pointer")).get();
 
   ValueObjectSP del_obj = tuple_frontend->GetChildAtIndex(1);
   if (del_obj)
-    m_del_obj = del_obj->Clone(ConstString("deleter"));
+    m_del_obj = del_obj->Clone(ConstString("deleter")).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"));
+      m_obj_obj = obj_obj->Clone(ConstString("object")).get();
     }
   }
 
@@ -114,11 +119,11 @@ bool LibStdcppUniquePtrSyntheticFrontEnd
 lldb::ValueObjectSP
 LibStdcppUniquePtrSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
   if (idx == 0)
-    return m_ptr_obj;
+    return m_ptr_obj->GetSP();
   if (idx == 1)
-    return m_del_obj;
+    return m_del_obj->GetSP();
   if (idx == 2)
-    return m_obj_obj;
+    return m_obj_obj->GetSP();
   return lldb::ValueObjectSP();
 }
 




More information about the lldb-commits mailing list