To catch bugs like the one fixed in

http://llvm.org/viewvc/llvm-project?view=rev&revision=78127, I'm changing the
ExecutionEngine's global mappings to hold AssertingVH<const GlobalValue>. That
way, if unregistering a mapping fails to actually unregister it, we'll get an
assert. Running the jit nightly tests didn't uncover any actual instances of
the problem.

This also uncovered the fact that AssertingVH<const X> didn't work, so I fixed
that too.

llvm-svn: 78400

include/llvm/ExecutionEngine/ExecutionEngine.h

@@ -37,26 +37,27 @@ class ModuleProvider;
 class MutexGuard;
 class TargetData;
 class Type;
+template<typename> class AssertingVH;
 
 class ExecutionEngineState {
 private:
   /// GlobalAddressMap - A mapping between LLVM global values and their
   /// actualized version...
-  std::map<const GlobalValue*, void *> GlobalAddressMap;
+  std::map<AssertingVH<const GlobalValue>, void *> GlobalAddressMap;
 
   /// GlobalAddressReverseMap - This is the reverse mapping of GlobalAddressMap,
   /// used to convert raw addresses into the LLVM global value that is emitted
   /// at the address.  This map is not computed unless getGlobalValueAtAddress
   /// is called at some point.
-  std::map<void *, const GlobalValue*> GlobalAddressReverseMap;
+  std::map<void *, AssertingVH<const GlobalValue> > GlobalAddressReverseMap;
 
 public:
-  std::map<const GlobalValue*, void *> &
+  std::map<AssertingVH<const GlobalValue>, void *> &
   getGlobalAddressMap(const MutexGuard &) {
     return GlobalAddressMap;
   }
 
-  std::map<void*, const GlobalValue*> & 
+  std::map<void*, AssertingVH<const GlobalValue> > &
   getGlobalAddressReverseMap(const MutexGuard &) {
     return GlobalAddressReverseMap;
   }

include/llvm/Support/ValueHandle.h

@@ -171,7 +171,7 @@ class AssertingVH
     return static_cast<ValueTy*>(ValueHandleBase::getValPtr());
   }
   void setValPtr(ValueTy *P) {
-    ValueHandleBase::operator=(P);
+    ValueHandleBase::operator=(GetAsValue(P));
   }
 #else
   ValueTy *ThePtr;
@@ -179,10 +179,15 @@ class AssertingVH
   void setValPtr(ValueTy *P) { ThePtr = P; }
 #endif
 
+  // Convert a ValueTy*, which may be const, to the type the base
+  // class expects.
+  static Value *GetAsValue(Value *V) { return V; }
+  static Value *GetAsValue(const Value *V) { return const_cast<Value*>(V); }
+
 public:
 #ifndef NDEBUG
   AssertingVH() : ValueHandleBase(Assert) {}
-  AssertingVH(ValueTy *P) : ValueHandleBase(Assert, P) {}
+  AssertingVH(ValueTy *P) : ValueHandleBase(Assert, GetAsValue(P)) {}
   AssertingVH(const AssertingVH &RHS) : ValueHandleBase(Assert, RHS) {}
 #else
   AssertingVH() : ThePtr(0) {}

lib/ExecutionEngine/ExecutionEngine.cpp

@@ -13,17 +13,19 @@
 //===----------------------------------------------------------------------===//
 
 #define DEBUG_TYPE "jit"
+#include "llvm/ExecutionEngine/ExecutionEngine.h"
+
 #include "llvm/Constants.h"
 #include "llvm/DerivedTypes.h"
 #include "llvm/Module.h"
 #include "llvm/ModuleProvider.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Config/alloca.h"
-#include "llvm/ExecutionEngine/ExecutionEngine.h"
 #include "llvm/ExecutionEngine/GenericValue.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MutexGuard.h"
+#include "llvm/Support/ValueHandle.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/System/DynamicLibrary.h"
 #include "llvm/System/Host.h"
