Implement va_arg handling for x86_64.

 - Based on a patch by Vladimir Kuznetsov!

 - x86_64 has a complicated calling convention for va_args; instead of dealing
   with this, this patch uses a clever workaround by initializing the va_list
   structure so that the callee believes all arguments were passed in the stack
   save area.


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

4 files changed, 101 insertions, 24 deletions

diff --git a/lib/Core/Executor.cpp b/lib/Core/Executor.cpp
index 370495eb..8338cb92 100644
--- a/lib/Core/Executor.cpp
+++ b/lib/Core/Executor.cpp
@@ -1114,20 +1114,54 @@ void Executor::executeCall(ExecutionState &state,
         callExternalFunction(state, ki, f, arguments);
         break;
           
-        // vararg is handled by caller and intrinsic lowering,
-        // see comment for ExecutionState::varargs
+        // va_arg is handled by caller and intrinsic lowering, see comment for
+        // ExecutionState::varargs
       case Intrinsic::vastart:  {
         StackFrame &sf = state.stack.back();
         assert(sf.varargs && 
                "vastart called in function with no vararg object");
-        executeMemoryOperation(state, true, arguments[0], 
-                               sf.varargs->getBaseExpr(), 0);
+
+        // FIXME: This is really specific to the architecture, not the pointer
+        // size. This happens to work fir x86-32 and x86-64, however.
+        Expr::Width WordSize = Context::get().getPointerWidth();
+        if (WordSize == Expr::Int32) {
+          executeMemoryOperation(state, true, arguments[0], 
+                                 sf.varargs->getBaseExpr(), 0);
+        } else {
+          assert(WordSize == Expr::Int64 && "Unknown word size!");
+
+          // X86-64 has quite complicated calling convention. However,
+          // instead of implementing it, we can do a simple hack: just
+          // make a function believe that all varargs are on stack.
+          executeMemoryOperation(state, true, arguments[0],
+                                 ConstantExpr::create(48, 32), 0); // gp_offset
+          executeMemoryOperation(state, true,
+                                 AddExpr::create(arguments[0], 
+                                                 ConstantExpr::create(4, 64)),
+                                 ConstantExpr::create(304, 32), 0); // fp_offset
+          executeMemoryOperation(state, true,
+                                 AddExpr::create(arguments[0], 
+                                                 ConstantExpr::create(8, 64)),
+                                 sf.varargs->getBaseExpr(), 0); // overflow_arg_area
+          executeMemoryOperation(state, true,
+                                 AddExpr::create(arguments[0], 
+                                                 ConstantExpr::create(16, 64)),
+                                 ConstantExpr::create(0, 64), 0); // reg_save_area
+        }
         break;
       }
-      case Intrinsic::vaend:    // va_end is a noop for the interpreter
+      case Intrinsic::vaend:
+        // va_end is a noop for the interpreter.
+        //
+        // FIXME: We should validate that the target didn't do something bad
+        // with vaeend, however (like call it twice).
         break;
           
-      case Intrinsic::vacopy: // should be lowered
+      case Intrinsic::vacopy:
+        // va_copy should have been lowered.
+        //
+        // FIXME: It would be nice to check for errors in the usage of this as
+        // well.
       default:
         klee_error("unknown intrinsic: %s", f->getName().data());
       }
