Make the ExecutionEngine automatically remove global mappings on when their

GlobalValue is destroyed.  Function destruction still leaks machine code and
can crash on leaked stubs, but this is some progress.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@83987 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ExecutionEngine/ExecutionEngine.h b/include/llvm/ExecutionEngine/ExecutionEngine.h
index cf2df6b8326b50a1f8090b81bf3c253c49376a20..b9da0fcfce1934a15b5e18c605a3643f93989624 100644 (file)
--- a/include/llvm/ExecutionEngine/ExecutionEngine.h
+++ b/include/llvm/ExecutionEngine/ExecutionEngine.h
@@ -19,6 +19,7 @@
 #include <map>
 #include <string>
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/ValueHandle.h"
 #include "llvm/System/Mutex.h"
 #include "llvm/Target/TargetMachine.h"
 
@@ -26,6 +27,7 @@ namespace llvm {
 
 struct GenericValue;
 class Constant;
+class ExecutionEngine;
 class Function;
 class GlobalVariable;
 class GlobalValue;
@@ -37,13 +39,29 @@ class ModuleProvider;
 class MutexGuard;
 class TargetData;
 class Type;
-template<typename> class AssertingVH;
 
 class ExecutionEngineState {
+public:
+  class MapUpdatingCVH : public CallbackVH {
+    ExecutionEngineState &EES;
+
+  public:
+    MapUpdatingCVH(ExecutionEngineState &EES, const GlobalValue *GV);
+
+    operator const GlobalValue*() const {
+      return cast<GlobalValue>(getValPtr());
+    }
+
+    virtual void deleted();
+    virtual void allUsesReplacedWith(Value *new_value);
+  };
+
 private:
+  ExecutionEngine &EE;
+
   /// GlobalAddressMap - A mapping between LLVM global values and their
   /// actualized version...
-  std::map<AssertingVH<const GlobalValue>, void *> GlobalAddressMap;
+  std::map<MapUpdatingCVH, void *> GlobalAddressMap;
 
   /// GlobalAddressReverseMap - This is the reverse mapping of GlobalAddressMap,
   /// used to convert raw addresses into the LLVM global value that is emitted
@@ -52,7 +70,13 @@ private:
   std::map<void *, AssertingVH<const GlobalValue> > GlobalAddressReverseMap;
 
 public:
-  std::map<AssertingVH<const GlobalValue>, void *> &
+  ExecutionEngineState(ExecutionEngine &EE) : EE(EE) {}
+
+  MapUpdatingCVH getVH(const GlobalValue *GV) {
+    return MapUpdatingCVH(*this, GV);
+  }
+
+  std::map<MapUpdatingCVH, void *> &
   getGlobalAddressMap(const MutexGuard &) {
     return GlobalAddressMap;
   }
@@ -69,7 +93,7 @@ public:
 
 class ExecutionEngine {
   const TargetData *TD;
-  ExecutionEngineState state;
+  ExecutionEngineState EEState;
   bool LazyCompilationDisabled;
   bool GVCompilationDisabled;
   bool SymbolSearchingDisabled;
@@ -213,8 +237,8 @@ public:
   /// at the specified location.  This is used internally as functions are JIT'd
   /// and as global variables are laid out in memory.  It can and should also be
   /// used by clients of the EE that want to have an LLVM global overlay
-  /// existing data in memory.  After adding a mapping for GV, you must not
-  /// destroy it until you've removed the mapping.
+  /// existing data in memory.  Mappings are automatically removed when their
+  /// GlobalValue is destroyed.
   void addGlobalMapping(const GlobalValue *GV, void *Addr);
   
   /// clearAllGlobalMappings - Clear all global mappings and start over again
@@ -238,29 +262,23 @@ public:
   void *getPointerToGlobalIfAvailable(const GlobalValue *GV);
 
   /// getPointerToGlobal - This returns the address of the specified global
-  /// value.  This may involve code generation if it's a function.  After
-  /// getting a pointer to GV, it and all globals it transitively refers to have
-  /// been passed to addGlobalMapping.  You must clear the mapping for each
-  /// referred-to global before destroying it.  If a referred-to global RTG is a
-  /// function and this ExecutionEngine is a JIT compiler, calling
-  /// updateGlobalMapping(RTG, 0) will leak the function's machine code, so you
-  /// should call freeMachineCodeForFunction(RTG) instead.  Note that
-  /// optimizations can move and delete non-external GlobalValues without
-  /// notifying the ExecutionEngine.
+  /// value.  This may involve code generation if it's a function.
   ///
   void *getPointerToGlobal(const GlobalValue *GV);
 
   /// getPointerToFunction - The different EE's represent function bodies in
   /// different ways.  They should each implement this to say what a function
-  /// pointer should look like.  See getPointerToGlobal for the requirements on
-  /// destroying F and any GlobalValues it refers to.
+  /// pointer should look like.  When F is destroyed, the ExecutionEngine will
+  /// remove its global mapping but will not yet free its machine code.  Call
+  /// freeMachineCodeForFunction(F) explicitly to do that.  Note that global
+  /// optimizations can destroy Functions without notifying the ExecutionEngine.
   ///
   virtual void *getPointerToFunction(Function *F) = 0;
 
   /// getPointerToFunctionOrStub - If the specified function has been
   /// code-gen'd, return a pointer to the function.  If not, compile it, or use
-  /// a stub to implement lazy compilation if available.  See getPointerToGlobal
-  /// for the requirements on destroying F and any GlobalValues it refers to.
+  /// a stub to implement lazy compilation if available.  See
+  /// getPointerToFunction for the requirements on destroying F.
   ///
   virtual void *getPointerToFunctionOrStub(Function *F) {
     // Default implementation, just codegen the function.
@@ -296,8 +314,7 @@ public:
 
   /// getOrEmitGlobalVariable - Return the address of the specified global
   /// variable, possibly emitting it to memory if needed.  This is used by the
-  /// Emitter.  See getPointerToGlobal for the requirements on destroying GV and
-  /// any GlobalValues it refers to.
+  /// Emitter.
   virtual void *getOrEmitGlobalVariable(const GlobalVariable *GV) {
     return getPointerToGlobal((GlobalValue*)GV);
   }
@@ -471,6 +488,12 @@ class EngineBuilder {
 
 };
 
+inline bool operator<(const ExecutionEngineState::MapUpdatingCVH& lhs,
+                      const ExecutionEngineState::MapUpdatingCVH& rhs) {
+    return static_cast<const GlobalValue*>(lhs) <
+        static_cast<const GlobalValue*>(rhs);
+}
+
 } // End llvm namespace
 
 #endif

diff --git a/lib/ExecutionEngine/ExecutionEngine.cpp b/lib/ExecutionEngine/ExecutionEngine.cpp
index 700ae5ea9c41964bac1e6b16fce2d760d4bb70eb..053d96020d37374df235044b739c16efb9fbf9e0 100644 (file)
--- a/lib/ExecutionEngine/ExecutionEngine.cpp
+++ b/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -46,7 +46,9 @@ ExecutionEngine *(*ExecutionEngine::InterpCtor)(ModuleProvider *MP,
 ExecutionEngine::EERegisterFn ExecutionEngine::ExceptionTableRegister = 0;
 
 
-ExecutionEngine::ExecutionEngine(ModuleProvider *P) : LazyFunctionCreator(0) {
+ExecutionEngine::ExecutionEngine(ModuleProvider *P)
+  : EEState(*this),
+    LazyFunctionCreator(0) {
   LazyCompilationDisabled = false;
   GVCompilationDisabled   = false;
   SymbolSearchingDisabled = false;
@@ -115,8 +117,8 @@ Function *ExecutionEngine::FindFunctionNamed(const char *FnName) {
 
 void *ExecutionEngineState::RemoveMapping(
   const MutexGuard &, const GlobalValue *ToUnmap) {
-  std::map<AssertingVH<const GlobalValue>, void *>::iterator I =
-    GlobalAddressMap.find(ToUnmap);
+  std::map<MapUpdatingCVH, void *>::iterator I =
+    GlobalAddressMap.find(getVH(ToUnmap));
   void *OldVal;
   if (I == GlobalAddressMap.end())
     OldVal = 0;
@@ -139,14 +141,14 @@ void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) {
 
   DEBUG(errs() << "JIT: Map \'" << GV->getName() 
         << "\' to [" << Addr << "]\n";);
-  void *&CurVal = state.getGlobalAddressMap(locked)[GV];
+  void *&CurVal = EEState.getGlobalAddressMap(locked)[EEState.getVH(GV)];
   assert((CurVal == 0 || Addr == 0) && "GlobalMapping already established!");
   CurVal = Addr;
   
   // If we are using the reverse mapping, add it too
-  if (!state.getGlobalAddressReverseMap(locked).empty()) {
+  if (!EEState.getGlobalAddressReverseMap(locked).empty()) {
     AssertingVH<const GlobalValue> &V =
-      state.getGlobalAddressReverseMap(locked)[Addr];
+      EEState.getGlobalAddressReverseMap(locked)[Addr];
     assert((V == 0 || GV == 0) && "GlobalMapping already established!");
     V = GV;
   }
@@ -157,8 +159,8 @@ void ExecutionEngine::addGlobalMapping(const GlobalValue *GV, void *Addr) {
 void ExecutionEngine::clearAllGlobalMappings() {
   MutexGuard locked(lock);
   
-  state.getGlobalAddressMap(locked).clear();
-  state.getGlobalAddressReverseMap(locked).clear();
+  EEState.getGlobalAddressMap(locked).clear();
+  EEState.getGlobalAddressReverseMap(locked).clear();
 }
 
 /// clearGlobalMappingsFromModule - Clear all global mappings that came from a
@@ -167,11 +169,11 @@ void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) {
   MutexGuard locked(lock);
   
   for (Module::iterator FI = M->begin(), FE = M->end(); FI != FE; ++FI) {
-    state.RemoveMapping(locked, FI);
+    EEState.RemoveMapping(locked, FI);
   }
   for (Module::global_iterator GI = M->global_begin(), GE = M->global_end(); 
        GI != GE; ++GI) {
-    state.RemoveMapping(locked, GI);
+    EEState.RemoveMapping(locked, GI);
   }
 }
 
@@ -181,25 +183,25 @@ void ExecutionEngine::clearGlobalMappingsFromModule(Module *M) {
 void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
   MutexGuard locked(lock);
 
-  std::map<AssertingVH<const GlobalValue>, void *> &Map =
-    state.getGlobalAddressMap(locked);
+  std::map<ExecutionEngineState::MapUpdatingCVH, void *> &Map =
+    EEState.getGlobalAddressMap(locked);
 
   // Deleting from the mapping?
   if (Addr == 0) {
-    return state.RemoveMapping(locked, GV);
+    return EEState.RemoveMapping(locked, GV);
   }
   
-  void *&CurVal = Map[GV];
+  void *&CurVal = Map[EEState.getVH(GV)];
   void *OldVal = CurVal;
 
-  if (CurVal && !state.getGlobalAddressReverseMap(locked).empty())
-    state.getGlobalAddressReverseMap(locked).erase(CurVal);
+  if (CurVal && !EEState.getGlobalAddressReverseMap(locked).empty())
+    EEState.getGlobalAddressReverseMap(locked).erase(CurVal);
   CurVal = Addr;
   
   // If we are using the reverse mapping, add it too
-  if (!state.getGlobalAddressReverseMap(locked).empty()) {
+  if (!EEState.getGlobalAddressReverseMap(locked).empty()) {
     AssertingVH<const GlobalValue> &V =
-      state.getGlobalAddressReverseMap(locked)[Addr];
+      EEState.getGlobalAddressReverseMap(locked)[Addr];
     assert((V == 0 || GV == 0) && "GlobalMapping already established!");
     V = GV;
   }
@@ -212,9 +214,9 @@ void *ExecutionEngine::updateGlobalMapping(const GlobalValue *GV, void *Addr) {
 void *ExecutionEngine::getPointerToGlobalIfAvailable(const GlobalValue *GV) {
   MutexGuard locked(lock);
   
-  std::map<AssertingVH<const GlobalValue>, void*>::iterator I =
-    state.getGlobalAddressMap(locked).find(GV);
-  return I != state.getGlobalAddressMap(locked).end() ? I->second : 0;
+  std::map<ExecutionEngineState::MapUpdatingCVH, void*>::iterator I =
+    EEState.getGlobalAddressMap(locked).find(EEState.getVH(GV));
+  return I != EEState.getGlobalAddressMap(locked).end() ? I->second : 0;
 }
 
 /// getGlobalValueAtAddress - Return the LLVM global value object that starts
@@ -224,17 +226,17 @@ const GlobalValue *ExecutionEngine::getGlobalValueAtAddress(void *Addr) {
   MutexGuard locked(lock);
 
   // If we haven't computed the reverse mapping yet, do so first.
-  if (state.getGlobalAddressReverseMap(locked).empty()) {
-    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,
+  if (EEState.getGlobalAddressReverseMap(locked).empty()) {
+    for (std::map<ExecutionEngineState::MapUpdatingCVH, void *>::iterator
+         I = EEState.getGlobalAddressMap(locked).begin(),
+         E = EEState.getGlobalAddressMap(locked).end(); I != E; ++I)
+      EEState.getGlobalAddressReverseMap(locked).insert(std::make_pair(I->second,
                                                                      I->first));
   }
 
   std::map<void *, AssertingVH<const GlobalValue> >::iterator I =
-    state.getGlobalAddressReverseMap(locked).find(Addr);
-  return I != state.getGlobalAddressReverseMap(locked).end() ? I->second : 0;
+    EEState.getGlobalAddressReverseMap(locked).find(Addr);
+  return I != EEState.getGlobalAddressReverseMap(locked).end() ? I->second : 0;
 }
 
 // CreateArgv - Turn a vector of strings into a nice argv style array of
@@ -474,7 +476,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) {
     return getPointerToFunction(F);
 
   MutexGuard locked(lock);
-  void *p = state.getGlobalAddressMap(locked)[GV];
+  void *p = EEState.getGlobalAddressMap(locked)[EEState.getVH(GV)];
   if (p)
     return p;
 
@@ -484,7 +486,7 @@ void *ExecutionEngine::getPointerToGlobal(const GlobalValue *GV) {
     EmitGlobalVariable(GVar);
   else
     llvm_unreachable("Global hasn't had an address allocated yet!");
-  return state.getGlobalAddressMap(locked)[GV];
+  return EEState.getGlobalAddressMap(locked)[EEState.getVH(GV)];
 }
 
 /// This function converts a Constant* into a GenericValue. The interesting 
@@ -1069,3 +1071,18 @@ void ExecutionEngine::EmitGlobalVariable(const GlobalVariable *GV) {
   NumInitBytes += (unsigned)GVSize;
   ++NumGlobals;
 }
+
+ExecutionEngineState::MapUpdatingCVH::MapUpdatingCVH(
+  ExecutionEngineState &EES, const GlobalValue *GV)
+  : CallbackVH(const_cast<GlobalValue*>(GV)), EES(EES) {}
+
+void ExecutionEngineState::MapUpdatingCVH::deleted() {
+  MutexGuard locked(EES.EE.lock);
+  EES.RemoveMapping(locked, *this);  // Destroys *this.
+}
+
+void ExecutionEngineState::MapUpdatingCVH::allUsesReplacedWith(
+  Value *new_value) {
+  assert(false && "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 97a8478311400bb130cdbfe6338b2f58cffd4786..904ee2b6c49fb3fcf23fc08fca758d7d4ac43291 100644 (file)
--- a/unittests/ExecutionEngine/ExecutionEngineTest.cpp
+++ b/unittests/ExecutionEngine/ExecutionEngineTest.cpp
@@ -113,4 +113,17 @@ TEST_F(ExecutionEngineTest, ClearModuleMappings) {
   EXPECT_EQ(G2, Engine->getGlobalValueAtAddress(&Mem1));
 }
 
+TEST_F(ExecutionEngineTest, DestructionRemovesGlobalMapping) {
+  GlobalVariable *G1 =
+    NewExtGlobal(Type::getInt32Ty(getGlobalContext()), "Global1");
+  int32_t Mem1 = 3;
+  Engine->addGlobalMapping(G1, &Mem1);
+  // Make sure the reverse mapping is enabled.
+  EXPECT_EQ(G1, Engine->getGlobalValueAtAddress(&Mem1));
+  // When the GV goes away, the ExecutionEngine should remove any
+  // mappings that refer to it.
+  G1->eraseFromParent();
+  EXPECT_EQ(NULL, Engine->getGlobalValueAtAddress(&Mem1));
+}
+
 }