cameron314 created this revision.
cameron314 added reviewers: labath, jingham.
cameron314 added a project: LLDB.
Herald added subscribers: lldb-commits, JDevlieghere, aprantl.
This fixes a memory leak with several synthetic children front-ends.
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.
The memory leak is actually much more important than just the lost memory,
since it prevents the file handle to the binary (.elf in my case) from being
closed due to references to the debug info from the ValueObject hierarchy. This
means a build system is blocked from rebuilding the binary even after LLDB has
disconnected.
Note that most of the existing synthetic children front-ends correctly avoid
this buggy shared-pointer pattern with explicit comments about this exact type
of memory leak. For example, in the NSErrorSyntheticFrontEnd:
// the child here can be "real" (i.e. an actual child of the root) or
// synthetized from raw memory if the former, I need to store a plain pointer
// to it - or else a loop of references will cause this entire hierarchy of
// values to leak if the latter, then I need to store a SharedPointer to it -
// so that it only goes away when everyone else in the cluster goes away oh
// joy!
ValueObject *m_child_ptr;
ValueObjectSP m_child_sp;
and the LibCxxMapIteratorSyntheticFrontEnd:
// this must be a ValueObject* because it is a child of the ValueObject we
// are producing children for it if were a ValueObjectSP, we would end up
// with a loop (iterator -> synthetic -> child -> parent == iterator) and
// that would in turn leak memory by never allowing the ValueObjects to die
// and free their memory
Apparently I'm not the first to lose a few hours debugging this :-)
Repository:
rLLDB LLDB
https://reviews.llvm.org/D68641
Files:
source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
Index: source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -39,9 +39,9 @@
bool GetSummary(Stream &stream, const TypeSummaryOptions &options);
private:
- ValueObjectSP m_ptr_obj;
- ValueObjectSP m_obj_obj;
- ValueObjectSP m_del_obj;
+ ValueObject* m_ptr_obj = nullptr;
+ ValueObject* m_obj_obj = nullptr;
+ ValueObject* m_del_obj = nullptr;
ValueObjectSP GetTuple();
};
@@ -92,17 +92,17 @@
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 +114,11 @@
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();
}
Index: source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp
@@ -37,7 +37,7 @@
size_t GetIndexOfChildWithName(ConstString name) override;
private:
- std::vector<ValueObjectSP> m_members;
+ std::vector<ValueObject*> m_members;
};
} // end of anonymous namespace
@@ -72,7 +72,7 @@
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 +86,7 @@
lldb::ValueObjectSP
LibStdcppTupleSyntheticFrontEnd::GetChildAtIndex(size_t idx) {
if (idx < m_members.size())
- return m_members[idx];
+ return m_members[idx]->GetSP();
return lldb::ValueObjectSP();
}
Index: source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -184,7 +184,6 @@
private:
size_t m_size = 0;
- ValueObjectSP m_base_sp;
};
} // namespace
Index: source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp
@@ -30,47 +30,51 @@
ValueObjectSP GetChildAtIndex(size_t idx) override;
private:
- std::vector<ValueObjectSP> m_elements;
- ValueObjectSP m_base_sp;
+ 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 *
Index: source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp
@@ -38,16 +38,16 @@
}
private:
- ValueObjectSP m_container_sp;
+ 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;
}
Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -31,7 +31,6 @@
private:
size_t m_size = 0;
- ValueObjectSP m_base_sp;
};
} // namespace
Index: source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
===================================================================
--- source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
@@ -31,7 +31,7 @@
private:
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 +50,7 @@
bool BitsetFrontEnd::Update() {
m_elements.clear();
- m_first.reset();
+ m_first = nullptr;
TargetSP target_sp = m_backend.GetTargetSP();
if (!target_sp)
@@ -63,7 +63,7 @@
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 +86,7 @@
chunk = m_first->GetChildAtIndex(idx / *bit_size, true);
} else {
type = m_first->GetCompilerType();
- chunk = m_first;
+ chunk = m_first->GetSP();
}
if (!type || !chunk)
return {};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
