From a53d7f2521cfe49435fb52d4bd62110a7fbe5a27 Mon Sep 17 00:00:00 2001 From: Argiris Kirtzidis Date: Sat, 28 Aug 2010 09:06:06 +0000 Subject: [PATCH] Fix the memory leak of FloatingLiteral/IntegerLiteral. For large floats/integers, APFloat/APInt will allocate memory from the heap to represent these numbers. Unfortunately, when we use a BumpPtrAllocator to allocate IntegerLiteral/FloatingLiteral nodes the memory associated with the APFloat/APInt values will never get freed. I introduce the class 'APNumericStorage' which uses ASTContext's allocator for memory allocation and is used internally by FloatingLiteral/IntegerLiteral. Fixes rdar://7637185 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@112361 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Expr.h | 97 +++++++++++++++++++++++++++++++------ lib/AST/ASTImporter.cpp | 5 +- lib/AST/Expr.cpp | 38 +++++++++++++++ lib/Rewrite/RewriteObjC.cpp | 11 +++-- lib/Sema/SemaDecl.cpp | 3 +- lib/Sema/SemaDeclCXX.cpp | 9 ++-- lib/Sema/SemaExpr.cpp | 6 +-- lib/Sema/SemaExprCXX.cpp | 8 +-- lib/Sema/SemaOverload.cpp | 4 +- lib/Sema/SemaTemplate.cpp | 2 +- lib/Sema/TreeTransform.h | 15 +++--- lib/Serialization/ASTReaderStmt.cpp | 8 +-- 12 files changed, 156 insertions(+), 50 deletions(-) diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index 61159fdc4..9ec1451e7 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -757,28 +757,84 @@ public: virtual child_iterator child_end(); }; +/// \brief Used by IntegerLiteral/FloatingLiteral to store the numeric without +/// leaking memory. +/// +/// For large floats/integers, APFloat/APInt will allocate memory from the heap +/// to represent these numbers. Unfortunately, when we use a BumpPtrAllocator +/// to allocate IntegerLiteral/FloatingLiteral nodes the memory associated with +/// the APFloat/APInt values will never get freed. APNumericStorage uses +/// ASTContext's allocator for memory allocation. +class APNumericStorage { + unsigned BitWidth; + union { + uint64_t VAL; ///< Used to store the <= 64 bits integer value. + uint64_t *pVal; ///< Used to store the >64 bits integer value. + }; + + bool hasAllocation() const { return llvm::APInt::getNumWords(BitWidth) > 1; } + + APNumericStorage(const APNumericStorage&); // do not implement + APNumericStorage& operator=(const APNumericStorage&); // do not implement + +protected: + APNumericStorage() : BitWidth(0), VAL(0) { } + + llvm::APInt getIntValue() const { + unsigned NumWords = llvm::APInt::getNumWords(BitWidth); + if (NumWords > 1) + return llvm::APInt(BitWidth, NumWords, pVal); + else + return llvm::APInt(BitWidth, VAL); + } + void setIntValue(ASTContext &C, const llvm::APInt &Val); +}; + +class APIntStorage : public APNumericStorage { +public: + llvm::APInt getValue() const { return getIntValue(); } + void setValue(ASTContext &C, const llvm::APInt &Val) { setIntValue(C, Val); } +}; + +class APFloatStorage : public APNumericStorage { +public: + llvm::APFloat getValue() const { return llvm::APFloat(getIntValue()); } + void setValue(ASTContext &C, const llvm::APFloat &Val) { + setIntValue(C, Val.bitcastToAPInt()); + } +}; + class IntegerLiteral : public Expr { - llvm::APInt Value; + APIntStorage Num; SourceLocation Loc; + + /// \brief Construct an empty integer literal. + explicit IntegerLiteral(EmptyShell Empty) + : Expr(IntegerLiteralClass, Empty) { } + public: // type should be IntTy, LongTy, LongLongTy, UnsignedIntTy, UnsignedLongTy, // or UnsignedLongLongTy - IntegerLiteral(const llvm::APInt &V, QualType type, SourceLocation l) - : Expr(IntegerLiteralClass, type, false, false), Value(V), Loc(l) { + IntegerLiteral(ASTContext &C, const llvm::APInt &V, + QualType type, SourceLocation l) + : Expr(IntegerLiteralClass, type, false, false), Loc(l) { assert(type->isIntegerType() && "Illegal type in IntegerLiteral"); + setValue(C, V); } - /// \brief Construct an empty integer literal. - explicit IntegerLiteral(EmptyShell Empty) - : Expr(IntegerLiteralClass, Empty) { } + // type should be IntTy, LongTy, LongLongTy, UnsignedIntTy, UnsignedLongTy, + // or UnsignedLongLongTy + static IntegerLiteral *Create(ASTContext &C, const llvm::APInt &V, + QualType type, SourceLocation l); + static IntegerLiteral *Create(ASTContext &C, EmptyShell Empty); - const llvm::APInt &getValue() const { return Value; } + llvm::APInt getValue() const { return Num.getValue(); } virtual SourceRange getSourceRange() const { return SourceRange(Loc); } /// \brief Retrieve the location of the literal. SourceLocation getLocation() const { return Loc; } - void setValue(const llvm::APInt &Val) { Value = Val; } + void setValue(ASTContext &C, const llvm::APInt &Val) { Num.setValue(C, Val); } void setLocation(SourceLocation Location) { Loc = Location; } static bool classof(const Stmt *T) { @@ -827,21 +883,30 @@ public: }; class FloatingLiteral : public Expr { - llvm::APFloat Value; + APFloatStorage Num; bool IsExact : 1; SourceLocation Loc; -public: - FloatingLiteral(const llvm::APFloat &V, bool isexact, + + FloatingLiteral(ASTContext &C, const llvm::APFloat &V, bool isexact, QualType Type, SourceLocation L) - : Expr(FloatingLiteralClass, Type, false, false), Value(V), - IsExact(isexact), Loc(L) {} + : Expr(FloatingLiteralClass, Type, false, false), + IsExact(isexact), Loc(L) { + setValue(C, V); + } /// \brief Construct an empty floating-point literal. explicit FloatingLiteral(EmptyShell Empty) - : Expr(FloatingLiteralClass, Empty), Value(0.0) { } + : Expr(FloatingLiteralClass, Empty), IsExact(false) { } + +public: + static FloatingLiteral *Create(ASTContext &C, const llvm::APFloat &V, + bool isexact, QualType Type, SourceLocation L); + static FloatingLiteral *Create(ASTContext &C, EmptyShell Empty); - const llvm::APFloat &getValue() const { return Value; } - void setValue(const llvm::APFloat &Val) { Value = Val; } + llvm::APFloat getValue() const { return Num.getValue(); } + void setValue(ASTContext &C, const llvm::APFloat &Val) { + Num.setValue(C, Val); + } bool isExact() const { return IsExact; } void setExact(bool E) { IsExact = E; } diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index f63df46f5..eee41a6c2 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -2818,8 +2818,9 @@ Expr *ASTNodeImporter::VisitIntegerLiteral(IntegerLiteral *E) { if (T.isNull()) return 0; - return new (Importer.getToContext()) - IntegerLiteral(E->getValue(), T, Importer.Import(E->getLocation())); + return IntegerLiteral::Create(Importer.getToContext(), + E->getValue(), T, + Importer.Import(E->getLocation())); } Expr *ASTNodeImporter::VisitCharacterLiteral(CharacterLiteral *E) { diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index d5cf4d67f..5feef1c80 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -374,6 +374,44 @@ std::string PredefinedExpr::ComputeName(IdentType IT, const Decl *CurrentDecl) { return ""; } +void APNumericStorage::setIntValue(ASTContext &C, const llvm::APInt &Val) { + if (hasAllocation()) + C.Deallocate(pVal); + + BitWidth = Val.getBitWidth(); + unsigned NumWords = Val.getNumWords(); + const uint64_t* Words = Val.getRawData(); + if (NumWords > 1) { + pVal = new (C) uint64_t[NumWords]; + std::copy(Words, Words + NumWords, pVal); + } else if (NumWords == 1) + VAL = Words[0]; + else + VAL = 0; +} + +IntegerLiteral * +IntegerLiteral::Create(ASTContext &C, const llvm::APInt &V, + QualType type, SourceLocation l) { + return new (C) IntegerLiteral(C, V, type, l); +} + +IntegerLiteral * +IntegerLiteral::Create(ASTContext &C, EmptyShell Empty) { + return new (C) IntegerLiteral(Empty); +} + +FloatingLiteral * +FloatingLiteral::Create(ASTContext &C, const llvm::APFloat &V, + bool isexact, QualType Type, SourceLocation L) { + return new (C) FloatingLiteral(C, V, isexact, Type, L); +} + +FloatingLiteral * +FloatingLiteral::Create(ASTContext &C, EmptyShell Empty) { + return new (C) FloatingLiteral(Empty); +} + /// getValueAsApproximateDouble - This returns the value as an inaccurate /// double. Note that this may cause loss of precision, but is useful for /// debugging dumps, etc. diff --git a/lib/Rewrite/RewriteObjC.cpp b/lib/Rewrite/RewriteObjC.cpp index 80b9681a7..4a7de4bed 100644 --- a/lib/Rewrite/RewriteObjC.cpp +++ b/lib/Rewrite/RewriteObjC.cpp @@ -3029,9 +3029,10 @@ Stmt *RewriteObjC::SynthMessageExpr(ObjCMessageExpr *Exp, // is needed to decide what to do. unsigned IntSize = static_cast(Context->getTypeSize(Context->IntTy)); - IntegerLiteral *limit = new (Context) IntegerLiteral(llvm::APInt(IntSize, 8), - Context->IntTy, - SourceLocation()); + IntegerLiteral *limit = IntegerLiteral::Create(*Context, + llvm::APInt(IntSize, 8), + Context->IntTy, + SourceLocation()); BinaryOperator *lessThanExpr = new (Context) BinaryOperator(sizeofExpr, limit, BO_LE, Context->IntTy, @@ -5268,8 +5269,8 @@ Stmt *RewriteObjC::SynthBlockInitExpr(BlockExpr *Exp, int flag = (BLOCK_HAS_COPY_DISPOSE | BLOCK_HAS_DESCRIPTOR); unsigned IntSize = static_cast(Context->getTypeSize(Context->IntTy)); - Expr *FlagExp = new (Context) IntegerLiteral(llvm::APInt(IntSize, flag), - Context->IntTy, SourceLocation()); + Expr *FlagExp = IntegerLiteral::Create(*Context, llvm::APInt(IntSize, flag), + Context->IntTy, SourceLocation()); InitExprs.push_back(FlagExp); } NewRep = new (Context) CallExpr(*Context, DRE, &InitExprs[0], InitExprs.size(), diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 26d494191..6f8d9a927 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -6480,8 +6480,7 @@ void Sema::ActOnLastBitfield(SourceLocation DeclLoc, Decl *EnclosingDecl, } // All conditions are met. Add a new bitfield to the tail end of ivars. llvm::APInt Zero(Context.getTypeSize(Context.CharTy), 0); - Expr * BW = - new (Context) IntegerLiteral(Zero, Context.CharTy, DeclLoc); + Expr * BW = IntegerLiteral::Create(Context, Zero, Context.CharTy, DeclLoc); Ivar = ObjCIvarDecl::Create(Context, cast(EnclosingDecl), DeclLoc, 0, diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 676af80c7..35920c279 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -4669,7 +4669,7 @@ BuildSingleCopyAssign(Sema &S, SourceLocation Loc, QualType T, // Initialize the iteration variable to zero. llvm::APInt Zero(S.Context.getTypeSize(SizeType), 0); - IterationVar->setInit(new (S.Context) IntegerLiteral(Zero, SizeType, Loc)); + IterationVar->setInit(IntegerLiteral::Create(S.Context, Zero, SizeType, Loc)); // Create a reference to the iteration variable; we'll use this several // times throughout. @@ -4685,8 +4685,9 @@ BuildSingleCopyAssign(Sema &S, SourceLocation Loc, QualType T, Upper.zextOrTrunc(S.Context.getTypeSize(SizeType)); Expr *Comparison = new (S.Context) BinaryOperator(IterationVarRef->Retain(), - new (S.Context) IntegerLiteral(Upper, SizeType, Loc), - BO_NE, S.Context.BoolTy, Loc); + IntegerLiteral::Create(S.Context, + Upper, SizeType, Loc), + BO_NE, S.Context.BoolTy, Loc); // Create the pre-increment of the iteration variable. Expr *Increment @@ -5135,7 +5136,7 @@ void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation, ASTOwningVector CallArgs(*this); CallArgs.push_back(To.takeAs()); CallArgs.push_back(From.takeAs()); - CallArgs.push_back(new (Context) IntegerLiteral(Size, SizeType, Loc)); + CallArgs.push_back(IntegerLiteral::Create(Context, Size, SizeType, Loc)); llvm::SmallVector Commas; // FIXME: Silly Commas.push_back(Loc); Commas.push_back(Loc); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index d0c2b8214..1a065eb63 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -1946,7 +1946,7 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok) { if (Tok.getLength() == 1) { const char Val = PP.getSpellingOfSingleCharacterNumericConstant(Tok); unsigned IntSize = Context.Target.getIntWidth(); - return Owned(new (Context) IntegerLiteral(llvm::APInt(IntSize, Val-'0'), + return Owned(IntegerLiteral::Create(Context, llvm::APInt(IntSize, Val-'0'), Context.IntTy, Tok.getLocation())); } @@ -2004,7 +2004,7 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok) { } bool isExact = (result == APFloat::opOK); - Res = new (Context) FloatingLiteral(Val, isExact, Ty, Tok.getLocation()); + Res = FloatingLiteral::Create(Context, Val, isExact, Ty, Tok.getLocation()); } else if (!Literal.isIntegerLiteral()) { return ExprError(); @@ -2091,7 +2091,7 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok) { if (ResultVal.getBitWidth() != Width) ResultVal.trunc(Width); } - Res = new (Context) IntegerLiteral(ResultVal, Ty, Tok.getLocation()); + Res = IntegerLiteral::Create(Context, ResultVal, Ty, Tok.getLocation()); } // If this is an imaginary literal, create the ImaginaryLiteral wrapper. diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 2f8cfe2d2..5720d931b 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -672,9 +672,9 @@ Sema::BuildCXXNew(SourceLocation StartLoc, bool UseGlobal, if (!ArraySize) { if (const ConstantArrayType *Array = Context.getAsConstantArrayType(AllocType)) { - ArraySize = new (Context) IntegerLiteral(Array->getSize(), - Context.getSizeType(), - TypeRange.getEnd()); + ArraySize = IntegerLiteral::Create(Context, Array->getSize(), + Context.getSizeType(), + TypeRange.getEnd()); AllocType = Array->getElementType(); } } @@ -922,7 +922,7 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, // We don't care about the actual value of this argument. // FIXME: Should the Sema create the expression and embed it in the syntax // tree? Or should the consumer just recalculate the value? - IntegerLiteral Size(llvm::APInt::getNullValue( + IntegerLiteral Size(Context, llvm::APInt::getNullValue( Context.Target.getPointerWidth(0)), Context.getSizeType(), SourceLocation()); diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index b5f1e528e..11b4bb3b9 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -6754,8 +6754,8 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, unsigned OpcIn, // post-decrement. if (Opc == UO_PostInc || Opc == UO_PostDec) { llvm::APSInt Zero(Context.getTypeSize(Context.IntTy), false); - Args[1] = new (Context) IntegerLiteral(Zero, Context.IntTy, - SourceLocation()); + Args[1] = IntegerLiteral::Create(Context, Zero, Context.IntTy, + SourceLocation()); NumArgs = 2; } diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index e293e9b39..09656bcd9 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -3149,7 +3149,7 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg, T, Loc)); - return Owned(new (Context) IntegerLiteral(*Arg.getAsIntegral(), T, Loc)); + return Owned(IntegerLiteral::Create(Context, *Arg.getAsIntegral(), T, Loc)); } diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index dbc02d872..08cfd68d6 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -5332,10 +5332,10 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr *E) { } else if (const ConstantArrayType *ConsArrayT = dyn_cast(ArrayT)) { ArraySize - = SemaRef.Owned(new (SemaRef.Context) IntegerLiteral( - ConsArrayT->getSize(), - SemaRef.Context.getSizeType(), - /*FIXME:*/E->getLocStart())); + = SemaRef.Owned(IntegerLiteral::Create(SemaRef.Context, + ConsArrayT->getSize(), + SemaRef.Context.getSizeType(), + /*FIXME:*/E->getLocStart())); AllocType = ConsArrayT->getElementType(); } else if (const DependentSizedArrayType *DepArrayT = dyn_cast(ArrayT)) { @@ -6352,7 +6352,8 @@ TreeTransform::RebuildArrayType(QualType ElementType, break; } - IntegerLiteral ArraySize(*Size, SizeType, /*FIXME*/BracketsRange.getBegin()); + IntegerLiteral ArraySize(SemaRef.Context, *Size, SizeType, + /*FIXME*/BracketsRange.getBegin()); return SemaRef.BuildArrayType(ElementType, SizeMod, &ArraySize, IndexTypeQuals, BracketsRange, getDerived().getBaseEntity()); @@ -6418,8 +6419,8 @@ QualType TreeTransform::RebuildExtVectorType(QualType ElementType, llvm::APInt numElements(SemaRef.Context.getIntWidth(SemaRef.Context.IntTy), NumElements, true); IntegerLiteral *VectorSize - = new (SemaRef.Context) IntegerLiteral(numElements, SemaRef.Context.IntTy, - AttributeLoc); + = IntegerLiteral::Create(SemaRef.Context, numElements, SemaRef.Context.IntTy, + AttributeLoc); return SemaRef.BuildExtVectorType(ElementType, VectorSize, AttributeLoc); } diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp index ff2d39e6e..ec227e283 100644 --- a/lib/Serialization/ASTReaderStmt.cpp +++ b/lib/Serialization/ASTReaderStmt.cpp @@ -409,12 +409,12 @@ void ASTStmtReader::VisitDeclRefExpr(DeclRefExpr *E) { void ASTStmtReader::VisitIntegerLiteral(IntegerLiteral *E) { VisitExpr(E); E->setLocation(SourceLocation::getFromRawEncoding(Record[Idx++])); - E->setValue(Reader.ReadAPInt(Record, Idx)); + E->setValue(*Reader.getContext(), Reader.ReadAPInt(Record, Idx)); } void ASTStmtReader::VisitFloatingLiteral(FloatingLiteral *E) { VisitExpr(E); - E->setValue(Reader.ReadAPFloat(Record, Idx)); + E->setValue(*Reader.getContext(), Reader.ReadAPFloat(Record, Idx)); E->setExact(Record[Idx++]); E->setLocation(SourceLocation::getFromRawEncoding(Record[Idx++])); } @@ -1401,11 +1401,11 @@ Stmt *ASTReader::ReadStmtFromStream(llvm::BitstreamCursor &Cursor) { break; case EXPR_INTEGER_LITERAL: - S = new (Context) IntegerLiteral(Empty); + S = IntegerLiteral::Create(*Context, Empty); break; case EXPR_FLOATING_LITERAL: - S = new (Context) FloatingLiteral(Empty); + S = FloatingLiteral::Create(*Context, Empty); break; case EXPR_IMAGINARY_LITERAL: -- 2.11.4.GIT