@@ -128,7 +130,8 @@ void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) {
   
   // If we are using the reverse mapping, add it too
   if (!state.getGlobalAddressReverseMap(locked).empty()) {
-    const GlobalValue *&V = state.getGlobalAddressReverseMap(locked)[Addr];
+    AssertingVH<const GlobalValue> &V =
+      state.getGlobalAddressReverseMap(locked)[Addr];
     assert((V == 0 || GV == 0) && "GlobalMapping already established!");
     V = GV;
   }
@@ -149,13 +152,13 @@ void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) {
   MutexGuard locked(lock);
   
   for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ++FI) {
-    state.getGlobalAddressMap(locked).erase(FI);
-    state.getGlobalAddressReverseMap(locked).erase(FI);
+    state.getGlobalAddressMap(locked).erase(&*FI);
+    state.getGlobalAddressReverseMap(locked).erase(&*FI);
   }
   for (Module::global_iterator GI = M->global_begin(), GE = M->global_end(); 
        GI != GE; ++GI) {
-    state.getGlobalAddressMap(locked).erase(GI);
-    state.getGlobalAddressReverseMap(locked).erase(GI);
+    state.getGlobalAddressMap(locked).erase(&*GI);
+    state.getGlobalAddressReverseMap(locked).erase(&*GI);
   }
 }
 
@@ -165,11 +168,12 @@ void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) {
 void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
   MutexGuard locked(lock);
 
-  std::map<const GlobalValue*, void *> &Map = state.getGlobalAddressMap(locked);
+  std::map<AssertingVH<const GlobalValue>, void *> &Map =
+    state.getGlobalAddressMap(locked);
 
   // Deleting from the mapping?
   if (Addr == 0) {
-    std::map<const GlobalValue*, void *>::iterator I = Map.find(GV);
+    std::map<AssertingVH<const GlobalValue>, void *>::iterator I = Map.find(GV);
     void *OldVal;
     if (I == Map.end())
       OldVal = 0;
@@ -192,7 +196,8 @@ void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
   
   // If we are using the reverse mapping, add it too
   if (!state.getGlobalAddressReverseMap(locked).empty()) {
-    const GlobalValue *&V = state.getGlobalAddressReverseMap(locked)[Addr];
+    AssertingVH<const GlobalValue> &V =
+      state.getGlobalAddressReverseMap(locked)[Addr];
     assert((V == 0 || GV == 0) && "GlobalMapping already established!");
     V = GV;
   }
@@ -205,8 +210,8 @@ void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
 void *ExecutionEngine::getPointerToGlobalIfAvailable(const GlobalValue *GV) {
   MutexGuard locked(lock);
   
-  std::map<const GlobalValue*, void*>::iterator I =
-  state.getGlobalAddressMap(locked).find(GV);
+  std::map<AssertingVH<const GlobalValue>, void*>::iterator I =
+    state.getGlobalAddressMap(locked).find(GV);
   return I != state.getGlobalAddressMap(locked).end() ? I->second : 0;
 }
 
@@ -218,14 +223,14 @@ const GlobalValue *ExecutionEngine::getGlobalValueAtAddress(void *Addr) {
 
   // If we haven't computed the reverse mapping yet, do so first.
   if (state.getGlobalAddressReverseMap(locked).empty()) {
-    for (std::map<const GlobalValue*, void *>::iterator
+    for (std::map<AssertingVH<const GlobalValue>, void *>::iterator
          I = state.getGlobalAddressMap(locked).begin(),
          E = state.getGlobalAddressMap(locked).end(); I != E; ++I)
       state.getGlobalAddressReverseMap(locked).insert(std::make_pair(I->second,
                                                                      I->first));
   }
 
-  std::map<void *, const GlobalValue*>::iterator I =
+  std::map<void *, AssertingVH<const GlobalValue> >::iterator I =
     state.getGlobalAddressReverseMap(locked).find(Addr);
   return I != state.getGlobalAddressReverseMap(locked).end() ? I->second : 0;
 }

unittests/Support/ValueHandleTest.cpp

@@ -120,6 +120,13 @@ TEST_F(ValueHandle, AssertingVH_BasicOperation) {
   EXPECT_FALSE((*AVH).mayWriteToMemory());
 }
 
+TEST_F(ValueHandle, AssertingVH_Const) {
+  const CastInst *ConstBitcast = BitcastV.get();
+  AssertingVH<const CastInst> AVH(ConstBitcast);
+  const CastInst *implicit_to_exact_type = AVH;
+  implicit_to_exact_type = implicit_to_exact_type;  // Avoid warning.
+}
+
 TEST_F(ValueHandle, AssertingVH_Comparisons) {
   AssertingVH<Value> BitcastAVH(BitcastV.get());
   AssertingVH<Value> ConstantAVH(ConstantV);