Fix memory leak for APValues that do memory allocation.
This patch ensures that APValues are deallocated with the ASTContext by registering a deallocation function for APValues to the ASTContext. Original version of the patch by James Dennett. llvm-svn: 183101
This commit is contained in:
parent
d0cf5b2de3
commit
a732899cb4
|
@ -168,6 +168,13 @@ public:
|
||||||
MakeUninit();
|
MakeUninit();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// \brief Returns whether the object performed allocations.
|
||||||
|
///
|
||||||
|
/// If APValues are constructed via placement new, \c needsCleanup()
|
||||||
|
/// indicates whether the destructor must be called in order to correctly
|
||||||
|
/// free all allocated memory.
|
||||||
|
bool needsCleanup() const;
|
||||||
|
|
||||||
/// \brief Swaps the contents of this and the given APValue.
|
/// \brief Swaps the contents of this and the given APValue.
|
||||||
void swap(APValue &RHS);
|
void swap(APValue &RHS);
|
||||||
|
|
||||||
|
|
|
@ -2209,10 +2209,12 @@ private:
|
||||||
const ObjCImplementationDecl *Impl) const;
|
const ObjCImplementationDecl *Impl) const;
|
||||||
|
|
||||||
private:
|
private:
|
||||||
/// \brief A set of deallocations that should be performed when the
|
/// \brief A set of deallocations that should be performed when the
|
||||||
/// ASTContext is destroyed.
|
/// ASTContext is destroyed.
|
||||||
SmallVector<std::pair<void (*)(void*), void *>, 16> Deallocations;
|
typedef llvm::SmallDenseMap<void(*)(void*), llvm::SmallVector<void*, 16> >
|
||||||
|
DeallocationMap;
|
||||||
|
DeallocationMap Deallocations;
|
||||||
|
|
||||||
// FIXME: This currently contains the set of StoredDeclMaps used
|
// FIXME: This currently contains the set of StoredDeclMaps used
|
||||||
// by DeclContext objects. This probably should not be in ASTContext,
|
// by DeclContext objects. This probably should not be in ASTContext,
|
||||||
// but we include it here so that ASTContext can quickly deallocate them.
|
// but we include it here so that ASTContext can quickly deallocate them.
|
||||||
|
|
|
@ -212,6 +212,39 @@ void APValue::DestroyDataAndMakeUninit() {
|
||||||
Kind = Uninitialized;
|
Kind = Uninitialized;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool APValue::needsCleanup() const {
|
||||||
|
switch (getKind()) {
|
||||||
|
case Uninitialized:
|
||||||
|
case AddrLabelDiff:
|
||||||
|
return false;
|
||||||
|
case Struct:
|
||||||
|
case Union:
|
||||||
|
case Array:
|
||||||
|
case Vector:
|
||||||
|
return true;
|
||||||
|
case Int:
|
||||||
|
return getInt().needsCleanup();
|
||||||
|
case Float:
|
||||||
|
return getFloat().needsCleanup();
|
||||||
|
case ComplexFloat:
|
||||||
|
assert(getComplexFloatImag().needsCleanup() ==
|
||||||
|
getComplexFloatReal().needsCleanup() &&
|
||||||
|
"In _Complex float types, real and imaginary values always have the "
|
||||||
|
"same size.");
|
||||||
|
return getComplexFloatReal().needsCleanup();
|
||||||
|
case ComplexInt:
|
||||||
|
assert(getComplexIntImag().needsCleanup() ==
|
||||||
|
getComplexIntReal().needsCleanup() &&
|
||||||
|
"In _Complex int types, real and imaginary values must have the "
|
||||||
|
"same size.");
|
||||||
|
return getComplexIntReal().needsCleanup();
|
||||||
|
case LValue:
|
||||||
|
return reinterpret_cast<const LV *>(Data)->hasPathPtr();
|
||||||
|
case MemberPointer:
|
||||||
|
return reinterpret_cast<const MemberPointerData *>(Data)->hasPathPtr();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void APValue::swap(APValue &RHS) {
|
void APValue::swap(APValue &RHS) {
|
||||||
std::swap(Kind, RHS.Kind);
|
std::swap(Kind, RHS.Kind);
|
||||||
char TmpData[MaxSize];
|
char TmpData[MaxSize];
|
||||||
|
|
|
@ -732,10 +732,12 @@ ASTContext::~ASTContext() {
|
||||||
// FIXME: Is this the ideal solution?
|
// FIXME: Is this the ideal solution?
|
||||||
ReleaseDeclContextMaps();
|
ReleaseDeclContextMaps();
|
||||||
|
|
||||||
// Call all of the deallocation functions.
|
// Call all of the deallocation functions on all of their targets.
|
||||||
for (unsigned I = 0, N = Deallocations.size(); I != N; ++I)
|
for (DeallocationMap::const_iterator I = Deallocations.begin(),
|
||||||
Deallocations[I].first(Deallocations[I].second);
|
E = Deallocations.end(); I != E; ++I)
|
||||||
|
for (unsigned J = 0, N = I->second.size(); J != N; ++J)
|
||||||
|
(I->first)((I->second)[J]);
|
||||||
|
|
||||||
// ASTRecordLayout objects in ASTRecordLayouts must always be destroyed
|
// ASTRecordLayout objects in ASTRecordLayouts must always be destroyed
|
||||||
// because they can contain DenseMaps.
|
// because they can contain DenseMaps.
|
||||||
for (llvm::DenseMap<const ObjCContainerDecl*,
|
for (llvm::DenseMap<const ObjCContainerDecl*,
|
||||||
|
@ -759,7 +761,7 @@ ASTContext::~ASTContext() {
|
||||||
}
|
}
|
||||||
|
|
||||||
void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) {
|
void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) {
|
||||||
Deallocations.push_back(std::make_pair(Callback, Data));
|
Deallocations[Callback].push_back(Data);
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
|
|
@ -1827,6 +1827,10 @@ EvaluatedStmt *VarDecl::ensureEvaluatedStmt() const {
|
||||||
EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>();
|
EvaluatedStmt *Eval = Init.dyn_cast<EvaluatedStmt *>();
|
||||||
if (!Eval) {
|
if (!Eval) {
|
||||||
Stmt *S = Init.get<Stmt *>();
|
Stmt *S = Init.get<Stmt *>();
|
||||||
|
// Note: EvaluatedStmt contains an APValue, which usually holds
|
||||||
|
// resources not allocated from the ASTContext. We need to do some
|
||||||
|
// work to avoid leaking those, but we do so in VarDecl::evaluateValue
|
||||||
|
// where we can detect whether there's anything to clean up or not.
|
||||||
Eval = new (getASTContext()) EvaluatedStmt;
|
Eval = new (getASTContext()) EvaluatedStmt;
|
||||||
Eval->Value = S;
|
Eval->Value = S;
|
||||||
Init = Eval;
|
Init = Eval;
|
||||||
|
@ -1839,6 +1843,13 @@ APValue *VarDecl::evaluateValue() const {
|
||||||
return evaluateValue(Notes);
|
return evaluateValue(Notes);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
namespace {
|
||||||
|
// Destroy an APValue that was allocated in an ASTContext.
|
||||||
|
void DestroyAPValue(void* UntypedValue) {
|
||||||
|
static_cast<APValue*>(UntypedValue)->~APValue();
|
||||||
|
}
|
||||||
|
} // namespace
|
||||||
|
|
||||||
APValue *VarDecl::evaluateValue(
|
APValue *VarDecl::evaluateValue(
|
||||||
SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
|
SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
|
||||||
EvaluatedStmt *Eval = ensureEvaluatedStmt();
|
EvaluatedStmt *Eval = ensureEvaluatedStmt();
|
||||||
|
@ -1864,9 +1875,13 @@ APValue *VarDecl::evaluateValue(
|
||||||
bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, getASTContext(),
|
bool Result = Init->EvaluateAsInitializer(Eval->Evaluated, getASTContext(),
|
||||||
this, Notes);
|
this, Notes);
|
||||||
|
|
||||||
// Ensure the result is an uninitialized APValue if evaluation fails.
|
// Ensure the computed APValue is cleaned up later if evaluation succeeded,
|
||||||
|
// or that it's empty (so that there's nothing to clean up) if evaluation
|
||||||
|
// failed.
|
||||||
if (!Result)
|
if (!Result)
|
||||||
Eval->Evaluated = APValue();
|
Eval->Evaluated = APValue();
|
||||||
|
else if (Eval->Evaluated.needsCleanup())
|
||||||
|
getASTContext().AddDeallocation(DestroyAPValue, &Eval->Evaluated);
|
||||||
|
|
||||||
Eval->IsEvaluating = false;
|
Eval->IsEvaluating = false;
|
||||||
Eval->WasEvaluated = true;
|
Eval->WasEvaluated = true;
|
||||||
|
|
|
@ -3,6 +3,7 @@ add_clang_unittest(ASTTests
|
||||||
CommentLexer.cpp
|
CommentLexer.cpp
|
||||||
CommentParser.cpp
|
CommentParser.cpp
|
||||||
DeclPrinterTest.cpp
|
DeclPrinterTest.cpp
|
||||||
|
DeclTest.cpp
|
||||||
SourceLocationTest.cpp
|
SourceLocationTest.cpp
|
||||||
StmtPrinterTest.cpp
|
StmtPrinterTest.cpp
|
||||||
)
|
)
|
||||||
|
|
|
@ -0,0 +1,56 @@
|
||||||
|
//===- unittests/AST/DeclTest.cpp --- Declaration tests -------------------===//
|
||||||
|
//
|
||||||
|
// The LLVM Compiler Infrastructure
|
||||||
|
//
|
||||||
|
// This file is distributed under the University of Illinois Open Source
|
||||||
|
// License. See LICENSE.TXT for details.
|
||||||
|
//
|
||||||
|
//===----------------------------------------------------------------------===//
|
||||||
|
//
|
||||||
|
// Unit tests for Decl nodes in the AST.
|
||||||
|
//
|
||||||
|
//===----------------------------------------------------------------------===//
|
||||||
|
|
||||||
|
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
||||||
|
#include "clang/Tooling/Tooling.h"
|
||||||
|
#include "gtest/gtest.h"
|
||||||
|
|
||||||
|
using namespace clang::ast_matchers;
|
||||||
|
using namespace clang::tooling;
|
||||||
|
|
||||||
|
TEST(Decl, CleansUpAPValues) {
|
||||||
|
MatchFinder Finder;
|
||||||
|
llvm::OwningPtr<FrontendActionFactory> Factory(
|
||||||
|
newFrontendActionFactory(&Finder));
|
||||||
|
|
||||||
|
// This is a regression test for a memory leak in APValues for structs that
|
||||||
|
// allocate memory. This test only fails if run under valgrind with full leak
|
||||||
|
// checking enabled.
|
||||||
|
std::vector<std::string> Args(1, "-std=c++11");
|
||||||
|
ASSERT_TRUE(runToolOnCodeWithArgs(
|
||||||
|
Factory->create(),
|
||||||
|
"struct X { int a; }; constexpr X x = { 42 };"
|
||||||
|
"union Y { constexpr Y(int a) : a(a) {} int a; }; constexpr Y y = { 42 };"
|
||||||
|
"constexpr int z[2] = { 42, 43 };"
|
||||||
|
"constexpr int __attribute__((vector_size(16))) v1 = {};"
|
||||||
|
"constexpr __uint128_t large_int = 0xffffffffffffffff;"
|
||||||
|
"constexpr __uint128_t small_int = 1;"
|
||||||
|
"constexpr double d1 = 42.42;"
|
||||||
|
"constexpr long double d2 = 42.42;"
|
||||||
|
"constexpr _Complex long double c1 = 42.0i;"
|
||||||
|
"constexpr _Complex long double c2 = 42.0;"
|
||||||
|
"template<int N> struct A : A<N-1> {};"
|
||||||
|
"template<> struct A<0> { int n; }; A<50> a;"
|
||||||
|
"constexpr int &r = a.n;"
|
||||||
|
"constexpr int A<50>::*p = &A<50>::n;"
|
||||||
|
"void f() { foo: bar: constexpr int k = __builtin_constant_p(0) ?"
|
||||||
|
" (char*)&&foo - (char*)&&bar : 0; }",
|
||||||
|
Args));
|
||||||
|
|
||||||
|
// FIXME: Once this test starts breaking we can test APValue::needsCleanup
|
||||||
|
// for ComplexInt.
|
||||||
|
ASSERT_FALSE(runToolOnCodeWithArgs(
|
||||||
|
Factory->create(),
|
||||||
|
"constexpr _Complex __uint128_t c = 0xffffffffffffffff;",
|
||||||
|
Args));
|
||||||
|
}
|
Loading…
Reference in New Issue