allocate memory objects for functions

Before, we reused the llvm::Function* value in the target program,
even though it stems from KLEE's own address space. This leads to
non-deterministic function pointers, even with --allocate-determ.
This issue was identified in the MoKLEE paper. Now, we allocate a
memory object per function, for its (potentially) deterministic
address. Mapping this address back to llvm::Functions is done by
the legalFunctions map.

Also, pointer width now depends on the target, not the host.

2 files changed, 15 insertions, 9 deletions

diff --git a/lib/Core/Executor.cpp b/lib/Core/Executor.cpp
index fb83c883..a81c4882 100644
--- a/lib/Core/Executor.cpp
+++ b/lib/Core/Executor.cpp
@@ -670,7 +670,7 @@ void Executor::initializeGlobals(ExecutionState &state) {
 }
 
 void Executor::allocateGlobalObjects(ExecutionState &state) {
-  const Module *m = kmodule->module.get();
+  Module *m = kmodule->module.get();
 
   if (m->getModuleInlineAsm() != "")
     klee_warning("executable has module level assembly (ignoring)");
@@ -678,7 +678,7 @@ void Executor::allocateGlobalObjects(ExecutionState &state) {
   // object. given that we use malloc to allocate memory in states this also
   // ensures that we won't conflict. we don't need to allocate a memory object
   // since reading/writing via a function pointer is unsupported anyway.
-  for (const Function &f : *m) {
+  for (Function &f : *m) {
     ref<ConstantExpr> addr;
 
     // If the symbol has external weak linkage then it is implicitly
@@ -688,8 +688,12 @@ void Executor::allocateGlobalObjects(ExecutionState &state) {
         !externalDispatcher->resolveSymbol(f.getName().str())) {
       addr = Expr::createPointer(0);
     } else {
-      addr = Expr::createPointer(reinterpret_cast<std::uint64_t>(&f));
-      legalFunctions.insert(reinterpret_cast<std::uint64_t>(&f));
+      // We allocate an object to represent each function,
+      // its address can be used for function pointers.
+      // TODO: Check whether the object is accessed?
+      auto mo = memory->allocate(8, false, true, &f, 8);
+      addr = Expr::createPointer(mo->address);
+      legalFunctions.emplace(mo->address, &f);
     }
 
     globalAddresses.emplace(&f, addr);
@@ -2462,8 +2466,9 @@ void Executor::executeInstruction(ExecutionState &state, KInstruction *ki) {
         StatePair res = fork(*free, EqExpr::create(v, value), true);
         if (res.first) {
           uint64_t addr = value->getZExtValue();
-          if (legalFunctions.count(addr)) {
-            f = (Function*) addr;
+          auto it = legalFunctions.find(addr);
+          if (it != legalFunctions.end()) {
+            f = it->second;
 
             // Don't give warning on unique resolution
             if (res.second || !first)
diff --git a/lib/Core/Executor.h b/lib/Core/Executor.h
index ae960731..987edf47 100644
--- a/lib/Core/Executor.h
+++ b/lib/Core/Executor.h
@@ -34,6 +34,7 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 struct KTest;
@@ -164,9 +165,9 @@ private:
   /// globals that have no representative object (i.e. functions).
   std::map<const llvm::GlobalValue*, ref<ConstantExpr> > globalAddresses;
 
-  /// The set of legal function addresses, used to validate function
-  /// pointers. We use the actual Function* address as the function address.
-  std::set<uint64_t> legalFunctions;
+  /// Map of legal function addresses to the corresponding Function.
+  /// Used to validate and dereference function pointers.
+  std::unordered_map<std::uint64_t, llvm::Function*> legalFunctions;
 
   /// When non-null the bindings that will be used for calls to
   /// klee_make_symbolic in order replay.