miri engine: also check return type before calling function

This commit is contained in:
Ralf Jung 2018-10-02 21:16:35 +02:00
parent d2b9b1de05
commit 616cb6356f
6 changed files with 36 additions and 2 deletions

View File

@ -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 {

View File

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

View File

@ -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),

View File

@ -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

View File

@ -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 {

View File

@ -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