miri engine: also check return type before calling function

Author: RalfJung

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 {

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 } =>

src/librustc/ty/structural_impls.rs

@@ -493,6 +493,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),

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

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 {

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