From: Lang Hames Date: Tue, 31 Mar 2015 20:31:14 +0000 (+0000) Subject: [ExecutionEngine] Fix MCJIT::addGlobalMapping. X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=83b5f345b27253598ecef7c10a6f4cae825d6b42 [ExecutionEngine] Fix MCJIT::addGlobalMapping. This patch fixes MCJIT::addGlobalMapping by changing the implementation of the ExecutionEngineState class. The new implementation maintains a bidirectional mapping between symbol names (std::strings) and addresses (uint64_ts), rather than a mapping between Value*s and void*s. This has fix has been made for backwards compatibility, however the strongly preferred way to resolve unknown symbols is by writing a custom RuntimeDyld::SymbolResolver (formerly RTDyldMemoryManager) and overriding the findSymbol method. The addGlobalMapping method is a hangover from the legacy JIT (which has was removed in 3.6), and may be deprecated in a future release as part of a clean-up of the ExecutionEngine interface. Patch by Murat Bolat. Thanks Murat! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233747 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h b/include/llvm/ExecutionEngine/ExecutionEngine.h index 6e022afae5f..4b2add8bf5d 100644 --- a/include/llvm/ExecutionEngine/ExecutionEngine.h +++ b/include/llvm/ExecutionEngine/ExecutionEngine.h @@ -59,46 +59,34 @@ namespace object { /// table. Access to this class should be serialized under a mutex. class ExecutionEngineState { public: - struct AddressMapConfig : public ValueMapConfig { - typedef ExecutionEngineState *ExtraData; - static sys::Mutex *getMutex(ExecutionEngineState *EES); - static void onDelete(ExecutionEngineState *EES, const GlobalValue *Old); - static void onRAUW(ExecutionEngineState *, const GlobalValue *, - const GlobalValue *); - }; - - typedef ValueMap - GlobalAddressMapTy; + typedef StringMap GlobalAddressMapTy; private: - ExecutionEngine &EE; - /// GlobalAddressMap - A mapping between LLVM global values and their - /// actualized version... + /// GlobalAddressMap - A mapping between LLVM global symbol names values and + /// their actualized version... GlobalAddressMapTy 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: - ExecutionEngineState(ExecutionEngine &EE); GlobalAddressMapTy &getGlobalAddressMap() { return GlobalAddressMap; } - std::map > & - getGlobalAddressReverseMap() { + std::map &getGlobalAddressReverseMap() { return GlobalAddressReverseMap; } /// \brief Erase an entry from the mapping table. /// /// \returns The address that \p ToUnmap was happed to. - void *RemoveMapping(const GlobalValue *ToUnmap); + uint64_t RemoveMapping(StringRef Name); }; /// \brief Abstract interface for implementation execution of LLVM modules, @@ -161,6 +149,9 @@ protected: /// abort. void *(*LazyFunctionCreator)(const std::string &); + /// getMangledName - Get mangled name. + std::string getMangledName(const GlobalValue *GV); + public: /// lock - This lock protects the ExecutionEngine and MCJIT classes. It must /// be held while changing the internal state of any of those classes. @@ -232,7 +223,8 @@ public: /// Map the address of a JIT section as returned from the memory manager /// to the address in the target process as the running code will see it. /// This is the address which will be used for relocation resolution. - virtual void mapSectionAddress(const void *LocalAddress, uint64_t TargetAddress) { + virtual void mapSectionAddress(const void *LocalAddress, + uint64_t TargetAddress) { llvm_unreachable("Re-mapping of section addresses not supported with this " "EE!"); } @@ -290,6 +282,7 @@ public: /// existing data in memory. Mappings are automatically removed when their /// GlobalValue is destroyed. void addGlobalMapping(const GlobalValue *GV, void *Addr); + void addGlobalMapping(StringRef Name, uint64_t Addr); /// clearAllGlobalMappings - Clear all global mappings and start over again, /// for use in dynamic compilation scenarios to move globals. @@ -303,14 +296,17 @@ public: /// address. This updates both maps as required. If "Addr" is null, the /// entry for the global is removed from the mappings. This returns the old /// value of the pointer, or null if it was not in the map. - void *updateGlobalMapping(const GlobalValue *GV, void *Addr); + uint64_t updateGlobalMapping(const GlobalValue *GV, void *Addr); + uint64_t updateGlobalMapping(StringRef Name, uint64_t Addr); + + /// getAddressToGlobalIfAvailable - This returns the address of the specified + /// global symbol. + uint64_t getAddressToGlobalIfAvailable(StringRef S); /// getPointerToGlobalIfAvailable - This returns the address of the specified /// global value if it is has already been codegen'd, otherwise it returns /// null. - /// - /// This function is deprecated for the MCJIT execution engine. It doesn't - /// seem to be needed in that case, but an equivalent can be added if it is. + void *getPointerToGlobalIfAvailable(StringRef S); void *getPointerToGlobalIfAvailable(const GlobalValue *GV); /// getPointerToGlobal - This returns the address of the specified global @@ -474,7 +470,7 @@ public: } protected: - ExecutionEngine() : EEState(*this) {} + ExecutionEngine() {} explicit ExecutionEngine(std::unique_ptr M); void emitGlobals(); diff --git a/lib/ExecutionEngine/ExecutionEngine.cpp b/lib/ExecutionEngine/ExecutionEngine.cpp index 238663a16be..d7038fd2c9a 100644 --- a/lib/ExecutionEngine/ExecutionEngine.cpp +++ b/lib/ExecutionEngine/ExecutionEngine.cpp @@ -22,6 +22,7 @@ #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Mangler.h" #include "llvm/IR/Module.h" #include "llvm/IR/Operator.h" #include "llvm/IR/ValueHandle.h" @@ -61,8 +62,7 @@ ExecutionEngine *(*ExecutionEngine::InterpCtor)(std::unique_ptr M, void JITEventListener::anchor() {} ExecutionEngine::ExecutionEngine(std::unique_ptr M) - : EEState(*this), - LazyFunctionCreator(nullptr) { + : LazyFunctionCreator(nullptr) { CompilingLazily = false; GVCompilationDisabled = false; SymbolSearchingDisabled = false; @@ -154,38 +154,52 @@ Function *ExecutionEngine::FindFunctionNamed(const char *FnName) { } -void *ExecutionEngineState::RemoveMapping(const GlobalValue *ToUnmap) { - GlobalAddressMapTy::iterator I = GlobalAddressMap.find(ToUnmap); - void *OldVal; +uint64_t ExecutionEngineState::RemoveMapping(StringRef Name) { + GlobalAddressMapTy::iterator I = GlobalAddressMap.find(Name); + uint64_t OldVal; // FIXME: This is silly, we shouldn't end up with a mapping -> 0 in the // GlobalAddressMap. if (I == GlobalAddressMap.end()) - OldVal = nullptr; + OldVal = 0; else { + GlobalAddressReverseMap.erase(I->second); OldVal = I->second; GlobalAddressMap.erase(I); } - GlobalAddressReverseMap.erase(OldVal); return OldVal; } +std::string ExecutionEngine::getMangledName(const GlobalValue *GV) { + MutexGuard locked(lock); + Mangler Mang(DL); + SmallString<128> FullName; + Mang.getNameWithPrefix(FullName, GV->getName()); + return FullName.str(); +} + void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) { MutexGuard locked(lock); + addGlobalMapping(getMangledName(GV), (uint64_t) Addr); +} - DEBUG(dbgs() << "JIT: Map \'" << GV->getName() - << "\' to [" << Addr << "]\n";); - void *&CurVal = EEState.getGlobalAddressMap()[GV]; +void ExecutionEngine::addGlobalMapping(StringRef Name, uint64_t Addr) { + MutexGuard locked(lock); + + assert(!Name.empty() && "Empty GlobalMapping symbol name!"); + + DEBUG(dbgs() << "JIT: Map \'" << Name << "\' to [" << Addr << "]\n";); + uint64_t &CurVal = EEState.getGlobalAddressMap()[Name]; assert((!CurVal || !Addr) && "GlobalMapping already established!"); CurVal = Addr; // If we are using the reverse mapping, add it too. if (!EEState.getGlobalAddressReverseMap().empty()) { - AssertingVH &V = - EEState.getGlobalAddressReverseMap()[Addr]; - assert((!V || !GV) && "GlobalMapping already established!"); - V = GV; + std::string &V = EEState.getGlobalAddressReverseMap()[CurVal]; + assert((!V.empty() || !Name.empty()) && + "GlobalMapping already established!"); + V = Name; } } @@ -200,13 +214,19 @@ void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) { MutexGuard locked(lock); for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ++FI) - EEState.RemoveMapping(FI); + EEState.RemoveMapping(getMangledName(FI)); for (Module::global_iterator GI = M->global_begin(), GE = M->global_end(); GI != GE; ++GI) - EEState.RemoveMapping(GI); + EEState.RemoveMapping(getMangledName(GI)); } -void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) { +uint64_t ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, + void *Addr) { + MutexGuard locked(lock); + return updateGlobalMapping(getMangledName(GV), (uint64_t) Addr); +} + +uint64_t ExecutionEngine::updateGlobalMapping(StringRef Name, uint64_t Addr) { MutexGuard locked(lock); ExecutionEngineState::GlobalAddressMapTy &Map = @@ -214,10 +234,10 @@ void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) { // Deleting from the mapping? if (!Addr) - return EEState.RemoveMapping(GV); + return EEState.RemoveMapping(Name); - void *&CurVal = Map[GV]; - void *OldVal = CurVal; + uint64_t &CurVal = Map[Name]; + uint64_t OldVal = CurVal; if (CurVal && !EEState.getGlobalAddressReverseMap().empty()) EEState.getGlobalAddressReverseMap().erase(CurVal); @@ -225,20 +245,35 @@ void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) { // If we are using the reverse mapping, add it too. if (!EEState.getGlobalAddressReverseMap().empty()) { - AssertingVH &V = - EEState.getGlobalAddressReverseMap()[Addr]; - assert((!V || !GV) && "GlobalMapping already established!"); - V = GV; + std::string &V = EEState.getGlobalAddressReverseMap()[CurVal]; + assert((!V.empty() || !Name.empty()) && + "GlobalMapping already established!"); + V = Name; } return OldVal; } -void *ExecutionEngine::getPointerToGlobalIfAvailable(const GlobalValue *GV) { +uint64_t ExecutionEngine::getAddressToGlobalIfAvailable(StringRef S) { MutexGuard locked(lock); - + uint64_t Address = 0; ExecutionEngineState::GlobalAddressMapTy::iterator I = - EEState.getGlobalAddressMap().find(GV); - return I != EEState.getGlobalAddressMap().end() ? I->second : nullptr; + EEState.getGlobalAddressMap().find(S); + if (I != EEState.getGlobalAddressMap().end()) + Address = I->second; + return Address; +} + + +void *ExecutionEngine::getPointerToGlobalIfAvailable(StringRef S) { + MutexGuard locked(lock); + if (void* Address = (void *) getAddressToGlobalIfAvailable(S)) + return Address; + return nullptr; +} + +void *ExecutionEngine::getPointerToGlobalIfAvailable(const GlobalValue *GV) { + MutexGuard locked(lock); + return getPointerToGlobalIfAvailable(getMangledName(GV)); } const GlobalValue *ExecutionEngine::getGlobalValueAtAddress(void *Addr) { @@ -247,15 +282,25 @@ const GlobalValue *ExecutionEngine::getGlobalValueAtAddress(void *Addr) { // If we haven't computed the reverse mapping yet, do so first. if (EEState.getGlobalAddressReverseMap().empty()) { for (ExecutionEngineState::GlobalAddressMapTy::iterator - I = EEState.getGlobalAddressMap().begin(), - E = EEState.getGlobalAddressMap().end(); I != E; ++I) + I = EEState.getGlobalAddressMap().begin(), + E = EEState.getGlobalAddressMap().end(); I != E; ++I) { + StringRef Name = I->first(); + uint64_t Addr = I->second; EEState.getGlobalAddressReverseMap().insert(std::make_pair( - I->second, I->first)); + Addr, Name)); + } } - std::map >::iterator I = - EEState.getGlobalAddressReverseMap().find(Addr); - return I != EEState.getGlobalAddressReverseMap().end() ? I->second : nullptr; + std::map::iterator I = + EEState.getGlobalAddressReverseMap().find((uint64_t) Addr); + + if (I != EEState.getGlobalAddressReverseMap().end()) { + StringRef Name = I->second; + for (unsigned i = 0, e = Modules.size(); i != e; ++i) + if (GlobalValue *GV = Modules[i]->getNamedValue(Name)) + return GV; + } + return nullptr; } namespace { @@ -511,7 +556,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) { return getPointerToFunction(F); MutexGuard locked(lock); - if (void *P = EEState.getGlobalAddressMap()[GV]) + if (void* P = getPointerToGlobalIfAvailable(GV)) return P; // Global variable might have been added since interpreter started. @@ -521,7 +566,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) { else llvm_unreachable("Global hasn't had an address allocated yet!"); - return EEState.getGlobalAddressMap()[GV]; + return getPointerToGlobalIfAvailable(GV); } /// \brief Converts a Constant* into a GenericValue, including handling of @@ -1293,25 +1338,3 @@ void ExecutionEngine::EmitGlobalVariable(const GlobalVariable *GV) { NumInitBytes += (unsigned)GVSize; ++NumGlobals; } - -ExecutionEngineState::ExecutionEngineState(ExecutionEngine &EE) - : EE(EE), GlobalAddressMap(this) { -} - -sys::Mutex * -ExecutionEngineState::AddressMapConfig::getMutex(ExecutionEngineState *EES) { - return &EES->EE.lock; -} - -void ExecutionEngineState::AddressMapConfig::onDelete(ExecutionEngineState *EES, - const GlobalValue *Old) { - void *OldVal = EES->GlobalAddressMap.lookup(Old); - EES->GlobalAddressReverseMap.erase(OldVal); -} - -void ExecutionEngineState::AddressMapConfig::onRAUW(ExecutionEngineState *, - const GlobalValue *, - const GlobalValue *) { - llvm_unreachable("The ExecutionEngine doesn't know how to handle a" - " RAUW on a value it has a global mapping for."); -} diff --git a/unittests/ExecutionEngine/ExecutionEngineTest.cpp b/unittests/ExecutionEngine/ExecutionEngineTest.cpp index 8ffc1c8f2c6..80d4ea2db45 100644 --- a/unittests/ExecutionEngine/ExecutionEngineTest.cpp +++ b/unittests/ExecutionEngine/ExecutionEngineTest.cpp @@ -54,6 +54,7 @@ TEST_F(ExecutionEngineTest, ForwardGlobalMapping) { int32_t Mem1 = 3; Engine->addGlobalMapping(G1, &Mem1); EXPECT_EQ(&Mem1, Engine->getPointerToGlobalIfAvailable(G1)); + EXPECT_EQ(&Mem1, Engine->getPointerToGlobalIfAvailable("Global1")); int32_t Mem2 = 4; Engine->updateGlobalMapping(G1, &Mem2); EXPECT_EQ(&Mem2, Engine->getPointerToGlobalIfAvailable(G1));