From 855243789cb44799c03f4c7216d3d6308805f549 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Thu, 7 Jun 2012 15:09:51 +0000 Subject: [PATCH] Plug a long standing memory leak in TemplateArgument. The integral APSInt value is now stored in a decomposed form and the backing store for large values is allocated via the ASTContext. This way its not leaked as TemplateArguments are never destructed when they are allocated in the ASTContext. Since the integral data is immutable it is now shared between instances, making copying TemplateArguments a trivial operation. Currently getting the integral data out of a TemplateArgument requires creating a new APSInt object. This is cheap when the value is small but can be expensive if it's not. If this turns out to be an issue a more efficient accessor could be added. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158150 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/TemplateBase.h | 96 ++++++++---------------------- include/clang/Sema/Template.h | 5 +- lib/AST/ASTContext.cpp | 3 +- lib/AST/ASTImporter.cpp | 4 +- lib/AST/DumpXML.cpp | 2 +- lib/AST/ItaniumMangle.cpp | 2 +- lib/AST/MicrosoftMangle.cpp | 2 +- lib/AST/StmtPrinter.cpp | 2 +- lib/AST/StmtProfile.cpp | 2 +- lib/AST/TemplateBase.cpp | 32 +++++++--- lib/CodeGen/CGDebugInfo.cpp | 2 +- lib/Sema/SemaExpr.cpp | 2 +- lib/Sema/SemaTemplate.cpp | 11 ++-- lib/Sema/SemaTemplateDeduction.cpp | 11 ++-- lib/Serialization/ASTReader.cpp | 2 +- lib/Serialization/ASTWriter.cpp | 2 +- tools/libclang/CIndexUSRs.cpp | 2 +- 17 files changed, 79 insertions(+), 103 deletions(-) diff --git a/include/clang/AST/TemplateBase.h b/include/clang/AST/TemplateBase.h index 65f54600c0..9d3f45f4b2 100644 --- a/include/clang/AST/TemplateBase.h +++ b/include/clang/AST/TemplateBase.h @@ -73,7 +73,15 @@ private: union { uintptr_t TypeOrValue; struct { - char Value[sizeof(llvm::APSInt)]; + // We store a decomposed APSInt with the data allocated by ASTContext if + // BitWidth > 64. The memory may be shared between multiple + // TemplateArgument instances. + union { + uint64_t VAL; ///< Used to store the <= 64 bits integer value. + const uint64_t *pVal; ///< Used to store the >64 bits integer value. + }; + unsigned BitWidth : 31; + unsigned IsUnsigned : 1; void *Type; } Integer; struct { @@ -104,11 +112,15 @@ public: TypeOrValue = reinterpret_cast(D); } - /// \brief Construct an integral constant template argument. - TemplateArgument(const llvm::APSInt &Value, QualType Type) : Kind(Integral) { - // FIXME: Large integral values will get leaked. Do something - // similar to what we did with IntegerLiteral. - new (Integer.Value) llvm::APSInt(Value); + /// \brief Construct an integral constant template argument. The memory to + /// store the value is allocated with Ctx. + TemplateArgument(ASTContext &Ctx, const llvm::APSInt &Value, QualType Type); + + /// \brief Construct an integral constant template argument with the same + /// value as Other but a different type. + TemplateArgument(const TemplateArgument &Other, QualType Type) + : Kind(Integral) { + Integer = Other.Integer; Integer.Type = Type.getAsOpaquePtr(); } @@ -165,62 +177,6 @@ public: this->Args.NumArgs = NumArgs; } - /// \brief Copy constructor for a template argument. - TemplateArgument(const TemplateArgument &Other) : Kind(Other.Kind) { - // FIXME: Large integral values will get leaked. Do something - // similar to what we did with IntegerLiteral. - if (Kind == Integral) { - new (Integer.Value) llvm::APSInt(*Other.getAsIntegral()); - Integer.Type = Other.Integer.Type; - } else if (Kind == Pack) { - Args.NumArgs = Other.Args.NumArgs; - Args.Args = Other.Args.Args; - } else if (Kind == Template || Kind == TemplateExpansion) { - TemplateArg.Name = Other.TemplateArg.Name; - TemplateArg.NumExpansions = Other.TemplateArg.NumExpansions; - } else - TypeOrValue = Other.TypeOrValue; - } - - TemplateArgument& operator=(const TemplateArgument& Other) { - using llvm::APSInt; - - if (Kind == Other.Kind && Kind == Integral) { - // Copy integral values. - *this->getAsIntegral() = *Other.getAsIntegral(); - Integer.Type = Other.Integer.Type; - return *this; - } - - // Destroy the current integral value, if that's what we're holding. - if (Kind == Integral) - getAsIntegral()->~APSInt(); - - Kind = Other.Kind; - - if (Other.Kind == Integral) { - new (Integer.Value) llvm::APSInt(*Other.getAsIntegral()); - Integer.Type = Other.Integer.Type; - } else if (Other.Kind == Pack) { - Args.NumArgs = Other.Args.NumArgs; - Args.Args = Other.Args.Args; - } else if (Kind == Template || Kind == TemplateExpansion) { - TemplateArg.Name = Other.TemplateArg.Name; - TemplateArg.NumExpansions = Other.TemplateArg.NumExpansions; - } else { - TypeOrValue = Other.TypeOrValue; - } - - return *this; - } - - ~TemplateArgument() { - using llvm::APSInt; - - if (Kind == Integral) - getAsIntegral()->~APSInt(); - } - /// \brief Create a new template argument pack by copying the given set of /// template arguments. static TemplateArgument CreatePackCopy(ASTContext &Context, @@ -286,14 +242,14 @@ public: llvm::Optional getNumTemplateExpansions() const; /// \brief Retrieve the template argument as an integral value. - llvm::APSInt *getAsIntegral() { - if (Kind != Integral) - return 0; - return reinterpret_cast(&Integer.Value[0]); - } - - const llvm::APSInt *getAsIntegral() const { - return const_cast(this)->getAsIntegral(); + // FIXME: Provide a way to read the integral data without copying the value. + llvm::APSInt getAsIntegral() const { + if (Integer.BitWidth <= 64) + return llvm::APSInt(llvm::APInt(Integer.BitWidth, Integer.VAL), + Integer.IsUnsigned); + return llvm::APSInt(llvm::APInt(Integer.BitWidth, + llvm::makeArrayRef(Integer.pVal, Integer.BitWidth / 8)), + Integer.IsUnsigned); } /// \brief Retrieve the type of the integral value. diff --git a/include/clang/Sema/Template.h b/include/clang/Sema/Template.h index c16823a6a1..87d71b8995 100644 --- a/include/clang/Sema/Template.h +++ b/include/clang/Sema/Template.h @@ -152,10 +152,11 @@ namespace clang { /// \brief Construct an integral non-type template argument that /// has been deduced, possibly from an array bound. - DeducedTemplateArgument(const llvm::APSInt &Value, + DeducedTemplateArgument(ASTContext &Ctx, + const llvm::APSInt &Value, QualType ValueType, bool DeducedFromArrayBound) - : TemplateArgument(Value, ValueType), + : TemplateArgument(Ctx, Value, ValueType), DeducedFromArrayBound(DeducedFromArrayBound) { } /// \brief For a non-type template argument, determine whether the diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index 164813b7c3..bd68d83250 100644 --- a/lib/AST/ASTContext.cpp +++ b/lib/AST/ASTContext.cpp @@ -3336,8 +3336,7 @@ ASTContext::getCanonicalTemplateArgument(const TemplateArgument &Arg) const { Arg.getNumTemplateExpansions()); case TemplateArgument::Integral: - return TemplateArgument(*Arg.getAsIntegral(), - getCanonicalType(Arg.getIntegralType())); + return TemplateArgument(Arg, getCanonicalType(Arg.getIntegralType())); case TemplateArgument::Type: return TemplateArgument(getCanonicalType(Arg.getAsType())); diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index 417f3cff58..9f016fe9bb 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -322,7 +322,7 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context, Arg2.getIntegralType())) return false; - return IsSameValue(*Arg1.getAsIntegral(), *Arg2.getAsIntegral()); + return IsSameValue(Arg1.getAsIntegral(), Arg2.getAsIntegral()); case TemplateArgument::Declaration: if (!Arg1.getAsDecl() || !Arg2.getAsDecl()) @@ -1992,7 +1992,7 @@ ASTNodeImporter::ImportTemplateArgument(const TemplateArgument &From) { QualType ToType = Importer.Import(From.getIntegralType()); if (ToType.isNull()) return TemplateArgument(); - return TemplateArgument(*From.getAsIntegral(), ToType); + return TemplateArgument(From, ToType); } case TemplateArgument::Declaration: diff --git a/lib/AST/DumpXML.cpp b/lib/AST/DumpXML.cpp index 4c7cd8a679..4df692d9a5 100644 --- a/lib/AST/DumpXML.cpp +++ b/lib/AST/DumpXML.cpp @@ -326,7 +326,7 @@ struct XMLDumper : public XMLDeclVisitor, } case TemplateArgument::Integral: { push("integer"); - setInteger("value", *A.getAsIntegral()); + setInteger("value", A.getAsIntegral()); completeAttrs(); pop(); break; diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp index e974ade6f6..7c7a5e5de3 100644 --- a/lib/AST/ItaniumMangle.cpp +++ b/lib/AST/ItaniumMangle.cpp @@ -3132,7 +3132,7 @@ void CXXNameMangler::mangleTemplateArg(const NamedDecl *P, break; } case TemplateArgument::Integral: - mangleIntegerLiteral(A.getIntegralType(), *A.getAsIntegral()); + mangleIntegerLiteral(A.getIntegralType(), A.getAsIntegral()); break; case TemplateArgument::Declaration: { assert(P && "Missing template parameter for declaration argument"); diff --git a/lib/AST/MicrosoftMangle.cpp b/lib/AST/MicrosoftMangle.cpp index 5984881702..b0d9316fe5 100644 --- a/lib/AST/MicrosoftMangle.cpp +++ b/lib/AST/MicrosoftMangle.cpp @@ -638,7 +638,7 @@ MicrosoftCXXNameMangler::mangleTemplateArgs(const TemplateArgument *TemplateArgs mangleType(TA.getAsType()); break; case TemplateArgument::Integral: - mangleIntegerLiteral(TA.getIntegralType(), *TA.getAsIntegral()); + mangleIntegerLiteral(TA.getIntegralType(), TA.getAsIntegral()); break; default: { // Issue a diagnostic. diff --git a/lib/AST/StmtPrinter.cpp b/lib/AST/StmtPrinter.cpp index 7309e7c889..2bd4d769e7 100644 --- a/lib/AST/StmtPrinter.cpp +++ b/lib/AST/StmtPrinter.cpp @@ -1275,7 +1275,7 @@ void StmtPrinter::VisitUserDefinedLiteral(UserDefinedLiteral *Node) { const TemplateArgument &Pack = Args->get(0); for (TemplateArgument::pack_iterator I = Pack.pack_begin(), E = Pack.pack_end(); I != E; ++I) { - char C = (char)I->getAsIntegral()->getZExtValue(); + char C = (char)I->getAsIntegral().getZExtValue(); OS << C; } break; diff --git a/lib/AST/StmtProfile.cpp b/lib/AST/StmtProfile.cpp index e6b378e16f..72b979d4c2 100644 --- a/lib/AST/StmtProfile.cpp +++ b/lib/AST/StmtProfile.cpp @@ -1161,7 +1161,7 @@ void StmtProfiler::VisitTemplateArgument(const TemplateArgument &Arg) { break; case TemplateArgument::Integral: - Arg.getAsIntegral()->Profile(ID); + Arg.getAsIntegral().Profile(ID); VisitType(Arg.getIntegralType()); break; diff --git a/lib/AST/TemplateBase.cpp b/lib/AST/TemplateBase.cpp index 531e03e302..284c1b5baa 100644 --- a/lib/AST/TemplateBase.cpp +++ b/lib/AST/TemplateBase.cpp @@ -36,17 +36,17 @@ using namespace clang; static void printIntegral(const TemplateArgument &TemplArg, raw_ostream &Out) { const ::clang::Type *T = TemplArg.getIntegralType().getTypePtr(); - const llvm::APSInt *Val = TemplArg.getAsIntegral(); + const llvm::APSInt &Val = TemplArg.getAsIntegral(); if (T->isBooleanType()) { - Out << (Val->getBoolValue() ? "true" : "false"); + Out << (Val.getBoolValue() ? "true" : "false"); } else if (T->isCharType()) { - const char Ch = Val->getZExtValue(); + const char Ch = Val.getZExtValue(); Out << ((Ch == '\'') ? "'\\" : "'"); Out.write_escaped(StringRef(&Ch, 1), /*UseHexEscapes=*/ true); Out << "'"; } else { - Out << Val->toString(10); + Out << Val; } } @@ -54,6 +54,24 @@ static void printIntegral(const TemplateArgument &TemplArg, // TemplateArgument Implementation //===----------------------------------------------------------------------===// +TemplateArgument::TemplateArgument(ASTContext &Ctx, const llvm::APSInt &Value, + QualType Type) + : Kind(Integral) { + // Copy the APSInt value into our decomposed form. + Integer.BitWidth = Value.getBitWidth(); + Integer.IsUnsigned = Value.isUnsigned(); + // If the value is large, we have to get additional memory from the ASTContext + if (Integer.BitWidth > 64) { + void *Mem = Ctx.Allocate(Integer.BitWidth / 8); + std::memcpy(Mem, Value.getRawData(), Integer.BitWidth / 8); + Integer.pVal = static_cast(Mem); + } else { + Integer.VAL = Value.getZExtValue(); + } + + Integer.Type = Type.getAsOpaquePtr(); +} + TemplateArgument TemplateArgument::CreatePackCopy(ASTContext &Context, const TemplateArgument *Args, unsigned NumArgs) { @@ -246,7 +264,7 @@ void TemplateArgument::Profile(llvm::FoldingSetNodeID &ID, } case Integral: - getAsIntegral()->Profile(ID); + getAsIntegral().Profile(ID); getIntegralType().Profile(ID); break; @@ -275,7 +293,7 @@ bool TemplateArgument::structurallyEquals(const TemplateArgument &Other) const { case Integral: return getIntegralType() == Other.getIntegralType() && - *getAsIntegral() == *Other.getAsIntegral(); + getAsIntegral() == Other.getAsIntegral(); case Pack: if (Args.NumArgs != Other.Args.NumArgs) return false; @@ -498,7 +516,7 @@ const DiagnosticBuilder &clang::operator<<(const DiagnosticBuilder &DB, return DB << "nullptr"; case TemplateArgument::Integral: - return DB << Arg.getAsIntegral()->toString(10); + return DB << Arg.getAsIntegral().toString(10); case TemplateArgument::Template: return DB << Arg.getAsTemplate(); diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index beea777a97..8d28322c44 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -1086,7 +1086,7 @@ CollectTemplateParams(const TemplateParameterList *TPList, llvm::DIType TTy = getOrCreateType(TA.getIntegralType(), Unit); llvm::DITemplateValueParameter TVP = DBuilder.createTemplateValueParameter(TheCU, ND->getName(), TTy, - TA.getAsIntegral()->getZExtValue()); + TA.getAsIntegral().getZExtValue()); TemplateParams.push_back(TVP); } } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 9f0378ac0e..d2377f5905 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -2605,7 +2605,7 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok, Scope *UDLScope) { llvm::APSInt Value(CharBits, CharIsUnsigned); for (unsigned I = 0, N = Literal.getUDSuffixOffset(); I != N; ++I) { Value = ThisTokBegin[I]; - TemplateArgument Arg(Value, Context.CharTy); + TemplateArgument Arg(Context, Value, Context.CharTy); TemplateArgumentLocInfo ArgInfo; ExplicitArgs.addArgument(TemplateArgumentLoc(Arg, ArgInfo)); } diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index 002b0b174e..dccb4b6a61 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -4125,7 +4125,8 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, IntegerType = Enum->getDecl()->getIntegerType(); Value = Value.extOrTrunc(Context.getTypeSize(IntegerType)); - Converted = TemplateArgument(Value, Context.getCanonicalType(ParamType)); + Converted = TemplateArgument(Context, Value, + Context.getCanonicalType(ParamType)); return ArgResult; } @@ -4251,7 +4252,7 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param, } } - Converted = TemplateArgument(Value, + Converted = TemplateArgument(Context, Value, ParamType->isEnumeralType() ? Context.getCanonicalType(ParamType) : IntegerType); @@ -4569,13 +4570,13 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg, Kind = CharacterLiteral::Ascii; return Owned(new (Context) CharacterLiteral( - Arg.getAsIntegral()->getZExtValue(), + Arg.getAsIntegral().getZExtValue(), Kind, T, Loc)); } if (T->isBooleanType()) return Owned(new (Context) CXXBoolLiteralExpr( - Arg.getAsIntegral()->getBoolValue(), + Arg.getAsIntegral().getBoolValue(), T, Loc)); if (T->isNullPtrType()) @@ -4590,7 +4591,7 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const TemplateArgument &Arg, else BT = T; - Expr *E = IntegerLiteral::Create(Context, *Arg.getAsIntegral(), BT, Loc); + Expr *E = IntegerLiteral::Create(Context, Arg.getAsIntegral(), BT, Loc); if (T->isEnumeralType()) { // FIXME: This is a hack. We need a better way to handle substituted // non-type template parameters. diff --git a/lib/Sema/SemaTemplateDeduction.cpp b/lib/Sema/SemaTemplateDeduction.cpp index e6f2b2d649..78a860c53d 100644 --- a/lib/Sema/SemaTemplateDeduction.cpp +++ b/lib/Sema/SemaTemplateDeduction.cpp @@ -193,7 +193,7 @@ checkDeducedTemplateArguments(ASTContext &Context, if (Y.getKind() == TemplateArgument::Expression || Y.getKind() == TemplateArgument::Declaration || (Y.getKind() == TemplateArgument::Integral && - hasSameExtendedValue(*X.getAsIntegral(), *Y.getAsIntegral()))) + hasSameExtendedValue(X.getAsIntegral(), Y.getAsIntegral()))) return DeducedTemplateArgument(X, X.wasDeducedFromArrayBound() && Y.wasDeducedFromArrayBound()); @@ -293,7 +293,8 @@ DeduceNonTypeTemplateArgument(Sema &S, assert(NTTP->getDepth() == 0 && "Cannot deduce non-type template argument with depth > 0"); - DeducedTemplateArgument NewDeduced(Value, ValueType, DeducedFromArrayBound); + DeducedTemplateArgument NewDeduced(S.Context, Value, ValueType, + DeducedFromArrayBound); DeducedTemplateArgument Result = checkDeducedTemplateArguments(S.Context, Deduced[NTTP->getIndex()], NewDeduced); @@ -1595,7 +1596,7 @@ DeduceTemplateArguments(Sema &S, case TemplateArgument::Integral: if (Arg.getKind() == TemplateArgument::Integral) { - if (hasSameExtendedValue(*Param.getAsIntegral(), *Arg.getAsIntegral())) + if (hasSameExtendedValue(Param.getAsIntegral(), Arg.getAsIntegral())) return Sema::TDK_Success; Info.FirstArg = Param; @@ -1618,7 +1619,7 @@ DeduceTemplateArguments(Sema &S, = getDeducedParameterFromExpr(Param.getAsExpr())) { if (Arg.getKind() == TemplateArgument::Integral) return DeduceNonTypeTemplateArgument(S, NTTP, - *Arg.getAsIntegral(), + Arg.getAsIntegral(), Arg.getIntegralType(), /*ArrayBound=*/false, Info, Deduced); @@ -1867,7 +1868,7 @@ static bool isSameTemplateArg(ASTContext &Context, Y.getAsTemplateOrTemplatePattern()).getAsVoidPointer(); case TemplateArgument::Integral: - return *X.getAsIntegral() == *Y.getAsIntegral(); + return X.getAsIntegral() == Y.getAsIntegral(); case TemplateArgument::Expression: { llvm::FoldingSetNodeID XID, YID; diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index a9092a43aa..3b59163d24 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -5905,7 +5905,7 @@ ASTReader::ReadTemplateArgument(ModuleFile &F, case TemplateArgument::Integral: { llvm::APSInt Value = ReadAPSInt(Record, Idx); QualType T = readType(F, Record, Idx); - return TemplateArgument(Value, T); + return TemplateArgument(Context, Value, T); } case TemplateArgument::Template: return TemplateArgument(ReadTemplateName(F, Record, Idx)); diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index edef1c026f..0eb2f1675b 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -4147,7 +4147,7 @@ void ASTWriter::AddTemplateArgument(const TemplateArgument &Arg, AddDeclRef(Arg.getAsDecl(), Record); break; case TemplateArgument::Integral: - AddAPSInt(*Arg.getAsIntegral(), Record); + AddAPSInt(Arg.getAsIntegral(), Record); AddTypeRef(Arg.getIntegralType(), Record); break; case TemplateArgument::Template: diff --git a/tools/libclang/CIndexUSRs.cpp b/tools/libclang/CIndexUSRs.cpp index 7c79b69315..c885dd546c 100644 --- a/tools/libclang/CIndexUSRs.cpp +++ b/tools/libclang/CIndexUSRs.cpp @@ -754,7 +754,7 @@ void USRGenerator::VisitTemplateArgument(const TemplateArgument &Arg) { case TemplateArgument::Integral: Out << 'V'; VisitType(Arg.getIntegralType()); - Out << *Arg.getAsIntegral(); + Out << Arg.getAsIntegral(); break; } } -- 2.40.0