From 89b9b3536eab99a673ae42ef71fc0c6a5f1ff1ae Mon Sep 17 00:00:00 2001 From: Scott Olson Date: Mon, 19 Sep 2016 19:40:56 -0600 Subject: [PATCH 1/3] Remove more eval_operand_to_ptr. --- src/interpreter/mod.rs | 55 +++++++++++++++------------- src/interpreter/terminator/mod.rs | 59 ++++++++++++++++++------------- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 9338044e5341..f7d6b464cda3 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -779,31 +779,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // remove it as soon as PrimVal can represent fat pointers. fn eval_operand_to_ptr(&mut self, op: &mir::Operand<'tcx>) -> EvalResult<'tcx, Pointer> { let value = self.eval_operand(op)?; - match value { - Value::ByRef(ptr) => Ok(ptr), - - Value::ByVal(primval) => { - let ty = self.operand_ty(op); - let size = self.type_size(ty); - let align = self.type_align(ty); - let ptr = self.memory.allocate(size, align)?; - self.memory.write_primval(ptr, primval)?; - Ok(ptr) - } - - Value::ByValPair(primval1, primval2) => { - let ty = self.operand_ty(op); - let size = self.type_size(ty); - let align = self.type_align(ty); - let ptr = self.memory.allocate(size, align)?; - - // FIXME(solson): Major dangerous assumptions here. Ideally obliterate this - // function. - self.memory.write_primval(ptr, primval1)?; - self.memory.write_primval(ptr.offset((size / 2) as isize), primval2)?; - Ok(ptr) - } - } + let ty = self.operand_ty(op); + self.value_to_ptr(value, ty) } fn eval_operand_to_primval(&mut self, op: &mir::Operand<'tcx>) -> EvalResult<'tcx, PrimVal> { @@ -980,6 +957,34 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { Ok(()) } + // FIXME(solson): This method unnecessarily allocates and should not be necessary. We can + // remove it as soon as PrimVal can represent fat pointers. + fn value_to_ptr(&mut self, value: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, Pointer> { + match value { + Value::ByRef(ptr) => Ok(ptr), + + Value::ByVal(primval) => { + let size = self.type_size(ty); + let align = self.type_align(ty); + let ptr = self.memory.allocate(size, align)?; + self.memory.write_primval(ptr, primval)?; + Ok(ptr) + } + + Value::ByValPair(primval1, primval2) => { + let size = self.type_size(ty); + let align = self.type_align(ty); + let ptr = self.memory.allocate(size, align)?; + + // FIXME(solson): Major dangerous assumptions here. Ideally obliterate this + // function. + self.memory.write_primval(ptr, primval1)?; + self.memory.write_primval(ptr.offset((size / 2) as isize), primval2)?; + Ok(ptr) + } + } + } + fn value_to_primval(&mut self, value: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, PrimVal> { match value { Value::ByRef(ptr) => self.read_primval(ptr, ty), diff --git a/src/interpreter/terminator/mod.rs b/src/interpreter/terminator/mod.rs index 8f0375936ccf..b8d46323bfa8 100644 --- a/src/interpreter/terminator/mod.rs +++ b/src/interpreter/terminator/mod.rs @@ -13,7 +13,8 @@ use syntax::{ast, attr}; use error::{EvalError, EvalResult}; use memory::Pointer; -use super::{EvalContext, IntegerExt, StackPopCleanup}; +use primval::PrimVal; +use super::{EvalContext, IntegerExt, StackPopCleanup, Value}; mod intrinsics; @@ -154,7 +155,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { substs: &'tcx Substs<'tcx>, fn_ty: &'tcx BareFnTy, destination: Option<(Pointer, mir::BasicBlock)>, - args: &[mir::Operand<'tcx>], + arg_operands: &[mir::Operand<'tcx>], span: Span, ) -> EvalResult<'tcx, ()> { use syntax::abi::Abi; @@ -163,7 +164,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let ty = fn_ty.sig.0.output; let layout = self.type_layout(ty); let (ret, target) = destination.unwrap(); - self.call_intrinsic(def_id, substs, args, ret, layout)?; + self.call_intrinsic(def_id, substs, arg_operands, ret, layout)?; self.goto_block(target); Ok(()) } @@ -172,23 +173,23 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let ty = fn_ty.sig.0.output; let size = self.type_size(ty); let (ret, target) = destination.unwrap(); - self.call_c_abi(def_id, args, ret, size)?; + self.call_c_abi(def_id, arg_operands, ret, size)?; self.goto_block(target); Ok(()) } Abi::Rust | Abi::RustCall => { - let mut arg_srcs = Vec::new(); - for arg in args { - let src = self.eval_operand_to_ptr(arg)?; - let src_ty = self.operand_ty(arg); - arg_srcs.push((src, src_ty)); + let mut args = Vec::new(); + for arg in arg_operands { + let arg_val = self.eval_operand(arg)?; + let arg_ty = self.operand_ty(arg); + args.push((arg_val, arg_ty)); } // Only trait methods can have a Self parameter. let (resolved_def_id, resolved_substs) = if let Some(trait_id) = self.tcx.trait_of_item(def_id) { - self.trait_method(trait_id, def_id, substs, &mut arg_srcs)? + self.trait_method(trait_id, def_id, substs, &mut args)? } else { (def_id, substs) }; @@ -200,9 +201,9 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { }; self.push_stack_frame(resolved_def_id, span, mir, resolved_substs, return_ptr, return_to_block)?; - for (i, (src, src_ty)) in arg_srcs.into_iter().enumerate() { + for (i, (arg_val, arg_ty)) in args.into_iter().enumerate() { let dest = self.frame().locals[i]; - self.move_(src, dest, src_ty)?; + self.write_value(arg_val, dest, arg_ty)?; } Ok(()) @@ -344,7 +345,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { }) } - fn unpack_fn_args(&self, args: &mut Vec<(Pointer, Ty<'tcx>)>) { + fn unpack_fn_args(&self, args: &mut Vec<(Value, Ty<'tcx>)>) { if let Some((last, last_ty)) = args.pop() { let last_layout = self.type_layout(last_ty); match (&last_ty.sty, last_layout) { @@ -353,9 +354,13 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let offsets = iter::once(0) .chain(variant.offset_after_field.iter() .map(|s| s.bytes())); + let last_ptr = match last { + Value::ByRef(ptr) => ptr, + _ => bug!("rust-call ABI tuple argument wasn't Value::ByRef"), + }; for (offset, ty) in offsets.zip(fields) { - let src = last.offset(offset as isize); - args.push((src, ty)); + let arg = Value::ByRef(last_ptr.offset(offset as isize)); + args.push((arg, ty)); } } ty => bug!("expected tuple as last argument in function with 'rust-call' ABI, got {:?}", ty), @@ -369,7 +374,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { trait_id: DefId, def_id: DefId, substs: &'tcx Substs<'tcx>, - args: &mut Vec<(Pointer, Ty<'tcx>)>, + args: &mut Vec<(Value, Ty<'tcx>)>, ) -> EvalResult<'tcx, (DefId, &'tcx Substs<'tcx>)> { let trait_ref = ty::TraitRef::from_method(self.tcx, trait_id, substs); let trait_ref = self.tcx.normalize_associated_type(&ty::Binder(trait_ref)); @@ -398,6 +403,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { (ty::ClosureKind::FnMut, ty::ClosureKind::FnMut) | (ty::ClosureKind::FnOnce, ty::ClosureKind::FnOnce) | (ty::ClosureKind::Fn, ty::ClosureKind::FnMut) => {} // No adapter needed. + (ty::ClosureKind::Fn, ty::ClosureKind::FnOnce) | (ty::ClosureKind::FnMut, ty::ClosureKind::FnOnce) => { // The closure fn is a `fn(&self, ...)` or `fn(&mut self, ...)`. @@ -409,13 +415,15 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // // These are both the same at trans time. - // interpreter magic: insert an intermediate pointer, so we can skip the intermediate function call - // FIXME: this is a memory leak, should probably add the pointer to the current stack - let ptr_size = self.memory.pointer_size(); - let first = self.memory.allocate(ptr_size, ptr_size)?; - self.memory.copy(args[0].0, first, ptr_size, ptr_size)?; - self.memory.write_ptr(args[0].0, first)?; + // Interpreter magic: insert an intermediate pointer, so we can skip the + // intermediate function call. + // FIXME: this is a memory leak, should probably add the pointer to the + // current stack. + let first = self.value_to_ptr(args[0].0, args[0].1)?; + args[0].0 = Value::ByVal(PrimVal::AbstractPtr(first)); + args[0].1 = self.tcx.mk_mut_ptr(args[0].1); } + _ => bug!("cannot convert {:?} to {:?}", closure_kind, trait_closure_kind), } Ok((vtable_closure.closure_def_id, vtable_closure.substs.func_substs)) @@ -433,8 +441,11 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { traits::VtableObject(ref data) => { let idx = self.tcx.get_vtable_index_of_object_method(data, def_id); - if let Some(&mut(first_arg, ref mut first_ty)) = args.get_mut(0) { - let (_, vtable) = self.get_fat_ptr(first_arg); + if let Some(&mut(ref mut first_arg, ref mut first_ty)) = args.get_mut(0) { + // FIXME(solson): Remove this allocating hack. + let ptr = self.value_to_ptr(*first_arg, *first_ty)?; + *first_arg = Value::ByRef(ptr); + let (_, vtable) = self.get_fat_ptr(ptr); let vtable = self.memory.read_ptr(vtable)?; let idx = idx + 3; let offset = idx * self.memory.pointer_size(); From 840594115dd6a992b45090ff986f8257503ee8a0 Mon Sep 17 00:00:00 2001 From: Scott Olson Date: Wed, 21 Sep 2016 23:16:31 -0600 Subject: [PATCH 2/3] Update for changes in rustc. --- src/interpreter/step.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/interpreter/step.rs b/src/interpreter/step.rs index 6f8a297a6d8f..f87f5e43a960 100644 --- a/src/interpreter/step.rs +++ b/src/interpreter/step.rs @@ -82,6 +82,10 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // Miri can safely ignore these. Only translation needs them. StorageLive(_) | StorageDead(_) => {} + + // Defined to do nothing. These are added by optimization passes, to avoid changing the + // size of MIR constantly. + Nop => {} } self.frame_mut().stmt += 1; @@ -186,12 +190,18 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for ConstantExtractor<'a, 'b, 'tcx> { } } - fn visit_lvalue(&mut self, lvalue: &mir::Lvalue<'tcx>, context: LvalueContext, location: mir::Location) { + fn visit_lvalue( + &mut self, + lvalue: &mir::Lvalue<'tcx>, + context: LvalueContext<'tcx>, + location: mir::Location + ) { self.super_lvalue(lvalue, context, location); if let mir::Lvalue::Static(def_id) = *lvalue { let substs = subst::Substs::empty(self.ecx.tcx); let span = self.span; - if let hir::map::Node::NodeItem(&hir::Item { ref node, .. }) = self.ecx.tcx.map.get_if_local(def_id).expect("static not found") { + let node_item = self.ecx.tcx.map.get_if_local(def_id).expect("static not found"); + if let hir::map::Node::NodeItem(&hir::Item { ref node, .. }) = node_item { if let hir::ItemStatic(_, m, _) = *node { self.global_item(def_id, substs, span, m == hir::MutImmutable); return; From 5b012edc7ab8690610b0502be24f3eb8ed30486b Mon Sep 17 00:00:00 2001 From: Scott Olson Date: Wed, 21 Sep 2016 23:23:50 -0600 Subject: [PATCH 3/3] Rename AbstractPtr to Ptr. --- src/interpreter/cast.rs | 4 ++-- src/interpreter/mod.rs | 6 +++--- src/interpreter/terminator/mod.rs | 2 +- src/memory.rs | 2 +- src/primval.rs | 12 ++++++------ 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/interpreter/cast.rs b/src/interpreter/cast.rs index 580f957b4491..06cbf2495dac 100644 --- a/src/interpreter/cast.rs +++ b/src/interpreter/cast.rs @@ -28,7 +28,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { U64(u) | IntegerPtr(u) => self.cast_const_int(u, ty, false), FnPtr(ptr) | - AbstractPtr(ptr) => self.cast_ptr(ptr, ty), + Ptr(ptr) => self.cast_ptr(ptr, ty), } } @@ -36,7 +36,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { use primval::PrimVal::*; match ty.sty { ty::TyRef(..) | - ty::TyRawPtr(_) => Ok(AbstractPtr(ptr)), + ty::TyRawPtr(_) => Ok(Ptr(ptr)), ty::TyFnPtr(_) => Ok(FnPtr(ptr)), _ => Err(EvalError::Unimplemented(format!("ptr to {:?} cast", ty))), } diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index f7d6b464cda3..c6bbe829b5ca 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -246,7 +246,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { self.memory.write_bytes(ptr, s.as_bytes())?; self.memory.freeze(ptr.alloc_id)?; Value::ByValPair( - PrimVal::AbstractPtr(ptr), + PrimVal::Ptr(ptr), self.target_usize_primval(s.len() as u64) ) } @@ -255,7 +255,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { let ptr = self.memory.allocate(bs.len(), 1)?; self.memory.write_bytes(ptr, bs)?; self.memory.freeze(ptr.alloc_id)?; - Value::ByVal(PrimVal::AbstractPtr(ptr)) + Value::ByVal(PrimVal::Ptr(ptr)) } Struct(_) => unimplemented!(), @@ -1050,7 +1050,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { &ty::TyRawPtr(ty::TypeAndMut { ty, .. }) => { if self.type_is_sized(ty) { match self.memory.read_ptr(ptr) { - Ok(p) => PrimVal::AbstractPtr(p), + Ok(p) => PrimVal::Ptr(p), Err(EvalError::ReadBytesAsPointer) => { PrimVal::IntegerPtr(self.memory.read_usize(ptr)?) } diff --git a/src/interpreter/terminator/mod.rs b/src/interpreter/terminator/mod.rs index b8d46323bfa8..10273881b67c 100644 --- a/src/interpreter/terminator/mod.rs +++ b/src/interpreter/terminator/mod.rs @@ -420,7 +420,7 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { // FIXME: this is a memory leak, should probably add the pointer to the // current stack. let first = self.value_to_ptr(args[0].0, args[0].1)?; - args[0].0 = Value::ByVal(PrimVal::AbstractPtr(first)); + args[0].0 = Value::ByVal(PrimVal::Ptr(first)); args[0].1 = self.tcx.mk_mut_ptr(args[0].1); } diff --git a/src/memory.rs b/src/memory.rs index 6042181cdf2e..0108f7e5d9c1 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -530,7 +530,7 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { PrimVal::F32(f) => self.write_f32(ptr, f), PrimVal::F64(f) => self.write_f64(ptr, f), PrimVal::FnPtr(p) | - PrimVal::AbstractPtr(p) => self.write_ptr(ptr, p), + PrimVal::Ptr(p) => self.write_ptr(ptr, p), } } diff --git a/src/primval.rs b/src/primval.rs index d75b7879c375..50ef05a245df 100644 --- a/src/primval.rs +++ b/src/primval.rs @@ -12,7 +12,7 @@ pub enum PrimVal { I8(i8), I16(i16), I32(i32), I64(i64), U8(u8), U16(u16), U32(u32), U64(u64), - AbstractPtr(Pointer), + Ptr(Pointer), FnPtr(Pointer), IntegerPtr(u64), Char(char), @@ -211,10 +211,10 @@ 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(_)) | - (FnPtr(_), AbstractPtr(_)) | - (AbstractPtr(_), FnPtr(_)) | + (Ptr(_), IntegerPtr(_)) | + (IntegerPtr(_), Ptr(_)) | + (FnPtr(_), Ptr(_)) | + (Ptr(_), FnPtr(_)) | (FnPtr(_), IntegerPtr(_)) | (IntegerPtr(_), FnPtr(_)) => unrelated_ptr_ops(bin_op)?, @@ -225,7 +225,7 @@ pub fn binary_op<'tcx>(bin_op: mir::BinOp, left: PrimVal, right: PrimVal) -> Eva _ => return Err(EvalError::Unimplemented(format!("unimplemented fn ptr comparison: {:?}", bin_op))), }, - (AbstractPtr(l_ptr), AbstractPtr(r_ptr)) => { + (Ptr(l_ptr), Ptr(r_ptr)) => { if l_ptr.alloc_id != r_ptr.alloc_id { return Ok((unrelated_ptr_ops(bin_op)?, false)); }