move module loading into external function

- having an explicit function which is defined for multiple llvm
  versions separately increases readability.
- also: error handling was simplified
- Personal motivation: being able to use this functionality in unit tests

fixes #561
related to #656

3 files changed, 107 insertions, 80 deletions

diff --git a/include/klee/Internal/Support/ModuleUtil.h b/include/klee/Internal/Support/ModuleUtil.h
index c85ba591..78998051 100644
--- a/include/klee/Internal/Support/ModuleUtil.h
+++ b/include/klee/Internal/Support/ModuleUtil.h
@@ -7,22 +7,37 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef KLEE_TRANSFORM_UTIL_H
-#define KLEE_TRANSFORM_UTIL_H
+#ifndef KLEE_MODULE_UTIL_H
+#define KLEE_MODULE_UTIL_H
+
+#include "klee/Config/Version.h"
+
+#if LLVM_VERSION_CODE > LLVM_VERSION(3, 2)
+#include "llvm/IR/Module.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/LLVMContext.h"
+#else
+#include "llvm/Module.h"
+#include "llvm/Function.h"
+#include "llvm/LLVMContext.h"
+#endif
 
-#include <string>
+#if LLVM_VERSION_CODE >= LLVM_VERSION(3, 5)
+#include "llvm/IR/CallSite.h"
+#else
+#include "llvm/Support/CallSite.h"
+#endif
 
