Keep track of stubs that are created. This fixes PR5162 and probably PR4822 and

4406. Patch by Nick Lewycky!


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

3 files changed, 105 insertions, 11 deletions

diff --git a/lib/ExecutionEngine/JIT/JIT.h b/lib/ExecutionEngine/JIT/JIT.h
index dfeffb516e..525cc84f94 100644
--- a/lib/ExecutionEngine/JIT/JIT.h
+++ b/lib/ExecutionEngine/JIT/JIT.h
@@ -16,6 +16,7 @@
 
 #include "llvm/ExecutionEngine/ExecutionEngine.h"
 #include "llvm/PassManager.h"
+#include "llvm/Support/ValueHandle.h"
 
 namespace llvm {
 
@@ -33,7 +34,7 @@ private:
 
   /// PendingFunctions - Functions which have not been code generated yet, but
   /// were called from a function being code generated.
-  std::vector<Function*> PendingFunctions;
+  std::vector<AssertingVH<Function> > PendingFunctions;
 
 public:
   explicit JITState(ModuleProvider *MP) : PM(MP), MP(MP) {}
@@ -43,7 +44,7 @@ public:
   }
   
   ModuleProvider *getMP() const { return MP; }
-  std::vector<Function*> &getPendingFunctions(const MutexGuard &L) {
+  std::vector<AssertingVH<Function> > &getPendingFunctions(const MutexGuard &L){
     return PendingFunctions;
   }
 };
