[llvm-commits] [llvm] r91611 - in /llvm/trunk: include/llvm/DerivedTypes.h lib/VMCore/LLVMContextImpl.h lib/VMCore/Type.cpp unittests/VMCore/DerivedTypesTest.cpp

[Jeffrey Yasskin]

Author: jyasskin
Date: Thu Dec 17 13:55:06 2009
New Revision: 91611

URL: http://llvm.org/viewvc/llvm-project?rev=91611&view=rev
Log:
This fixes a memory leak in OpaqueType found by Google's internal heapchecker.

Added:
    llvm/trunk/unittests/VMCore/DerivedTypesTest.cpp
Modified:
    llvm/trunk/include/llvm/DerivedTypes.h
    llvm/trunk/lib/VMCore/LLVMContextImpl.h
    llvm/trunk/lib/VMCore/Type.cpp

Modified: llvm/trunk/include/llvm/DerivedTypes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DerivedTypes.h?rev=91611&r1=91610&r2=91611&view=diff

==============================================================================
--- llvm/trunk/include/llvm/DerivedTypes.h (original)
+++ llvm/trunk/include/llvm/DerivedTypes.h Thu Dec 17 13:55:06 2009
@@ -502,9 +502,7 @@
 public:
   /// OpaqueType::get - Static factory method for the OpaqueType class...
   ///
-  static OpaqueType *get(LLVMContext &C) {
-    return new OpaqueType(C);           // All opaque types are distinct
-  }
+  static OpaqueType *get(LLVMContext &C);
 
   // Implement support for type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const OpaqueType *) { return true; }

Modified: llvm/trunk/lib/VMCore/LLVMContextImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/LLVMContextImpl.h?rev=91611&r1=91610&r2=91611&view=diff

==============================================================================
--- llvm/trunk/lib/VMCore/LLVMContextImpl.h (original)
+++ llvm/trunk/lib/VMCore/LLVMContextImpl.h Thu Dec 17 13:55:06 2009
@@ -27,6 +27,7 @@
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringMap.h"
 #include <vector>
 
@@ -159,6 +160,11 @@
   TypeMap<StructValType, StructType> StructTypes;
   TypeMap<IntegerValType, IntegerType> IntegerTypes;
 
+  // Opaque types are not structurally uniqued, so don't use TypeMap.
+  typedef SmallPtrSet<const OpaqueType*, 8> OpaqueTypesTy;
+  OpaqueTypesTy OpaqueTypes;
+  
+
   /// ValueHandles - This map keeps track of all of the value handles that are
   /// watching a Value*.  The Value::HasValueHandle bit is used to know
   // whether or not a value has an entry in this map.
@@ -201,6 +207,11 @@
         delete I->second;
     }
     MDNodeSet.clear();
+    for (OpaqueTypesTy::iterator I = OpaqueTypes.begin(), E = OpaqueTypes.end();
+        I != E; ++I) {
+      (*I)->AbstractTypeUsers.clear();
+      delete *I;
+    }
   }
 };
 

Modified: llvm/trunk/lib/VMCore/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/VMCore/Type.cpp?rev=91611&r1=91610&r2=91611&view=diff

==============================================================================
--- llvm/trunk/lib/VMCore/Type.cpp (original)
+++ llvm/trunk/lib/VMCore/Type.cpp Thu Dec 17 13:55:06 2009
@@ -79,6 +79,9 @@
     operator delete(const_cast<Type *>(this));
 
     return;
+  } else if (const OpaqueType *opaque_this = dyn_cast<OpaqueType>(this)) {
+    LLVMContextImpl *pImpl = this->getContext().pImpl;
+    pImpl->OpaqueTypes.erase(opaque_this);
   }
 
   // For all the other type subclasses, there is either no contained types or 
@@ -955,6 +958,20 @@
 
 
 //===----------------------------------------------------------------------===//
+// Opaque Type Factory...
+//
+
+OpaqueType *OpaqueType::get(LLVMContext &C) {
+  OpaqueType *OT = new OpaqueType(C);           // All opaque types are distinct
+  
+  LLVMContextImpl *pImpl = C.pImpl;
+  pImpl->OpaqueTypes.insert(OT);
+  return OT;
+}
+
+
+
+//===----------------------------------------------------------------------===//
 //                     Derived Type Refinement Functions
 //===----------------------------------------------------------------------===//
 

Added: llvm/trunk/unittests/VMCore/DerivedTypesTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/VMCore/DerivedTypesTest.cpp?rev=91611&view=auto

==============================================================================
--- llvm/trunk/unittests/VMCore/DerivedTypesTest.cpp (added)
+++ llvm/trunk/unittests/VMCore/DerivedTypesTest.cpp Thu Dec 17 13:55:06 2009
@@ -0,0 +1,31 @@
+//===- llvm/unittest/VMCore/DerivedTypesTest.cpp - Types unit tests -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+#include "../lib/VMCore/LLVMContextImpl.h"
+#include "llvm/Type.h"
+#include "llvm/DerivedTypes.h"
+#include "llvm/LLVMContext.h"
+using namespace llvm;
+
+namespace {
+
+TEST(OpaqueTypeTest, RegisterWithContext) {
+  LLVMContext C;
+  LLVMContextImpl *pImpl = C.pImpl;  
+
+  EXPECT_EQ(0u, pImpl->OpaqueTypes.size());
+  {
+    PATypeHolder Type = OpaqueType::get(C);
+    EXPECT_EQ(1u, pImpl->OpaqueTypes.size());
+  }
+  EXPECT_EQ(0u, pImpl->OpaqueTypes.size());
+}
+
+}  // namespace