[cfe-commits] r133610 - in /cfe/trunk: include/clang/Basic/DelayedCleanupPool.h include/clang/Parse/Parser.h lib/Parse/ParseDecl.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseExpr.cpp lib/Parse/ParseExprCXX.cpp lib/Parse/ParseTemplate.cpp lib/Parse/ParseTentative.cpp lib/Parse/Parser.cpp lib/Sema/DeclSpec.cpp

Argyrios Kyrtzidis akyrtzi at gmail.com
Tue Jun 21 23:09:49 PDT 2011


Author: akirtzidis
Date: Wed Jun 22 01:09:49 2011
New Revision: 133610

URL: http://llvm.org/viewvc/llvm-project?rev=133610&view=rev
Log:
Introduce DelayedCleanupPool useful for simplifying clean-up of certain resources that, while their
lifetime is well-known and restricted, cleaning them up manually is easy to miss and cause a leak.

Use it to plug the leaking of TemplateIdAnnotation objects. rdar://9634138.

Added:
    cfe/trunk/include/clang/Basic/DelayedCleanupPool.h
Modified:
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/lib/Parse/ParseDecl.cpp
    cfe/trunk/lib/Parse/ParseDeclCXX.cpp
    cfe/trunk/lib/Parse/ParseExpr.cpp
    cfe/trunk/lib/Parse/ParseExprCXX.cpp
    cfe/trunk/lib/Parse/ParseTemplate.cpp
    cfe/trunk/lib/Parse/ParseTentative.cpp
    cfe/trunk/lib/Parse/Parser.cpp
    cfe/trunk/lib/Sema/DeclSpec.cpp

Added: cfe/trunk/include/clang/Basic/DelayedCleanupPool.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DelayedCleanupPool.h?rev=133610&view=auto
==============================================================================
--- cfe/trunk/include/clang/Basic/DelayedCleanupPool.h (added)
+++ cfe/trunk/include/clang/Basic/DelayedCleanupPool.h Wed Jun 22 01:09:49 2011
@@ -0,0 +1,109 @@
+//=== DelayedCleanupPool.h - Delayed Clean-up Pool Implementation *- C++ -*===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a facility to delay calling cleanup methods until specific
+// points.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_DELAYEDCLEANUPPOOL_H
+#define LLVM_CLANG_BASIC_DELAYEDCLEANUPPOOL_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+
+/// \brief Gathers pairs of pointer-to-object/pointer-to-cleanup-function
+/// allowing the cleanup functions to get called (with the pointer as parameter)
+/// at specific points.
+///
+/// The use case is to simplify clean-up of certain resources that, while their
+/// lifetime is well-known and restricted, cleaning them up manually is easy to
+/// miss and cause a leak.
+///
+/// The same pointer can be added multiple times; its clean-up function will
+/// only be called once.
+class DelayedCleanupPool {
+public:
+  typedef void (*CleanupFn)(void *ptr);
+
+  /// \brief Adds a pointer and its associated cleanup function to be called
+  /// at a later point.
+  ///
+  /// \returns false if the pointer is already added, true otherwise.
+  bool delayCleanup(void *ptr, CleanupFn fn) {
+    assert(ptr && "Expected valid pointer to object");
+    assert(fn && "Expected valid pointer to function");
+
+    CleanupFn &mapFn = Ptrs[ptr];
+    assert((!mapFn || mapFn == fn) &&
+           "Adding a pointer with different cleanup function!");
+
+    if (!mapFn) {
+      mapFn = fn;
+      Cleanups.push_back(std::make_pair(ptr, fn));
+      return true;
+    }
+
+    return false;
+  }
+
+  template <typename T>
+  bool delayDelete(T *ptr) {
+    return delayCleanup(ptr, cleanupWithDelete<T>);
+  }
+
+  template <typename T, void (T::*Fn)()>
+  bool delayMemberFunc(T *ptr) {
+    return delayCleanup(ptr, cleanupWithMemberFunc<T, Fn>);
+  }
+
+  void doCleanup() {
+    for (llvm::SmallVector<std::pair<void *, CleanupFn>, 8>::reverse_iterator
+           I = Cleanups.rbegin(), E = Cleanups.rend(); I != E; ++I)
+      I->second(I->first);
+    Cleanups.clear();
+    Ptrs.clear();
+  }
+
+  ~DelayedCleanupPool() {
+    doCleanup();
+  }
+
+private:
+  llvm::DenseMap<void *, CleanupFn> Ptrs;
+  llvm::SmallVector<std::pair<void *, CleanupFn>, 8> Cleanups;
+
+  template <typename T>
+  static void cleanupWithDelete(void *ptr) {
+    delete static_cast<T *>(ptr);
+  }
+
+  template <typename T, void (T::*Fn)()>
+  static void cleanupWithMemberFunc(void *ptr) {
+    (static_cast<T *>(ptr)->*Fn)();
+  }
+};
+
+/// \brief RAII object for triggering a cleanup of a DelayedCleanupPool.
+class DelayedCleanupPoint {
+  DelayedCleanupPool &Pool;
+
+public:
+  DelayedCleanupPoint(DelayedCleanupPool &pool) : Pool(pool) { }
+
+  ~DelayedCleanupPoint() {
+    Pool.doCleanup();
+  }
+};
+
+} // end namespace clang
+
+#endif

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Jun 22 01:09:49 2011
@@ -15,6 +15,7 @@
 #define LLVM_CLANG_PARSE_PARSER_H
 
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/DelayedCleanupPool.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/CodeCompletionHandler.h"
 #include "clang/Sema/Sema.h"