diff --git a/lib/ExecutionEngine/JIT/JITEmitter.cpp b/lib/ExecutionEngine/JIT/JITEmitter.cpp
index e8314a1d86..3d69c3ff5e 100644
--- a/lib/ExecutionEngine/JIT/JITEmitter.cpp
+++ b/lib/ExecutionEngine/JIT/JITEmitter.cpp
@@ -64,7 +64,7 @@ namespace {
   class JITResolverState {
   public:
     typedef std::map<AssertingVH<Function>, void*> FunctionToStubMapTy;
-    typedef std::map<void*, Function*> StubToFunctionMapTy;
+    typedef std::map<void*, AssertingVH<Function> > StubToFunctionMapTy;
     typedef std::map<AssertingVH<GlobalValue>, void*> GlobalToIndirectSymMapTy;
   private:
     /// FunctionToStubMap - Keep track of the stub created for a particular
@@ -198,9 +198,9 @@ void *JITResolver::getFunctionStub(Function *F) {
 
   // Call the lazy resolver function unless we are JIT'ing non-lazily, in which
   // case we must resolve the symbol now.
-  void *Actual =  TheJIT->isLazyCompilationDisabled() 
+  void *Actual = TheJIT->isLazyCompilationDisabled()
     ? (void *)0 : (void *)(intptr_t)LazyResolverFn;
-   
+  
   // If this is an external declaration, attempt to resolve the address now
   // to place in the stub.
   if (F->isDeclaration() && !F->hasNotBeenReadFromBitcode()) {
@@ -231,14 +231,14 @@ void *JITResolver::getFunctionStub(Function *F) {
   // Finally, keep track of the stub-to-Function mapping so that the
   // JITCompilerFn knows which function to compile!
   state.getStubToFunctionMap(locked)[Stub] = F;
-  
+
   // If we are JIT'ing non-lazily but need to call a function that does not
   // exist yet, add it to the JIT's work list so that we can fill in the stub
   // address later.
   if (!Actual && TheJIT->isLazyCompilationDisabled())
     if (!F->isDeclaration() || F->hasNotBeenReadFromBitcode())
       TheJIT->addPendingFunction(F);
-  
+
   return Stub;
 }
 
@@ -696,11 +696,8 @@ void *JITEmitter::getPointerToGVIndirectSym(GlobalValue *V, void *Reference,
 }
 
 void JITEmitter::AddStubToCurrentFunction(void *StubAddr) {
-  if (!TheJIT->areDlsymStubsEnabled())
-    return;
-  
   assert(CurFn && "Stub added to current function, but current function is 0!");
-  
+
   SmallVectorImpl<void*> &StubsUsed = CurFnStubUses[CurFn];
   StubsUsed.push_back(StubAddr);
 
diff --git a/unittests/ExecutionEngine/JIT/JITTest.cpp b/unittests/ExecutionEngine/JIT/JITTest.cpp
index a4812bc176..55d37493ea 100644
--- a/unittests/ExecutionEngine/JIT/JITTest.cpp
+++ b/unittests/ExecutionEngine/JIT/JITTest.cpp
@@ -166,6 +166,102 @@ TEST_F(JITTest, FarCallToKnownFunction) {
   EXPECT_EQ(8, TestFunctionPtr());
 }
 
+// Test a function C which calls A and B which call each other.
+TEST_F(JITTest, NonLazyCompilationStillNeedsStubs) {
+  TheJIT->DisableLazyCompilation();
+
+  const FunctionType *Func1Ty =
+      cast<FunctionType>(TypeBuilder<void(void), false>::get(Context));
+  std::vector<const Type*> arg_types;
+  arg_types.push_back(Type::getInt1Ty(Context));
+  const FunctionType *FuncTy = FunctionType::get(
+      Type::getVoidTy(Context), arg_types, false);
+  Function *Func1 = Function::Create(Func1Ty, Function::ExternalLinkage,
+                                     "func1", M);
+  Function *Func2 = Function::Create(FuncTy, Function::InternalLinkage,
+                                     "func2", M);
+  Function *Func3 = Function::Create(FuncTy, Function::InternalLinkage,
+                                     "func3", M);
+  BasicBlock *Block1 = BasicBlock::Create(Context, "block1", Func1);
+  BasicBlock *Block2 = BasicBlock::Create(Context, "block2", Func2);
+  BasicBlock *True2 = BasicBlock::Create(Context, "cond_true", Func2);
+  BasicBlock *False2 = BasicBlock::Create(Context, "cond_false", Func2);
+  BasicBlock *Block3 = BasicBlock::Create(Context, "block3", Func3);
+  BasicBlock *True3 = BasicBlock::Create(Context, "cond_true", Func3);
+  BasicBlock *False3 = BasicBlock::Create(Context, "cond_false", Func3);
+
+  // Make Func1 call Func2(0) and Func3(0).
+  IRBuilder<> Builder(Block1);
+  Builder.CreateCall(Func2, ConstantInt::getTrue(Context));
+  Builder.CreateCall(Func3, ConstantInt::getTrue(Context));
+  Builder.CreateRetVoid();
+
+  // void Func2(bool b) { if (b) { Func3(false); return; } return; }
+  Builder.SetInsertPoint(Block2);
+  Builder.CreateCondBr(Func2->arg_begin(), True2, False2);
+  Builder.SetInsertPoint(True2);
+  Builder.CreateCall(Func3, ConstantInt::getFalse(Context));
+  Builder.CreateRetVoid();
+  Builder.SetInsertPoint(False2);
+  Builder.CreateRetVoid();
+
+  // void Func3(bool b) { if (b) { Func2(false); return; } return; }
+  Builder.SetInsertPoint(Block3);
+  Builder.CreateCondBr(Func3->arg_begin(), True3, False3);
+  Builder.SetInsertPoint(True3);
+  Builder.CreateCall(Func2, ConstantInt::getFalse(Context));
+  Builder.CreateRetVoid();
+  Builder.SetInsertPoint(False3);
+  Builder.CreateRetVoid();
+
+  // Compile the function to native code
+  void (*F1Ptr)() =
+     reinterpret_cast<void(*)()>((intptr_t)TheJIT->getPointerToFunction(Func1));
+
+  F1Ptr();
+}
+
+// Regression test for PR5162.  This used to trigger an AssertingVH inside the
+// JIT's Function to stub mapping.
+TEST_F(JITTest, NonLazyLeaksNoStubs) {
+  TheJIT->DisableLazyCompilation();
+
+  // Create two functions with a single basic block each.
+  const FunctionType *FuncTy =
+      cast<FunctionType>(TypeBuilder<int(), false>::get(Context));
+  Function *Func1 = Function::Create(FuncTy, Function::ExternalLinkage,
+                                     "func1", M);
+  Function *Func2 = Function::Create(FuncTy, Function::InternalLinkage,
+                                     "func2", M);
+  BasicBlock *Block1 = BasicBlock::Create(Context, "block1", Func1);
+  BasicBlock *Block2 = BasicBlock::Create(Context, "block2", Func2);
+
+  // The first function calls the second and returns the result
+  IRBuilder<> Builder(Block1);
+  Value *Result = Builder.CreateCall(Func2);
+  Builder.CreateRet(Result);
+
+  // The second function just returns a constant
+  Builder.SetInsertPoint(Block2);
+  Builder.CreateRet(ConstantInt::get(TypeBuilder<int, false>::get(Context),42));
+
+  // Compile the function to native code
+  (void)TheJIT->getPointerToFunction(Func1);
+
+  // Free the JIT state for the functions
+  TheJIT->freeMachineCodeForFunction(Func1);
+  TheJIT->freeMachineCodeForFunction(Func2);
+
+  // Delete the first function (and show that is has no users)
+  EXPECT_EQ(Func1->getNumUses(), 0u);
+  Func1->eraseFromParent();
+
+  // Delete the second function (and show that it has no users - it had one,
+  // func1 but that's gone now)
+  EXPECT_EQ(Func2->getNumUses(), 0u);
+  Func2->eraseFromParent();
+}
+
 // This code is copied from JITEventListenerTest, but it only runs once for all
 // the tests in this directory.  Everything seems fine, but that's strange
 // behavior.