]> granicus.if.org Git - clang/commitdiff
Plug a long standing memory leak in TemplateArgument.
authorBenjamin Kramer <benny.kra@googlemail.com>
Thu, 7 Jun 2012 15:09:51 +0000 (15:09 +0000)
committerBenjamin Kramer <benny.kra@googlemail.com>
Thu, 7 Jun 2012 15:09:51 +0000 (15:09 +0000)
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

17 files changed:
include/clang/AST/TemplateBase.h
include/clang/Sema/Template.h
lib/AST/ASTContext.cpp
lib/AST/ASTImporter.cpp
lib/AST/DumpXML.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/StmtPrinter.cpp
lib/AST/StmtProfile.cpp
lib/AST/TemplateBase.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateDeduction.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
tools/libclang/CIndexUSRs.cpp

index 65f54600c00981e84888e6bfbac7c79d789b05e9..9d3f45f4b2cfcecc9ea4c446e32bf4ff1cbdd763 100644 (file)
@@ -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<uintptr_t>(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<unsigned> getNumTemplateExpansions() const;
   
   /// \brief Retrieve the template argument as an integral value.
-  llvm::APSInt *getAsIntegral() {
-    if (Kind != Integral)
-      return 0;
-    return reinterpret_cast<llvm::APSInt*>(&Integer.Value[0]);
-  }
-
-  const llvm::APSInt *getAsIntegral() const {
-    return const_cast<TemplateArgument*>(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.
index c16823a6a14b93256867308fd61d0b3036913fb9..87d71b89952572e056c6070f70a1ea315aea2e26 100644 (file)
@@ -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
index 164813b7c35796044041e767c443c1f75cf27215..bd68d8325009bd6103d8e14fa181dadd956359ff 100644 (file)
@@ -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()));
index 417f3cff58bfb447f229ec821f6959427ffb84be..9f016fe9bbbf0e5b43fe974cde05862a632a3621 100644 (file)
@@ -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:
index 4c7cd8a6793bb7ba5748c4795b07ea5b350d75c9..4df692d9a511f24fe6b13f0de9df66e17e1f9b27 100644 (file)
@@ -326,7 +326,7 @@ struct XMLDumper : public XMLDeclVisitor<XMLDumper>,
     }
     case TemplateArgument::Integral: {
       push("integer");
-      setInteger("value", *A.getAsIntegral());
+      setInteger("value", A.getAsIntegral());
       completeAttrs();
       pop();
       break;
index e974ade6f60d8712d508cb2b257d2ff3721e1423..7c7a5e5de38711910694fdaa6299f8eb6ca51685 100644 (file)
@@ -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");
index 598488170288cd96b3825436dac2e67b0f9243e8..b0d9316fe5cec13e0c7e05455ba73814f90abbcb 100644 (file)
@@ -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.
index 7309e7c889a2d52fc7878a56630024ef4ac75fcd..2bd4d769e795afc2cdf795f6a091c4c37693323c 100644 (file)
@@ -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;
index e6b378e16f0360e1e5a73480ed1d22ba875405dc..72b979d4c2f7952e57f97495b01fe5d1fb7ca2c4 100644 (file)
@@ -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;
 
index 531e03e302b2472dd729b0527b6c3368c5c14bef..284c1b5baae019f161d963d451bfc3f7a9a58c73 100644 (file)
@@ -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<uint64_t *>(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();
index beea777a97833645442fae0a80955e2e597fe531..8d28322c44c5b876b8defc4f5706631199b82a54 100644 (file)
@@ -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);          
     }
   }
index 9f0378ac0ef482c5cf9a46361f676f6e82787860..d2377f59056e73475063ff2fdb1aae92cc99468c 100644 (file)
@@ -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));
       }
index 002b0b174e5ffb63bac7bb6043e2a9c50fe9322f..dccb4b6a61f9a249e64402b13c6f8e7be11edc75 100644 (file)
@@ -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.
index e6f2b2d649a06962a5d184aed1ffa12c0a7277a9..78a860c53da39dfa1fdd88519fc0f57260a83fcd 100644 (file)
@@ -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;
index a9092a43aa164a1f24b3abf6aa90341a7384b56c..3b59163d24f534fbd7ea3a8512508f8b92f45b65 100644 (file)
@@ -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));
index edef1c026f33c278553361e389558ec2bb0d88b9..0eb2f1675beeeb80d25d893d9d6df75f2aac9621 100644 (file)
@@ -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:
index 7c79b69315ddbf15d86bb7e90da94e6705d0b6d8..c885dd546c39f23996fcb3e9820f3467f89e18b1 100644 (file)
@@ -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;
   }
 }