[cfe-commits] r95861 - in /cfe/trunk: include/clang/AST/ASTContext.h lib/AST/ASTContext.cpp lib/AST/DeclBase.cpp

[Ted Kremenek]

Author: kremenek
Date: Thu Feb 11 01:12:28 2010
New Revision: 95861

URL: http://llvm.org/viewvc/llvm-project?rev=95861&view=rev
Log:
Have ~ASTContext() delete StoredDeclsMap (internal to DeclContext) by
storing the set of StoredDeclsMaps in an internal vector of void*.
This isn't an ideal solution, but for the time being this fixes a
major memory leak with these DenseMaps not being freed.

Fixes: <rdar://problem/7634755>


Modified:
    cfe/trunk/include/clang/AST/ASTContext.h
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/DeclBase.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=95861&r1=95860&r2=95861&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Thu Feb 11 01:12:28 2010
@@ -46,6 +46,7 @@
   class SourceManager;
   class TargetInfo;
   // Decls
+  class DeclContext;
   class CXXMethodDecl;
   class CXXRecordDecl;
   class Decl;
@@ -1212,6 +1213,15 @@
 
   const ASTRecordLayout &getObjCLayout(const ObjCInterfaceDecl *D,
                                        const ObjCImplementationDecl *Impl);
+  
+private:
+  // FIXME: This currently contains the set of StoredDeclMaps used
+  // by DeclContext objects.  This probably should not be in ASTContext,
+  // but we include it here so that ASTContext can quickly deallocate them.
+  std::vector<void*> SDMs; 
+  friend class DeclContext;
+  void *CreateStoredDeclsMap();
+  void ReleaseDeclContextMaps();
 };
   
 /// @brief Utility function for constructing a nullary selector.

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=95861&r1=95860&r2=95861&view=diff

==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Feb 11 01:12:28 2010
@@ -56,6 +56,10 @@
 }
 
 ASTContext::~ASTContext() {
+  // Release the DenseMaps associated with DeclContext objects.
+  // FIXME: Is this the ideal solution?
+  ReleaseDeclContextMaps();
+  
   if (FreeMemory) {
     // Deallocate all the types.
     while (!Types.empty()) {

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=95861&r1=95860&r2=95861&view=diff

==============================================================================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Thu Feb 11 01:12:28 2010
@@ -448,7 +448,10 @@
 }
 
 DeclContext::~DeclContext() {
-  delete static_cast<StoredDeclsMap*>(LookupPtr);
+  // FIXME: Currently ~ASTContext will delete the StoredDeclsMaps because
+  // ~DeclContext() is not guaranteed to be called when ASTContext uses
+  // a BumpPtrAllocator.
+  // delete static_cast<StoredDeclsMap*>(LookupPtr);
 }
 
 void DeclContext::DestroyDecls(ASTContext &C) {
@@ -622,7 +625,8 @@
   // Load the declaration IDs for all of the names visible in this
   // context.
   assert(!LookupPtr && "Have a lookup map before de-serialization?");
-  StoredDeclsMap *Map = new StoredDeclsMap;
+  StoredDeclsMap *Map =
+    (StoredDeclsMap*) getParentASTContext().CreateStoredDeclsMap();
   LookupPtr = Map;
   for (unsigned I = 0, N = Decls.size(); I != N; ++I) {
     (*Map)[Decls[I].Name].setFromDeclIDs(Decls[I].Declarations);
@@ -830,8 +834,11 @@
   if (isa<ClassTemplateSpecializationDecl>(D))
     return;
 
-  if (!LookupPtr)
-    LookupPtr = new StoredDeclsMap;
+  ASTContext *C = 0;
+  if (!LookupPtr) {
+    C = &getParentASTContext();
+    LookupPtr = (StoredDeclsMap*) C->CreateStoredDeclsMap();
+  }
 
   // Insert this declaration into the map.
   StoredDeclsMap &Map = *static_cast<StoredDeclsMap*>(LookupPtr);
@@ -844,7 +851,10 @@
   // If it is possible that this is a redeclaration, check to see if there is
   // already a decl for which declarationReplaces returns true.  If there is
   // one, just replace it and return.
-  if (DeclNameEntries.HandleRedeclaration(getParentASTContext(), D))
+  if (!C)
+    C = &getParentASTContext();
+  
+  if (DeclNameEntries.HandleRedeclaration(*C, D))
     return;
 
   // Put this declaration into the appropriate slot.
@@ -896,3 +906,18 @@
   }
   }
 }
+
+//===----------------------------------------------------------------------===//
+// Creation and Destruction of StoredDeclsMaps.                               //
+//===----------------------------------------------------------------------===//
+
+void *ASTContext::CreateStoredDeclsMap() {
+  StoredDeclsMap *M = new StoredDeclsMap();
+  SDMs.push_back(M);
+  return M;
+}
+
+void ASTContext::ReleaseDeclContextMaps() {
+  for (std::vector<void*>::iterator I = SDMs.begin(), E = SDMs.end(); I!=E; ++I)
+    delete (StoredDeclsMap*) *I;
+}