miri engine: also check return type before calling function

6 files changed, 36 insertions, 2 deletions

diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs
index 3ff0034fbbe..e83b514c691 100644
--- a/src/librustc/ich/impls_ty.rs
+++ b/src/librustc/ich/impls_ty.rs
@@ -560,6 +560,10 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
                 a.hash_stable(hcx, hasher);
                 b.hash_stable(hcx, hasher)
             },
+            FunctionRetMismatch(a, b) => {
+                a.hash_stable(hcx, hasher);
+                b.hash_stable(hcx, hasher)
+            },
             NoMirFor(ref s) => s.hash_stable(hcx, hasher),
             UnterminatedCString(ptr) => ptr.hash_stable(hcx, hasher),
             PointerOutOfBounds {
diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs
index 566673857b9..fe466e247c9 100644
--- a/src/librustc/mir/interpret/error.rs
+++ b/src/librustc/mir/interpret/error.rs
@@ -186,6 +186,7 @@ pub enum EvalErrorKind<'tcx, O> {
 
     FunctionAbiMismatch(Abi, Abi),
     FunctionArgMismatch(Ty<'tcx>, Ty<'tcx>),
+    FunctionRetMismatch(Ty<'tcx>, Ty<'tcx>),
     FunctionArgCountMismatch,
     NoMirFor(String),
     UnterminatedCString(Pointer),
@@ -294,7 +295,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
         use self::EvalErrorKind::*;
         match *self {
             MachineError(ref inner) => inner,
-            FunctionAbiMismatch(..) | FunctionArgMismatch(..) | FunctionArgCountMismatch =>
+            FunctionAbiMismatch(..) | FunctionArgMismatch(..) | FunctionRetMismatch(..)
+            | FunctionArgCountMismatch =>
                 "tried to call a function through a function pointer of incompatible type",
             InvalidMemoryAccess =>
                 "tried to access memory through an invalid pointer",
@@ -470,6 +472,10 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
                 write!(f, "tried to call a function with argument of type {:?} \
                            passing data of type {:?}",
                     callee_ty, caller_ty),
+            FunctionRetMismatch(caller_ty, callee_ty) =>
+                write!(f, "tried to call a function with return type {:?} \
+                           passing return place of type {:?}",
+                    callee_ty, caller_ty),
             FunctionArgCountMismatch =>
                 write!(f, "tried to call a function with incorrect number of arguments"),
             BoundsCheck { ref len, ref index } =>
diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs
index 83a9491cf46..3f62883c6a5 100644
--- a/src/librustc/ty/structural_impls.rs
+++ b/src/librustc/ty/structural_impls.rs
@@ -492,6 +492,10 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
                 tcx.lift(&a)?,
                 tcx.lift(&b)?,
             ),
+            FunctionRetMismatch(a, b) => FunctionRetMismatch(
+                tcx.lift(&a)?,
+                tcx.lift(&b)?,
+            ),
             FunctionArgCountMismatch => FunctionArgCountMismatch,
             NoMirFor(ref s) => NoMirFor(s.clone()),
             UnterminatedCString(ptr) => UnterminatedCString(ptr),
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index ff059e7d185..e15a721731e 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -231,6 +231,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     /// Mark a storage as live, killing the previous content and returning it.
     /// Remember to deallocate that!
     pub fn storage_live(&mut self, local: mir::Local) -> EvalResult<'tcx, LocalValue> {
+        assert!(local != mir::RETURN_PLACE, "Cannot make return place live");
         trace!("{:?} is now live", local);
 
         let layout = self.layout_of_local(self.cur_frame(), local)?;
@@ -242,6 +243,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
     /// Returns the old value of the local.
     /// Remember to deallocate that!
     pub fn storage_dead(&mut self, local: mir::Local) -> LocalValue {
+        assert!(local != mir::RETURN_PLACE, "Cannot make return place dead");
         trace!("{:?} is now dead", local);
 
         mem::replace(&mut self.frame_mut().locals[local], LocalValue::Dead)
@@ -446,6 +448,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc
             let dummy =
                 LocalValue::Live(Operand::Immediate(Value::Scalar(ScalarMaybeUndef::Undef)));
             let mut locals = IndexVec::from_elem(dummy, &mir.local_decls);
+            // Return place is handled specially by the `eval_place` functions, and the
+            // entry in `locals` should never be used. Make it dead, to be sure.
+            locals[mir::RETURN_PLACE] = LocalValue::Dead;
             // Now mark those locals as dead that we do not want to initialize
             match self.tcx.describe_def(instance.def_id()) {
                 // statics and constants don't have `Storage*` statements, no need to look for them
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
index c7ed69e0cb6..e82d60af639 100644
--- a/src/librustc_mir/interpret/terminator.rs
+++ b/src/librustc_mir/interpret/terminator.rs
@@ -287,7 +287,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
 
                 let return_place = match dest {
                     Some(place) => *place,
-                    None => Place::null(&self),
+                    None => Place::null(&self), // any access will error. good!
                 };
                 self.push_stack_frame(
                     instance,
@@ -373,6 +373,20 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                         trace!("Caller has too many args over");
                         return err!(FunctionArgCountMismatch);
                     }
+                    // Don't forget to check the return type!
+                    if let Some(caller_ret) = dest {
+                        let callee_ret = self.eval_place(&mir::Place::Local(mir::RETURN_PLACE))?;
+                        if !Self::check_argument_compat(caller_ret.layout, callee_ret.layout) {
+                            return err!(FunctionRetMismatch(
+                                caller_ret.layout.ty, callee_ret.layout.ty
+                            ));
+                        }
+                    } else {
+                        // FIXME: The caller thinks this function cannot return. How do
+                        // we verify that the callee agrees?
+                        // On the plus side, the the callee every writes to its return place,
+                        // that will be detected as UB (because we set that to NULL above).
+                    }
                     Ok(())
                 })();
                 match res {
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
index 70d50d589d1..626baf207ee 100644
--- a/src/librustc_mir/transform/const_prop.rs
+++ b/src/librustc_mir/transform/const_prop.rs
@@ -154,6 +154,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                     // FIXME: figure out the rules and start linting
                     | FunctionAbiMismatch(..)
                     | FunctionArgMismatch(..)
+                    | FunctionRetMismatch(..)
                     | FunctionArgCountMismatch
                     // fine at runtime, might be a register address or sth
                     | ReadBytesAsPointer