@@ -170,6 +171,10 @@
   /// Factory object for creating AttributeList objects.
   AttributeFactory AttrFactory;
 
+  /// \brief Gathers and cleans up objects when parsing of a top-level
+  /// declaration is finished.
+  DelayedCleanupPool TopLevelDeclCleanupPool;
+
 public:
   Parser(Preprocessor &PP, Sema &Actions);
   ~Parser();
@@ -467,7 +472,12 @@
   bool TryAltiVecTokenOutOfLine(DeclSpec &DS, SourceLocation Loc,
                                 const char *&PrevSpec, unsigned &DiagID,
                                 bool &isInvalid);
-    
+
+  /// \brief Get the TemplateIdAnnotation from the token and put it in the
+  /// cleanup pool so that it gets destroyed when parsing the current top level
+  /// declaration is finished.
+  TemplateIdAnnotation *takeTemplateIdAnnotation(const Token &tok);
+
   /// TentativeParsingAction - An object that is used as a kind of "tentative
   /// parsing transaction". It gets instantiated to mark the token position and
   /// after the token consumption is done, Commit() or Revert() is called to

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Jun 22 01:09:49 2011
@@ -1396,8 +1396,7 @@
         // Thus, if the template-name is actually the constructor
         // name, then the code is ill-formed; this interpretation is
         // reinforced by the NAD status of core issue 635. 
-        TemplateIdAnnotation *TemplateId
-          = static_cast<TemplateIdAnnotation *>(Next.getAnnotationValue());
+        TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Next);
         if ((DSContext == DSC_top_level ||
              (DSContext == DSC_class && DS.isFriendSpecified())) &&
             TemplateId->Name &&
@@ -1599,8 +1598,7 @@
 
       // type-name
     case tok::annot_template_id: {
-      TemplateIdAnnotation *TemplateId
-        = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+      TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
       if (TemplateId->Kind != TNK_Type_template) {
         // This template-id does not refer to a type name, so we're
         // done with the type-specifiers.

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Wed Jun 22 01:09:49 2011
@@ -701,8 +701,7 @@
                                           CXXScopeSpec &SS) {
   // Check whether we have a template-id that names a type.
   if (Tok.is(tok::annot_template_id)) {
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
     if (TemplateId->Kind == TNK_Type_template ||
         TemplateId->Kind == TNK_Dependent_template_name) {
       AnnotateTemplateIdTokenAsType();
@@ -976,7 +975,7 @@
       }
     }
   } else if (Tok.is(tok::annot_template_id)) {
-    TemplateId = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateId = takeTemplateIdAnnotation(Tok);
     NameLoc = ConsumeToken();
 
     if (TemplateId->Kind != TNK_Type_template &&
@@ -993,7 +992,6 @@
 
       DS.SetTypeSpecError();
       SkipUntil(tok::semi, false, true);
-      TemplateId->Destroy();
       if (SuppressingAccessChecks)
         Actions.ActOnStopSuppressingAccessChecks();
 
@@ -1051,9 +1049,6 @@
     }
 
     SkipUntil(tok::comma, true);
-
-    if (TemplateId)
-      TemplateId->Destroy();
     return;
   }
 
@@ -1149,7 +1144,6 @@
                                     TemplateParams? &(*TemplateParams)[0] : 0,
                                  TemplateParams? TemplateParams->size() : 0));
     }
