From 3ba4f6db04487929b2eecfc645fa9f4d8c54dfea Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 17 Jun 2016 15:16:41 +0200 Subject: [PATCH] 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 6d367a3be2ef..be8648ee3028 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 c881e79e6145..c983a3f71634 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 6869eb1eef04..b6c46ba4b00b 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 76091be3581e..4f23b5ec9c38 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)); }