[bugpoint] Fix crash when testing for miscompilation.

Method BugDriver::performFinalCleanups(...) would delete Module object
it worked on, which was also deleted by its caller
(e.g. TestCodeGenerator(...)). Changed the code to avoid double delete
and make Module ownership slightly clearer.

Patch by Andrzej Janik.

llvm-svn: 330763

4 files changed, 15 insertions, 12 deletions

diff --git a/llvm/tools/bugpoint/BugDriver.h b/llvm/tools/bugpoint/BugDriver.h
index 815931258ae..bc60ae75354 100644
--- a/llvm/tools/bugpoint/BugDriver.h
+++ b/llvm/tools/bugpoint/BugDriver.h
@@ -197,7 +197,7 @@ public:
   /// MayModifySemantics argument is true, then the cleanups is allowed to
   /// modify how the code behaves.
   ///
-  std::unique_ptr<Module> performFinalCleanups(Module *M,
+  std::unique_ptr<Module> performFinalCleanups(std::unique_ptr<Module> M,
                                                bool MayModifySemantics = false);
 
   /// Given a module, extract up to one loop from it into a new function. This
diff --git a/llvm/tools/bugpoint/CrashDebugger.cpp b/llvm/tools/bugpoint/CrashDebugger.cpp
index 3b7f2af436c..631784a2c65 100644
--- a/llvm/tools/bugpoint/CrashDebugger.cpp
+++ b/llvm/tools/bugpoint/CrashDebugger.cpp
@@ -1161,7 +1161,7 @@ static Error DebugACrash(BugDriver &BD, BugTester TestFn) {
   if (!BugpointIsInterrupted) {
     outs() << "\n*** Attempting to perform final cleanups: ";
     std::unique_ptr<Module> M = CloneModule(BD.getProgram());
-    M = BD.performFinalCleanups(M.release(), true);
+    M = BD.performFinalCleanups(std::move(M), true);
 
     // Find out if the pass still crashes on the cleaned up program...
     if (M && TestFn(BD, M.get()))
diff --git a/llvm/tools/bugpoint/ExtractFunction.cpp b/llvm/tools/bugpoint/ExtractFunction.cpp
index ab55dbeb324..343ecd54b48 100644
--- a/llvm/tools/bugpoint/ExtractFunction.cpp
+++ b/llvm/tools/bugpoint/ExtractFunction.cpp
@@ -127,7 +127,8 @@ BugDriver::deleteInstructionFromProgram(const Instruction *I,
 }
 
 std::unique_ptr<Module>
-BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) {
+BugDriver::performFinalCleanups(std::unique_ptr<Module> M,
+                                bool MayModifySemantics) {
   // Make all functions external, so GlobalDCE doesn't delete them...
   for (Module::iterator I = M->begin(), E = M->end(); I != E; ++I)
     I->setLinkage(GlobalValue::ExternalLinkage);
@@ -140,12 +141,11 @@ BugDriver::performFinalCleanups(Module *M, bool MayModifySemantics) {
   else
     CleanupPasses.push_back("deadargelim");
 
-  std::unique_ptr<Module> New = runPassesOn(M, CleanupPasses);
+  std::unique_ptr<Module> New = runPassesOn(M.get(), CleanupPasses);
   if (!New) {
     errs() << "Final cleanups failed.  Sorry. :(  Please report a bug!\n";
     return nullptr;
   }
-  delete M;
   return New;
 }
 
diff --git a/llvm/tools/bugpoint/Miscompilation.cpp b/llvm/tools/bugpoint/Miscompilation.cpp
index 627b14ce195..375bee7a0d5 100644
--- a/llvm/tools/bugpoint/Miscompilation.cpp
+++ b/llvm/tools/bugpoint/Miscompilation.cpp
@@ -776,15 +776,15 @@ Error BugDriver::debugMiscompilation() {
 
 /// Get the specified modules ready for code generator testing.
 ///
-static void CleanupAndPrepareModules(BugDriver &BD,
-                                     std::unique_ptr<Module> &Test,
-                                     Module *Safe) {
+static std::unique_ptr<Module>
+CleanupAndPrepareModules(BugDriver &BD, std::unique_ptr<Module> Test,
+                         Module *Safe) {
   // Clean up the modules, removing extra cruft that we don't need anymore...
-  Test = BD.performFinalCleanups(Test.get());
+  Test = BD.performFinalCleanups(std::move(Test));
 
   // If we are executing the JIT, we have several nasty issues to take care of.
   if (!BD.isExecutingJIT())
-    return;
+    return Test;
 
   // First, if the main function is in the Safe module, we must add a stub to
   // the Test module to call into it.  Thus, we create a new function `main'
@@ -930,6 +930,8 @@ static void CleanupAndPrepareModules(BugDriver &BD,
     errs() << "Bugpoint has a bug, which corrupted a module!!\n";
     abort();
   }
+
+  return Test;
 }
 
 /// This is the predicate function used to check to see if the "Test" portion of
@@ -939,7 +941,7 @@ static void CleanupAndPrepareModules(BugDriver &BD,
 static Expected<bool> TestCodeGenerator(BugDriver &BD,
                                         std::unique_ptr<Module> Test,
                                         std::unique_ptr<Module> Safe) {
-  CleanupAndPrepareModules(BD, Test, Safe.get());
+  Test = CleanupAndPrepareModules(BD, std::move(Test), Safe.get());
 
   SmallString<128> TestModuleBC;
   int TestModuleFD;
@@ -1030,7 +1032,8 @@ Error BugDriver::debugCodeGenerator() {
       SplitFunctionsOutOfModule(ToNotCodeGen.get(), *Funcs, VMap);
 
   // Condition the modules
-  CleanupAndPrepareModules(*this, ToCodeGen, ToNotCodeGen.get());
+  ToCodeGen =
+      CleanupAndPrepareModules(*this, std::move(ToCodeGen), ToNotCodeGen.get());
 
   SmallString<128> TestModuleBC;
   int TestModuleFD;