From: Jeffrey Yasskin Date: Fri, 7 Aug 2009 19:54:29 +0000 (+0000) Subject: To catch bugs like the one fixed in X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=0d5bd59553375dc85ac04c81ef48ef74c9e7193e;p=oota-llvm.git 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. 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 didn't work, so I fixed that too. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@78400 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h b/include/llvm/ExecutionEngine/ExecutionEngine.h index 1dc9d1dee5e..6a3a9144d57 100644 --- a/include/llvm/ExecutionEngine/ExecutionEngine.h +++ b/include/llvm/ExecutionEngine/ExecutionEngine.h @@ -37,26 +37,27 @@ class ModuleProvider; class MutexGuard; class TargetData; class Type; +template class AssertingVH; class ExecutionEngineState { private: /// GlobalAddressMap - A mapping between LLVM global values and their /// actualized version... - std::map GlobalAddressMap; + std::map, 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 GlobalAddressReverseMap; + std::map > GlobalAddressReverseMap; public: - std::map & + std::map, void *> & getGlobalAddressMap(const MutexGuard &) { return GlobalAddressMap; } - std::map & + std::map > & getGlobalAddressReverseMap(const MutexGuard &) { return GlobalAddressReverseMap; } diff --git a/include/llvm/Support/ValueHandle.h b/include/llvm/Support/ValueHandle.h index 84745ff2c34..e0e06e11863 100644 --- a/include/llvm/Support/ValueHandle.h +++ b/include/llvm/Support/ValueHandle.h @@ -171,7 +171,7 @@ class AssertingVH return static_cast(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(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) {} diff --git a/lib/ExecutionEngine/ExecutionEngine.cpp b/lib/ExecutionEngine/ExecutionEngine.cpp index e827d36f861..348190a75ba 100644 --- a/lib/ExecutionEngine/ExecutionEngine.cpp +++ b/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 &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 &Map = state.getGlobalAddressMap(locked); + std::map, void *> &Map = + state.getGlobalAddressMap(locked); // Deleting from the mapping? if (Addr == 0) { - std::map::iterator I = Map.find(GV); + std::map, 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 &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::iterator I = - state.getGlobalAddressMap(locked).find(GV); + std::map, 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::iterator + for (std::map, 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::iterator I = + std::map >::iterator I = state.getGlobalAddressReverseMap(locked).find(Addr); return I != state.getGlobalAddressReverseMap(locked).end() ? I->second : 0; } diff --git a/unittests/Support/ValueHandleTest.cpp b/unittests/Support/ValueHandleTest.cpp index d9a5515f78e..5e6cd61035e 100644 --- a/unittests/Support/ValueHandleTest.cpp +++ b/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 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 BitcastAVH(BitcastV.get()); AssertingVH ConstantAVH(ConstantV);