-namespace llvm {
-  class Function;
-  class Instruction;
-  class Module; 
-  class CallSite; 
-}
+#include <string>
 
 namespace klee {
- 
+  /// Load llvm module from a bitcode archive file.
+  llvm::Module *loadModule(llvm::LLVMContext &ctx,
+                           const std::string &path,
+                           std::string &errorMsg);
+
   /// Link a module with a specified bitcode archive.
-  llvm::Module *linkWithLibrary(llvm::Module *module, 
+  llvm::Module *linkWithLibrary(llvm::Module *module,
                                 const std::string &libraryName);
 
   /// Return the Function* target of a Call or Invoke instruction, or
diff --git a/lib/Module/ModuleUtil.cpp b/lib/Module/ModuleUtil.cpp
index c7f1c6d9..1642f6d7 100644
--- a/lib/Module/ModuleUtil.cpp
+++ b/lib/Module/ModuleUtil.cpp
@@ -19,7 +19,6 @@
 #endif
 
 #if LLVM_VERSION_CODE >= LLVM_VERSION(3, 3)
-#include "llvm/Bitcode/ReaderWriter.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
@@ -47,16 +46,17 @@
 #include "llvm/IR/AssemblyAnnotationWriter.h"
 #endif
 
-#include "llvm/Support/raw_ostream.h"
+#if LLVM_VERSION_CODE <= LLVM_VERSION(2, 9)
+// for llvm::error_code
+#include "llvm/Support/system_error.h"
+#endif
+
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Bitcode/ReaderWriter.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Path.h"
 
-#if LLVM_VERSION_CODE < LLVM_VERSION(3, 5)
-#include "llvm/Support/CallSite.h"
-#else
-#include "llvm/IR/CallSite.h"
-#endif
-
 #include <map>
 #include <set>
 #include <fstream>
@@ -399,7 +399,7 @@ static bool linkBCA(object::Archive* archive, Module* composite, std::string& er
 #endif
 
 
-Module *klee::linkWithLibrary(Module *module, 
+Module *klee::linkWithLibrary(Module *module,
                               const std::string &libraryName) {
   KLEE_DEBUG_WITH_TYPE("klee_linker", dbgs() << "Linking file " << libraryName << "\n");
 #if LLVM_VERSION_CODE >= LLVM_VERSION(3, 3)
@@ -519,11 +519,11 @@ Module *klee::linkWithLibrary(Module *module,
 
   llvm::sys::Path libraryPath(libraryName);
   bool native = false;
-    
+
   if (linker.LinkInFile(libraryPath, native)) {
     klee_error("Linking library %s failed", libraryName.c_str());
   }
-    
+
   return linker.releaseModule();
 #endif
 }
@@ -566,12 +566,12 @@ static bool valueIsOnlyCalled(const Value *v) {
       // Make sure the instruction is a call or invoke.
       CallSite cs(const_cast<Instruction *>(instr));
       if (!cs) return false;
-      
+
       // Make sure that the value is only the target of this call and
       // not an argument.
       if (cs.hasArgument(v))
         return false;
-    } else if (const llvm::ConstantExpr *ce = 
+    } else if (const llvm::ConstantExpr *ce =
                dyn_cast<llvm::ConstantExpr>(*it)) {
       if (ce->getOpcode()==Instruction::BitCast)
         if (valueIsOnlyCalled(ce))
@@ -592,3 +592,66 @@ static bool valueIsOnlyCalled(const Value *v) {
 bool klee::functionEscapes(const Function *f) {
   return !valueIsOnlyCalled(f);
 }
+
+#if LLVM_VERSION_CODE < LLVM_VERSION(3, 5)
+
+Module *klee::loadModule(LLVMContext &ctx, const std::string &path, std::string &errorMsg) {
+    OwningPtr<MemoryBuffer> bufferPtr;
+    error_code ec = MemoryBuffer::getFileOrSTDIN(path.c_str(), bufferPtr);
+    if (ec) {
+        errorMsg = ec.message();
+        return 0;
+    }
+
+    Module *module = getLazyBitcodeModule(bufferPtr.get(), ctx, &errorMsg);
+
+    if (!module) {
+        return 0;
+    }
+    if (module->MaterializeAllPermanently(&errorMsg)) {
+        delete module;
+        return 0;
+    }
+
+    // In the case of success LLVM will take ownership of the module.
+    // Therefore we need to take ownership away from the `bufferPtr` otherwise the
+    // allocated memory will be deleted twice.
+    bufferPtr.take();
+
+    errorMsg = "";
+    return module;
+}
+
+#else
+
+Module *klee::loadModule(LLVMContext &ctx, const std::string &path, std::string &errorMsg) {
+  auto buffer = MemoryBuffer::getFileOrSTDIN(path.c_str());
+  if (!buffer) {
+    errorMsg = buffer.getError().message().c_str();
+    return nullptr;
+  }
+
+#if LLVM_VERSION_CODE >= LLVM_VERSION(3, 6)
+  auto errorOrModule = getLazyBitcodeModule(std::move(buffer.get()), ctx);
+#else
+  auto errorOrModule = getLazyBitcodeModule(buffer->get(), ctx);
+#endif
+
+  if (!errorOrModule) {
+    errorMsg = errorOrModule.getError().message().c_str();
+    return nullptr;
+  }
+  // The module has taken ownership of the MemoryBuffer so release it
+  // from the std::unique_ptr
+  buffer->release();
+  auto module = *errorOrModule;
+
+  if (auto ec = module->materializeAllPermanently()) {
+    errorMsg = ec.message();
+    return nullptr;
+  }
+
+  errorMsg = "";
+  return module;
+}
+#endif
diff --git a/tools/klee/main.cpp b/tools/klee/main.cpp
index 94798dad..3662cbee 100644
--- a/tools/klee/main.cpp
+++ b/tools/klee/main.cpp
@@ -25,7 +25,6 @@
 
 #if LLVM_VERSION_CODE > LLVM_VERSION(3, 2)
 #include "llvm/IR/Constants.h"
-#include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
@@ -33,7 +32,6 @@
 #include "llvm/IR/LLVMContext.h"
 #else
 #include "llvm/Constants.h"
-#include "llvm/Module.h"
 #include "llvm/Type.h"
 #include "llvm/InstrTypes.h"
 #include "llvm/Instruction.h"
@@ -606,7 +604,7 @@ std::string KleeHandler::getRunTimeLibraryPath(const char *argv0) {
   {
     KLEE_DEBUG_WITH_TYPE("klee_runtime", llvm::dbgs() <<
                          "Using installed KLEE library runtime: ");
-    libDir = toolRoot.str().substr(0, 
+    libDir = toolRoot.str().substr(0,
                toolRoot.str().size() - strlen( KLEE_INSTALL_BIN_DIR ));
     llvm::sys::path::append(libDir, KLEE_INSTALL_RUNTIME_DIR);
   }
@@ -1235,56 +1233,13 @@ int main(int argc, char **argv, char **envp) {
   sys::SetInterruptFunction(interrupt_handle);
 
   // Load the bytecode...
-  std::string ErrorMsg;
+  std::string errorMsg;
   LLVMContext ctx;
-  Module *mainModule = 0;
-#if LLVM_VERSION_CODE < LLVM_VERSION(3, 5)
-  OwningPtr<MemoryBuffer> BufferPtr;
-  error_code ec=MemoryBuffer::getFileOrSTDIN(InputFile.c_str(), BufferPtr);
-  if (ec) {
+  Module *mainModule = klee::loadModule(ctx, InputFile, errorMsg);
+  if (!mainModule) {
     klee_error("error loading program '%s': %s", InputFile.c_str(),
-               ec.message().c_str());
-  }
-
-  mainModule = getLazyBitcodeModule(BufferPtr.get(), ctx, &ErrorMsg);
-
-  if (mainModule) {
-    if (mainModule->MaterializeAllPermanently(&ErrorMsg)) {
-      delete mainModule;
-      mainModule = 0;
-    }
+               errorMsg.c_str());
   }
-  if (!mainModule)
-    klee_error("error loading program '%s': %s", InputFile.c_str(),
-               ErrorMsg.c_str());
-#else
-  auto Buffer = MemoryBuffer::getFileOrSTDIN(InputFile.c_str());
-  if (!Buffer)
-    klee_error("error loading program '%s': %s", InputFile.c_str(),
-               Buffer.getError().message().c_str());
-
-#if LLVM_VERSION_CODE >= LLVM_VERSION(3, 6)
-  auto mainModuleOrError = getLazyBitcodeModule(std::move(Buffer.get()), ctx);
-#else
-  auto mainModuleOrError = getLazyBitcodeModule(Buffer->get(), ctx);
-#endif
-
-  if (!mainModuleOrError) {
-    klee_error("error loading program '%s': %s", InputFile.c_str(),
-               mainModuleOrError.getError().message().c_str());
-  }
-  else {
-    // The module has taken ownership of the MemoryBuffer so release it
-    // from the std::unique_ptr
-    Buffer->release();
-  }
-
-  mainModule = *mainModuleOrError;
-  if (auto ec = mainModule->materializeAllPermanently()) {
-    klee_error("error loading program '%s': %s", InputFile.c_str(),
-               ec.message().c_str());
-  }
-#endif
 
   if (WithPOSIXRuntime) {
     int r = initEnv(mainModule);
@@ -1578,12 +1533,6 @@ int main(int argc, char **argv, char **envp) {
 
   handler->getInfoStream() << stats.str();
 
-#if LLVM_VERSION_CODE < LLVM_VERSION(3, 5)
-  // FIXME: This really doesn't look right
-  // This is preventing the module from being
-  // deleted automatically
-  BufferPtr.take();
-#endif
   delete handler;
 
   return 0;