[llvm] r330763 - [bugpoint] Fix crash when testing for miscompilation.

[Rafael Espindola via llvm-commits]

Author: rafael
Date: Tue Apr 24 13:15:27 2018
New Revision: 330763

URL: http://llvm.org/viewvc/llvm-project?rev=330763&view=rev
Log:
[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.

Modified:
    llvm/trunk/tools/bugpoint/BugDriver.h
    llvm/trunk/tools/bugpoint/CrashDebugger.cpp
    llvm/trunk/tools/bugpoint/ExtractFunction.cpp
    llvm/trunk/tools/bugpoint/Miscompilation.cpp

Modified: llvm/trunk/tools/bugpoint/BugDriver.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/BugDriver.h?rev=330763&r1=330762&r2=330763&view=diff
==============================================================================
--- llvm/trunk/tools/bugpoint/BugDriver.h (original)
+++ llvm/trunk/tools/bugpoint/BugDriver.h Tue Apr 24 13:15:27 2018
@@ -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

Modified: llvm/trunk/tools/bugpoint/CrashDebugger.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/CrashDebugger.cpp?rev=330763&r1=330762&r2=330763&view=diff
==============================================================================
--- llvm/trunk/tools/bugpoint/CrashDebugger.cpp (original)
+++ llvm/trunk/tools/bugpoint/CrashDebugger.cpp Tue Apr 24 13:15:27 2018
@@ -1161,7 +1161,7 @@ static Error DebugACrash(BugDriver &BD,
   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()))

Modified: llvm/trunk/tools/bugpoint/ExtractFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/ExtractFunction.cpp?rev=330763&r1=330762&r2=330763&view=diff
==============================================================================
--- llvm/trunk/tools/bugpoint/ExtractFunction.cpp (original)
+++ llvm/trunk/tools/bugpoint/ExtractFunction.cpp Tue Apr 24 13:15:27 2018
@@ -127,7 +127,8 @@ BugDriver::deleteInstructionFromProgram(
 }
 
 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 *
   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;
 }
 

Modified: llvm/trunk/tools/bugpoint/Miscompilation.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/bugpoint/Miscompilation.cpp?rev=330763&r1=330762&r2=330763&view=diff
==============================================================================
--- llvm/trunk/tools/bugpoint/Miscompilation.cpp (original)
+++ llvm/trunk/tools/bugpoint/Miscompilation.cpp Tue Apr 24 13:15:27 2018
@@ -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(Bug
     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(Bug
 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;