-    TemplateId->Destroy();
   } else if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation &&
              TUK == Sema::TUK_Declaration) {
     // Explicit instantiation of a member of a class template
@@ -2248,8 +2242,7 @@
   ParseOptionalCXXScopeSpecifier(SS, ParsedType(), false);
   ParsedType TemplateTypeTy;
   if (Tok.is(tok::annot_template_id)) {
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
     if (TemplateId->Kind == TNK_Type_template ||
         TemplateId->Kind == TNK_Dependent_template_name) {
       AnnotateTemplateIdTokenAsType();

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Wed Jun 22 01:09:49 2011
@@ -955,8 +955,7 @@
 
     Token Next = NextToken();
     if (Next.is(tok::annot_template_id)) {
-      TemplateIdAnnotation *TemplateId
-        = static_cast<TemplateIdAnnotation *>(Next.getAnnotationValue());
+      TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Next);
       if (TemplateId->Kind == TNK_Type_template) {
         // We have a qualified template-id that we know refers to a
         // type, translate it into a type and continue parsing as a
@@ -975,8 +974,7 @@
   }
 
   case tok::annot_template_id: { // [C++]          template-id
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
     if (TemplateId->Kind == TNK_Type_template) {
       // We have a template-id that we know refers to a type,
       // translate it into a type and continue parsing as a cast

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Wed Jun 22 01:09:49 2011
@@ -245,8 +245,7 @@
       // So we need to check whether the simple-template-id is of the
       // right kind (it should name a type or be dependent), and then
       // convert it into a type within the nested-name-specifier.
-      TemplateIdAnnotation *TemplateId
-        = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+      TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
       if (CheckForDestructor && GetLookAheadToken(2).is(tok::tilde)) {
         *MayBePseudoDestructor = true;
         return false;
@@ -281,10 +280,6 @@
         SS.SetInvalid(SourceRange(StartLoc, CCLoc));
       }
 
-      // If we are caching tokens we will process the TemplateId again,
-      // otherwise destroy it.
-      if (!PP.isBacktrackEnabled())
-        TemplateId->Destroy();
       continue;
     }
 
@@ -1606,8 +1601,7 @@
   // unqualified-id:
   //   template-id (already parsed and annotated)
   if (Tok.is(tok::annot_template_id)) {
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation*>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
 
     // If the template-name names the current class, then this is a constructor 
     if (AllowConstructorName && TemplateId->Name &&
@@ -1630,7 +1624,6 @@
                                             /*NontrivialTypeSourceInfo=*/true),
                                   TemplateId->TemplateNameLoc, 
                                   TemplateId->RAngleLoc);
-        TemplateId->Destroy();
         ConsumeToken();
         return false;
       }

Modified: cfe/trunk/lib/Parse/ParseTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTemplate.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseTemplate.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTemplate.cpp Wed Jun 22 01:09:49 2011
@@ -861,8 +861,7 @@
 void Parser::AnnotateTemplateIdTokenAsType() {
   assert(Tok.is(tok::annot_template_id) && "Requires template-id tokens");
 
-  TemplateIdAnnotation *TemplateId
-    = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+  TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
   assert((TemplateId->Kind == TNK_Type_template ||
           TemplateId->Kind == TNK_Dependent_template_name) &&
          "Only works for type and dependent templates");
@@ -888,7 +887,6 @@
   // Replace the template-id annotation token, and possible the scope-specifier
   // that precedes it, with the typename annotation token.
   PP.AnnotateCachedTokens(Tok);
-  TemplateId->Destroy();
 }
 
 /// \brief Determine whether the given token can end a template argument.

Modified: cfe/trunk/lib/Parse/ParseTentative.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseTentative.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseTentative.cpp (original)
+++ cfe/trunk/lib/Parse/ParseTentative.cpp Wed Jun 22 01:09:49 2011
@@ -908,8 +908,7 @@
     return TPResult::True();
 
   case tok::annot_template_id: {
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
     if (TemplateId->Kind != TNK_Type_template)
       return TPResult::False();
     CXXScopeSpec SS;

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Wed Jun 22 01:09:49 2011
@@ -486,6 +486,7 @@
 /// ParseTopLevelDecl - Parse one top-level declaration, return whatever the
 /// action tells us to.  This returns true if the EOF was encountered.
 bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result) {
+  DelayedCleanupPoint CleanupRAII(TopLevelDeclCleanupPool);
 
   while (Tok.is(tok::annot_pragma_unused))
     HandlePragmaUnused();
@@ -548,6 +549,7 @@
 Parser::DeclGroupPtrTy
 Parser::ParseExternalDeclaration(ParsedAttributesWithRange &attrs,
                                  ParsingDeclSpec *DS) {
+  DelayedCleanupPoint CleanupRAII(TopLevelDeclCleanupPool);
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
   
   Decl *SingleDecl = 0;
@@ -1155,6 +1157,18 @@
   return move(Result);
 }
 
+/// \brief Get the TemplateIdAnnotation from the token and put it in the
+/// cleanup pool so that it gets destroyed when parsing the current top level
+/// declaration is finished.
+TemplateIdAnnotation *Parser::takeTemplateIdAnnotation(const Token &tok) {
+  assert(tok.is(tok::annot_template_id) && "Expected template-id token");
+  TemplateIdAnnotation *
+      Id = static_cast<TemplateIdAnnotation *>(tok.getAnnotationValue());
+  TopLevelDeclCleanupPool.delayMemberFunc< TemplateIdAnnotation,
+                                          &TemplateIdAnnotation::Destroy>(Id);
+  return Id;
+}
+
 /// TryAnnotateTypeOrScopeToken - If the current token position is on a
 /// typename (possibly qualified in C++) or a C++ scope specifier not followed
 /// by a typename, TryAnnotateTypeOrScopeToken will replace one or more tokens
@@ -1209,8 +1223,7 @@
                                      *Tok.getIdentifierInfo(),
                                      Tok.getLocation());
     } else if (Tok.is(tok::annot_template_id)) {
-      TemplateIdAnnotation *TemplateId
-        = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+      TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
       if (TemplateId->Kind == TNK_Function_template) {
         Diag(Tok, diag::err_typename_refers_to_non_type_template)
           << Tok.getAnnotationRange();
@@ -1228,7 +1241,6 @@
                                      TemplateId->LAngleLoc,
                                      TemplateArgsPtr, 
                                      TemplateId->RAngleLoc);
-      TemplateId->Destroy();
     } else {
       Diag(Tok, diag::err_expected_type_name_after_typename)
         << SS.getRange();
@@ -1311,8 +1323,7 @@
   }
 
   if (Tok.is(tok::annot_template_id)) {
-    TemplateIdAnnotation *TemplateId
-      = static_cast<TemplateIdAnnotation *>(Tok.getAnnotationValue());
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
     if (TemplateId->Kind == TNK_Type_template) {
       // A template-id that refers to a type was parsed into a
       // template-id annotation in a context where we weren't allowed

Modified: cfe/trunk/lib/Sema/DeclSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/DeclSpec.cpp?rev=133610&r1=133609&r2=133610&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/DeclSpec.cpp (original)
+++ cfe/trunk/lib/Sema/DeclSpec.cpp Wed Jun 22 01:09:49 2011
@@ -792,9 +792,6 @@
 }
 
 void UnqualifiedId::clear() {
-  if (Kind == IK_TemplateId)
-    TemplateId->Destroy();
-  
   Kind = IK_Identifier;
   Identifier = 0;
   StartLocation = SourceLocation();





More information about the cfe-commits mailing list