[llvm] r193094 - Improving MCJIT/RuntimeDyld thread safety

Andrew Kaylor andrew.kaylor at intel.com
Mon Oct 21 10:42:06 PDT 2013


Author: akaylor
Date: Mon Oct 21 12:42:06 2013
New Revision: 193094

URL: http://llvm.org/viewvc/llvm-project?rev=193094&view=rev
Log:
Improving MCJIT/RuntimeDyld thread safety

Modified:
    llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
    llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
    llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
    llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h

Modified: llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h?rev=193094&r1=193093&r2=193094&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h Mon Oct 21 12:42:06 2013
@@ -153,7 +153,7 @@ protected:
   void *(*LazyFunctionCreator)(const std::string &);
 
 public:
-  /// lock - This lock protects the ExecutionEngine, JIT, JITResolver and
+  /// lock - This lock protects the ExecutionEngine, MCJIT, JIT, JITResolver and
   /// JITEmitter classes.  It must be held while changing the internal state of
   /// any of those classes.
   sys::Mutex lock;

Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=193094&r1=193093&r2=193094&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Mon Oct 21 12:42:06 2013
@@ -62,6 +62,7 @@ MCJIT::MCJIT(Module *m, TargetMachine *t
 }
 
 MCJIT::~MCJIT() {
+  MutexGuard locked(lock);
   Dyld.deregisterEHFrames();
 
   LoadedObjectMap::iterator it, end = LoadedObjects.end();
@@ -77,22 +78,23 @@ MCJIT::~MCJIT() {
 }
 
 void MCJIT::addModule(Module *M) {
+  MutexGuard locked(lock);
   Modules.push_back(M);
   ModuleStates[M] = MCJITModuleState();
 }
 
 void MCJIT::setObjectCache(ObjectCache* NewCache) {
+  MutexGuard locked(lock);
   ObjCache = NewCache;
 }
 
 ObjectBufferStream* MCJIT::emitObject(Module *M) {
+  MutexGuard locked(lock);
+
   // This must be a module which has already been added to this MCJIT instance.
   assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
   assert(ModuleStates.find(M) != ModuleStates.end());
 
-  // Get a thread lock to make sure we aren't trying to compile multiple times
-  MutexGuard locked(lock);
-
   // Re-compilation is not supported
   assert(!ModuleStates[M].hasBeenEmitted());
 
@@ -127,13 +129,13 @@ ObjectBufferStream* MCJIT::emitObject(Mo
 }
 
 void MCJIT::generateCodeForModule(Module *M) {
+  // Get a thread lock to make sure we aren't trying to load multiple times
+  MutexGuard locked(lock);
+
   // This must be a module which has already been added to this MCJIT instance.
   assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
   assert(ModuleStates.find(M) != ModuleStates.end());
 
-  // Get a thread lock to make sure we aren't trying to load multiple times
-  MutexGuard locked(lock);
-
   // Re-compilation is not supported
   if (ModuleStates[M].hasBeenLoaded())
     return;
@@ -168,6 +170,8 @@ void MCJIT::generateCodeForModule(Module
 }
 
 void MCJIT::finalizeLoadedModules() {
+  MutexGuard locked(lock);
+
   // Resolve any outstanding relocations.
   Dyld.resolveRelocations();
 
@@ -192,6 +196,8 @@ void MCJIT::finalizeLoadedModules() {
 
 // FIXME: Rename this.
 void MCJIT::finalizeObject() {
+  MutexGuard locked(lock);
+
   // FIXME: This is a temporary hack to get around problems with calling
   // finalize multiple times.
   bool finalizeNeeded = false;
@@ -232,6 +238,8 @@ void MCJIT::finalizeObject() {
 }
 
 void MCJIT::finalizeModule(Module *M) {
+  MutexGuard locked(lock);
+
   // This must be a module which has already been added to this MCJIT instance.
   assert(std::find(Modules.begin(), Modules.end(), M) != Modules.end());
   assert(ModuleStates.find(M) != ModuleStates.end());
@@ -268,6 +276,8 @@ uint64_t MCJIT::getExistingSymbolAddress
 
 Module *MCJIT::findModuleForSymbol(const std::string &Name,
                                    bool CheckFunctionsOnly) {
+  MutexGuard locked(lock);
+
   // If it hasn't already been generated, see if it's in one of our modules.
   SmallVector<Module *, 1>::iterator end = Modules.end();
   SmallVector<Module *, 1>::iterator it;
@@ -290,6 +300,8 @@ Module *MCJIT::findModuleForSymbol(const
 uint64_t MCJIT::getSymbolAddress(const std::string &Name,
                                  bool CheckFunctionsOnly)
 {
+  MutexGuard locked(lock);
+
   // First, check to see if we already have this symbol.
   uint64_t Addr = getExistingSymbolAddress(Name);
   if (Addr)
@@ -306,8 +318,6 @@ uint64_t MCJIT::getSymbolAddress(const s
   if (ModuleStates[M].hasBeenLoaded())
     return 0;
 
-  // FIXME: We probably need to make sure we aren't in the process of
-  //        loading or finalizing this module.
   generateCodeForModule(M);
 
   // Check the RuntimeDyld table again, it should be there now.
@@ -315,6 +325,7 @@ uint64_t MCJIT::getSymbolAddress(const s
 }
 
 uint64_t MCJIT::getGlobalValueAddress(const std::string &Name) {
+  MutexGuard locked(lock);
   uint64_t Result = getSymbolAddress(Name, false);
   if (Result != 0)
     finalizeLoadedModules();
@@ -322,6 +333,7 @@ uint64_t MCJIT::getGlobalValueAddress(co
 }
 
 uint64_t MCJIT::getFunctionAddress(const std::string &Name) {
+  MutexGuard locked(lock);
   uint64_t Result = getSymbolAddress(Name, true);
   if (Result != 0)
     finalizeLoadedModules();
@@ -330,6 +342,7 @@ uint64_t MCJIT::getFunctionAddress(const
 
 // Deprecated.  Use getFunctionAddress instead.
 void *MCJIT::getPointerToFunction(Function *F) {
+  MutexGuard locked(lock);
 
   if (F->isDeclaration() || F->hasAvailableExternallyLinkage()) {
     bool AbortOnFailure = !F->hasExternalWeakLinkage();

Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h?rev=193094&r1=193093&r2=193094&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h (original)
+++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h Mon Oct 21 12:42:06 2013
@@ -70,10 +70,6 @@ private:
   OwningPtr<RTDyldMemoryManager> ClientMM;
 };
 
-// FIXME: This makes all kinds of horrible assumptions for the time being,
-// like only having one module, not needing to worry about multi-threading,
-// blah blah. Purely in get-it-up-and-limping mode for now.
-
 // About Module states:
 //
 // The purpose of the "added" state is having modules in standby. (added=known

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=193094&r1=193093&r2=193094&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Mon Oct 21 12:42:06 2013
@@ -19,6 +19,7 @@
 #include "RuntimeDyldMachO.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MathExtras.h"
+#include "llvm/Support/MutexGuard.h"
 #include "llvm/Object/ELF.h"
 
 using namespace llvm;
@@ -37,6 +38,8 @@ void RuntimeDyldImpl::deregisterEHFrames
 
 // Resolve the relocations for all symbols we currently know about.
 void RuntimeDyldImpl::resolveRelocations() {
+  MutexGuard locked(lock);
+
   // First, resolve relocations associated with external symbols.
   resolveExternalSymbols();
 
@@ -57,6 +60,7 @@ void RuntimeDyldImpl::resolveRelocations
 
 void RuntimeDyldImpl::mapSectionAddress(const void *LocalAddress,
                                         uint64_t TargetAddress) {
+  MutexGuard locked(lock);
   for (unsigned i = 0, e = Sections.size(); i != e; ++i) {
     if (Sections[i].Address == LocalAddress) {
       reassignSectionAddress(i, TargetAddress);
@@ -73,6 +77,8 @@ ObjectImage *RuntimeDyldImpl::createObje
 }
 
 ObjectImage *RuntimeDyldImpl::loadObject(ObjectBuffer *InputBuffer) {
+  MutexGuard locked(lock);
+
   OwningPtr<ObjectImage> obj(createObjectImage(InputBuffer));
   if (!obj)
     report_fatal_error("Unable to create object image from memory buffer!");

Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h?rev=193094&r1=193093&r2=193094&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h (original)
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h Mon Oct 21 12:42:06 2013
@@ -25,6 +25,7 @@
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/SwapByteOrder.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/system_error.h"
@@ -186,6 +187,18 @@ protected:
   Triple::ArchType Arch;
   bool IsTargetLittleEndian;
 
+  // This mutex prevents simultaneously loading objects from two different
+  // threads.  This keeps us from having to protect individual data structures
+  // and guarantees that section allocation requests to the memory manager
+  // won't be interleaved between modules.  It is also used in mapSectionAddress
+  // and resolveRelocations to protect write access to internal data structures.
+  //
+  // loadObject may be called on the same thread during the handling of of
+  // processRelocations, and that's OK.  The handling of the relocation lists
+  // is written in such a way as to work correctly if new elements are added to
+  // the end of the list while the list is being processed.
+  sys::Mutex lock;
+
   virtual unsigned getMaxStubSize() = 0;
   virtual unsigned getStubAlignment() = 0;
 





More information about the llvm-commits mailing list