From 58b4fac1ceca3d75e59091b09e71a8914d6bd7e2 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 13:09:20 +0200 Subject: [PATCH 01/18] implement overflowing ops --- src/error.rs | 21 ++++++++++++++ src/interpreter/mod.rs | 47 ++++++++++++++++++++++-------- src/lib.rs | 1 + src/primval.rs | 66 +++++++++++++++++++++++++++++++++++++----- tests/compiletest.rs | 6 ++-- 5 files changed, 118 insertions(+), 23 deletions(-) diff --git a/src/error.rs b/src/error.rs index 35a978f2d2..885f5af632 100644 --- a/src/error.rs +++ b/src/error.rs @@ -3,6 +3,9 @@ use std::fmt; use rustc::mir::repr as mir; use rustc::ty::BareFnTy; use memory::Pointer; +use rustc_const_math::ConstMathErr; +use syntax::codemap::Span; +use primval::PrimVal; #[derive(Clone, Debug)] pub enum EvalError<'tcx> { @@ -24,6 +27,10 @@ pub enum EvalError<'tcx> { Unimplemented(String), DerefFunctionPointer, ExecuteMemory, + ArrayIndexOutOfBounds(Span, u64, u64), + Math(Span, ConstMathErr), + InvalidBitShiftRhs(PrimVal), + Overflow(PrimVal, PrimVal, mir::BinOp, PrimVal), } pub type EvalResult<'tcx, T> = Result>; @@ -58,6 +65,14 @@ impl<'tcx> Error for EvalError<'tcx> { "tried to dereference a function pointer", EvalError::ExecuteMemory => "tried to treat a memory pointer as a function pointer", + EvalError::ArrayIndexOutOfBounds(..) => + "array index out of bounds", + EvalError::Math(..) => + "mathematical operation failed", + EvalError::InvalidBitShiftRhs(..) => + "bit shift rhs negative or not an int", + EvalError::Overflow(..) => + "mathematical operation overflowed", } } @@ -73,6 +88,12 @@ impl<'tcx> fmt::Display for EvalError<'tcx> { }, EvalError::FunctionPointerTyMismatch(expected, got) => write!(f, "tried to call a function of type {:?} through a function pointer of type {:?}", expected, got), + EvalError::ArrayIndexOutOfBounds(span, len, index) => + write!(f, "array index {} out of bounds {} at {:?}", index, len, span), + EvalError::Math(span, ref err) => + write!(f, "mathematical operation at {:?} failed with {:?}", span, err), + EvalError::Overflow(l, r, op, val) => + write!(f, "mathematical operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val), _ => write!(f, "{}", self.description()), } } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 213f813529..6d367a3be2 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -474,15 +474,23 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.frame_mut().block = target; } - Assert { ref cond, expected, ref msg, target, cleanup } => { - let actual_ptr = self.eval_operand(cond)?; - let actual = self.memory.read_bool(actual_ptr)?; - if actual == expected { + Assert { ref cond, expected, ref msg, target, .. } => { + let cond_ptr = self.eval_operand(cond)?; + if expected == self.memory.read_bool(cond_ptr)? { self.frame_mut().block = target; } else { - panic!("unimplemented: jump to {:?} and print {:?}", cleanup, msg); + return match *msg { + mir::AssertMessage::BoundsCheck { ref len, ref index } => { + let len = self.eval_operand(len).expect("can't eval len"); + let len = self.memory.read_usize(len).expect("can't read len"); + let index = self.eval_operand(index).expect("can't eval index"); + let index = self.memory.read_usize(index).expect("can't read index"); + Err(EvalError::ArrayIndexOutOfBounds(terminator.source_info.span, len, index)) + }, + mir::AssertMessage::Math(ref err) => Err(EvalError::Math(terminator.source_info.span, err.clone())), + } } - } + }, DropAndReplace { .. } => unimplemented!(), Resume => unimplemented!(), @@ -922,13 +930,28 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let right_ty = self.operand_ty(right); let right_val = self.read_primval(right_ptr, right_ty)?; - let val = primval::binary_op(bin_op, left_val, right_val)?; - self.memory.write_primval(dest, val)?; + use rustc::ty::layout::Layout::*; + let tup_layout = match *dest_layout { + Univariant { ref variant, .. } => variant, + _ => panic!("checked bin op returns something other than a tuple"), + }; - // FIXME(solson): Find the result type size properly. Perhaps refactor out - // Projection calculations so we can do the equivalent of `dest.1` here. - let s = self.type_size(left_ty); - self.memory.write_bool(dest.offset(s as isize), false)?; + match primval::binary_op(bin_op, left_val, right_val) { + Ok(val) => { + let offset = tup_layout.field_offset(0).bytes() as isize; + self.memory.write_primval(dest.offset(offset), val)?; + let offset = tup_layout.field_offset(1).bytes() as isize; + self.memory.write_bool(dest.offset(offset), false)?; + }, + Err(EvalError::Overflow(l, r, op, val)) => { + debug!("mathematical operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val); + let offset = tup_layout.field_offset(0).bytes() as isize; + self.memory.write_primval(dest.offset(offset), val)?; + let offset = tup_layout.field_offset(1).bytes() as isize; + self.memory.write_bool(dest.offset(offset), true)?; + }, + Err(other) => return Err(other), + } } UnaryOp(un_op, ref operand) => { diff --git a/src/lib.rs b/src/lib.rs index 4bc5a07e3c..c881e79e61 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,6 +14,7 @@ extern crate rustc_data_structures; extern crate rustc_mir; extern crate rustc_trans; +extern crate rustc_const_math; extern crate syntax; #[macro_use] extern crate log; extern crate log_settings; diff --git a/src/primval.rs b/src/primval.rs index 5e1cdac45a..b1900874a9 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -17,21 +17,32 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva use rustc::mir::repr::BinOp::*; use self::PrimVal::*; + macro_rules! overflow { + ($v:ident, $v2:ident, $l:ident, $op:ident, $r:ident) => ({ + let (val, of) = $l.$op($r); + if of { + return Err(EvalError::Overflow($v($l), $v2($r), bin_op, $v(val))); + } else { + $v(val) + } + }) + } + macro_rules! int_binops { ($v:ident, $l:ident, $r:ident) => ({ match bin_op { - Add => $v($l + $r), - Sub => $v($l - $r), - Mul => $v($l * $r), - Div => $v($l / $r), - Rem => $v($l % $r), + Add => overflow!($v, $v, $l, overflowing_add, $r), + Sub => overflow!($v, $v, $l, overflowing_sub, $r), + Mul => overflow!($v, $v, $l, overflowing_mul, $r), + Div => overflow!($v, $v, $l, overflowing_div, $r), + Rem => overflow!($v, $v, $l, overflowing_rem, $r), BitXor => $v($l ^ $r), BitAnd => $v($l & $r), BitOr => $v($l | $r), - // TODO(solson): Can have differently-typed RHS. - Shl => $v($l << $r), - Shr => $v($l >> $r), + // these have already been handled + Shl => unreachable!(), + Shr => unreachable!(), Eq => Bool($l == $r), Ne => Bool($l != $r), @@ -53,6 +64,45 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva } } + match bin_op { + // can have rhs with a different numeric type + Shl | Shr => { + let r = match right { + I8(i) if i >= 0 => i as u32, + I16(i) if i >= 0 => i as u32, + I32(i) if i >= 0 => i as u32, + I64(i) if i >= 0 && i as i32 as i64 == i => i as u32, + U8(i) => i as u32, + U16(i) => i as u32, + U32(i) => i, + U64(i) if i as u32 as u64 == i => i as u32, + _ => return Err(EvalError::InvalidBitShiftRhs(right)), + }; + macro_rules! shift { + ($v:ident, $l:ident, $r:ident) => ({ + match bin_op { + Shl => overflow!($v, U32, $l, overflowing_shl, $r), + Shr => overflow!($v, U32, $l, overflowing_shr, $r), + _ => unreachable!(), + } + }) + } + let val = match left { + I8(l) => shift!(I8, l, r), + I16(l) => shift!(I16, l, r), + I32(l) => shift!(I32, l, r), + I64(l) => shift!(I64, l, r), + U8(l) => shift!(U8, l, r), + U16(l) => shift!(U16, l, r), + U32(l) => shift!(U32, l, r), + U64(l) => shift!(U64, l, r), + _ => unreachable!(), + }; + return Ok(val); + }, + _ => {}, + } + let val = match (left, right) { (I8(l), I8(r)) => int_binops!(I8, l, r), (I16(l), I16(r)) => int_binops!(I16, l, r), diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 66b94ad88d..6869eb1eef 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -79,11 +79,11 @@ fn compile_test() { writeln!(stderr.lock(), "FAILED: {}", e).unwrap(); }, } + if failed { + panic!("some tests failed"); + } } let stderr = std::io::stderr(); writeln!(stderr.lock(), "").unwrap(); }); - if failed { - panic!("some tests failed"); - } } From 3ba4f6db04487929b2eecfc645fa9f4d8c54dfea Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 15:16:41 +0200 Subject: [PATCH 02/18] remove code repetition and fix overflowing intrinsics --- src/interpreter/mod.rs | 191 +++++++++++++++++++++-------------------- src/lib.rs | 1 - tests/compiletest.rs | 8 +- tests/run-pass/ints.rs | 15 +--- 4 files changed, 102 insertions(+), 113 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 6d367a3be2..be8648ee30 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -515,9 +515,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let name = self.tcx.item_name(def_id).as_str(); match fn_ty.sig.0.output { ty::FnConverging(ty) => { - let size = self.type_size(ty); + let layout = self.type_layout(ty); let ret = return_ptr.unwrap(); - self.call_intrinsic(&name, substs, args, ret, size) + self.call_intrinsic(&name, substs, args, ret, layout) } ty::FnDiverging => unimplemented!(), } @@ -667,87 +667,126 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(if not_null { nndiscr } else { 1 - nndiscr }) } + fn intrinsic_with_overflow( + &mut self, + op: mir::BinOp, + left: &mir::Operand<'tcx>, + right: &mir::Operand<'tcx>, + dest: Pointer, + dest_layout: &'tcx Layout, + ) -> EvalResult<'tcx, ()> { + use rustc::ty::layout::Layout::*; + let tup_layout = match *dest_layout { + Univariant { ref variant, .. } => variant, + _ => panic!("checked bin op returns something other than a tuple"), + }; + + let overflowed = self.intrinsic_overflowing(op, left, right, dest)?; + let offset = tup_layout.field_offset(1).bytes() as isize; + self.memory.write_bool(dest.offset(offset), overflowed) + } + + fn math( + &mut self, + op: mir::BinOp, + left: &mir::Operand<'tcx>, + right: &mir::Operand<'tcx>, + ) -> EvalResult<'tcx, PrimVal> { + let left_ptr = self.eval_operand(left)?; + let left_ty = self.operand_ty(left); + let left_val = self.read_primval(left_ptr, left_ty)?; + + let right_ptr = self.eval_operand(right)?; + let right_ty = self.operand_ty(right); + let right_val = self.read_primval(right_ptr, right_ty)?; + + primval::binary_op(op, left_val, right_val) + } + + fn intrinsic_overflowing( + &mut self, + op: mir::BinOp, + left: &mir::Operand<'tcx>, + right: &mir::Operand<'tcx>, + dest: Pointer, + ) -> EvalResult<'tcx, bool> { + match self.math(op, left, right) { + Ok(val) => { + self.memory.write_primval(dest, val)?; + Ok(false) + }, + Err(EvalError::Overflow(l, r, op, val)) => { + debug!("operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val); + self.memory.write_primval(dest, val)?; + Ok(true) + }, + Err(other) => Err(other), + } + } + fn call_intrinsic( &mut self, name: &str, substs: &'tcx Substs<'tcx>, args: &[mir::Operand<'tcx>], dest: Pointer, - dest_size: usize + dest_layout: &'tcx Layout, ) -> EvalResult<'tcx, ()> { let args_res: EvalResult> = args.iter() .map(|arg| self.eval_operand(arg)) .collect(); - let args = args_res?; + let args_ptrs = args_res?; + + let pointer_size = self.memory.pointer_size; match name { - // FIXME(solson): Handle different integer types correctly. - "add_with_overflow" => { - let ty = *substs.types.get(subst::FnSpace, 0); - let size = self.type_size(ty); - let left = self.memory.read_int(args[0], size)?; - let right = self.memory.read_int(args[1], size)?; - let (n, overflowed) = unsafe { - ::std::intrinsics::add_with_overflow::(left, right) - }; - self.memory.write_int(dest, n, size)?; - self.memory.write_bool(dest.offset(size as isize), overflowed)?; - } + "add_with_overflow" => self.intrinsic_with_overflow(mir::BinOp::Add, &args[0], &args[1], dest, dest_layout)?, + "sub_with_overflow" => self.intrinsic_with_overflow(mir::BinOp::Sub, &args[0], &args[1], dest, dest_layout)?, + "mul_with_overflow" => self.intrinsic_with_overflow(mir::BinOp::Mul, &args[0], &args[1], dest, dest_layout)?, + // FIXME: turn into an assertion to catch wrong `assume` that would cause UB in llvm "assume" => {} "copy_nonoverlapping" => { let elem_ty = *substs.types.get(subst::FnSpace, 0); let elem_size = self.type_size(elem_ty); - let src = self.memory.read_ptr(args[0])?; - let dest = self.memory.read_ptr(args[1])?; - let count = self.memory.read_isize(args[2])?; + let src = self.memory.read_ptr(args_ptrs[0])?; + let dest = self.memory.read_ptr(args_ptrs[1])?; + let count = self.memory.read_isize(args_ptrs[2])?; self.memory.copy(src, dest, count as usize * elem_size)?; } "discriminant_value" => { let ty = *substs.types.get(subst::FnSpace, 0); - let adt_ptr = self.memory.read_ptr(args[0])?; + let adt_ptr = self.memory.read_ptr(args_ptrs[0])?; let discr_val = self.read_discriminant_value(adt_ptr, ty)?; - self.memory.write_uint(dest, discr_val, dest_size)?; + self.memory.write_uint(dest, discr_val, 8)?; } "forget" => { let arg_ty = *substs.types.get(subst::FnSpace, 0); let arg_size = self.type_size(arg_ty); - self.memory.drop_fill(args[0], arg_size)?; + self.memory.drop_fill(args_ptrs[0], arg_size)?; } - "init" => self.memory.write_repeat(dest, 0, dest_size)?, + "init" => self.memory.write_repeat(dest, 0, dest_layout.size(&self.tcx.data_layout).bytes() as usize)?, "min_align_of" => { - self.memory.write_int(dest, 1, dest_size)?; + // FIXME: use correct value + self.memory.write_int(dest, 1, pointer_size)?; } "move_val_init" => { let ty = *substs.types.get(subst::FnSpace, 0); - let ptr = self.memory.read_ptr(args[0])?; - self.move_(args[1], ptr, ty)?; - } - - // FIXME(solson): Handle different integer types correctly. - "mul_with_overflow" => { - let ty = *substs.types.get(subst::FnSpace, 0); - let size = self.type_size(ty); - let left = self.memory.read_int(args[0], size)?; - let right = self.memory.read_int(args[1], size)?; - let (n, overflowed) = unsafe { - ::std::intrinsics::mul_with_overflow::(left, right) - }; - self.memory.write_int(dest, n, size)?; - self.memory.write_bool(dest.offset(size as isize), overflowed)?; + let ptr = self.memory.read_ptr(args_ptrs[0])?; + self.move_(args_ptrs[1], ptr, ty)?; } "offset" => { let pointee_ty = *substs.types.get(subst::FnSpace, 0); let pointee_size = self.type_size(pointee_ty) as isize; - let ptr_arg = args[0]; - let offset = self.memory.read_isize(args[1])?; + let ptr_arg = args_ptrs[0]; + let offset = self.memory.read_isize(args_ptrs[1])?; match self.memory.read_ptr(ptr_arg) { Ok(ptr) => { @@ -763,35 +802,35 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } } - // FIXME(solson): Handle different integer types correctly. Use primvals? "overflowing_sub" => { - let ty = *substs.types.get(subst::FnSpace, 0); - let size = self.type_size(ty); - let left = self.memory.read_int(args[0], size)?; - let right = self.memory.read_int(args[1], size)?; - let n = left.wrapping_sub(right); - self.memory.write_int(dest, n, size)?; + self.intrinsic_overflowing(mir::BinOp::Sub, &args[0], &args[1], dest)?; + } + "overflowing_mul" => { + self.intrinsic_overflowing(mir::BinOp::Mul, &args[0], &args[1], dest)?; + } + "overflowing_add" => { + self.intrinsic_overflowing(mir::BinOp::Add, &args[0], &args[1], dest)?; } "size_of" => { let ty = *substs.types.get(subst::FnSpace, 0); let size = self.type_size(ty) as u64; - self.memory.write_uint(dest, size, dest_size)?; + self.memory.write_uint(dest, size, pointer_size)?; } "size_of_val" => { let ty = *substs.types.get(subst::FnSpace, 0); if self.type_is_sized(ty) { let size = self.type_size(ty) as u64; - self.memory.write_uint(dest, size, dest_size)?; + self.memory.write_uint(dest, size, pointer_size)?; } else { match ty.sty { ty::TySlice(_) | ty::TyStr => { let elem_ty = ty.sequence_element_type(self.tcx); let elem_size = self.type_size(elem_ty) as u64; let ptr_size = self.memory.pointer_size as isize; - let n = self.memory.read_usize(args[0].offset(ptr_size))?; - self.memory.write_uint(dest, n * elem_size, dest_size)?; + let n = self.memory.read_usize(args_ptrs[0].offset(ptr_size))?; + self.memory.write_uint(dest, n * elem_size, pointer_size)?; } _ => return Err(EvalError::Unimplemented(format!("unimplemented: size_of_val::<{:?}>", ty))), @@ -801,9 +840,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { "transmute" => { let ty = *substs.types.get(subst::FnSpace, 0); - self.move_(args[0], dest, ty)?; + self.move_(args_ptrs[0], dest, ty)?; } - "uninit" => self.memory.mark_definedness(dest, dest_size, false)?, + "uninit" => self.memory.mark_definedness(dest, dest_layout.size(&self.tcx.data_layout).bytes() as usize, false)?, name => return Err(EvalError::Unimplemented(format!("unimplemented intrinsic: {}", name))), } @@ -908,50 +947,12 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } BinaryOp(bin_op, ref left, ref right) => { - let left_ptr = self.eval_operand(left)?; - let left_ty = self.operand_ty(left); - let left_val = self.read_primval(left_ptr, left_ty)?; - - let right_ptr = self.eval_operand(right)?; - let right_ty = self.operand_ty(right); - let right_val = self.read_primval(right_ptr, right_ty)?; - - let val = primval::binary_op(bin_op, left_val, right_val)?; - self.memory.write_primval(dest, val)?; + let result = self.math(bin_op, left, right)?; + self.memory.write_primval(dest, result)?; } - // FIXME(solson): Factor this out with BinaryOp. CheckedBinaryOp(bin_op, ref left, ref right) => { - let left_ptr = self.eval_operand(left)?; - let left_ty = self.operand_ty(left); - let left_val = self.read_primval(left_ptr, left_ty)?; - - let right_ptr = self.eval_operand(right)?; - let right_ty = self.operand_ty(right); - let right_val = self.read_primval(right_ptr, right_ty)?; - - use rustc::ty::layout::Layout::*; - let tup_layout = match *dest_layout { - Univariant { ref variant, .. } => variant, - _ => panic!("checked bin op returns something other than a tuple"), - }; - - match primval::binary_op(bin_op, left_val, right_val) { - Ok(val) => { - let offset = tup_layout.field_offset(0).bytes() as isize; - self.memory.write_primval(dest.offset(offset), val)?; - let offset = tup_layout.field_offset(1).bytes() as isize; - self.memory.write_bool(dest.offset(offset), false)?; - }, - Err(EvalError::Overflow(l, r, op, val)) => { - debug!("mathematical operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val); - let offset = tup_layout.field_offset(0).bytes() as isize; - self.memory.write_primval(dest.offset(offset), val)?; - let offset = tup_layout.field_offset(1).bytes() as isize; - self.memory.write_bool(dest.offset(offset), true)?; - }, - Err(other) => return Err(other), - } + self.intrinsic_with_overflow(bin_op, left, right, dest, dest_layout)?; } UnaryOp(un_op, ref operand) => { diff --git a/src/lib.rs b/src/lib.rs index c881e79e61..c983a3f716 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,6 @@ btree_range, collections, collections_bound, - core_intrinsics, filling_drop, question_mark, rustc_private, diff --git a/tests/compiletest.rs b/tests/compiletest.rs index 6869eb1eef..b6c46ba4b0 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -38,7 +38,6 @@ fn for_all_targets(sysroot: &str, mut f: F) { #[test] fn compile_test() { - let mut failed = false; // Taken from https://github.com/Manishearth/rust-clippy/pull/911. let home = option_env!("RUSTUP_HOME").or(option_env!("MULTIRUST_HOME")); let toolchain = option_env!("RUSTUP_TOOLCHAIN").or(option_env!("MULTIRUST_TOOLCHAIN")); @@ -69,19 +68,16 @@ fn compile_test() { match cmd.output() { Ok(ref output) if output.status.success() => writeln!(stderr.lock(), "ok").unwrap(), Ok(output) => { - failed = true; writeln!(stderr.lock(), "FAILED with exit code {}", output.status.code().unwrap_or(0)).unwrap(); writeln!(stderr.lock(), "stdout: \n {}", std::str::from_utf8(&output.stdout).unwrap()).unwrap(); writeln!(stderr.lock(), "stderr: \n {}", std::str::from_utf8(&output.stderr).unwrap()).unwrap(); + panic!("some tests failed"); } Err(e) => { - failed = true; writeln!(stderr.lock(), "FAILED: {}", e).unwrap(); + panic!("some tests failed"); }, } - if failed { - panic!("some tests failed"); - } } let stderr = std::io::stderr(); writeln!(stderr.lock(), "").unwrap(); diff --git a/tests/run-pass/ints.rs b/tests/run-pass/ints.rs index 76091be358..4f23b5ec9c 100644 --- a/tests/run-pass/ints.rs +++ b/tests/run-pass/ints.rs @@ -1,34 +1,25 @@ -#![feature(custom_attribute)] -#![allow(dead_code, unused_attributes)] - -#[miri_run] fn ret() -> i64 { 1 } -#[miri_run] fn neg() -> i64 { -1 } -#[miri_run] fn add() -> i64 { 1 + 2 } -#[miri_run] fn indirect_add() -> i64 { let x = 1; let y = 2; x + y } -#[miri_run] fn arith() -> i32 { 3*3 + 4*4 } -#[miri_run] fn match_int() -> i16 { let n = 2; match n { @@ -40,7 +31,6 @@ fn match_int() -> i16 { } } -#[miri_run] fn match_int_range() -> i64 { let n = 42; match n { @@ -53,7 +43,6 @@ fn match_int_range() -> i64 { } } -#[miri_run] fn main() { assert_eq!(ret(), 1); assert_eq!(neg(), -1); @@ -62,4 +51,8 @@ fn main() { assert_eq!(arith(), 5*5); assert_eq!(match_int(), 20); assert_eq!(match_int_range(), 4); + assert_eq!(i64::min_value().overflowing_mul(-1), (i64::min_value(), true)); + assert_eq!(i32::min_value().overflowing_mul(-1), (i32::min_value(), true)); + assert_eq!(i16::min_value().overflowing_mul(-1), (i16::min_value(), true)); + assert_eq!(i8::min_value().overflowing_mul(-1), (i8::min_value(), true)); } From 6376ef42286d83aed3dbb8778db98abf1ef0a39a Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 15:21:01 +0200 Subject: [PATCH 03/18] run the *compiled* run-pass tests on the host machine --- tests/compiletest.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index b6c46ba4b0..e57869766b 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -3,23 +3,32 @@ extern crate compiletest_rs as compiletest; use std::path::{PathBuf, Path}; use std::io::Write; -fn run_mode(dir: &'static str, mode: &'static str, sysroot: &str) { +fn compile_fail(sysroot: &str) { // Disable rustc's new error fomatting. It breaks these tests. std::env::remove_var("RUST_NEW_ERROR_FORMAT"); let flags = format!("--sysroot {} -Dwarnings", sysroot); for_all_targets(sysroot, |target| { let mut config = compiletest::default_config(); config.host_rustcflags = Some(flags.clone()); - config.mode = mode.parse().expect("Invalid mode"); + config.mode = "compile-fail".parse().expect("Invalid mode"); config.run_lib_path = Path::new(sysroot).join("lib").join("rustlib").join(&target).join("lib"); config.rustc_path = "target/debug/miri".into(); - config.src_base = PathBuf::from(format!("tests/{}", dir)); + config.src_base = PathBuf::from("tests/compile-fail".to_string()); config.target = target.to_owned(); config.target_rustcflags = Some(flags.clone()); compiletest::run_tests(&config); }); } +fn run_pass() { + // Disable rustc's new error fomatting. It breaks these tests. + std::env::remove_var("RUST_NEW_ERROR_FORMAT"); + let mut config = compiletest::default_config(); + config.mode = "run-pass".parse().expect("Invalid mode"); + config.src_base = PathBuf::from("tests/run-pass".to_string()); + compiletest::run_tests(&config); +} + fn for_all_targets(sysroot: &str, mut f: F) { for target in std::fs::read_dir(format!("{}/lib/rustlib/", sysroot)).unwrap() { let target = target.unwrap(); @@ -47,7 +56,8 @@ fn compile_test() { .expect("need to specify RUST_SYSROOT env var or use rustup or multirust") .to_owned(), }; - run_mode("compile-fail", "compile-fail", &sysroot); + compile_fail(&sysroot); + run_pass(); for_all_targets(&sysroot, |target| { for file in std::fs::read_dir("tests/run-pass").unwrap() { let file = file.unwrap(); From 4f48bef8969120da73154ceb7bf16aa07dc94657 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 15:48:15 +0200 Subject: [PATCH 04/18] cfail test for std::env::args() --- tests/compile-fail/env_args.rs | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/compile-fail/env_args.rs diff --git a/tests/compile-fail/env_args.rs b/tests/compile-fail/env_args.rs new file mode 100644 index 0000000000..b0068785e1 --- /dev/null +++ b/tests/compile-fail/env_args.rs @@ -0,0 +1,6 @@ +//error-pattern: no mir for `std + +fn main() { + let x = std::env::args(); + assert_eq!(x.count(), 1); +} From e3a2bf84e26f0be966297d75b47b9d121feb3bf5 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 16:03:11 +0200 Subject: [PATCH 05/18] clippy --- src/interpreter/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index be8648ee30..abc695de4d 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1127,9 +1127,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // Hack to support fat pointer -> thin pointer casts to keep tests for // other things passing for now. - let is_fat_ptr_cast = pointee_type(src_ty).map(|ty| { - !self.type_is_sized(ty) - }).unwrap_or(false); + let is_fat_ptr_cast = pointee_type(src_ty).map_or(false, |ty| !self.type_is_sized(ty)); if dest_size == src_size || is_fat_ptr_cast { self.memory.copy(src, dest, dest_size)?; From 00eb198a82376eeb608c32e6d4252743f6dcfc87 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 16:49:06 +0200 Subject: [PATCH 06/18] implement fn -> unsafe fn pointer casts --- src/interpreter/mod.rs | 11 ++++++++++- tests/compile-fail/cast_fn_ptr_unsafe.rs | 10 ++++++++++ tests/compile-fail/cast_fn_ptr_unsafe2.rs | 10 ++++++++++ tests/run-pass/cast_fn_ptr_unsafe.rs | 8 ++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/compile-fail/cast_fn_ptr_unsafe.rs create mode 100644 tests/compile-fail/cast_fn_ptr_unsafe2.rs create mode 100644 tests/run-pass/cast_fn_ptr_unsafe.rs diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index abc695de4d..7ae0867522 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1144,7 +1144,16 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { ref other => panic!("reify fn pointer on {:?}", other), }, - _ => return Err(EvalError::Unimplemented(format!("can't handle cast: {:?}", rvalue))), + UnsafeFnPointer => match dest_ty.sty { + ty::TyFnPtr(unsafe_fn_ty) => { + let src = self.eval_operand(operand)?; + let ptr = self.memory.read_ptr(src)?; + let fn_def = self.memory.get_fn(ptr.alloc_id)?; + let fn_ptr = self.memory.create_fn_ptr(fn_def.def_id, fn_def.substs, unsafe_fn_ty); + self.memory.write_ptr(dest, fn_ptr)?; + }, + ref other => panic!("fn to unsafe fn cast on {:?}", other), + }, } } diff --git a/tests/compile-fail/cast_fn_ptr_unsafe.rs b/tests/compile-fail/cast_fn_ptr_unsafe.rs new file mode 100644 index 0000000000..225cd1391b --- /dev/null +++ b/tests/compile-fail/cast_fn_ptr_unsafe.rs @@ -0,0 +1,10 @@ +// just making sure that fn -> unsafe fn casts are handled by rustc so miri doesn't have to +fn main() { + fn f() {} + + let g = f as fn() as unsafe fn(i32); //~ERROR: non-scalar cast: `fn()` as `unsafe fn(i32)` + + unsafe { + g(42); + } +} diff --git a/tests/compile-fail/cast_fn_ptr_unsafe2.rs b/tests/compile-fail/cast_fn_ptr_unsafe2.rs new file mode 100644 index 0000000000..c3a2fb9556 --- /dev/null +++ b/tests/compile-fail/cast_fn_ptr_unsafe2.rs @@ -0,0 +1,10 @@ +// just making sure that fn -> unsafe fn casts are handled by rustc so miri doesn't have to +fn main() { + fn f() {} + + let g = f as fn() as fn(i32) as unsafe fn(i32); //~ERROR: non-scalar cast: `fn()` as `fn(i32)` + + unsafe { + g(42); + } +} diff --git a/tests/run-pass/cast_fn_ptr_unsafe.rs b/tests/run-pass/cast_fn_ptr_unsafe.rs new file mode 100644 index 0000000000..0cabb369bf --- /dev/null +++ b/tests/run-pass/cast_fn_ptr_unsafe.rs @@ -0,0 +1,8 @@ +fn main() { + fn f() {} + + let g = f as fn() as unsafe fn(); + unsafe { + g(); + } +} From 874d683bfa1b916cb9e3bc57d132a62afdd411fe Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 10:34:34 +0200 Subject: [PATCH 07/18] improve method names and add documentation --- src/interpreter/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 7ae0867522..0492a3dfc7 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -667,6 +667,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(if not_null { nndiscr } else { 1 - nndiscr }) } + /// applies the binary operation `op` to the two operands and writes a tuple of the result + /// and a boolean signifying the potential overflow to the destination fn intrinsic_with_overflow( &mut self, op: mir::BinOp, @@ -686,7 +688,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.memory.write_bool(dest.offset(offset), overflowed) } - fn math( + /// extracts the lhs and rhs primval from the operands and applies the binary op + fn eval_binop( &mut self, op: mir::BinOp, left: &mir::Operand<'tcx>, @@ -703,6 +706,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { primval::binary_op(op, left_val, right_val) } + /// applies the binary operation `op` to the arguments and writes the result to the destination fn intrinsic_overflowing( &mut self, op: mir::BinOp, @@ -710,7 +714,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { right: &mir::Operand<'tcx>, dest: Pointer, ) -> EvalResult<'tcx, bool> { - match self.math(op, left, right) { + match self.eval_binop(op, left, right) { Ok(val) => { self.memory.write_primval(dest, val)?; Ok(false) @@ -947,7 +951,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } BinaryOp(bin_op, ref left, ref right) => { - let result = self.math(bin_op, left, right)?; + let result = self.eval_binop(bin_op, left, right)?; self.memory.write_primval(dest, result)?; } From d9776427b465283bb2368be0301bf0e5cd1459eb Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 10:34:55 +0200 Subject: [PATCH 08/18] compiletest 2.0 uses json errors and doesn't depend on the output format anymore --- tests/compiletest.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index e57869766b..ffe35ad7e4 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -4,8 +4,6 @@ use std::path::{PathBuf, Path}; use std::io::Write; fn compile_fail(sysroot: &str) { - // Disable rustc's new error fomatting. It breaks these tests. - std::env::remove_var("RUST_NEW_ERROR_FORMAT"); let flags = format!("--sysroot {} -Dwarnings", sysroot); for_all_targets(sysroot, |target| { let mut config = compiletest::default_config(); @@ -21,8 +19,6 @@ fn compile_fail(sysroot: &str) { } fn run_pass() { - // Disable rustc's new error fomatting. It breaks these tests. - std::env::remove_var("RUST_NEW_ERROR_FORMAT"); let mut config = compiletest::default_config(); config.mode = "run-pass".parse().expect("Invalid mode"); config.src_base = PathBuf::from("tests/run-pass".to_string()); From e90ee1674ae2fd32bc5e07a3a52562c45b273c56 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 10:35:15 +0200 Subject: [PATCH 09/18] fix comparing of function pointers --- src/interpreter/mod.rs | 4 ++++ src/memory.rs | 26 ++++++++++++++++++-------- src/primval.rs | 14 +++++++++++++- tests/run-pass/function_pointers.rs | 2 ++ 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 0492a3dfc7..739e987a37 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -1404,6 +1404,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { (8, &ty::TyUint(UintTy::Us)) | (_, &ty::TyUint(UintTy::U64)) => PrimVal::U64(self.memory.read_uint(ptr, 8)? as u64), + (_, &ty::TyFnDef(def_id, substs, fn_ty)) => { + PrimVal::FnPtr(self.memory.create_fn_ptr(def_id, substs, fn_ty)) + }, + (_, &ty::TyFnPtr(_)) => self.memory.read_ptr(ptr).map(PrimVal::FnPtr)?, (_, &ty::TyRef(_, ty::TypeAndMut { ty, .. })) | (_, &ty::TyRawPtr(ty::TypeAndMut { ty, .. })) => { if self.type_is_sized(ty) { diff --git a/src/memory.rs b/src/memory.rs index e20d92207d..a7fecd773c 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -42,7 +42,7 @@ impl Pointer { } } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] pub struct FunctionDefinition<'tcx> { pub def_id: DefId, pub substs: &'tcx Substs<'tcx>, @@ -59,6 +59,8 @@ pub struct Memory<'tcx> { /// Function "allocations". They exist solely so pointers have something to point to, and /// we can figure out what they point to. functions: HashMap>, + /// Inverse map of `functions` so we don't allocate a new pointer every time we need one + function_definitions: HashMap, AllocId>, next_id: AllocId, pub pointer_size: usize, } @@ -69,22 +71,29 @@ impl<'tcx> Memory<'tcx> { Memory { alloc_map: HashMap::new(), functions: HashMap::new(), + function_definitions: HashMap::new(), next_id: AllocId(0), pointer_size: pointer_size, } } - // FIXME: never create two pointers to the same def_id + substs combination - // maybe re-use the statics cache of the EvalContext? pub fn create_fn_ptr(&mut self, def_id: DefId, substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy<'tcx>) -> Pointer { - let id = self.next_id; - debug!("creating fn ptr: {}", id); - self.next_id.0 += 1; - self.functions.insert(id, FunctionDefinition { + let def = FunctionDefinition { def_id: def_id, substs: substs, fn_ty: fn_ty, - }); + }; + if let Some(&alloc_id) = self.function_definitions.get(&def) { + return Pointer { + alloc_id: alloc_id, + offset: 0, + }; + } + let id = self.next_id; + debug!("creating fn ptr: {}", id); + self.next_id.0 += 1; + self.functions.insert(id, def); + self.function_definitions.insert(def, id); Pointer { alloc_id: id, offset: 0, @@ -361,6 +370,7 @@ impl<'tcx> Memory<'tcx> { PrimVal::U32(n) => self.write_uint(ptr, n as u64, 4), PrimVal::U64(n) => self.write_uint(ptr, n as u64, 8), PrimVal::IntegerPtr(n) => self.write_uint(ptr, n as u64, pointer_size), + PrimVal::FnPtr(_p) | PrimVal::AbstractPtr(_p) => unimplemented!(), } } diff --git a/src/primval.rs b/src/primval.rs index b1900874a9..67ef58948f 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -10,6 +10,7 @@ pub enum PrimVal { U8(u8), U16(u16), U32(u32), U64(u64), AbstractPtr(Pointer), + FnPtr(Pointer), IntegerPtr(u64), } @@ -130,9 +131,20 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva (IntegerPtr(l), IntegerPtr(r)) => int_binops!(IntegerPtr, l, r), - (AbstractPtr(_), IntegerPtr(_)) | (IntegerPtr(_), AbstractPtr(_)) => + (AbstractPtr(_), IntegerPtr(_)) | + (IntegerPtr(_), AbstractPtr(_)) | + (FnPtr(_), AbstractPtr(_)) | + (AbstractPtr(_), FnPtr(_)) | + (FnPtr(_), IntegerPtr(_)) | + (IntegerPtr(_), FnPtr(_)) => return unrelated_ptr_ops(bin_op), + (FnPtr(l_ptr), FnPtr(r_ptr)) => match bin_op { + Eq => Bool(l_ptr == r_ptr), + Ne => Bool(l_ptr != r_ptr), + _ => return Err(EvalError::Unimplemented(format!("unimplemented fn ptr comparison: {:?}", bin_op))), + }, + (AbstractPtr(l_ptr), AbstractPtr(r_ptr)) => { if l_ptr.alloc_id != r_ptr.alloc_id { return unrelated_ptr_ops(bin_op); diff --git a/tests/run-pass/function_pointers.rs b/tests/run-pass/function_pointers.rs index 8361a58ea4..2e75a5a3ea 100644 --- a/tests/run-pass/function_pointers.rs +++ b/tests/run-pass/function_pointers.rs @@ -12,4 +12,6 @@ fn call_fn_ptr() -> i32 { fn main() { assert_eq!(call_fn_ptr(), 42); + assert!(return_fn_ptr() == f); + assert!(return_fn_ptr() as unsafe fn() -> i32 == f as fn() -> i32 as unsafe fn() -> i32); } From ed4af21605ca5939a8d45f52975632be95cd210a Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 12:33:54 +0200 Subject: [PATCH 10/18] fix master --- tests/run-pass/specialization.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/run-pass/specialization.rs b/tests/run-pass/specialization.rs index 9570b5b58e..13894926d3 100644 --- a/tests/run-pass/specialization.rs +++ b/tests/run-pass/specialization.rs @@ -1,3 +1,5 @@ +#![feature(specialization)] + trait IsUnit { fn is_unit() -> bool; } From b9ac85d2a94e94dd5c81d5988b0adf60c1868e0c Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 16:52:43 +0200 Subject: [PATCH 11/18] rustc does overflow checking for us, don't duplicate it. --- src/error.rs | 5 ----- src/interpreter/mod.rs | 23 ++++++++--------------- src/primval.rs | 13 +++++++------ 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/src/error.rs b/src/error.rs index 885f5af632..3962c09dee 100644 --- a/src/error.rs +++ b/src/error.rs @@ -30,7 +30,6 @@ pub enum EvalError<'tcx> { ArrayIndexOutOfBounds(Span, u64, u64), Math(Span, ConstMathErr), InvalidBitShiftRhs(PrimVal), - Overflow(PrimVal, PrimVal, mir::BinOp, PrimVal), } pub type EvalResult<'tcx, T> = Result>; @@ -71,8 +70,6 @@ impl<'tcx> Error for EvalError<'tcx> { "mathematical operation failed", EvalError::InvalidBitShiftRhs(..) => "bit shift rhs negative or not an int", - EvalError::Overflow(..) => - "mathematical operation overflowed", } } @@ -92,8 +89,6 @@ impl<'tcx> fmt::Display for EvalError<'tcx> { write!(f, "array index {} out of bounds {} at {:?}", index, len, span), EvalError::Math(span, ref err) => write!(f, "mathematical operation at {:?} failed with {:?}", span, err), - EvalError::Overflow(l, r, op, val) => - write!(f, "mathematical operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val), _ => write!(f, "{}", self.description()), } } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index eaadfbd1df..4815421f24 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -688,13 +688,14 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.memory.write_bool(dest.offset(offset), overflowed) } - /// extracts the lhs and rhs primval from the operands and applies the binary op + /// Extracts the lhs and rhs primval from the operands and applies the binary op. + /// Returns the result and whether the operation overflowed fn eval_binop( &mut self, op: mir::BinOp, left: &mir::Operand<'tcx>, right: &mir::Operand<'tcx>, - ) -> EvalResult<'tcx, PrimVal> { + ) -> EvalResult<'tcx, (PrimVal, bool)> { let left_ptr = self.eval_operand(left)?; let left_ty = self.operand_ty(left); let left_val = self.read_primval(left_ptr, left_ty)?; @@ -714,18 +715,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { right: &mir::Operand<'tcx>, dest: Pointer, ) -> EvalResult<'tcx, bool> { - match self.eval_binop(op, left, right) { - Ok(val) => { - self.memory.write_primval(dest, val)?; - Ok(false) - }, - Err(EvalError::Overflow(l, r, op, val)) => { - debug!("operation overflowed: {:?} {} {:?} => {:?}", l, op.to_hir_binop().as_str(), r, val); - self.memory.write_primval(dest, val)?; - Ok(true) - }, - Err(other) => Err(other), - } + let (val, overflow) = self.eval_binop(op, left, right)?; + self.memory.write_primval(dest, val)?; + Ok(overflow) } fn call_intrinsic( @@ -951,7 +943,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { } BinaryOp(bin_op, ref left, ref right) => { - let result = self.eval_binop(bin_op, left, right)?; + // ignore overflow bit, rustc inserts check branches for us + let result = self.eval_binop(bin_op, left, right)?.0; self.memory.write_primval(dest, result)?; } diff --git a/src/primval.rs b/src/primval.rs index 67ef58948f..673aea4244 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -14,7 +14,8 @@ pub enum PrimVal { IntegerPtr(u64), } -pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> EvalResult<'tcx, PrimVal> { +/// returns the result of the operation and whether the operation overflowed +pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> EvalResult<'tcx, (PrimVal, bool)> { use rustc::mir::repr::BinOp::*; use self::PrimVal::*; @@ -22,7 +23,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva ($v:ident, $v2:ident, $l:ident, $op:ident, $r:ident) => ({ let (val, of) = $l.$op($r); if of { - return Err(EvalError::Overflow($v($l), $v2($r), bin_op, $v(val))); + return Ok(($v(val), true)); } else { $v(val) } @@ -99,7 +100,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva U64(l) => shift!(U64, l, r), _ => unreachable!(), }; - return Ok(val); + return Ok((val, false)); }, _ => {}, } @@ -137,7 +138,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva (AbstractPtr(_), FnPtr(_)) | (FnPtr(_), IntegerPtr(_)) | (IntegerPtr(_), FnPtr(_)) => - return unrelated_ptr_ops(bin_op), + unrelated_ptr_ops(bin_op)?, (FnPtr(l_ptr), FnPtr(r_ptr)) => match bin_op { Eq => Bool(l_ptr == r_ptr), @@ -147,7 +148,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva (AbstractPtr(l_ptr), AbstractPtr(r_ptr)) => { if l_ptr.alloc_id != r_ptr.alloc_id { - return unrelated_ptr_ops(bin_op); + return Ok((unrelated_ptr_ops(bin_op)?, false)); } let l = l_ptr.offset; @@ -167,7 +168,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva (l, r) => return Err(EvalError::Unimplemented(format!("unimplemented binary op: {:?}, {:?}, {:?}", l, r, bin_op))), }; - Ok(val) + Ok((val, false)) } pub fn unary_op<'tcx>(un_op: mir::UnOp, val: PrimVal) -> EvalResult<'tcx, PrimVal> { From 68469be89bd7652cf12a0fe74fb6cd6c5915498d Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 16:52:53 +0200 Subject: [PATCH 12/18] rename function cache member --- src/memory.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/memory.rs b/src/memory.rs index a7fecd773c..be0cd0ef4f 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -60,7 +60,7 @@ pub struct Memory<'tcx> { /// we can figure out what they point to. functions: HashMap>, /// Inverse map of `functions` so we don't allocate a new pointer every time we need one - function_definitions: HashMap, AllocId>, + function_alloc_cache: HashMap, AllocId>, next_id: AllocId, pub pointer_size: usize, } @@ -71,7 +71,7 @@ impl<'tcx> Memory<'tcx> { Memory { alloc_map: HashMap::new(), functions: HashMap::new(), - function_definitions: HashMap::new(), + function_alloc_cache: HashMap::new(), next_id: AllocId(0), pointer_size: pointer_size, } @@ -83,7 +83,7 @@ impl<'tcx> Memory<'tcx> { substs: substs, fn_ty: fn_ty, }; - if let Some(&alloc_id) = self.function_definitions.get(&def) { + if let Some(&alloc_id) = self.function_alloc_cache.get(&def) { return Pointer { alloc_id: alloc_id, offset: 0, @@ -93,7 +93,7 @@ impl<'tcx> Memory<'tcx> { debug!("creating fn ptr: {}", id); self.next_id.0 += 1; self.functions.insert(id, def); - self.function_definitions.insert(def, id); + self.function_alloc_cache.insert(def, id); Pointer { alloc_id: id, offset: 0, From 0821a15476ad85975445fdfde3bb8016b8d8c848 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 16:57:36 +0200 Subject: [PATCH 13/18] no need for EvalContext::eval_binop --- src/interpreter/mod.rs | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 4815421f24..0a8c82c5df 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -688,14 +688,15 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.memory.write_bool(dest.offset(offset), overflowed) } - /// Extracts the lhs and rhs primval from the operands and applies the binary op. - /// Returns the result and whether the operation overflowed - fn eval_binop( + /// Applies the binary operation `op` to the arguments and writes the result to the destination. + /// Returns `true` if the operation overflowed. + fn intrinsic_overflowing( &mut self, op: mir::BinOp, left: &mir::Operand<'tcx>, right: &mir::Operand<'tcx>, - ) -> EvalResult<'tcx, (PrimVal, bool)> { + dest: Pointer, + ) -> EvalResult<'tcx, bool> { let left_ptr = self.eval_operand(left)?; let left_ty = self.operand_ty(left); let left_val = self.read_primval(left_ptr, left_ty)?; @@ -704,18 +705,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let right_ty = self.operand_ty(right); let right_val = self.read_primval(right_ptr, right_ty)?; - primval::binary_op(op, left_val, right_val) - } - - /// applies the binary operation `op` to the arguments and writes the result to the destination - fn intrinsic_overflowing( - &mut self, - op: mir::BinOp, - left: &mir::Operand<'tcx>, - right: &mir::Operand<'tcx>, - dest: Pointer, - ) -> EvalResult<'tcx, bool> { - let (val, overflow) = self.eval_binop(op, left, right)?; + let (val, overflow) = primval::binary_op(op, left_val, right_val)?; self.memory.write_primval(dest, val)?; Ok(overflow) } @@ -944,8 +934,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { BinaryOp(bin_op, ref left, ref right) => { // ignore overflow bit, rustc inserts check branches for us - let result = self.eval_binop(bin_op, left, right)?.0; - self.memory.write_primval(dest, result)?; + self.intrinsic_overflowing(bin_op, left, right, dest)?; } CheckedBinaryOp(bin_op, ref left, ref right) => { From 3e3aeab0ed29e7da7aa151465d5ed63d6689069b Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 17:16:45 +0200 Subject: [PATCH 14/18] implement bit masks as the compiler would translate them --- src/error.rs | 2 +- src/primval.rs | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3962c09dee..9e5e899746 100644 --- a/src/error.rs +++ b/src/error.rs @@ -69,7 +69,7 @@ impl<'tcx> Error for EvalError<'tcx> { EvalError::Math(..) => "mathematical operation failed", EvalError::InvalidBitShiftRhs(..) => - "bit shift rhs negative or not an int", + "bit shift rhs not an int", } } diff --git a/src/primval.rs b/src/primval.rs index 673aea4244..586aaeee4e 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -69,15 +69,26 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva match bin_op { // can have rhs with a different numeric type Shl | Shr => { + let mask_bits = match left { + I8(_) => 3, + I16(_) => 4, + I32(_) => 5, + I64(_) => 6, + U8(_) => 3, + U16(_) => 4, + U32(_) => 5, + U64(_) => 6, + _ => unreachable!(), + }; let r = match right { - I8(i) if i >= 0 => i as u32, - I16(i) if i >= 0 => i as u32, - I32(i) if i >= 0 => i as u32, - I64(i) if i >= 0 && i as i32 as i64 == i => i as u32, - U8(i) => i as u32, - U16(i) => i as u32, - U32(i) => i, - U64(i) if i as u32 as u64 == i => i as u32, + I8(i) => (i & ((1 << mask_bits) - 1)) as u32, + I16(i) => (i & ((1 << mask_bits) - 1)) as u32, + I32(i) => (i & ((1 << mask_bits) - 1)) as u32, + I64(i) => (i & ((1 << mask_bits) - 1)) as u32, + U8(i) => (i & ((1 << mask_bits) - 1)) as u32, + U16(i) => (i & ((1 << mask_bits) - 1)) as u32, + U32(i) => (i & ((1 << mask_bits) - 1)) as u32, + U64(i) => (i & ((1 << mask_bits) - 1)) as u32, _ => return Err(EvalError::InvalidBitShiftRhs(right)), }; macro_rules! shift { From a088f105aa2e099ba027e47f9bc29c30b06b416e Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 17:52:14 +0200 Subject: [PATCH 15/18] add a comment explaining the magic numbers --- src/primval.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/primval.rs b/src/primval.rs index 586aaeee4e..29d79abbbf 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -69,6 +69,8 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva match bin_op { // can have rhs with a different numeric type Shl | Shr => { + // these numbers are the maximum number of bits a bitshift rhs could possibly have + // e.g. u16 can be bitshifted by 0..16, so 2^4 - 1 is the largest possible bitshift let mask_bits = match left { I8(_) => 3, I16(_) => 4, From 001ae69212a58766c007bf8464927e18fe406d0f Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 17:52:36 +0200 Subject: [PATCH 16/18] remove the bad rhs value error and panic instead. the typechecker prevent this --- src/error.rs | 4 ---- src/primval.rs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/error.rs b/src/error.rs index 9e5e899746..b19f63231a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -5,7 +5,6 @@ use rustc::ty::BareFnTy; use memory::Pointer; use rustc_const_math::ConstMathErr; use syntax::codemap::Span; -use primval::PrimVal; #[derive(Clone, Debug)] pub enum EvalError<'tcx> { @@ -29,7 +28,6 @@ pub enum EvalError<'tcx> { ExecuteMemory, ArrayIndexOutOfBounds(Span, u64, u64), Math(Span, ConstMathErr), - InvalidBitShiftRhs(PrimVal), } pub type EvalResult<'tcx, T> = Result>; @@ -68,8 +66,6 @@ impl<'tcx> Error for EvalError<'tcx> { "array index out of bounds", EvalError::Math(..) => "mathematical operation failed", - EvalError::InvalidBitShiftRhs(..) => - "bit shift rhs not an int", } } diff --git a/src/primval.rs b/src/primval.rs index 29d79abbbf..fbb9301288 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -91,7 +91,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva U16(i) => (i & ((1 << mask_bits) - 1)) as u32, U32(i) => (i & ((1 << mask_bits) - 1)) as u32, U64(i) => (i & ((1 << mask_bits) - 1)) as u32, - _ => return Err(EvalError::InvalidBitShiftRhs(right)), + _ => panic!("bad MIR: bitshift rhs is not integral"), }; macro_rules! shift { ($v:ident, $l:ident, $r:ident) => ({ From c7039dbb2b601cff4a929e01c4941b00becadab2 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 18:01:35 +0200 Subject: [PATCH 17/18] simplify the masked rhs computation --- src/primval.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/primval.rs b/src/primval.rs index fbb9301288..3399d697ed 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -82,17 +82,19 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva U64(_) => 6, _ => unreachable!(), }; + let mask = (1 << mask_bits) - 1; let r = match right { - I8(i) => (i & ((1 << mask_bits) - 1)) as u32, - I16(i) => (i & ((1 << mask_bits) - 1)) as u32, - I32(i) => (i & ((1 << mask_bits) - 1)) as u32, - I64(i) => (i & ((1 << mask_bits) - 1)) as u32, - U8(i) => (i & ((1 << mask_bits) - 1)) as u32, - U16(i) => (i & ((1 << mask_bits) - 1)) as u32, - U32(i) => (i & ((1 << mask_bits) - 1)) as u32, - U64(i) => (i & ((1 << mask_bits) - 1)) as u32, + I8(i) => i as u8 & mask, + I16(i) => i as u8 & mask, + I32(i) => i as u8 & mask, + I64(i) => i as u8 & mask, + U8(i) => i as u8 & mask, + U16(i) => i as u8 & mask, + U32(i) => i as u8 & mask, + U64(i) => i as u8 & mask, _ => panic!("bad MIR: bitshift rhs is not integral"), }; + let r = r as u32; macro_rules! shift { ($v:ident, $l:ident, $r:ident) => ({ match bin_op { From 65de5dd2d0fd66c5277787558c3af9421545af24 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 20 Jun 2016 18:15:33 +0200 Subject: [PATCH 18/18] simplify even more --- src/primval.rs | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/primval.rs b/src/primval.rs index 3399d697ed..6b17a63b65 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -69,32 +69,30 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva match bin_op { // can have rhs with a different numeric type Shl | Shr => { - // these numbers are the maximum number of bits a bitshift rhs could possibly have - // e.g. u16 can be bitshifted by 0..16, so 2^4 - 1 is the largest possible bitshift - let mask_bits = match left { - I8(_) => 3, - I16(_) => 4, - I32(_) => 5, - I64(_) => 6, - U8(_) => 3, - U16(_) => 4, - U32(_) => 5, - U64(_) => 6, + // these numbers are the maximum number a bitshift rhs could possibly have + // e.g. u16 can be bitshifted by 0..16, so masking with 0b1111 (16 - 1) will ensure we are in that range + let type_bits: u32 = match left { + I8(_) | U8(_) => 8, + I16(_) | U16(_) => 16, + I32(_) | U32(_) => 32, + I64(_) | U64(_) => 64, _ => unreachable!(), }; - let mask = (1 << mask_bits) - 1; + assert!(type_bits.is_power_of_two()); + // turn into `u32` because `overflowing_sh{l,r}` only take `u32` let r = match right { - I8(i) => i as u8 & mask, - I16(i) => i as u8 & mask, - I32(i) => i as u8 & mask, - I64(i) => i as u8 & mask, - U8(i) => i as u8 & mask, - U16(i) => i as u8 & mask, - U32(i) => i as u8 & mask, - U64(i) => i as u8 & mask, + I8(i) => i as u32, + I16(i) => i as u32, + I32(i) => i as u32, + I64(i) => i as u32, + U8(i) => i as u32, + U16(i) => i as u32, + U32(i) => i as u32, + U64(i) => i as u32, _ => panic!("bad MIR: bitshift rhs is not integral"), }; - let r = r as u32; + // apply mask + let r = r & (type_bits - 1); macro_rules! shift { ($v:ident, $l:ident, $r:ident) => ({ match bin_op {