@@ -1137,16 +1171,19 @@ void Executor::executeCall(ExecutionState &state,
       transferToBasicBlock(ii->getNormalDest(), i->getParent(), state);
     }
   } else {
-    // XXX not really happy about this reliance on prevPC but is ok I
-    // guess. This just done to avoid having to pass KInstIterator
-    // everywhere instead of the actual instruction, since we can't
-    // make a KInstIterator from just an instruction (unlike LLVM).
+    // FIXME: I'm not really happy about this reliance on prevPC but it is ok, I
+    // guess. This just done to avoid having to pass KInstIterator everywhere
+    // instead of the actual instruction, since we can't make a KInstIterator
+    // from just an instruction (unlike LLVM).
     KFunction *kf = kmodule->functionMap[f];
     state.pushFrame(state.prevPC, kf);
     state.pc = kf->instructions;
         
     if (statsTracker)
       statsTracker->framePushed(state, &state.stack[state.stack.size()-2]);
+ 
+     // TODO: support "byval" parameter attribute
+     // TODO: support zeroext, signext, sret attributes
         
     unsigned callingArgs = arguments.size();
     unsigned funcArgs = f->arg_size();
@@ -1168,8 +1205,17 @@ void Executor::executeCall(ExecutionState &state,
             
       StackFrame &sf = state.stack.back();
       unsigned size = 0;
-      for (unsigned i = funcArgs; i < callingArgs; i++)
-        size += Expr::getMinBytesForWidth(arguments[i]->getWidth());
+      for (unsigned i = funcArgs; i < callingArgs; i++) {
+        // FIXME: This is really specific to the architecture, not the pointer
+        // size. This happens to work fir x86-32 and x86-64, however.
+        Expr::Width WordSize = Context::get().getPointerWidth();
+        if (WordSize == Expr::Int32) {
+          size += Expr::getMinBytesForWidth(arguments[i]->getWidth());
+        } else {
+          size += llvm::RoundUpToAlignment(arguments[i]->getWidth(), 
+                                           WordSize) / 8;
+        }
+      }
 
       MemoryObject *mo = sf.varargs = memory->allocate(size, true, false, 
                                                        state.prevPC->inst);
@@ -1180,9 +1226,18 @@ void Executor::executeCall(ExecutionState &state,
       ObjectState *os = bindObjectInState(state, mo, true);
       unsigned offset = 0;
       for (unsigned i = funcArgs; i < callingArgs; i++) {
-        // XXX: DRE: i think we bind memory objects here?
-        os->write(offset, arguments[i]);
-        offset += Expr::getMinBytesForWidth(arguments[i]->getWidth());
+        // FIXME: This is really specific to the architecture, not the pointer
+        // size. This happens to work fir x86-32 and x86-64, however.
+        Expr::Width WordSize = Context::get().getPointerWidth();
+        if (WordSize == Expr::Int32) {
+          os->write(offset, arguments[i]);
+          offset += Expr::getMinBytesForWidth(arguments[i]->getWidth());
+        } else {
+          assert(WordSize == Expr::Int64 && "Unknown word size!");
+          os->write(offset, arguments[i]);
+          offset += llvm::RoundUpToAlignment(arguments[i]->getWidth(), 
+                                             WordSize) / 8;
+        }
       }
     }
 
diff --git a/lib/Module/IntrinsicCleaner.cpp b/lib/Module/IntrinsicCleaner.cpp
index e59b7ff6..4f490e8e 100644
--- a/lib/Module/IntrinsicCleaner.cpp
+++ b/lib/Module/IntrinsicCleaner.cpp
@@ -33,13 +33,14 @@ bool IntrinsicCleanerPass::runOnModule(Module &M) {
   bool dirty = false;
   for (Module::iterator f = M.begin(), fe = M.end(); f != fe; ++f)
     for (Function::iterator b = f->begin(), be = f->end(); b != be; ++b)
-        dirty |= runOnBasicBlock(*b);
+      dirty |= runOnBasicBlock(*b);
   return dirty;
 }
 
 bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b) { 
   bool dirty = false;
-
+  
+  unsigned WordSize = TargetData.getPointerSizeInBits() / 8;
   for (BasicBlock::iterator i = b.begin(), ie = b.end(); i != ie;) {     
     IntrinsicInst *ii = dyn_cast<IntrinsicInst>(&*i);
     // increment now since LowerIntrinsic deletion makes iterator invalid.
@@ -51,15 +52,34 @@ bool IntrinsicCleanerPass::runOnBasicBlock(BasicBlock &b) {
         break;
         
         // Lower vacopy so that object resolution etc is handled by
-        // normal instructions.  FIXME: This is broken for non-x86_32.
+        // normal instructions.
+        //
+        // FIXME: This is much more target dependent than just the word size,
+        // however this works for x86-32 and x86-64.
       case Intrinsic::vacopy: { // (dst, src) -> *((i8**) dst) = *((i8**) src)
         Value *dst = ii->getOperand(1);
         Value *src = ii->getOperand(2);
-        Type *i8pp = PointerType::getUnqual(PointerType::getUnqual(Type::Int8Ty));
-        Value *castedDst = CastInst::CreatePointerCast(dst, i8pp, "vacopy.cast.dst", ii);
-        Value *castedSrc = CastInst::CreatePointerCast(src, i8pp, "vacopy.cast.src", ii);
-        Value *load = new LoadInst(castedSrc, "vacopy.read", ii);
-        new StoreInst(load, castedDst, false, ii);
+
+        if (WordSize == 4) {
+          Type *i8pp = PointerType::getUnqual(PointerType::getUnqual(Type::Int8Ty));
+          Value *castedDst = CastInst::CreatePointerCast(dst, i8pp, "vacopy.cast.dst", ii);
+          Value *castedSrc = CastInst::CreatePointerCast(src, i8pp, "vacopy.cast.src", ii);
+          Value *load = new LoadInst(castedSrc, "vacopy.read", ii);
+          new StoreInst(load, castedDst, false, ii);
+        } else {
+          assert(WordSize == 8 && "Invalid word size!");
+          Type *i64p = PointerType::getUnqual(Type::Int64Ty);
+          Value *pDst = CastInst::CreatePointerCast(dst, i64p, "vacopy.cast.dst", ii);
+          Value *pSrc = CastInst::CreatePointerCast(src, i64p, "vacopy.cast.src", ii);
+          Value *val = new LoadInst(pSrc, std::string(), ii); new StoreInst(val, pDst, ii);
+          Value *off = ConstantInt::get(Type::Int64Ty, 1);
+          pDst = GetElementPtrInst::Create(pDst, off, std::string(), ii);
+          pSrc = GetElementPtrInst::Create(pSrc, off, std::string(), ii);
+          val = new LoadInst(pSrc, std::string(), ii); new StoreInst(val, pDst, ii);
+          pDst = GetElementPtrInst::Create(pDst, off, std::string(), ii);
+          pSrc = GetElementPtrInst::Create(pSrc, off, std::string(), ii);
+          val = new LoadInst(pSrc, std::string(), ii); new StoreInst(val, pDst, ii);
+        }
         ii->removeFromParent();
         delete ii;
         break;
diff --git a/lib/Module/Passes.h b/lib/Module/Passes.h
index 23205f75..37a9ac8b 100644
--- a/lib/Module/Passes.h
+++ b/lib/Module/Passes.h
@@ -53,6 +53,7 @@ public:
   // variables (via intrinsic lowering).
 class IntrinsicCleanerPass : public llvm::ModulePass {
   static char ID;
+  const llvm::TargetData &TargetData;
   llvm::IntrinsicLowering *IL;
   bool LowerIntrinsics;
 
@@ -61,6 +62,7 @@ public:
   IntrinsicCleanerPass(const llvm::TargetData &TD,
                        bool LI=true)
     : llvm::ModulePass((intptr_t) &ID),
+      TargetData(TD),
       IL(new llvm::IntrinsicLowering(TD)),
       LowerIntrinsics(LI) {}
   ~IntrinsicCleanerPass() { delete IL; } 
diff --git a/test/Feature/Vararg.c b/test/Feature/Vararg.c
index f782c15e..9f6643bc 100644
--- a/test/Feature/Vararg.c
+++ b/test/Feature/Vararg.c
@@ -76,7 +76,7 @@ int main() {
   assert(va_array(5, 0, 5, 1, 1, 2, 1)==45); // 15 + 30
 
   // should give memory error
-  test1(-1, 52, 37, 2.0, p);
+  test1(-1, 52, 2.0, p);
 
   return 0;
 }