From 5b5e076b473eb31736381f3c2cd73a169a66cbf5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 16:46:33 +0100 Subject: [PATCH 01/21] generalize the traversal part of validation to a ValueVisitor --- src/librustc_mir/const_eval.rs | 6 +- src/librustc_mir/interpret/eval_context.rs | 2 +- src/librustc_mir/interpret/mod.rs | 3 + src/librustc_mir/interpret/place.rs | 10 +- src/librustc_mir/interpret/validity.rs | 583 ++++++++++----------- src/librustc_mir/interpret/visitor.rs | 125 +++++ 6 files changed, 424 insertions(+), 305 deletions(-) create mode 100644 src/librustc_mir/interpret/visitor.rs diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 2658d7f59a07f..34684587db207 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -535,14 +535,14 @@ fn validate_const<'a, 'tcx>( key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { let cid = key.value; - let ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap(); + let mut ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap(); let val = (|| { let op = ecx.const_to_op(constant)?; let mut ref_tracking = RefTracking::new(op); - while let Some((op, mut path)) = ref_tracking.todo.pop() { + while let Some((op, path)) = ref_tracking.todo.pop() { ecx.validate_operand( op, - &mut path, + path, Some(&mut ref_tracking), /* const_mode */ true, )?; diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index fc13c5fef2dda..e6267012dc275 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -521,7 +521,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tc // return place is always a local and then this cannot happen. self.validate_operand( self.place_to_op(return_place)?, - &mut vec![], + vec![], None, /*const_mode*/false, )?; diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 6b31c675cc7d6..82fe08fa038a4 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -23,6 +23,7 @@ mod terminator; mod traits; mod validity; mod intrinsics; +mod visitor; pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one place: here @@ -38,4 +39,6 @@ pub use self::machine::{Machine, AllocMap, MayLeak}; pub use self::operand::{ScalarMaybeUndef, Immediate, ImmTy, Operand, OpTy}; +pub use self::visitor::ValueVisitor; + pub use self::validity::RefTracking; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index fa4d31846df4a..35276fa6265d2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -489,6 +489,8 @@ where /// Get the place of a field inside the place, and also the field's type. /// Just a convenience function, but used quite a bit. + /// This is the only projection that might have a side-effect: We cannot project + /// into the field of a local `ScalarPair`, we have to first allocate it. pub fn place_field( &mut self, base: PlaceTy<'tcx, M::PointerTag>, @@ -501,7 +503,7 @@ where } pub fn place_downcast( - &mut self, + &self, base: PlaceTy<'tcx, M::PointerTag>, variant: usize, ) -> EvalResult<'tcx, PlaceTy<'tcx, M::PointerTag>> { @@ -643,7 +645,7 @@ where if M::enforce_validity(self) { // Data got changed, better make sure it matches the type! - self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?; + self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?; } Ok(()) @@ -765,7 +767,7 @@ where if M::enforce_validity(self) { // Data got changed, better make sure it matches the type! - self.validate_operand(self.place_to_op(dest)?, &mut vec![], None, /*const_mode*/false)?; + self.validate_operand(self.place_to_op(dest)?, vec![], None, /*const_mode*/false)?; } Ok(()) @@ -843,7 +845,7 @@ where if M::enforce_validity(self) { // Data got changed, better make sure it matches the type! - self.validate_operand(dest.into(), &mut vec![], None, /*const_mode*/false)?; + self.validate_operand(dest.into(), vec![], None, /*const_mode*/false)?; } Ok(()) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 4dbae3c8c3d28..8ac3f992cfd96 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -8,24 +8,24 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::fmt::Write; +use std::fmt::{self, Write}; use std::hash::Hash; use syntax_pos::symbol::Symbol; use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf}; -use rustc::ty; +use rustc::ty::{self, TyCtxt}; use rustc_data_structures::fx::FxHashSet; use rustc::mir::interpret::{ Scalar, AllocType, EvalResult, EvalErrorKind }; use super::{ - ImmTy, OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef + OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef, ValueVisitor }; macro_rules! validation_failure { ($what:expr, $where:expr, $details:expr) => {{ - let where_ = path_format($where); + let where_ = path_format(&$where); let where_ = if where_.is_empty() { String::new() } else { @@ -37,7 +37,7 @@ macro_rules! validation_failure { ))) }}; ($what:expr, $where:expr) => {{ - let where_ = path_format($where); + let where_ = path_format(&$where); let where_ = if where_.is_empty() { String::new() } else { @@ -129,6 +129,43 @@ fn path_format(path: &Vec) -> String { out } +fn aggregate_field_path_elem<'a, 'tcx>( + layout: TyLayout<'tcx>, + field: usize, + tcx: TyCtxt<'a, 'tcx, 'tcx>, +) -> PathElem { + match layout.ty.sty { + // generators and closures. + ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { + if let Some(upvar) = tcx.optimized_mir(def_id).upvar_decls.get(field) { + PathElem::ClosureVar(upvar.debug_name) + } else { + // Sometimes the index is beyond the number of freevars (seen + // for a generator). + PathElem::ClosureVar(Symbol::intern(&field.to_string())) + } + } + + // tuples + ty::Tuple(_) => PathElem::TupleElem(field), + + // enums + ty::Adt(def, ..) if def.is_enum() => { + let variant = match layout.variants { + layout::Variants::Single { index } => &def.variants[index], + _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"), + }; + PathElem::Field(variant.fields[field].ident.name) + } + + // other ADTs + ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name), + + // nothing else has an aggregate layout + _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty), + } +} + fn scalar_format(value: ScalarMaybeUndef) -> String { match value { ScalarMaybeUndef::Undef => @@ -140,37 +177,92 @@ fn scalar_format(value: ScalarMaybeUndef) -> String { } } -impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - /// Make sure that `value` is valid for `ty`, *assuming* `ty` is a primitive type. - fn validate_primitive_type( - &self, - value: ImmTy<'tcx, M::PointerTag>, - path: &Vec, - ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, - const_mode: bool, - ) -> EvalResult<'tcx> { +struct ValidityVisitor<'rt, 'tcx, Tag> { + op: OpTy<'tcx, Tag>, + /// The `path` may be pushed to, but the part that is present when a function + /// starts must not be changed! `visit_fields` and `visit_array` rely on + /// this stack discipline. + path: Vec, + ref_tracking: Option<&'rt mut RefTracking<'tcx, Tag>>, + const_mode: bool, +} + +impl fmt::Debug for ValidityVisitor<'_, '_, Tag> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:?} ({:?})", *self.op, self.op.layout.ty) + } +} + +impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> + ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'tcx, M::PointerTag> +{ + #[inline(always)] + fn layout(&self) -> TyLayout<'tcx> { + self.op.layout + } + + fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + let variant = match ectx.read_discriminant(self.op) { + Ok(res) => res.1, + Err(err) => return match err.kind { + EvalErrorKind::InvalidDiscriminant(val) => + validation_failure!( + format!("invalid enum discriminant {}", val), self.path + ), + _ => + validation_failure!( + format!("non-integer enum discriminant"), self.path + ), + } + }; + // Put the variant projection onto the path, as a field + self.path.push(PathElem::Field(self.op.layout.ty + .ty_adt_def() + .unwrap() + .variants[variant].name)); + // Proceed with this variant + self.op = ectx.operand_downcast(self.op, variant)?; + Ok(()) + } + + fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + // FIXME: Should we reflect this in `self.path`? + let dest = self.op.to_mem_place(); // immediate trait objects are not a thing + self.op = ectx.unpack_dyn_trait(dest)?.1.into(); + Ok(()) + } + + fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + let value = try_validation!(ectx.read_immediate(self.op), + "uninitialized or unrepresentable data", self.path); // Go over all the primitive types let ty = value.layout.ty; match ty.sty { ty::Bool => { let value = value.to_scalar_or_undef(); try_validation!(value.to_bool(), - scalar_format(value), path, "a boolean"); + scalar_format(value), self.path, "a boolean"); }, ty::Char => { let value = value.to_scalar_or_undef(); try_validation!(value.to_char(), - scalar_format(value), path, "a valid unicode codepoint"); + scalar_format(value), self.path, "a valid unicode codepoint"); }, ty::Float(_) | ty::Int(_) | ty::Uint(_) => { // NOTE: Keep this in sync with the array optimization for int/float // types below! let size = value.layout.size; let value = value.to_scalar_or_undef(); - if const_mode { + if self.const_mode { // Integers/floats in CTFE: Must be scalar bits, pointers are dangerous try_validation!(value.to_bits(size), - scalar_format(value), path, "initialized plain bits"); + scalar_format(value), self.path, "initialized plain bits"); } else { // At run-time, for now, we accept *anything* for these types, including // undef. We should fix that, but let's start low. @@ -180,33 +272,33 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // No undef allowed here. Eventually this should be consistent with // the integer types. let _ptr = try_validation!(value.to_scalar_ptr(), - "undefined address in pointer", path); + "undefined address in pointer", self.path); let _meta = try_validation!(value.to_meta(), - "uninitialized data in fat pointer metadata", path); + "uninitialized data in fat pointer metadata", self.path); } _ if ty.is_box() || ty.is_region_ptr() => { // Handle fat pointers. // Check metadata early, for better diagnostics let ptr = try_validation!(value.to_scalar_ptr(), - "undefined address in pointer", path); + "undefined address in pointer", self.path); let meta = try_validation!(value.to_meta(), - "uninitialized data in fat pointer metadata", path); - let layout = self.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; + "uninitialized data in fat pointer metadata", self.path); + let layout = ectx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; if layout.is_unsized() { - let tail = self.tcx.struct_tail(layout.ty); + let tail = ectx.tcx.struct_tail(layout.ty); match tail.sty { ty::Dynamic(..) => { let vtable = try_validation!(meta.unwrap().to_ptr(), - "non-pointer vtable in fat pointer", path); - try_validation!(self.read_drop_type_from_vtable(vtable), - "invalid drop fn in vtable", path); - try_validation!(self.read_size_and_align_from_vtable(vtable), - "invalid size or align in vtable", path); + "non-pointer vtable in fat pointer", self.path); + try_validation!(ectx.read_drop_type_from_vtable(vtable), + "invalid drop fn in vtable", self.path); + try_validation!(ectx.read_size_and_align_from_vtable(vtable), + "invalid size or align in vtable", self.path); // FIXME: More checks for the vtable. } ty::Slice(..) | ty::Str => { - try_validation!(meta.unwrap().to_usize(self), - "non-integer slice length in fat pointer", path); + try_validation!(meta.unwrap().to_usize(ectx), + "non-integer slice length in fat pointer", self.path); } ty::Foreign(..) => { // Unsized, but not fat. @@ -216,25 +308,25 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } // Make sure this is non-NULL and aligned - let (size, align) = self.size_and_align_of(meta, layout)? + let (size, align) = ectx.size_and_align_of(meta, layout)? // for the purpose of validity, consider foreign types to have // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| layout.size_and_align()); - match self.memory.check_align(ptr, align) { + match ectx.memory.check_align(ptr, align) { Ok(_) => {}, Err(err) => { error!("{:?} is not aligned to {:?}", ptr, align); match err.kind { EvalErrorKind::InvalidNullPointerUsage => - return validation_failure!("NULL reference", path), + return validation_failure!("NULL reference", self.path), EvalErrorKind::AlignmentCheckFailed { .. } => - return validation_failure!("unaligned reference", path), + return validation_failure!("unaligned reference", self.path), _ => return validation_failure!( "dangling (out-of-bounds) reference (might be NULL at \ run-time)", - path + self.path ), } } @@ -242,29 +334,29 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Turn ptr into place. // `ref_to_mplace` also calls the machine hook for (re)activating the tag, // which in turn will (in full miri) check if the pointer is dereferencable. - let place = self.ref_to_mplace(value)?; + let place = ectx.ref_to_mplace(value)?; // Recursive checking - if let Some(ref_tracking) = ref_tracking { - assert!(const_mode, "We should only do recursie checking in const mode"); + if let Some(ref mut ref_tracking) = self.ref_tracking { + assert!(self.const_mode, "We should only do recursie checking in const mode"); if size != Size::ZERO { // Non-ZST also have to be dereferencable let ptr = try_validation!(place.ptr.to_ptr(), - "integer pointer in non-ZST reference", path); + "integer pointer in non-ZST reference", self.path); // Skip validation entirely for some external statics - let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id); + let alloc_kind = ectx.tcx.alloc_map.lock().get(ptr.alloc_id); if let Some(AllocType::Static(did)) = alloc_kind { // `extern static` cannot be validated as they have no body. // FIXME: Statics from other crates are also skipped. // They might be checked at a different type, but for now we // want to avoid recursing too deeply. This is not sound! - if !did.is_local() || self.tcx.is_foreign_item(did) { + if !did.is_local() || ectx.tcx.is_foreign_item(did) { return Ok(()); } } // Maintain the invariant that the place we are checking is // already verified to be in-bounds. - try_validation!(self.memory.check_bounds(ptr, size, false), - "dangling (not entirely in bounds) reference", path); + try_validation!(ectx.memory.check_bounds(ptr, size, false), + "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination // before. Proceed recursively even for integer pointers, no @@ -273,16 +365,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let op = place.into(); if ref_tracking.seen.insert(op) { trace!("Recursing below ptr {:#?}", *op); - ref_tracking.todo.push((op, path_clone_and_deref(path))); + ref_tracking.todo.push((op, path_clone_and_deref(&self.path))); } } } ty::FnPtr(_sig) => { let value = value.to_scalar_or_undef(); let ptr = try_validation!(value.to_ptr(), - scalar_format(value), path, "a pointer"); - let _fn = try_validation!(self.memory.get_fn(ptr), - scalar_format(value), path, "a function pointer"); + scalar_format(value), self.path, "a pointer"); + let _fn = try_validation!(ectx.memory.get_fn(ptr), + scalar_format(value), self.path, "a function pointer"); // FIXME: Check if the signature matches } // This should be all the primitive types @@ -292,16 +384,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok(()) } - /// Make sure that `value` matches the - fn validate_scalar_layout( - &self, - value: ScalarMaybeUndef, - size: Size, - path: &Vec, - layout: &layout::Scalar, - ) -> EvalResult<'tcx> { + fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) + -> EvalResult<'tcx> + { + let value = try_validation!(ectx.read_scalar(self.op), + "uninitialized or unrepresentable data", self.path); + // Determine the allowed range let (lo, hi) = layout.valid_range.clone().into_inner(); - let max_hi = u128::max_value() >> (128 - size.bits()); // as big as the size fits + // `max_hi` is as big as the size fits + let max_hi = u128::max_value() >> (128 - self.op.layout.size.bits()); assert!(hi <= max_hi); // We could also write `(hi + 1) % (max_hi + 1) == lo` but `max_hi + 1` overflows for `u128` if (lo == 0 && hi == max_hi) || (hi + 1 == lo) { @@ -310,7 +401,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // At least one value is excluded. Get the bits. let value = try_validation!(value.not_undef(), - scalar_format(value), path, format!("something in the range {:?}", layout.valid_range)); + scalar_format(value), self.path, + format!("something in the range {:?}", layout.valid_range)); let bits = match value { Scalar::Ptr(ptr) => { if lo == 1 && hi == max_hi { @@ -318,13 +410,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // We can call `check_align` to check non-NULL-ness, but have to also look // for function pointers. let non_null = - self.memory.check_align( + ectx.memory.check_align( Scalar::Ptr(ptr), Align::from_bytes(1, 1).unwrap() ).is_ok() || - self.memory.get_fn(ptr).is_ok(); + ectx.memory.get_fn(ptr).is_ok(); if !non_null { // could be NULL - return validation_failure!("a potentially NULL pointer", path); + return validation_failure!("a potentially NULL pointer", self.path); } return Ok(()); } else { @@ -332,7 +424,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // value. return validation_failure!( "a pointer", - path, + self.path, format!( "something that cannot possibly be outside the (wrapping) range {:?}", layout.valid_range @@ -340,8 +432,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ); } } - Scalar::Bits { bits, size: value_size } => { - assert_eq!(value_size as u64, size.bytes()); + Scalar::Bits { bits, size } => { + assert_eq!(size as u64, self.op.layout.size.bytes()); bits } }; @@ -355,7 +447,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } else { validation_failure!( bits, - path, + self.path, format!("something in the range {:?} or {:?}", 0..=hi, lo..=max_hi) ) } @@ -365,7 +457,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } else { validation_failure!( bits, - path, + self.path, if hi == max_hi { format!("something greater or equal to {}", lo) } else { @@ -376,250 +468,147 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - /// This function checks the data at `op`. `op` is assumed to cover valid memory if it - /// is an indirect operand. - /// It will error if the bits at the destination do not match the ones described by the layout. - /// The `path` may be pushed to, but the part that is present when the function - /// starts must not be changed! - /// - /// `ref_tracking` can be None to avoid recursive checking below references. - /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion) - /// validation (e.g., pointer values are fine in integers at runtime). - pub fn validate_operand( - &self, - dest: OpTy<'tcx, M::PointerTag>, - path: &mut Vec, - mut ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, - const_mode: bool, - ) -> EvalResult<'tcx> { - trace!("validate_operand: {:?}, {:?}", *dest, dest.layout.ty); - - // If this is a multi-variant layout, we have find the right one and proceed with that. - // (No good reasoning to make this recursion, but it is equivalent to that.) - let dest = match dest.layout.variants { - layout::Variants::NicheFilling { .. } | - layout::Variants::Tagged { .. } => { - let variant = match self.read_discriminant(dest) { - Ok(res) => res.1, - Err(err) => match err.kind { - EvalErrorKind::InvalidDiscriminant(val) => - return validation_failure!( - format!("invalid enum discriminant {}", val), path - ), - _ => - return validation_failure!( - String::from("non-integer enum discriminant"), path - ), - } - }; - // Put the variant projection onto the path, as a field - path.push(PathElem::Field(dest.layout.ty - .ty_adt_def() - .unwrap() - .variants[variant].name)); - // Proceed with this variant - let dest = self.operand_downcast(dest, variant)?; - trace!("variant layout: {:#?}", dest.layout); - dest - }, - layout::Variants::Single { .. } => dest, - }; - - // First thing, find the real type: - // If it is a trait object, switch to the actual type that was used to create it. - let dest = match dest.layout.ty.sty { - ty::Dynamic(..) => { - let dest = dest.to_mem_place(); // immediate trait objects are not a thing - self.unpack_dyn_trait(dest)?.1.into() - }, - _ => dest - }; - - // If this is a scalar, validate the scalar layout. - // Things can be aggregates and have scalar layout at the same time, and that - // is very relevant for `NonNull` and similar structs: We need to validate them - // at their scalar layout *before* descending into their fields. - // FIXME: We could avoid some redundant checks here. For newtypes wrapping - // scalars, we do the same check on every "level" (e.g. first we check - // MyNewtype and then the scalar in there). - match dest.layout.abi { - layout::Abi::Uninhabited => - return validation_failure!("a value of an uninhabited type", path), - layout::Abi::Scalar(ref layout) => { - let value = try_validation!(self.read_scalar(dest), - "uninitialized or unrepresentable data", path); - self.validate_scalar_layout(value, dest.layout.size, &path, layout)?; - } - // FIXME: Should we do something for ScalarPair? Vector? - _ => {} + fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize) + -> EvalResult<'tcx> + { + // Remember some stuff that will change for the recursive calls + let op = self.op; + let path_len = self.path.len(); + // Go look at all the fields + for i in 0..num_fields { + // Adapt our state + self.op = ectx.operand_field(op, i as u64)?; + self.path.push(aggregate_field_path_elem(op.layout, i, *ectx.tcx)); + // Recursive visit + ectx.visit_value(self)?; + // Restore original state + self.op = op; + self.path.truncate(path_len); } + Ok(()) + } - // Check primitive types. We do this after checking the scalar layout, - // just to have that done as well. Primitives can have varying layout, - // so we check them separately and before aggregate handling. - // It is CRITICAL that we get this check right, or we might be - // validating the wrong thing! - let primitive = match dest.layout.fields { - // Primitives appear as Union with 0 fields -- except for fat pointers. - layout::FieldPlacement::Union(0) => true, - _ => dest.layout.ty.builtin_deref(true).is_some(), - }; - if primitive { - let value = try_validation!(self.read_immediate(dest), - "uninitialized or unrepresentable data", path); - return self.validate_primitive_type( - value, - &path, - ref_tracking, - const_mode, - ); - } + fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + let mplace = self.op.to_mem_place(); // strings are never immediate + try_validation!(ectx.read_str(mplace), + "uninitialized or non-UTF-8 data in str", self.path); + Ok(()) + } - // Validate all fields of compound data structures - let path_len = path.len(); // Remember the length, in case we need to truncate - match dest.layout.fields { - layout::FieldPlacement::Union(fields) => { - // Empty unions are not accepted by rustc. That's great, it means we can - // use that as an unambiguous signal for detecting primitives. Make sure - // we did not miss any primitive. - debug_assert!(fields > 0); - // We can't check unions, their bits are allowed to be anything. - // The fields don't need to correspond to any bit pattern of the union's fields. - // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389 - }, - layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - // Go look at all the fields - for i in 0..offsets.len() { - let field = self.operand_field(dest, i as u64)?; - path.push(self.aggregate_field_path_elem(dest.layout, i)); - self.validate_operand( - field, - path, - ref_tracking.as_mut().map(|r| &mut **r), - const_mode, - )?; - path.truncate(path_len); + fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> + { + let mplace = if self.op.layout.is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(self.op.layout, ectx) + } else { + // non-ZST array/slice/str cannot be immediate + self.op.to_mem_place() + }; + match self.op.layout.ty.sty { + ty::Str => bug!("Strings should be handled separately"), + // Special handling for arrays/slices of builtin integer types + ty::Array(tys, ..) | ty::Slice(tys) if { + // This optimization applies only for integer and floating point types + // (i.e., types that can hold arbitrary bytes). + match tys.sty { + ty::Int(..) | ty::Uint(..) | ty::Float(..) => true, + _ => false, } - } - layout::FieldPlacement::Array { stride, .. } => { - let dest = if dest.layout.is_zst() { - // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(dest.layout, self) - } else { - // non-ZST array/slice/str cannot be immediate - dest.to_mem_place() - }; - match dest.layout.ty.sty { - // Special handling for strings to verify UTF-8 - ty::Str => { - try_validation!(self.read_str(dest), - "uninitialized or non-UTF-8 data in str", path); - } - // Special handling for arrays/slices of builtin integer types - ty::Array(tys, ..) | ty::Slice(tys) if { - // This optimization applies only for integer and floating point types - // (i.e., types that can hold arbitrary bytes). - match tys.sty { - ty::Int(..) | ty::Uint(..) | ty::Float(..) => true, - _ => false, - } - } => { - // This is the length of the array/slice. - let len = dest.len(self)?; - // Since primitive types are naturally aligned and tightly packed in arrays, - // we can use the stride to get the size of the integral type. - let ty_size = stride.bytes(); - // This is the size in bytes of the whole array. - let size = Size::from_bytes(ty_size * len); - - // NOTE: Keep this in sync with the handling of integer and float - // types above, in `validate_primitive_type`. - // In run-time mode, we accept pointers in here. This is actually more - // permissive than a per-element check would be, e.g. we accept - // an &[u8] that contains a pointer even though bytewise checking would - // reject it. However, that's good: We don't inherently want - // to reject those pointers, we just do not have the machinery to - // talk about parts of a pointer. - // We also accept undef, for consistency with the type-based checks. - match self.memory.check_bytes( - dest.ptr, - size, - /*allow_ptr_and_undef*/!const_mode, - ) { - // In the happy case, we needn't check anything else. - Ok(()) => {}, - // Some error happened, try to provide a more detailed description. - Err(err) => { - // For some errors we might be able to provide extra information - match err.kind { - EvalErrorKind::ReadUndefBytes(offset) => { - // Some byte was undefined, determine which - // element that byte belongs to so we can - // provide an index. - let i = (offset.bytes() / ty_size) as usize; - path.push(PathElem::ArrayElem(i)); - - return validation_failure!( - "undefined bytes", path - ) - }, - // Other errors shouldn't be possible - _ => return Err(err), - } - } - } - }, - _ => { - // This handles the unsized case correctly as well, as well as - // SIMD an all sorts of other array-like types. - for (i, field) in self.mplace_array_fields(dest)?.enumerate() { - let field = field?; - path.push(PathElem::ArrayElem(i)); - self.validate_operand( - field.into(), - path, - ref_tracking.as_mut().map(|r| &mut **r), - const_mode, - )?; - path.truncate(path_len); + } => { + // This is the length of the array/slice. + let len = mplace.len(ectx)?; + // This is the element type size. + let ty_size = ectx.layout_of(tys)?.size; + // This is the size in bytes of the whole array. + let size = ty_size * len; + + // NOTE: Keep this in sync with the handling of integer and float + // types above, in `visit_primitive`. + // In run-time mode, we accept pointers in here. This is actually more + // permissive than a per-element check would be, e.g. we accept + // an &[u8] that contains a pointer even though bytewise checking would + // reject it. However, that's good: We don't inherently want + // to reject those pointers, we just do not have the machinery to + // talk about parts of a pointer. + // We also accept undef, for consistency with the type-based checks. + match ectx.memory.check_bytes( + mplace.ptr, + size, + /*allow_ptr_and_undef*/!self.const_mode, + ) { + // In the happy case, we needn't check anything else. + Ok(()) => {}, + // Some error happened, try to provide a more detailed description. + Err(err) => { + // For some errors we might be able to provide extra information + match err.kind { + EvalErrorKind::ReadUndefBytes(offset) => { + // Some byte was undefined, determine which + // element that byte belongs to so we can + // provide an index. + let i = (offset.bytes() / ty_size.bytes()) as usize; + self.path.push(PathElem::ArrayElem(i)); + + return validation_failure!( + "undefined bytes", self.path + ) + }, + // Other errors shouldn't be possible + _ => return Err(err), } } } }, + _ => { + // Remember some stuff that will change for the recursive calls + let op = self.op; + let path_len = self.path.len(); + // This handles the unsized case correctly as well, as well as + // SIMD and all sorts of other array-like types. + for (i, field) in ectx.mplace_array_fields(mplace)?.enumerate() { + // Adapt our state + self.op = field?.into(); + self.path.push(PathElem::ArrayElem(i)); + // Recursive visit + ectx.visit_value(self)?; + // Restore original state + self.op = op; + self.path.truncate(path_len); + } + } } Ok(()) } +} - fn aggregate_field_path_elem(&self, layout: TyLayout<'tcx>, field: usize) -> PathElem { - match layout.ty.sty { - // generators and closures. - ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { - if let Some(upvar) = self.tcx.optimized_mir(def_id).upvar_decls.get(field) { - PathElem::ClosureVar(upvar.debug_name) - } else { - // Sometimes the index is beyond the number of freevars (seen - // for a generator). - PathElem::ClosureVar(Symbol::intern(&field.to_string())) - } - } - - // tuples - ty::Tuple(_) => PathElem::TupleElem(field), - - // enums - ty::Adt(def, ..) if def.is_enum() => { - let variant = match layout.variants { - layout::Variants::Single { index } => &def.variants[index], - _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"), - }; - PathElem::Field(variant.fields[field].ident.name) - } +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { + /// This function checks the data at `op`. `op` is assumed to cover valid memory if it + /// is an indirect operand. + /// It will error if the bits at the destination do not match the ones described by the layout. + /// + /// `ref_tracking` can be None to avoid recursive checking below references. + /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion) + /// validation (e.g., pointer values are fine in integers at runtime). + pub fn validate_operand( + &mut self, + op: OpTy<'tcx, M::PointerTag>, + path: Vec, + ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, + const_mode: bool, + ) -> EvalResult<'tcx> { + trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty); - // other ADTs - ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name), + // Construct a visitor + let mut visitor = ValidityVisitor { + op, + path, + ref_tracking, + const_mode + }; - // nothing else has an aggregate layout - _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty), - } + // Run it + self.visit_value(&mut visitor) } } diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs new file mode 100644 index 0000000000000..892eaceff1bc6 --- /dev/null +++ b/src/librustc_mir/interpret/visitor.rs @@ -0,0 +1,125 @@ +//! Visitor for a run-time value with a given layout: Traverse enums, structs and other compound +//! types until we arrive at the leaves, with custom handling for primitive types. + +use std::fmt; + +use rustc::ty::layout::{self, TyLayout}; +use rustc::ty; +use rustc::mir::interpret::{ + EvalResult, +}; + +use super::{ + Machine, EvalContext, +}; + +// How to traverse a value and what to do when we are at the leaves. +// In the future, we might want to turn this into two traits, but so far the +// only implementations we have couldn't share any code anyway. +pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { + // Get this value's layout. + fn layout(&self) -> TyLayout<'tcx>; + + // Downcast functions. These change the value as a side-effect. + fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx>; + fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx>; + + // Visit all fields of a compound. + // Just call `visit_value` if you want to go on recursively. + fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize) + -> EvalResult<'tcx>; + // Optimized handling for arrays -- avoid computing the layout for every field. + // Also it is the value's responsibility to figure out the length. + fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>; + // Special handling for strings. + fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx>; + + // Actions on the leaves. + fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) + -> EvalResult<'tcx>; + fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx>; +} + +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { + pub fn visit_value>(&mut self, v: &mut V) -> EvalResult<'tcx> { + trace!("visit_value: {:?}", v); + + // If this is a multi-variant layout, we have find the right one and proceed with that. + // (No benefit from making this recursion, but it is equivalent to that.) + match v.layout().variants { + layout::Variants::NicheFilling { .. } | + layout::Variants::Tagged { .. } => { + v.downcast_enum(self)?; + trace!("variant layout: {:#?}", v.layout()); + } + layout::Variants::Single { .. } => {} + } + + // Even for single variants, we might be able to get a more refined type: + // If it is a trait object, switch to the actual type that was used to create it. + match v.layout().ty.sty { + ty::Dynamic(..) => { + v.downcast_dyn_trait(self)?; + }, + _ => {}, + }; + + // If this is a scalar, visit it as such. + // Things can be aggregates and have scalar layout at the same time, and that + // is very relevant for `NonNull` and similar structs: We need to visit them + // at their scalar layout *before* descending into their fields. + // FIXME: We could avoid some redundant checks here. For newtypes wrapping + // scalars, we do the same check on every "level" (e.g. first we check + // MyNewtype and then the scalar in there). + match v.layout().abi { + layout::Abi::Scalar(ref layout) => { + v.visit_scalar(self, layout)?; + } + // FIXME: Should we do something for ScalarPair? Vector? + _ => {} + } + + // Check primitive types. We do this after checking the scalar layout, + // just to have that done as well. Primitives can have varying layout, + // so we check them separately and before aggregate handling. + // It is CRITICAL that we get this check right, or we might be + // validating the wrong thing! + let primitive = match v.layout().fields { + // Primitives appear as Union with 0 fields -- except for Boxes and fat pointers. + layout::FieldPlacement::Union(0) => true, + _ => v.layout().ty.builtin_deref(true).is_some(), + }; + if primitive { + return v.visit_primitive(self); + } + + // Proceed into the fields. + match v.layout().fields { + layout::FieldPlacement::Union(fields) => { + // Empty unions are not accepted by rustc. That's great, it means we can + // use that as an unambiguous signal for detecting primitives. Make sure + // we did not miss any primitive. + debug_assert!(fields > 0); + // We can't traverse unions, their bits are allowed to be anything. + // The fields don't need to correspond to any bit pattern of the union's fields. + // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389 + Ok(()) + }, + layout::FieldPlacement::Arbitrary { ref offsets, .. } => { + v.visit_fields(self, offsets.len()) + }, + layout::FieldPlacement::Array { .. } => { + match v.layout().ty.sty { + // Strings have properties that cannot be expressed pointwise. + ty::Str => v.visit_str(self), + // General case -- might also be SIMD vector or so + _ => v.visit_array(self), + } + } + } + } +} From 7d7bd9b6c24a89ebafd4400a710a868fb3e70242 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 18:44:00 +0100 Subject: [PATCH 02/21] reduce the amount of traversal/projection code that the visitor has to implement itself --- src/librustc_mir/interpret/validity.rs | 249 +++++++++--------- src/librustc_mir/interpret/visitor.rs | 144 ++++++++-- .../ui/consts/const-eval/ub-upvars.stderr | 2 +- .../consts/const-eval/union-ub-fat-ptr.stderr | 2 +- 4 files changed, 237 insertions(+), 160 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8ac3f992cfd96..7caf81788dd23 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -78,6 +78,7 @@ pub enum PathElem { TupleElem(usize), Deref, Tag, + DynDowncast, } /// State for tracking recursive validation of references @@ -97,15 +98,6 @@ impl<'tcx, Tag: Copy+Eq+Hash> RefTracking<'tcx, Tag> { } } -// Adding a Deref and making a copy of the path to be put into the queue -// always go together. This one does it with only new allocation. -fn path_clone_and_deref(path: &Vec) -> Vec { - let mut new_path = Vec::with_capacity(path.len()+1); - new_path.clone_from(path); - new_path.push(PathElem::Deref); - new_path -} - /// Format a path fn path_format(path: &Vec) -> String { use self::PathElem::*; @@ -113,59 +105,23 @@ fn path_format(path: &Vec) -> String { let mut out = String::new(); for elem in path.iter() { match elem { - Field(name) => write!(out, ".{}", name).unwrap(), - ClosureVar(name) => write!(out, ".", name).unwrap(), - TupleElem(idx) => write!(out, ".{}", idx).unwrap(), - ArrayElem(idx) => write!(out, "[{}]", idx).unwrap(), + Field(name) => write!(out, ".{}", name), + ClosureVar(name) => write!(out, ".", name), + TupleElem(idx) => write!(out, ".{}", idx), + ArrayElem(idx) => write!(out, "[{}]", idx), Deref => // This does not match Rust syntax, but it is more readable for long paths -- and // some of the other items here also are not Rust syntax. Actually we can't // even use the usual syntax because we are just showing the projections, // not the root. - write!(out, ".").unwrap(), - Tag => write!(out, ".").unwrap(), - } + write!(out, "."), + Tag => write!(out, "."), + DynDowncast => write!(out, "."), + }.unwrap() } out } -fn aggregate_field_path_elem<'a, 'tcx>( - layout: TyLayout<'tcx>, - field: usize, - tcx: TyCtxt<'a, 'tcx, 'tcx>, -) -> PathElem { - match layout.ty.sty { - // generators and closures. - ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { - if let Some(upvar) = tcx.optimized_mir(def_id).upvar_decls.get(field) { - PathElem::ClosureVar(upvar.debug_name) - } else { - // Sometimes the index is beyond the number of freevars (seen - // for a generator). - PathElem::ClosureVar(Symbol::intern(&field.to_string())) - } - } - - // tuples - ty::Tuple(_) => PathElem::TupleElem(field), - - // enums - ty::Adt(def, ..) if def.is_enum() => { - let variant = match layout.variants { - layout::Variants::Single { index } => &def.variants[index], - _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"), - }; - PathElem::Field(variant.fields[field].ident.name) - } - - // other ADTs - ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name), - - // nothing else has an aggregate layout - _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty), - } -} - fn scalar_format(value: ScalarMaybeUndef) -> String { match value { ScalarMaybeUndef::Undef => @@ -177,7 +133,7 @@ fn scalar_format(value: ScalarMaybeUndef) -> String { } } -struct ValidityVisitor<'rt, 'tcx, Tag> { +struct ValidityVisitor<'rt, 'a, 'tcx, Tag> { op: OpTy<'tcx, Tag>, /// The `path` may be pushed to, but the part that is present when a function /// starts must not be changed! `visit_fields` and `visit_array` rely on @@ -185,20 +141,89 @@ struct ValidityVisitor<'rt, 'tcx, Tag> { path: Vec, ref_tracking: Option<&'rt mut RefTracking<'tcx, Tag>>, const_mode: bool, + tcx: TyCtxt<'a, 'tcx, 'tcx>, } -impl fmt::Debug for ValidityVisitor<'_, '_, Tag> { +impl fmt::Debug for ValidityVisitor<'_, '_, '_, Tag> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?} ({:?})", *self.op, self.op.layout.ty) + write!(f, "{:?}, {:?}", *self.op, self.op.layout.ty) + } +} + +impl<'rt, 'a, 'tcx, Tag> ValidityVisitor<'rt, 'a, 'tcx, Tag> { + fn push_aggregate_field_path_elem( + &mut self, + layout: TyLayout<'tcx>, + field: usize, + ) { + let elem = match layout.ty.sty { + // generators and closures. + ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { + if let Some(upvar) = self.tcx.optimized_mir(def_id).upvar_decls.get(field) { + PathElem::ClosureVar(upvar.debug_name) + } else { + // Sometimes the index is beyond the number of freevars (seen + // for a generator). + PathElem::ClosureVar(Symbol::intern(&field.to_string())) + } + } + + // tuples + ty::Tuple(_) => PathElem::TupleElem(field), + + // enums + ty::Adt(def, ..) if def.is_enum() => { + let variant = match layout.variants { + layout::Variants::Single { index } => &def.variants[index], + _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"), + }; + PathElem::Field(variant.fields[field].ident.name) + } + + // other ADTs + ty::Adt(def, _) => PathElem::Field(def.non_enum_variant().fields[field].ident.name), + + // arrays/slices + ty::Array(..) | ty::Slice(..) => PathElem::ArrayElem(field), + + // dyn traits + ty::Dynamic(..) => PathElem::DynDowncast, + + // nothing else has an aggregate layout + _ => bug!("aggregate_field_path_elem: got non-aggregate type {:?}", layout.ty), + }; + self.path.push(elem); } } impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> - ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'tcx, M::PointerTag> + ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'a, 'tcx, M::PointerTag> { + type V = OpTy<'tcx, M::PointerTag>; + #[inline(always)] - fn layout(&self) -> TyLayout<'tcx> { - self.op.layout + fn value(&self) -> &OpTy<'tcx, M::PointerTag> { + &self.op + } + + #[inline] + fn with_field( + &mut self, + val: Self::V, + field: usize, + f: impl FnOnce(&mut Self) -> EvalResult<'tcx>, + ) -> EvalResult<'tcx> { + // Remember the old state + let path_len = self.path.len(); + let op = self.op; + // Perform operation + self.push_aggregate_field_path_elem(op.layout, field); + self.op = val; + f(self)?; + // Undo changes + self.path.truncate(path_len); + self.op = op; + Ok(()) } fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) @@ -227,15 +252,6 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Ok(()) } - fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> - { - // FIXME: Should we reflect this in `self.path`? - let dest = self.op.to_mem_place(); // immediate trait objects are not a thing - self.op = ectx.unpack_dyn_trait(dest)?.1.into(); - Ok(()) - } - fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { @@ -365,7 +381,13 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> let op = place.into(); if ref_tracking.seen.insert(op) { trace!("Recursing below ptr {:#?}", *op); - ref_tracking.todo.push((op, path_clone_and_deref(&self.path))); + // We need to clone the path anyway, make sure it gets created + // with enough space for the additional `Deref`. + let mut new_path = Vec::with_capacity(self.path.len()+1); + new_path.clone_from(&self.path); + new_path.push(PathElem::Deref); + // Remember to come back to this later. + ref_tracking.todo.push((op, new_path)); } } } @@ -378,12 +400,17 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // FIXME: Check if the signature matches } // This should be all the primitive types - ty::Never => bug!("Uninhabited type should have been caught earlier"), _ => bug!("Unexpected primitive type {}", value.layout.ty) } Ok(()) } + fn visit_uninhabited(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + validation_failure!("a value of an uninhabited type", self.path) + } + fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) -> EvalResult<'tcx> { @@ -468,47 +495,16 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } - fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize) - -> EvalResult<'tcx> - { - // Remember some stuff that will change for the recursive calls - let op = self.op; - let path_len = self.path.len(); - // Go look at all the fields - for i in 0..num_fields { - // Adapt our state - self.op = ectx.operand_field(op, i as u64)?; - self.path.push(aggregate_field_path_elem(op.layout, i, *ectx.tcx)); - // Recursive visit - ectx.visit_value(self)?; - // Restore original state - self.op = op; - self.path.truncate(path_len); - } - Ok(()) - } - - fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> - { - let mplace = self.op.to_mem_place(); // strings are never immediate - try_validation!(ectx.read_str(mplace), - "uninitialized or non-UTF-8 data in str", self.path); - Ok(()) - } - - fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> + fn handle_array(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx, bool> { - let mplace = if self.op.layout.is_zst() { - // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(self.op.layout, ectx) - } else { - // non-ZST array/slice/str cannot be immediate - self.op.to_mem_place() - }; - match self.op.layout.ty.sty { - ty::Str => bug!("Strings should be handled separately"), - // Special handling for arrays/slices of builtin integer types + Ok(match self.op.layout.ty.sty { + ty::Str => { + let mplace = self.op.to_mem_place(); // strings are never immediate + try_validation!(ectx.read_str(mplace), + "uninitialized or non-UTF-8 data in str", self.path); + true + } ty::Array(tys, ..) | ty::Slice(tys) if { // This optimization applies only for integer and floating point types // (i.e., types that can hold arbitrary bytes). @@ -517,6 +513,13 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> _ => false, } } => { + let mplace = if self.op.layout.is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(self.op.layout, ectx) + } else { + // non-ZST array/slice/str cannot be immediate + self.op.to_mem_place() + }; // This is the length of the array/slice. let len = mplace.len(ectx)?; // This is the element type size. @@ -539,7 +542,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> /*allow_ptr_and_undef*/!self.const_mode, ) { // In the happy case, we needn't check anything else. - Ok(()) => {}, + Ok(()) => true, // handled these arrays // Some error happened, try to provide a more detailed description. Err(err) => { // For some errors we might be able to provide extra information @@ -560,26 +563,9 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } } - }, - _ => { - // Remember some stuff that will change for the recursive calls - let op = self.op; - let path_len = self.path.len(); - // This handles the unsized case correctly as well, as well as - // SIMD and all sorts of other array-like types. - for (i, field) in ectx.mplace_array_fields(mplace)?.enumerate() { - // Adapt our state - self.op = field?.into(); - self.path.push(PathElem::ArrayElem(i)); - // Recursive visit - ectx.visit_value(self)?; - // Restore original state - self.op = op; - self.path.truncate(path_len); - } } - } - Ok(()) + _ => false, // not handled + }) } } @@ -605,7 +591,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> op, path, ref_tracking, - const_mode + const_mode, + tcx: *self.tcx, }; // Run it diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 892eaceff1bc6..7d6029d6424f7 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -10,34 +10,103 @@ use rustc::mir::interpret::{ }; use super::{ - Machine, EvalContext, + Machine, EvalContext, MPlaceTy, OpTy, }; -// How to traverse a value and what to do when we are at the leaves. -// In the future, we might want to turn this into two traits, but so far the -// only implementations we have couldn't share any code anyway. -pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { +// A thing that we can project into, and that has a layout. +// This wouldn't have to depend on `Machine` but with the current type inference, +// that's just more convenient to work with (avoids repeading all the `Machine` bounds). +pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy +{ // Get this value's layout. fn layout(&self) -> TyLayout<'tcx>; - // Downcast functions. These change the value as a side-effect. + // Get the underlying `MPlaceTy`, or panic if there is no such thing. + fn to_mem_place(self) -> MPlaceTy<'tcx, M::PointerTag>; + + // Create this from an `MPlaceTy` + fn from_mem_place(MPlaceTy<'tcx, M::PointerTag>) -> Self; + + // Project to the n-th field + fn project_field( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + field: u64, + ) -> EvalResult<'tcx, Self>; +} + +// Operands and places are both values +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> + for OpTy<'tcx, M::PointerTag> +{ + #[inline(always)] + fn layout(&self) -> TyLayout<'tcx> { + self.layout + } + + #[inline(always)] + fn to_mem_place(self) -> MPlaceTy<'tcx, M::PointerTag> { + self.to_mem_place() + } + + #[inline(always)] + fn from_mem_place(mplace: MPlaceTy<'tcx, M::PointerTag>) -> Self { + mplace.into() + } + + #[inline(always)] + fn project_field( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + field: u64, + ) -> EvalResult<'tcx, Self> { + ectx.operand_field(self, field) + } +} + +// How to traverse a value and what to do when we are at the leaves. +pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { + type V: Value<'a, 'mir, 'tcx, M>; + + // There's a value in here. + fn value(&self) -> &Self::V; + + // The value's layout (not meant to be overwritten). + #[inline(always)] + fn layout(&self) -> TyLayout<'tcx> { + self.value().layout() + } + + // Replace the value by `val`, which must be the `field`th field of `self`, + // then call `f` and then un-do everything that might have happened to the visitor state. + // The point of this is that some visitors keep a stack of fields that we projected below, + // and this lets us avoid copying that stack; instead they will pop the stack after + // executing `f`. + fn with_field( + &mut self, + val: Self::V, + field: usize, + f: impl FnOnce(&mut Self) -> EvalResult<'tcx>, + ) -> EvalResult<'tcx>; + + // This is an enum, downcast it to whatever the current variant is. + // (We do this here and not in `Value` to keep error handling + // under control of th visitor.) fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>; - fn downcast_dyn_trait(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx>; - // Visit all fields of a compound. - // Just call `visit_value` if you want to go on recursively. - fn visit_fields(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, num_fields: usize) - -> EvalResult<'tcx>; - // Optimized handling for arrays -- avoid computing the layout for every field. - // Also it is the value's responsibility to figure out the length. - fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>; - // Special handling for strings. - fn visit_str(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx>; + // A chance for the visitor to do special (different or more efficient) handling for some + // array types. Return `true` if the value was handled and we should return. + #[inline] + fn handle_array(&mut self, _ectx: &EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx, bool> + { + Ok(false) + } // Actions on the leaves. + fn visit_uninhabited(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx>; fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) -> EvalResult<'tcx>; fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) @@ -45,7 +114,10 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - pub fn visit_value>(&mut self, v: &mut V) -> EvalResult<'tcx> { + pub fn visit_value>( + &mut self, + v: &mut V, + ) -> EvalResult<'tcx> { trace!("visit_value: {:?}", v); // If this is a multi-variant layout, we have find the right one and proceed with that. @@ -63,7 +135,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // If it is a trait object, switch to the actual type that was used to create it. match v.layout().ty.sty { ty::Dynamic(..) => { - v.downcast_dyn_trait(self)?; + let dest = v.value().to_mem_place(); // immediate trait objects are not a thing + let inner = self.unpack_dyn_trait(dest)?.1; + // recurse with the inner type + return v.with_field(Value::from_mem_place(inner), 0, |v| self.visit_value(v)); }, _ => {}, }; @@ -76,6 +151,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // scalars, we do the same check on every "level" (e.g. first we check // MyNewtype and then the scalar in there). match v.layout().abi { + layout::Abi::Uninhabited => { + v.visit_uninhabited(self)?; + } layout::Abi::Scalar(ref layout) => { v.visit_scalar(self, layout)?; } @@ -107,19 +185,31 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // We can't traverse unions, their bits are allowed to be anything. // The fields don't need to correspond to any bit pattern of the union's fields. // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389 - Ok(()) }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - v.visit_fields(self, offsets.len()) + for i in 0..offsets.len() { + let val = v.value().project_field(self, i as u64)?; + v.with_field(val, i, |v| self.visit_value(v))?; + } }, layout::FieldPlacement::Array { .. } => { - match v.layout().ty.sty { - // Strings have properties that cannot be expressed pointwise. - ty::Str => v.visit_str(self), - // General case -- might also be SIMD vector or so - _ => v.visit_array(self), + if !v.handle_array(self)? { + // We still have to work! + // Let's get an mplace first. + let mplace = if v.layout().is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(v.layout(), self) + } else { + // non-ZST array/slice/str cannot be immediate + v.value().to_mem_place() + }; + // Now iterate over it. + for (i, field) in self.mplace_array_fields(mplace)?.enumerate() { + v.with_field(Value::from_mem_place(field?), i, |v| self.visit_value(v))?; + } } } } + Ok(()) } } diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr index 178f80f88e8d6..947af20b8891f 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.stderr +++ b/src/test/ui/consts/const-eval/ub-upvars.stderr @@ -6,7 +6,7 @@ LL | | let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) }; LL | | let another_var = 13; LL | | move || { let _ = bad_ref; let _ = another_var; } LL | | }; - | |__^ type validation failed: encountered 0 at .., but expected something greater or equal to 1 + | |__^ type validation failed: encountered 0 at ..., but expected something greater or equal to 1 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr b/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr index b61ea9ca6f95b..13683ead0d5c5 100644 --- a/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr +++ b/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr @@ -66,7 +66,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/union-ub-fat-ptr.rs:116:1 | LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ., but expected something in the range 0..=1 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .., but expected something in the range 0..=1 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior From c8e471fac3f803d7c332ffea677dcd2b585910a3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 19:52:10 +0100 Subject: [PATCH 03/21] also allow visiting places and mplaces --- src/librustc_mir/interpret/visitor.rs | 85 ++++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 7d6029d6424f7..79b400a821ffa 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -10,7 +10,7 @@ use rustc::mir::interpret::{ }; use super::{ - Machine, EvalContext, MPlaceTy, OpTy, + Machine, EvalContext, MPlaceTy, PlaceTy, OpTy, }; // A thing that we can project into, and that has a layout. @@ -21,13 +21,16 @@ pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy // Get this value's layout. fn layout(&self) -> TyLayout<'tcx>; - // Get the underlying `MPlaceTy`, or panic if there is no such thing. - fn to_mem_place(self) -> MPlaceTy<'tcx, M::PointerTag>; + // Make this a `MPlaceTy`, or panic if that's not possible. + fn force_allocation( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>>; - // Create this from an `MPlaceTy` + // Create this from an `MPlaceTy`. fn from_mem_place(MPlaceTy<'tcx, M::PointerTag>) -> Self; - // Project to the n-th field + // Project to the n-th field. fn project_field( self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, @@ -45,8 +48,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn to_mem_place(self) -> MPlaceTy<'tcx, M::PointerTag> { - self.to_mem_place() + fn force_allocation( + self, + _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { + Ok(self.to_mem_place()) } #[inline(always)] @@ -63,6 +69,66 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> ectx.operand_field(self, field) } } +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> + for MPlaceTy<'tcx, M::PointerTag> +{ + #[inline(always)] + fn layout(&self) -> TyLayout<'tcx> { + self.layout + } + + #[inline(always)] + fn force_allocation( + self, + _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { + Ok(self) + } + + #[inline(always)] + fn from_mem_place(mplace: MPlaceTy<'tcx, M::PointerTag>) -> Self { + mplace + } + + #[inline(always)] + fn project_field( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + field: u64, + ) -> EvalResult<'tcx, Self> { + ectx.mplace_field(self, field) + } +} +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> + for PlaceTy<'tcx, M::PointerTag> +{ + #[inline(always)] + fn layout(&self) -> TyLayout<'tcx> { + self.layout + } + + #[inline(always)] + fn force_allocation( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { + ectx.force_allocation(self) + } + + #[inline(always)] + fn from_mem_place(mplace: MPlaceTy<'tcx, M::PointerTag>) -> Self { + mplace.into() + } + + #[inline(always)] + fn project_field( + self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + field: u64, + ) -> EvalResult<'tcx, Self> { + ectx.place_field(self, field) + } +} // How to traverse a value and what to do when we are at the leaves. pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { @@ -135,7 +201,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // If it is a trait object, switch to the actual type that was used to create it. match v.layout().ty.sty { ty::Dynamic(..) => { - let dest = v.value().to_mem_place(); // immediate trait objects are not a thing + // immediate trait objects are not a thing + let dest = v.value().force_allocation(self)?; let inner = self.unpack_dyn_trait(dest)?.1; // recurse with the inner type return v.with_field(Value::from_mem_place(inner), 0, |v| self.visit_value(v)); @@ -201,7 +268,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> MPlaceTy::dangling(v.layout(), self) } else { // non-ZST array/slice/str cannot be immediate - v.value().to_mem_place() + v.value().force_allocation(self)? }; // Now iterate over it. for (i, field) in self.mplace_array_fields(mplace)?.enumerate() { From fdc3a3ed0cf7ac9082bed9a70b733751e40c8137 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 21:07:17 +0100 Subject: [PATCH 04/21] fix for pre-NLL rustc --- src/librustc_mir/interpret/validity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 7caf81788dd23..8919db5a6d353 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -133,7 +133,7 @@ fn scalar_format(value: ScalarMaybeUndef) -> String { } } -struct ValidityVisitor<'rt, 'a, 'tcx, Tag> { +struct ValidityVisitor<'rt, 'a, 'tcx: 'a+'rt, Tag: 'static> { op: OpTy<'tcx, Tag>, /// The `path` may be pushed to, but the part that is present when a function /// starts must not be changed! `visit_fields` and `visit_array` rely on From 33770abbe249b91e40e5507d85855f8f844222b9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:11:23 +0100 Subject: [PATCH 05/21] add visit() hook to the trait --- src/librustc_mir/interpret/validity.rs | 8 +++---- src/librustc_mir/interpret/visitor.rs | 30 ++++++++++++++++---------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8919db5a6d353..2d977d109440e 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -207,11 +207,11 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } #[inline] - fn with_field( + fn visit_field( &mut self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, val: Self::V, field: usize, - f: impl FnOnce(&mut Self) -> EvalResult<'tcx>, ) -> EvalResult<'tcx> { // Remember the old state let path_len = self.path.len(); @@ -219,7 +219,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // Perform operation self.push_aggregate_field_path_elem(op.layout, field); self.op = val; - f(self)?; + self.visit(ectx)?; // Undo changes self.path.truncate(path_len); self.op = op; @@ -596,6 +596,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> }; // Run it - self.visit_value(&mut visitor) + visitor.visit(self) } } diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 79b400a821ffa..5eb9ff7627d6c 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -131,7 +131,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } // How to traverse a value and what to do when we are at the leaves. -pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { +pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + Sized { type V: Value<'a, 'mir, 'tcx, M>; // There's a value in here. @@ -143,16 +143,16 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { self.value().layout() } - // Replace the value by `val`, which must be the `field`th field of `self`, - // then call `f` and then un-do everything that might have happened to the visitor state. + // Replace the value by `val`, which must be the `field`th field of `self`, then call + // `visit_value` and then un-do everything that might have happened to the visitor state. // The point of this is that some visitors keep a stack of fields that we projected below, // and this lets us avoid copying that stack; instead they will pop the stack after - // executing `f`. - fn with_field( + // executing `visit_value`. + fn visit_field( &mut self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, val: Self::V, field: usize, - f: impl FnOnce(&mut Self) -> EvalResult<'tcx>, ) -> EvalResult<'tcx>; // This is an enum, downcast it to whatever the current variant is. @@ -170,6 +170,14 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { Ok(false) } + // Execute visitor on the current value. Used for recursing. + #[inline] + fn visit(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + ectx.walk_value(self) + } + // Actions on the leaves. fn visit_uninhabited(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx>; @@ -180,11 +188,11 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug { } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - pub fn visit_value>( + pub fn walk_value>( &mut self, v: &mut V, ) -> EvalResult<'tcx> { - trace!("visit_value: {:?}", v); + trace!("walk_value: {:?}", v); // If this is a multi-variant layout, we have find the right one and proceed with that. // (No benefit from making this recursion, but it is equivalent to that.) @@ -205,7 +213,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let dest = v.value().force_allocation(self)?; let inner = self.unpack_dyn_trait(dest)?.1; // recurse with the inner type - return v.with_field(Value::from_mem_place(inner), 0, |v| self.visit_value(v)); + return v.visit_field(self, Value::from_mem_place(inner), 0); }, _ => {}, }; @@ -256,7 +264,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> layout::FieldPlacement::Arbitrary { ref offsets, .. } => { for i in 0..offsets.len() { let val = v.value().project_field(self, i as u64)?; - v.with_field(val, i, |v| self.visit_value(v))?; + v.visit_field(self, val, i)?; } }, layout::FieldPlacement::Array { .. } => { @@ -272,7 +280,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> }; // Now iterate over it. for (i, field) in self.mplace_array_fields(mplace)?.enumerate() { - v.with_field(Value::from_mem_place(field?), i, |v| self.visit_value(v))?; + v.visit_field(self, Value::from_mem_place(field?), i)?; } } } From b0f1b1a73ee1c5be48d3a288eff50a46b773a9df Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:23:25 +0100 Subject: [PATCH 06/21] provide some default implementations --- src/librustc_mir/interpret/visitor.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 5eb9ff7627d6c..96e76ffd972db 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -179,12 +179,15 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + } // Actions on the leaves. - fn visit_uninhabited(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx>; - fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) - -> EvalResult<'tcx>; - fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx>; + fn visit_uninhabited(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { Ok(()) } + fn visit_scalar(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, _layout: &layout::Scalar) + -> EvalResult<'tcx> + { Ok(()) } + fn visit_primitive(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { Ok(()) } } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { From 6d24b37a70de0afc2b145aff72964162ec8a4291 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:53:21 +0100 Subject: [PATCH 07/21] let the Value handle enum projections, so the visitor does not have to care --- src/librustc_mir/interpret/validity.rs | 39 ++++++++++----------- src/librustc_mir/interpret/visitor.rs | 47 +++++++++++++++++++++----- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 2d977d109440e..f7ba92c40ebf1 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -173,11 +173,15 @@ impl<'rt, 'a, 'tcx, Tag> ValidityVisitor<'rt, 'a, 'tcx, Tag> { // enums ty::Adt(def, ..) if def.is_enum() => { - let variant = match layout.variants { - layout::Variants::Single { index } => &def.variants[index], - _ => bug!("aggregate_field_path_elem: got enum but not in a specific variant"), - }; - PathElem::Field(variant.fields[field].ident.name) + // we might be projecting *to* a variant, or to a field *in*a variant. + match layout.variants { + layout::Variants::Single { index } => + // Inside a variant + PathElem::Field(def.variants[index].fields[field].ident.name), + _ => + // To a variant + PathElem::Field(def.variants[field].name) + } } // other ADTs @@ -226,30 +230,21 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Ok(()) } - fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) + #[inline] + fn visit(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { - let variant = match ectx.read_discriminant(self.op) { - Ok(res) => res.1, - Err(err) => return match err.kind { + // Translate enum discriminant errors to something nicer. + match ectx.walk_value(self) { + Ok(()) => Ok(()), + Err(err) => match err.kind { EvalErrorKind::InvalidDiscriminant(val) => validation_failure!( format!("invalid enum discriminant {}", val), self.path ), - _ => - validation_failure!( - format!("non-integer enum discriminant"), self.path - ), + _ => Err(err), } - }; - // Put the variant projection onto the path, as a field - self.path.push(PathElem::Field(self.op.layout.ty - .ty_adt_def() - .unwrap() - .variants[variant].name)); - // Proceed with this variant - self.op = ectx.operand_downcast(self.op, variant)?; - Ok(()) + } } fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 96e76ffd972db..863eeb3b25002 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -30,6 +30,13 @@ pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy // Create this from an `MPlaceTy`. fn from_mem_place(MPlaceTy<'tcx, M::PointerTag>) -> Self; + // Read the current enum discriminant, and downcast to that. Also return the + // variant index. + fn project_downcast( + self, + ectx: &EvalContext<'a, 'mir, 'tcx, M> + ) -> EvalResult<'tcx, (Self, usize)>; + // Project to the n-th field. fn project_field( self, @@ -60,6 +67,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> mplace.into() } + #[inline(always)] + fn project_downcast( + self, + ectx: &EvalContext<'a, 'mir, 'tcx, M> + ) -> EvalResult<'tcx, (Self, usize)> { + let idx = ectx.read_discriminant(self)?.1; + Ok((ectx.operand_downcast(self, idx)?, idx)) + } + #[inline(always)] fn project_field( self, @@ -90,6 +106,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> mplace } + #[inline(always)] + fn project_downcast( + self, + ectx: &EvalContext<'a, 'mir, 'tcx, M> + ) -> EvalResult<'tcx, (Self, usize)> { + let idx = ectx.read_discriminant(self.into())?.1; + Ok((ectx.mplace_downcast(self, idx)?, idx)) + } + #[inline(always)] fn project_field( self, @@ -120,6 +145,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> mplace.into() } + #[inline(always)] + fn project_downcast( + self, + ectx: &EvalContext<'a, 'mir, 'tcx, M> + ) -> EvalResult<'tcx, (Self, usize)> { + let idx = ectx.read_discriminant(ectx.place_to_op(self)?)?.1; + Ok((ectx.place_downcast(self, idx)?, idx)) + } + #[inline(always)] fn project_field( self, @@ -155,12 +189,6 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + field: usize, ) -> EvalResult<'tcx>; - // This is an enum, downcast it to whatever the current variant is. - // (We do this here and not in `Value` to keep error handling - // under control of th visitor.) - fn downcast_enum(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx>; - // A chance for the visitor to do special (different or more efficient) handling for some // array types. Return `true` if the value was handled and we should return. #[inline] @@ -202,8 +230,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> match v.layout().variants { layout::Variants::NicheFilling { .. } | layout::Variants::Tagged { .. } => { - v.downcast_enum(self)?; - trace!("variant layout: {:#?}", v.layout()); + let (inner, idx) = v.value().project_downcast(self)?; + trace!("variant layout: {:#?}", inner.layout()); + // recurse with the inner type + return v.visit_field(self, inner, idx); } layout::Variants::Single { .. } => {} } @@ -215,6 +245,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // immediate trait objects are not a thing let dest = v.value().force_allocation(self)?; let inner = self.unpack_dyn_trait(dest)?.1; + trace!("dyn object layout: {:#?}", inner.layout); // recurse with the inner type return v.visit_field(self, Value::from_mem_place(inner), 0); }, From fa01e04fbb5aad536c4d5e89581af00adbd6e0cf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 14:14:51 +0100 Subject: [PATCH 08/21] fix validation error on non-integer enum discriminants --- src/librustc/mir/interpret/error.rs | 4 +-- src/librustc/mir/interpret/value.rs | 11 +++++++- src/librustc_mir/interpret/operand.rs | 20 +++++++++++-- src/librustc_mir/interpret/validity.rs | 27 ++++++------------ .../ui/consts/const-eval/double_check2.stderr | 2 +- src/test/ui/consts/const-eval/ub-enum.rs | 2 ++ src/test/ui/consts/const-eval/ub-enum.stderr | 10 +++---- src/test/ui/consts/const-eval/ub-nonnull.rs | 1 + .../ui/consts/const-eval/ub-nonnull.stderr | 6 ++-- src/test/ui/consts/const-eval/ub-ref.rs | 1 + src/test/ui/consts/const-eval/ub-ref.stderr | 10 +++---- src/test/ui/consts/const-eval/ub-uninhabit.rs | 1 + .../ui/consts/const-eval/ub-uninhabit.stderr | 6 ++-- src/test/ui/consts/const-eval/ub-upvars.rs | 1 + .../ui/consts/const-eval/ub-upvars.stderr | 2 +- .../ui/consts/const-eval/union-ub-fat-ptr.rs | 1 + .../consts/const-eval/union-ub-fat-ptr.stderr | 28 +++++++++---------- src/test/ui/consts/const-eval/union-ub.rs | 2 ++ src/test/ui/consts/const-eval/union-ub.stderr | 2 +- 19 files changed, 79 insertions(+), 58 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 8e025f0d029b7..f28aa41ed4222 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -15,7 +15,7 @@ use ty::{Ty, layout}; use ty::layout::{Size, Align, LayoutError}; use rustc_target::spec::abi::Abi; -use super::Pointer; +use super::{Pointer, Scalar}; use backtrace::Backtrace; @@ -240,7 +240,7 @@ pub enum EvalErrorKind<'tcx, O> { InvalidMemoryAccess, InvalidFunctionPointer, InvalidBool, - InvalidDiscriminant(u128), + InvalidDiscriminant(Scalar), PointerOutOfBounds { ptr: Pointer, access: bool, diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 3b8e19c6ecaa9..64723405b0358 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![allow(unknown_lints)] +use std::fmt; use ty::layout::{HasDataLayout, Size}; use ty::subst::Substs; @@ -99,6 +99,15 @@ pub enum Scalar { Ptr(Pointer), } +impl fmt::Display for Scalar { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Scalar::Ptr(_) => write!(f, "a pointer"), + Scalar::Bits { bits, .. } => write!(f, "{}", bits), + } + } +} + impl<'tcx> Scalar<()> { #[inline] pub fn with_default_tag(self) -> Scalar diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 6f66dd1e70a55..83a2d14b7ca4c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,6 +12,7 @@ //! All high-level functions to read from memory work on operands as sources. use std::convert::TryInto; +use std::fmt; use rustc::{mir, ty}; use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt}; @@ -36,6 +37,15 @@ impl From> for ScalarMaybeUndef { } } +impl fmt::Display for ScalarMaybeUndef { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ScalarMaybeUndef::Undef => write!(f, "uninitialized bytes"), + ScalarMaybeUndef::Scalar(s) => write!(f, "{}", s), + } + } +} + impl<'tcx> ScalarMaybeUndef<()> { #[inline] pub fn with_default_tag(self) -> ScalarMaybeUndef @@ -732,8 +742,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok(match rval.layout.variants { layout::Variants::Single { .. } => bug!(), layout::Variants::Tagged { .. } => { + let bits_discr = match raw_discr.to_bits(discr_val.layout.size) { + Ok(raw_discr) => raw_discr, + Err(_) => return err!(InvalidDiscriminant(raw_discr.erase_tag())), + }; let real_discr = if discr_val.layout.ty.is_signed() { - let i = raw_discr.to_bits(discr_val.layout.size)? as i128; + let i = bits_discr as i128; // going from layout tag type to typeck discriminant type // requires first sign extending with the layout discriminant let shift = 128 - discr_val.layout.size.bits(); @@ -748,7 +762,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let truncatee = sexted as u128; (truncatee << shift) >> shift } else { - raw_discr.to_bits(discr_val.layout.size)? + bits_discr }; // Make sure we catch invalid discriminants let index = rval.layout.ty @@ -756,7 +770,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .expect("tagged layout for non adt") .discriminants(self.tcx.tcx) .position(|var| var.val == real_discr) - .ok_or_else(|| EvalErrorKind::InvalidDiscriminant(real_discr))?; + .ok_or_else(|| EvalErrorKind::InvalidDiscriminant(raw_discr.erase_tag()))?; (real_discr, index) }, layout::Variants::NicheFilling { diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index f7ba92c40ebf1..f69b882bef867 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -20,7 +20,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef, ValueVisitor + OpTy, MPlaceTy, Machine, EvalContext, ValueVisitor }; macro_rules! validation_failure { @@ -122,17 +122,6 @@ fn path_format(path: &Vec) -> String { out } -fn scalar_format(value: ScalarMaybeUndef) -> String { - match value { - ScalarMaybeUndef::Undef => - "uninitialized bytes".to_owned(), - ScalarMaybeUndef::Scalar(Scalar::Ptr(_)) => - "a pointer".to_owned(), - ScalarMaybeUndef::Scalar(Scalar::Bits { bits, .. }) => - bits.to_string(), - } -} - struct ValidityVisitor<'rt, 'a, 'tcx: 'a+'rt, Tag: 'static> { op: OpTy<'tcx, Tag>, /// The `path` may be pushed to, but the part that is present when a function @@ -240,7 +229,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Err(err) => match err.kind { EvalErrorKind::InvalidDiscriminant(val) => validation_failure!( - format!("invalid enum discriminant {}", val), self.path + val, self.path, "a valid enum discriminant" ), _ => Err(err), } @@ -258,12 +247,12 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> ty::Bool => { let value = value.to_scalar_or_undef(); try_validation!(value.to_bool(), - scalar_format(value), self.path, "a boolean"); + value, self.path, "a boolean"); }, ty::Char => { let value = value.to_scalar_or_undef(); try_validation!(value.to_char(), - scalar_format(value), self.path, "a valid unicode codepoint"); + value, self.path, "a valid unicode codepoint"); }, ty::Float(_) | ty::Int(_) | ty::Uint(_) => { // NOTE: Keep this in sync with the array optimization for int/float @@ -273,7 +262,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> if self.const_mode { // Integers/floats in CTFE: Must be scalar bits, pointers are dangerous try_validation!(value.to_bits(size), - scalar_format(value), self.path, "initialized plain bits"); + value, self.path, "initialized plain bits"); } else { // At run-time, for now, we accept *anything* for these types, including // undef. We should fix that, but let's start low. @@ -389,9 +378,9 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> ty::FnPtr(_sig) => { let value = value.to_scalar_or_undef(); let ptr = try_validation!(value.to_ptr(), - scalar_format(value), self.path, "a pointer"); + value, self.path, "a pointer"); let _fn = try_validation!(ectx.memory.get_fn(ptr), - scalar_format(value), self.path, "a function pointer"); + value, self.path, "a function pointer"); // FIXME: Check if the signature matches } // This should be all the primitive types @@ -423,7 +412,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } // At least one value is excluded. Get the bits. let value = try_validation!(value.not_undef(), - scalar_format(value), self.path, + value, self.path, format!("something in the range {:?}", layout.valid_range)); let bits = match value { Scalar::Ptr(ptr) => { diff --git a/src/test/ui/consts/const-eval/double_check2.stderr b/src/test/ui/consts/const-eval/double_check2.stderr index 28825477c8102..7e82d4fc264f5 100644 --- a/src/test/ui/consts/const-eval/double_check2.stderr +++ b/src/test/ui/consts/const-eval/double_check2.stderr @@ -5,7 +5,7 @@ LL | / static FOO: (&Foo, &Bar) = unsafe {( //~ undefined behavior LL | | Union { u8: &BAR }.foo, LL | | Union { u8: &BAR }.bar, LL | | )}; - | |___^ type validation failed: encountered invalid enum discriminant 5 at .1. + | |___^ type validation failed: encountered 5 at .1., but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior diff --git a/src/test/ui/consts/const-eval/ub-enum.rs b/src/test/ui/consts/const-eval/ub-enum.rs index 2f7a5dda9ffcb..7b6527464c548 100644 --- a/src/test/ui/consts/const-eval/ub-enum.rs +++ b/src/test/ui/consts/const-eval/ub-enum.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![allow(const_err)] // make sure we cannot allow away the errors tested here + #[repr(usize)] #[derive(Copy, Clone)] enum Enum { diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 4cbaa2f3a906f..4ba30f1a9911a 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -1,21 +1,21 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:22:1 + --> $DIR/ub-enum.rs:24:1 | LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.b }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-integer enum discriminant + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:35:1 + --> $DIR/ub-enum.rs:37:1 | LL | const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid enum discriminant 0 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:45:1 + --> $DIR/ub-enum.rs:47:1 | LL | const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at .Some.0.1, but expected something in the range 0..=1114111 diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs index 6d6ad38afdb96..8b83a1747cabd 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.rs +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -9,6 +9,7 @@ // except according to those terms. #![feature(const_transmute)] +#![allow(const_err)] // make sure we cannot allow away the errors tested here use std::mem; use std::ptr::NonNull; diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index 1fdea3f38d38d..7c0ff851d8848 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:17:1 + --> $DIR/ub-nonnull.rs:18:1 | LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -7,7 +7,7 @@ LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:20:1 + --> $DIR/ub-nonnull.rs:21:1 | LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -15,7 +15,7 @@ LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:22:1 + --> $DIR/ub-nonnull.rs:23:1 | LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 diff --git a/src/test/ui/consts/const-eval/ub-ref.rs b/src/test/ui/consts/const-eval/ub-ref.rs index 2c2abd9127582..c0dd94a375bcb 100644 --- a/src/test/ui/consts/const-eval/ub-ref.rs +++ b/src/test/ui/consts/const-eval/ub-ref.rs @@ -9,6 +9,7 @@ // except according to those terms. #![feature(const_transmute)] +#![allow(const_err)] // make sure we cannot allow away the errors tested here use std::mem; diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 4ae4640d074e3..359d1abb5008e 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/src/test/ui/consts/const-eval/ub-ref.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-ref.rs:15:1 + --> $DIR/ub-ref.rs:16:1 | LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference @@ -7,7 +7,7 @@ LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-ref.rs:18:1 + --> $DIR/ub-ref.rs:19:1 | LL | const NULL: &u16 = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -15,7 +15,7 @@ LL | const NULL: &u16 = unsafe { mem::transmute(0usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-ref.rs:21:1 + --> $DIR/ub-ref.rs:22:1 | LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits @@ -23,7 +23,7 @@ LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-ref.rs:24:1 + --> $DIR/ub-ref.rs:25:1 | LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a raw memory access tried to access part of a pointer value as raw bytes @@ -31,7 +31,7 @@ LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }]; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-ref.rs:27:1 + --> $DIR/ub-ref.rs:28:1 | LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered integer pointer in non-ZST reference diff --git a/src/test/ui/consts/const-eval/ub-uninhabit.rs b/src/test/ui/consts/const-eval/ub-uninhabit.rs index 22262fd0b9774..74713af2ea033 100644 --- a/src/test/ui/consts/const-eval/ub-uninhabit.rs +++ b/src/test/ui/consts/const-eval/ub-uninhabit.rs @@ -9,6 +9,7 @@ // except according to those terms. #![feature(const_transmute)] +#![allow(const_err)] // make sure we cannot allow away the errors tested here use std::mem; diff --git a/src/test/ui/consts/const-eval/ub-uninhabit.stderr b/src/test/ui/consts/const-eval/ub-uninhabit.stderr index 2ac0a6e4e8673..c5ac72b639c05 100644 --- a/src/test/ui/consts/const-eval/ub-uninhabit.stderr +++ b/src/test/ui/consts/const-eval/ub-uninhabit.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-uninhabit.rs:18:1 + --> $DIR/ub-uninhabit.rs:19:1 | LL | const BAD_BAD_BAD: Bar = unsafe { mem::transmute(()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type @@ -7,7 +7,7 @@ LL | const BAD_BAD_BAD: Bar = unsafe { mem::transmute(()) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-uninhabit.rs:21:1 + --> $DIR/ub-uninhabit.rs:22:1 | LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at . @@ -15,7 +15,7 @@ LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-uninhabit.rs:24:1 + --> $DIR/ub-uninhabit.rs:25:1 | LL | const BAD_BAD_ARRAY: [Bar; 1] = unsafe { mem::transmute(()) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at [0] diff --git a/src/test/ui/consts/const-eval/ub-upvars.rs b/src/test/ui/consts/const-eval/ub-upvars.rs index f591a5affe5da..6661de4ab2cb5 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.rs +++ b/src/test/ui/consts/const-eval/ub-upvars.rs @@ -9,6 +9,7 @@ // except according to those terms. #![feature(const_transmute,const_let)] +#![allow(const_err)] // make sure we cannot allow away the errors tested here use std::mem; diff --git a/src/test/ui/consts/const-eval/ub-upvars.stderr b/src/test/ui/consts/const-eval/ub-upvars.stderr index 947af20b8891f..3617a53a9788d 100644 --- a/src/test/ui/consts/const-eval/ub-upvars.stderr +++ b/src/test/ui/consts/const-eval/ub-upvars.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-upvars.rs:15:1 + --> $DIR/ub-upvars.rs:16:1 | LL | / const BAD_UPVAR: &FnOnce() = &{ //~ ERROR it is undefined behavior to use this value LL | | let bad_ref: &'static u16 = unsafe { mem::transmute(0usize) }; diff --git a/src/test/ui/consts/const-eval/union-ub-fat-ptr.rs b/src/test/ui/consts/const-eval/union-ub-fat-ptr.rs index 479cee92b94b3..31540b46631c0 100644 --- a/src/test/ui/consts/const-eval/union-ub-fat-ptr.rs +++ b/src/test/ui/consts/const-eval/union-ub-fat-ptr.rs @@ -9,6 +9,7 @@ // except according to those terms. #![allow(unused)] +#![allow(const_err)] // make sure we cannot allow away the errors tested here // normalize-stderr-test "alignment \d+" -> "alignment N" // normalize-stderr-test "offset \d+" -> "offset N" diff --git a/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr b/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr index 13683ead0d5c5..9a799b98cfc57 100644 --- a/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr +++ b/src/test/ui/consts/const-eval/union-ub-fat-ptr.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:87:1 + --> $DIR/union-ub-fat-ptr.rs:88:1 | LL | const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling (not entirely in bounds) reference @@ -7,7 +7,7 @@ LL | const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:90:1 + --> $DIR/union-ub-fat-ptr.rs:91:1 | LL | const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.str}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-integer slice length in fat pointer @@ -15,7 +15,7 @@ LL | const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:93:1 + --> $DIR/union-ub-fat-ptr.rs:94:1 | LL | const C2: &MyStr = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.my_str}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-integer slice length in fat pointer @@ -23,7 +23,7 @@ LL | const C2: &MyStr = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:99:1 + --> $DIR/union-ub-fat-ptr.rs:100:1 | LL | const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling (not entirely in bounds) reference @@ -31,7 +31,7 @@ LL | const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:102:1 + --> $DIR/union-ub-fat-ptr.rs:103:1 | LL | const C3: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-integer slice length in fat pointer @@ -39,7 +39,7 @@ LL | const C3: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, l = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:106:1 + --> $DIR/union-ub-fat-ptr.rs:107:1 | LL | const D: &Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: &3 } }.rust}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable @@ -47,7 +47,7 @@ LL | const D: &Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:109:1 + --> $DIR/union-ub-fat-ptr.rs:110:1 | LL | const E: &Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtable: &3 } }.rust}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable @@ -55,7 +55,7 @@ LL | const E: &Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtabl = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:112:1 + --> $DIR/union-ub-fat-ptr.rs:113:1 | LL | const F: &Trait = unsafe { DynTransmute { bad: BadDynRepr { ptr: &92, vtable: 3 } }.rust}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-pointer vtable in fat pointer @@ -63,7 +63,7 @@ LL | const F: &Trait = unsafe { DynTransmute { bad: BadDynRepr { ptr: &92, vtabl = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:116:1 + --> $DIR/union-ub-fat-ptr.rs:117:1 | LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .., but expected something in the range 0..=1 @@ -71,7 +71,7 @@ LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:120:1 + --> $DIR/union-ub-fat-ptr.rs:121:1 | LL | const H: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .[0], but expected something in the range 0..=1 @@ -79,7 +79,7 @@ LL | const H: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }]; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:126:1 + --> $DIR/union-ub-fat-ptr.rs:127:1 | LL | const I2: &MySliceBool = &MySlice(unsafe { BoolTransmute { val: 3 }.bl }, [false]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..0, but expected something in the range 0..=1 @@ -87,7 +87,7 @@ LL | const I2: &MySliceBool = &MySlice(unsafe { BoolTransmute { val: 3 }.bl }, [ = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:129:1 + --> $DIR/union-ub-fat-ptr.rs:130:1 | LL | const I3: &MySliceBool = &MySlice(true, [unsafe { BoolTransmute { val: 3 }.bl }]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..1[0], but expected something in the range 0..=1 @@ -95,7 +95,7 @@ LL | const I3: &MySliceBool = &MySlice(true, [unsafe { BoolTransmute { val: 3 }. = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:133:1 + --> $DIR/union-ub-fat-ptr.rs:134:1 | LL | const J1: &str = unsafe { SliceTransmute { slice: &[0xFF] }.str }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at . @@ -103,7 +103,7 @@ LL | const J1: &str = unsafe { SliceTransmute { slice: &[0xFF] }.str }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub-fat-ptr.rs:136:1 + --> $DIR/union-ub-fat-ptr.rs:137:1 | LL | const J2: &MyStr = unsafe { SliceTransmute { slice: &[0xFF] }.my_str }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at ..0 diff --git a/src/test/ui/consts/const-eval/union-ub.rs b/src/test/ui/consts/const-eval/union-ub.rs index 008f1f2364866..712147b52e964 100644 --- a/src/test/ui/consts/const-eval/union-ub.rs +++ b/src/test/ui/consts/const-eval/union-ub.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#![allow(const_err)] // make sure we cannot allow away the errors tested here + union DummyUnion { u8: u8, bool: bool, diff --git a/src/test/ui/consts/const-eval/union-ub.stderr b/src/test/ui/consts/const-eval/union-ub.stderr index bb916ddbbcfd7..2b6f6a47cd92f 100644 --- a/src/test/ui/consts/const-eval/union-ub.stderr +++ b/src/test/ui/consts/const-eval/union-ub.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/union-ub.rs:36:1 + --> $DIR/union-ub.rs:38:1 | LL | const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 0..=1 From 77c283465c567d72868714c4ee4f2328e0800346 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 14:41:36 +0100 Subject: [PATCH 09/21] Also test for undef in enum discriminant The error message is sub-par, but fixing that requries moving ScalarMaybeUndef to librustc which would conflict badly with another PR that is in flight. --- src/test/ui/consts/const-eval/ub-enum.rs | 5 +++++ src/test/ui/consts/const-eval/ub-enum.stderr | 14 +++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/test/ui/consts/const-eval/ub-enum.rs b/src/test/ui/consts/const-eval/ub-enum.rs index 7b6527464c548..309627581d051 100644 --- a/src/test/ui/consts/const-eval/ub-enum.rs +++ b/src/test/ui/consts/const-eval/ub-enum.rs @@ -33,10 +33,15 @@ enum Enum2 { union TransmuteEnum2 { a: usize, b: Enum2, + c: (), } const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b }; //~^ ERROR is undefined behavior +// Undef enum discriminant. In an arry to avoid `Scalar` layout. +const BAD_ENUM3 : [Enum2; 2] = [unsafe { TransmuteEnum2 { c: () }.b }; 2]; +//~^ ERROR is undefined behavior + // Invalid enum field content (mostly to test printing of apths for enum tuple // variants and tuples). union TransmuteChar { diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 4ba30f1a9911a..57c41711536d9 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -7,7 +7,7 @@ LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { a: &1 }.b }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:37:1 + --> $DIR/ub-enum.rs:38:1 | LL | const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected a valid enum discriminant @@ -15,13 +15,21 @@ LL | const BAD_ENUM2 : Enum2 = unsafe { TransmuteEnum2 { a: 0 }.b }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:47:1 + --> $DIR/ub-enum.rs:42:1 + | +LL | const BAD_ENUM3 : [Enum2; 2] = [unsafe { TransmuteEnum2 { c: () }.b }; 2]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to read undefined bytes + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior + +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-enum.rs:52:1 | LL | const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at .Some.0.1, but expected something in the range 0..=1114111 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0080`. From aea61e398c4e65b8572d71188cc098e5ec5fab54 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 22:27:55 +0100 Subject: [PATCH 10/21] converting a VisitorValue to a MemPlace must not mutate anything --- src/librustc_mir/interpret/visitor.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 863eeb3b25002..fe61cd78a4623 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -22,9 +22,9 @@ pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy fn layout(&self) -> TyLayout<'tcx>; // Make this a `MPlaceTy`, or panic if that's not possible. - fn force_allocation( + fn to_mem_place( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ectx: &EvalContext<'a, 'mir, 'tcx, M>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>>; // Create this from an `MPlaceTy`. @@ -55,9 +55,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn force_allocation( + fn to_mem_place( self, - _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + _ectx: &EvalContext<'a, 'mir, 'tcx, M>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { Ok(self.to_mem_place()) } @@ -94,9 +94,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn force_allocation( + fn to_mem_place( self, - _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + _ectx: &EvalContext<'a, 'mir, 'tcx, M>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { Ok(self) } @@ -133,11 +133,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn force_allocation( + fn to_mem_place( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ectx: &EvalContext<'a, 'mir, 'tcx, M>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - ectx.force_allocation(self) + // If this refers to a local, assert that it already has an allocation. + Ok(ectx.place_to_op(self)?.to_mem_place()) } #[inline(always)] @@ -243,7 +244,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> match v.layout().ty.sty { ty::Dynamic(..) => { // immediate trait objects are not a thing - let dest = v.value().force_allocation(self)?; + let dest = v.value().to_mem_place(self)?; let inner = self.unpack_dyn_trait(dest)?.1; trace!("dyn object layout: {:#?}", inner.layout); // recurse with the inner type @@ -310,7 +311,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> MPlaceTy::dangling(v.layout(), self) } else { // non-ZST array/slice/str cannot be immediate - v.value().force_allocation(self)? + v.value().to_mem_place(self)? }; // Now iterate over it. for (i, field) in self.mplace_array_fields(mplace)?.enumerate() { From 98295e9eb2e9fdc3cf1e9c819865e515a3125bc4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 08:17:40 +0100 Subject: [PATCH 11/21] use more traditional walk_array/visit_array instead of the handle_array hook --- src/librustc_mir/interpret/validity.rs | 24 ++--- src/librustc_mir/interpret/visitor.rs | 120 ++++++++++++------------- 2 files changed, 73 insertions(+), 71 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index f69b882bef867..45645a714d0aa 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -212,7 +212,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // Perform operation self.push_aggregate_field_path_elem(op.layout, field); self.op = val; - self.visit(ectx)?; + self.visit_value(ectx)?; // Undo changes self.path.truncate(path_len); self.op = op; @@ -220,11 +220,11 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } #[inline] - fn visit(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + fn visit_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { // Translate enum discriminant errors to something nicer. - match ectx.walk_value(self) { + match self.walk_value(ectx) { Ok(()) => Ok(()), Err(err) => match err.kind { EvalErrorKind::InvalidDiscriminant(val) => @@ -479,15 +479,14 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } - fn handle_array(&mut self, ectx: &EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx, bool> + fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> { - Ok(match self.op.layout.ty.sty { + match self.op.layout.ty.sty { ty::Str => { let mplace = self.op.to_mem_place(); // strings are never immediate try_validation!(ectx.read_str(mplace), "uninitialized or non-UTF-8 data in str", self.path); - true } ty::Array(tys, ..) | ty::Slice(tys) if { // This optimization applies only for integer and floating point types @@ -526,7 +525,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> /*allow_ptr_and_undef*/!self.const_mode, ) { // In the happy case, we needn't check anything else. - Ok(()) => true, // handled these arrays + Ok(()) => {}, // Some error happened, try to provide a more detailed description. Err(err) => { // For some errors we might be able to provide extra information @@ -548,8 +547,11 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } } - _ => false, // not handled - }) + _ => { + self.walk_array(ectx)? // default handler + } + } + Ok(()) } } @@ -580,6 +582,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> }; // Run it - visitor.visit(self) + visitor.visit_value(self) } } diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index fe61cd78a4623..8b153e17c1e5d 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -178,77 +178,91 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + self.value().layout() } - // Replace the value by `val`, which must be the `field`th field of `self`, then call - // `visit_value` and then un-do everything that might have happened to the visitor state. - // The point of this is that some visitors keep a stack of fields that we projected below, - // and this lets us avoid copying that stack; instead they will pop the stack after - // executing `visit_value`. - fn visit_field( - &mut self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, - val: Self::V, - field: usize, - ) -> EvalResult<'tcx>; - - // A chance for the visitor to do special (different or more efficient) handling for some - // array types. Return `true` if the value was handled and we should return. + // Recursie actions, ready to be overloaded. + /// Visit the current value, dispatching as appropriate to more speicalized visitors. #[inline] - fn handle_array(&mut self, _ectx: &EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx, bool> + fn visit_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> { - Ok(false) + self.walk_value(ectx) } - - // Execute visitor on the current value. Used for recursing. + /// Visit the current value as an array. #[inline] - fn visit(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { - ectx.walk_value(self) + self.walk_array(ectx) } + /// Called each time we recurse down to a field of the value, to (a) let + /// the visitor change its internal state (recording the new current value), + /// and (b) let the visitor track the "stack" of fields that we descended below. + fn visit_field( + &mut self, + ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + val: Self::V, + field: usize, + ) -> EvalResult<'tcx>; - // Actions on the leaves. + // Actions on the leaves, ready to be overloaded. + #[inline] fn visit_uninhabited(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { Ok(()) } + #[inline] fn visit_scalar(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, _layout: &layout::Scalar) -> EvalResult<'tcx> { Ok(()) } + #[inline] fn visit_primitive(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) -> EvalResult<'tcx> { Ok(()) } -} -impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - pub fn walk_value>( - &mut self, - v: &mut V, - ) -> EvalResult<'tcx> { - trace!("walk_value: {:?}", v); + // Default recursors. Not meant to be overloaded. + fn walk_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + // Let's get an mplace first. + let mplace = if self.layout().is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(self.layout(), ectx) + } else { + // non-ZST array/slice/str cannot be immediate + self.value().to_mem_place(ectx)? + }; + // Now iterate over it. + for (i, field) in ectx.mplace_array_fields(mplace)?.enumerate() { + self.visit_field(ectx, Value::from_mem_place(field?), i)?; + } + Ok(()) + } + fn walk_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + -> EvalResult<'tcx> + { + trace!("walk_value: {:?}", self); // If this is a multi-variant layout, we have find the right one and proceed with that. // (No benefit from making this recursion, but it is equivalent to that.) - match v.layout().variants { + match self.layout().variants { layout::Variants::NicheFilling { .. } | layout::Variants::Tagged { .. } => { - let (inner, idx) = v.value().project_downcast(self)?; + let (inner, idx) = self.value().project_downcast(ectx)?; trace!("variant layout: {:#?}", inner.layout()); // recurse with the inner type - return v.visit_field(self, inner, idx); + return self.visit_field(ectx, inner, idx); } layout::Variants::Single { .. } => {} } // Even for single variants, we might be able to get a more refined type: // If it is a trait object, switch to the actual type that was used to create it. - match v.layout().ty.sty { + match self.layout().ty.sty { ty::Dynamic(..) => { // immediate trait objects are not a thing - let dest = v.value().to_mem_place(self)?; - let inner = self.unpack_dyn_trait(dest)?.1; + let dest = self.value().to_mem_place(ectx)?; + let inner = ectx.unpack_dyn_trait(dest)?.1; trace!("dyn object layout: {:#?}", inner.layout); // recurse with the inner type - return v.visit_field(self, Value::from_mem_place(inner), 0); + return self.visit_field(ectx, Value::from_mem_place(inner), 0); }, _ => {}, }; @@ -260,12 +274,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // FIXME: We could avoid some redundant checks here. For newtypes wrapping // scalars, we do the same check on every "level" (e.g. first we check // MyNewtype and then the scalar in there). - match v.layout().abi { + match self.layout().abi { layout::Abi::Uninhabited => { - v.visit_uninhabited(self)?; + self.visit_uninhabited(ectx)?; } layout::Abi::Scalar(ref layout) => { - v.visit_scalar(self, layout)?; + self.visit_scalar(ectx, layout)?; } // FIXME: Should we do something for ScalarPair? Vector? _ => {} @@ -276,17 +290,17 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // so we check them separately and before aggregate handling. // It is CRITICAL that we get this check right, or we might be // validating the wrong thing! - let primitive = match v.layout().fields { + let primitive = match self.layout().fields { // Primitives appear as Union with 0 fields -- except for Boxes and fat pointers. layout::FieldPlacement::Union(0) => true, - _ => v.layout().ty.builtin_deref(true).is_some(), + _ => self.layout().ty.builtin_deref(true).is_some(), }; if primitive { - return v.visit_primitive(self); + return self.visit_primitive(ectx); } // Proceed into the fields. - match v.layout().fields { + match self.layout().fields { layout::FieldPlacement::Union(fields) => { // Empty unions are not accepted by rustc. That's great, it means we can // use that as an unambiguous signal for detecting primitives. Make sure @@ -298,26 +312,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { for i in 0..offsets.len() { - let val = v.value().project_field(self, i as u64)?; - v.visit_field(self, val, i)?; + let val = self.value().project_field(ectx, i as u64)?; + self.visit_field(ectx, val, i)?; } }, layout::FieldPlacement::Array { .. } => { - if !v.handle_array(self)? { - // We still have to work! - // Let's get an mplace first. - let mplace = if v.layout().is_zst() { - // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(v.layout(), self) - } else { - // non-ZST array/slice/str cannot be immediate - v.value().to_mem_place(self)? - }; - // Now iterate over it. - for (i, field) in self.mplace_array_fields(mplace)?.enumerate() { - v.visit_field(self, Value::from_mem_place(field?), i)?; - } - } + self.visit_array(ectx)?; } } Ok(()) From b096f0846ea9423bf44db0d2fa87ede6cb9a7cf1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 09:33:26 +0100 Subject: [PATCH 12/21] finally this actually looks like a visitor --- src/librustc_mir/interpret/validity.rs | 120 +++++++++++------------ src/librustc_mir/interpret/visitor.rs | 130 ++++++++++++------------- 2 files changed, 118 insertions(+), 132 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 45645a714d0aa..c303d2a1e67ca 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -8,12 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::fmt::{self, Write}; +use std::fmt::Write; use std::hash::Hash; use syntax_pos::symbol::Symbol; use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf}; -use rustc::ty::{self, TyCtxt}; +use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc::mir::interpret::{ Scalar, AllocType, EvalResult, EvalErrorKind @@ -122,24 +122,17 @@ fn path_format(path: &Vec) -> String { out } -struct ValidityVisitor<'rt, 'a, 'tcx: 'a+'rt, Tag: 'static> { - op: OpTy<'tcx, Tag>, +struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a, 'mir, 'tcx>+'rt> { /// The `path` may be pushed to, but the part that is present when a function /// starts must not be changed! `visit_fields` and `visit_array` rely on /// this stack discipline. path: Vec, - ref_tracking: Option<&'rt mut RefTracking<'tcx, Tag>>, + ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, const_mode: bool, - tcx: TyCtxt<'a, 'tcx, 'tcx>, + ecx: &'rt mut EvalContext<'a, 'mir, 'tcx, M>, } -impl fmt::Debug for ValidityVisitor<'_, '_, '_, Tag> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{:?}, {:?}", *self.op, self.op.layout.ty) - } -} - -impl<'rt, 'a, 'tcx, Tag> ValidityVisitor<'rt, 'a, 'tcx, Tag> { +impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> ValidityVisitor<'rt, 'a, 'mir, 'tcx, M> { fn push_aggregate_field_path_elem( &mut self, layout: TyLayout<'tcx>, @@ -148,7 +141,7 @@ impl<'rt, 'a, 'tcx, Tag> ValidityVisitor<'rt, 'a, 'tcx, Tag> { let elem = match layout.ty.sty { // generators and closures. ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => { - if let Some(upvar) = self.tcx.optimized_mir(def_id).upvar_decls.get(field) { + if let Some(upvar) = self.ecx.tcx.optimized_mir(def_id).upvar_decls.get(field) { PathElem::ClosureVar(upvar.debug_name) } else { // Sometimes the index is beyond the number of freevars (seen @@ -190,41 +183,38 @@ impl<'rt, 'a, 'tcx, Tag> ValidityVisitor<'rt, 'a, 'tcx, Tag> { } impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> - ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'a, 'tcx, M::PointerTag> + ValueVisitor<'a, 'mir, 'tcx, M> for ValidityVisitor<'rt, 'a, 'mir, 'tcx, M> { type V = OpTy<'tcx, M::PointerTag>; #[inline(always)] - fn value(&self) -> &OpTy<'tcx, M::PointerTag> { - &self.op + fn ecx(&mut self) -> &mut EvalContext<'a, 'mir, 'tcx, M> { + &mut self.ecx } #[inline] fn visit_field( &mut self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, - val: Self::V, + old_op: OpTy<'tcx, M::PointerTag>, field: usize, + new_op: OpTy<'tcx, M::PointerTag> ) -> EvalResult<'tcx> { // Remember the old state let path_len = self.path.len(); - let op = self.op; // Perform operation - self.push_aggregate_field_path_elem(op.layout, field); - self.op = val; - self.visit_value(ectx)?; + self.push_aggregate_field_path_elem(old_op.layout, field); + self.visit_value(new_op)?; // Undo changes self.path.truncate(path_len); - self.op = op; Ok(()) } #[inline] - fn visit_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + fn visit_value(&mut self, op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { + trace!("visit_value: {:?}, {:?}", *op, op.layout); // Translate enum discriminant errors to something nicer. - match self.walk_value(ectx) { + match self.walk_value(op) { Ok(()) => Ok(()), Err(err) => match err.kind { EvalErrorKind::InvalidDiscriminant(val) => @@ -236,10 +226,10 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } - fn visit_primitive(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + fn visit_primitive(&mut self, op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { - let value = try_validation!(ectx.read_immediate(self.op), + let value = try_validation!(self.ecx.read_immediate(op), "uninitialized or unrepresentable data", self.path); // Go over all the primitive types let ty = value.layout.ty; @@ -283,21 +273,21 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> "undefined address in pointer", self.path); let meta = try_validation!(value.to_meta(), "uninitialized data in fat pointer metadata", self.path); - let layout = ectx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; + let layout = self.ecx.layout_of(value.layout.ty.builtin_deref(true).unwrap().ty)?; if layout.is_unsized() { - let tail = ectx.tcx.struct_tail(layout.ty); + let tail = self.ecx.tcx.struct_tail(layout.ty); match tail.sty { ty::Dynamic(..) => { let vtable = try_validation!(meta.unwrap().to_ptr(), "non-pointer vtable in fat pointer", self.path); - try_validation!(ectx.read_drop_type_from_vtable(vtable), + try_validation!(self.ecx.read_drop_type_from_vtable(vtable), "invalid drop fn in vtable", self.path); - try_validation!(ectx.read_size_and_align_from_vtable(vtable), + try_validation!(self.ecx.read_size_and_align_from_vtable(vtable), "invalid size or align in vtable", self.path); // FIXME: More checks for the vtable. } ty::Slice(..) | ty::Str => { - try_validation!(meta.unwrap().to_usize(ectx), + try_validation!(meta.unwrap().to_usize(self.ecx), "non-integer slice length in fat pointer", self.path); } ty::Foreign(..) => { @@ -308,12 +298,12 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } // Make sure this is non-NULL and aligned - let (size, align) = ectx.size_and_align_of(meta, layout)? + let (size, align) = self.ecx.size_and_align_of(meta, layout)? // for the purpose of validity, consider foreign types to have // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| layout.size_and_align()); - match ectx.memory.check_align(ptr, align) { + match self.ecx.memory.check_align(ptr, align) { Ok(_) => {}, Err(err) => { error!("{:?} is not aligned to {:?}", ptr, align); @@ -334,7 +324,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // Turn ptr into place. // `ref_to_mplace` also calls the machine hook for (re)activating the tag, // which in turn will (in full miri) check if the pointer is dereferencable. - let place = ectx.ref_to_mplace(value)?; + let place = self.ecx.ref_to_mplace(value)?; // Recursive checking if let Some(ref mut ref_tracking) = self.ref_tracking { assert!(self.const_mode, "We should only do recursie checking in const mode"); @@ -343,19 +333,19 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> let ptr = try_validation!(place.ptr.to_ptr(), "integer pointer in non-ZST reference", self.path); // Skip validation entirely for some external statics - let alloc_kind = ectx.tcx.alloc_map.lock().get(ptr.alloc_id); + let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id); if let Some(AllocType::Static(did)) = alloc_kind { // `extern static` cannot be validated as they have no body. // FIXME: Statics from other crates are also skipped. // They might be checked at a different type, but for now we // want to avoid recursing too deeply. This is not sound! - if !did.is_local() || ectx.tcx.is_foreign_item(did) { + if !did.is_local() || self.ecx.tcx.is_foreign_item(did) { return Ok(()); } } // Maintain the invariant that the place we are checking is // already verified to be in-bounds. - try_validation!(ectx.memory.check_bounds(ptr, size, false), + try_validation!(self.ecx.memory.check_bounds(ptr, size, false), "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination @@ -379,7 +369,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> let value = value.to_scalar_or_undef(); let ptr = try_validation!(value.to_ptr(), value, self.path, "a pointer"); - let _fn = try_validation!(ectx.memory.get_fn(ptr), + let _fn = try_validation!(self.ecx.memory.get_fn(ptr), value, self.path, "a function pointer"); // FIXME: Check if the signature matches } @@ -389,21 +379,23 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Ok(()) } - fn visit_uninhabited(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) + fn visit_uninhabited(&mut self, _op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { validation_failure!("a value of an uninhabited type", self.path) } - fn visit_scalar(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, layout: &layout::Scalar) - -> EvalResult<'tcx> - { - let value = try_validation!(ectx.read_scalar(self.op), + fn visit_scalar( + &mut self, + op: OpTy<'tcx, M::PointerTag>, + layout: &layout::Scalar, + ) -> EvalResult<'tcx> { + let value = try_validation!(self.ecx.read_scalar(op), "uninitialized or unrepresentable data", self.path); // Determine the allowed range let (lo, hi) = layout.valid_range.clone().into_inner(); // `max_hi` is as big as the size fits - let max_hi = u128::max_value() >> (128 - self.op.layout.size.bits()); + let max_hi = u128::max_value() >> (128 - op.layout.size.bits()); assert!(hi <= max_hi); // We could also write `(hi + 1) % (max_hi + 1) == lo` but `max_hi + 1` overflows for `u128` if (lo == 0 && hi == max_hi) || (hi + 1 == lo) { @@ -421,10 +413,10 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // We can call `check_align` to check non-NULL-ness, but have to also look // for function pointers. let non_null = - ectx.memory.check_align( + self.ecx.memory.check_align( Scalar::Ptr(ptr), Align::from_bytes(1, 1).unwrap() ).is_ok() || - ectx.memory.get_fn(ptr).is_ok(); + self.ecx.memory.get_fn(ptr).is_ok(); if !non_null { // could be NULL return validation_failure!("a potentially NULL pointer", self.path); @@ -444,7 +436,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } Scalar::Bits { bits, size } => { - assert_eq!(size as u64, self.op.layout.size.bytes()); + assert_eq!(size as u64, op.layout.size.bytes()); bits } }; @@ -479,13 +471,12 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } - fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + fn visit_array(&mut self, op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { - match self.op.layout.ty.sty { + match op.layout.ty.sty { ty::Str => { - let mplace = self.op.to_mem_place(); // strings are never immediate - try_validation!(ectx.read_str(mplace), + let mplace = op.to_mem_place(); // strings are never immediate + try_validation!(self.ecx.read_str(mplace), "uninitialized or non-UTF-8 data in str", self.path); } ty::Array(tys, ..) | ty::Slice(tys) if { @@ -496,17 +487,17 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> _ => false, } } => { - let mplace = if self.op.layout.is_zst() { + let mplace = if op.layout.is_zst() { // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(self.op.layout, ectx) + MPlaceTy::dangling(op.layout, self.ecx) } else { // non-ZST array/slice/str cannot be immediate - self.op.to_mem_place() + op.to_mem_place() }; // This is the length of the array/slice. - let len = mplace.len(ectx)?; + let len = mplace.len(self.ecx)?; // This is the element type size. - let ty_size = ectx.layout_of(tys)?.size; + let ty_size = self.ecx.layout_of(tys)?.size; // This is the size in bytes of the whole array. let size = ty_size * len; @@ -519,7 +510,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // to reject those pointers, we just do not have the machinery to // talk about parts of a pointer. // We also accept undef, for consistency with the type-based checks. - match ectx.memory.check_bytes( + match self.ecx.memory.check_bytes( mplace.ptr, size, /*allow_ptr_and_undef*/!self.const_mode, @@ -548,7 +539,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } _ => { - self.walk_array(ectx)? // default handler + self.walk_array(op)? // default handler } } Ok(()) @@ -574,14 +565,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Construct a visitor let mut visitor = ValidityVisitor { - op, path, ref_tracking, const_mode, - tcx: *self.tcx, + ecx: self, }; // Run it - visitor.visit_value(self) + visitor.visit_value(op) } } diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 8b153e17c1e5d..f50ef96775f9d 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -1,8 +1,6 @@ //! Visitor for a run-time value with a given layout: Traverse enums, structs and other compound //! types until we arrive at the leaves, with custom handling for primitive types. -use std::fmt; - use rustc::ty::layout::{self, TyLayout}; use rustc::ty; use rustc::mir::interpret::{ @@ -166,103 +164,103 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } // How to traverse a value and what to do when we are at the leaves. -pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + Sized { +pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Sized { type V: Value<'a, 'mir, 'tcx, M>; - // There's a value in here. - fn value(&self) -> &Self::V; + // The visitor must have an `EvalContext` in it. + fn ecx(&mut self) -> &mut EvalContext<'a, 'mir, 'tcx, M>; - // The value's layout (not meant to be overwritten). + // Recursie actions, ready to be overloaded. + /// Visit the given value, dispatching as appropriate to more speicalized visitors. #[inline(always)] - fn layout(&self) -> TyLayout<'tcx> { - self.value().layout() + fn visit_value(&mut self, v: Self::V) -> EvalResult<'tcx> + { + self.walk_value(v) } - - // Recursie actions, ready to be overloaded. - /// Visit the current value, dispatching as appropriate to more speicalized visitors. - #[inline] - fn visit_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + /// Visit the given value as a union. + #[inline(always)] + fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> { - self.walk_value(ectx) + Ok(()) } - /// Visit the current value as an array. - #[inline] - fn visit_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + /// Visit the given value as an array. + #[inline(always)] + fn visit_array(&mut self, v: Self::V) -> EvalResult<'tcx> { - self.walk_array(ectx) + self.walk_array(v) } - /// Called each time we recurse down to a field of the value, to (a) let - /// the visitor change its internal state (recording the new current value), - /// and (b) let the visitor track the "stack" of fields that we descended below. + /// Called each time we recurse down to a field, passing in old and new value. + /// This gives the visitor the chance to track the stack of nested fields that + /// we are descending through. + #[inline(always)] fn visit_field( &mut self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, - val: Self::V, - field: usize, - ) -> EvalResult<'tcx>; + _old_val: Self::V, + _field: usize, + new_val: Self::V, + ) -> EvalResult<'tcx> { + self.visit_value(new_val) + } // Actions on the leaves, ready to be overloaded. - #[inline] - fn visit_uninhabited(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + /// Called whenever we reach a value with uninhabited layout. + /// Recursing to fields will continue after this! + #[inline(always)] + fn visit_uninhabited(&mut self, _v: Self::V) -> EvalResult<'tcx> { Ok(()) } - #[inline] - fn visit_scalar(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, _layout: &layout::Scalar) - -> EvalResult<'tcx> + /// Called whenever we reach a value with scalar layout. + /// Recursing to fields will continue after this! + #[inline(always)] + fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> EvalResult<'tcx> { Ok(()) } - #[inline] - fn visit_primitive(&mut self, _ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + /// Called whenever we reach a value of primitive type. There can be no recursion + /// below such a value. + #[inline(always)] + fn visit_primitive(&mut self, _v: Self::V) -> EvalResult<'tcx> { Ok(()) } // Default recursors. Not meant to be overloaded. - fn walk_array(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + fn walk_array(&mut self, v: Self::V) -> EvalResult<'tcx> { // Let's get an mplace first. - let mplace = if self.layout().is_zst() { + let mplace = if v.layout().is_zst() { // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(self.layout(), ectx) + MPlaceTy::dangling(v.layout(), self.ecx()) } else { // non-ZST array/slice/str cannot be immediate - self.value().to_mem_place(ectx)? + v.to_mem_place(self.ecx())? }; // Now iterate over it. - for (i, field) in ectx.mplace_array_fields(mplace)?.enumerate() { - self.visit_field(ectx, Value::from_mem_place(field?), i)?; + for (i, field) in self.ecx().mplace_array_fields(mplace)?.enumerate() { + self.visit_field(v, i, Value::from_mem_place(field?))?; } Ok(()) } - fn walk_value(&mut self, ectx: &mut EvalContext<'a, 'mir, 'tcx, M>) - -> EvalResult<'tcx> + fn walk_value(&mut self, v: Self::V) -> EvalResult<'tcx> { - trace!("walk_value: {:?}", self); - // If this is a multi-variant layout, we have find the right one and proceed with that. // (No benefit from making this recursion, but it is equivalent to that.) - match self.layout().variants { + match v.layout().variants { layout::Variants::NicheFilling { .. } | layout::Variants::Tagged { .. } => { - let (inner, idx) = self.value().project_downcast(ectx)?; + let (inner, idx) = v.project_downcast(self.ecx())?; trace!("variant layout: {:#?}", inner.layout()); // recurse with the inner type - return self.visit_field(ectx, inner, idx); + return self.visit_field(v, idx, inner); } layout::Variants::Single { .. } => {} } // Even for single variants, we might be able to get a more refined type: // If it is a trait object, switch to the actual type that was used to create it. - match self.layout().ty.sty { + match v.layout().ty.sty { ty::Dynamic(..) => { // immediate trait objects are not a thing - let dest = self.value().to_mem_place(ectx)?; - let inner = ectx.unpack_dyn_trait(dest)?.1; + let dest = v.to_mem_place(self.ecx())?; + let inner = self.ecx().unpack_dyn_trait(dest)?.1; trace!("dyn object layout: {:#?}", inner.layout); // recurse with the inner type - return self.visit_field(ectx, Value::from_mem_place(inner), 0); + return self.visit_field(v, 0, Value::from_mem_place(inner)); }, _ => {}, }; @@ -274,12 +272,12 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + // FIXME: We could avoid some redundant checks here. For newtypes wrapping // scalars, we do the same check on every "level" (e.g. first we check // MyNewtype and then the scalar in there). - match self.layout().abi { + match v.layout().abi { layout::Abi::Uninhabited => { - self.visit_uninhabited(ectx)?; + self.visit_uninhabited(v)?; } layout::Abi::Scalar(ref layout) => { - self.visit_scalar(ectx, layout)?; + self.visit_scalar(v, layout)?; } // FIXME: Should we do something for ScalarPair? Vector? _ => {} @@ -290,34 +288,32 @@ pub trait ValueVisitor<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: fmt::Debug + // so we check them separately and before aggregate handling. // It is CRITICAL that we get this check right, or we might be // validating the wrong thing! - let primitive = match self.layout().fields { + let primitive = match v.layout().fields { // Primitives appear as Union with 0 fields -- except for Boxes and fat pointers. layout::FieldPlacement::Union(0) => true, - _ => self.layout().ty.builtin_deref(true).is_some(), + _ => v.layout().ty.builtin_deref(true).is_some(), }; if primitive { - return self.visit_primitive(ectx); + return self.visit_primitive(v); } // Proceed into the fields. - match self.layout().fields { + match v.layout().fields { layout::FieldPlacement::Union(fields) => { // Empty unions are not accepted by rustc. That's great, it means we can // use that as an unambiguous signal for detecting primitives. Make sure // we did not miss any primitive. debug_assert!(fields > 0); - // We can't traverse unions, their bits are allowed to be anything. - // The fields don't need to correspond to any bit pattern of the union's fields. - // See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389 + self.visit_union(v)?; }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { for i in 0..offsets.len() { - let val = self.value().project_field(ectx, i as u64)?; - self.visit_field(ectx, val, i)?; + let val = v.project_field(self.ecx(), i as u64)?; + self.visit_field(v, i, val)?; } }, layout::FieldPlacement::Array { .. } => { - self.visit_array(ectx)?; + self.visit_array(v)?; } } Ok(()) From c2677211f65758f19d7051d6f24a34ce0ec44119 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 09:52:17 +0100 Subject: [PATCH 13/21] all values can convert to operators --- src/librustc_mir/interpret/visitor.rs | 91 ++++++++++++++------------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index f50ef96775f9d..7ae747a735790 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -19,26 +19,26 @@ pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy // Get this value's layout. fn layout(&self) -> TyLayout<'tcx>; - // Make this a `MPlaceTy`, or panic if that's not possible. - fn to_mem_place( + // Make this into an `OpTy`. + fn to_op( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M>, - ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>>; + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>>; // Create this from an `MPlaceTy`. fn from_mem_place(MPlaceTy<'tcx, M::PointerTag>) -> Self; - // Read the current enum discriminant, and downcast to that. Also return the - // variant index. + // Project to the given enum variant. fn project_downcast( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M> - ) -> EvalResult<'tcx, (Self, usize)>; + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + variant: usize, + ) -> EvalResult<'tcx, Self>; // Project to the n-th field. fn project_field( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self>; } @@ -53,11 +53,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn to_mem_place( + fn to_op( self, - _ectx: &EvalContext<'a, 'mir, 'tcx, M>, - ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - Ok(self.to_mem_place()) + _ecx: &EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + Ok(self) } #[inline(always)] @@ -68,19 +68,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> #[inline(always)] fn project_downcast( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M> - ) -> EvalResult<'tcx, (Self, usize)> { - let idx = ectx.read_discriminant(self)?.1; - Ok((ectx.operand_downcast(self, idx)?, idx)) + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + variant: usize, + ) -> EvalResult<'tcx, Self> { + ecx.operand_downcast(self, variant) } #[inline(always)] fn project_field( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self> { - ectx.operand_field(self, field) + ecx.operand_field(self, field) } } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> @@ -92,11 +92,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn to_mem_place( + fn to_op( self, - _ectx: &EvalContext<'a, 'mir, 'tcx, M>, - ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - Ok(self) + _ecx: &EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + Ok(self.into()) } #[inline(always)] @@ -107,19 +107,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> #[inline(always)] fn project_downcast( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M> - ) -> EvalResult<'tcx, (Self, usize)> { - let idx = ectx.read_discriminant(self.into())?.1; - Ok((ectx.mplace_downcast(self, idx)?, idx)) + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + variant: usize, + ) -> EvalResult<'tcx, Self> { + ecx.mplace_downcast(self, variant) } #[inline(always)] fn project_field( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self> { - ectx.mplace_field(self, field) + ecx.mplace_field(self, field) } } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> @@ -131,12 +131,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> } #[inline(always)] - fn to_mem_place( + fn to_op( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M>, - ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // If this refers to a local, assert that it already has an allocation. - Ok(ectx.place_to_op(self)?.to_mem_place()) + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + ecx.place_to_op(self) } #[inline(always)] @@ -147,19 +146,19 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> #[inline(always)] fn project_downcast( self, - ectx: &EvalContext<'a, 'mir, 'tcx, M> - ) -> EvalResult<'tcx, (Self, usize)> { - let idx = ectx.read_discriminant(ectx.place_to_op(self)?)?.1; - Ok((ectx.place_downcast(self, idx)?, idx)) + ecx: &EvalContext<'a, 'mir, 'tcx, M>, + variant: usize, + ) -> EvalResult<'tcx, Self> { + ecx.place_downcast(self, variant) } #[inline(always)] fn project_field( self, - ectx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self> { - ectx.place_field(self, field) + ecx.place_field(self, field) } } @@ -228,7 +227,7 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz MPlaceTy::dangling(v.layout(), self.ecx()) } else { // non-ZST array/slice/str cannot be immediate - v.to_mem_place(self.ecx())? + v.to_op(self.ecx())?.to_mem_place() }; // Now iterate over it. for (i, field) in self.ecx().mplace_array_fields(mplace)?.enumerate() { @@ -243,8 +242,10 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz match v.layout().variants { layout::Variants::NicheFilling { .. } | layout::Variants::Tagged { .. } => { - let (inner, idx) = v.project_downcast(self.ecx())?; - trace!("variant layout: {:#?}", inner.layout()); + let op = v.to_op(self.ecx())?; + let idx = self.ecx().read_discriminant(op)?.1; + let inner = v.project_downcast(self.ecx(), idx)?; + trace!("walk_value: variant layout: {:#?}", inner.layout()); // recurse with the inner type return self.visit_field(v, idx, inner); } @@ -256,9 +257,9 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz match v.layout().ty.sty { ty::Dynamic(..) => { // immediate trait objects are not a thing - let dest = v.to_mem_place(self.ecx())?; + let dest = v.to_op(self.ecx())?.to_mem_place(); let inner = self.ecx().unpack_dyn_trait(dest)?.1; - trace!("dyn object layout: {:#?}", inner.layout); + trace!("walk_value: dyn object layout: {:#?}", inner.layout); // recurse with the inner type return self.visit_field(v, 0, Value::from_mem_place(inner)); }, From 996a42557e2816cd577120f464c52b865ef8a924 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 10:01:22 +0100 Subject: [PATCH 14/21] the visitor can already load the value for visit_primitive --- src/librustc_mir/interpret/validity.rs | 19 +++++++++---------- src/librustc_mir/interpret/visitor.rs | 14 +++++++++----- src/test/ui/consts/const-eval/ub-ref.stderr | 2 +- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index c303d2a1e67ca..5ba15f4bb99af 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -20,7 +20,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, MPlaceTy, Machine, EvalContext, ValueVisitor + OpTy, MPlaceTy, ImmTy, Machine, EvalContext, ValueVisitor }; macro_rules! validation_failure { @@ -213,7 +213,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> fn visit_value(&mut self, op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { trace!("visit_value: {:?}, {:?}", *op, op.layout); - // Translate enum discriminant errors to something nicer. + // Translate some possible errors to something nicer. match self.walk_value(op) { Ok(()) => Ok(()), Err(err) => match err.kind { @@ -221,16 +221,17 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> validation_failure!( val, self.path, "a valid enum discriminant" ), + EvalErrorKind::ReadPointerAsBytes => + validation_failure!( + "a pointer", self.path, "plain bytes" + ), _ => Err(err), } } } - fn visit_primitive(&mut self, op: OpTy<'tcx, M::PointerTag>) - -> EvalResult<'tcx> + fn visit_primitive(&mut self, value: ImmTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { - let value = try_validation!(self.ecx.read_immediate(op), - "uninitialized or unrepresentable data", self.path); // Go over all the primitive types let ty = value.layout.ty; match ty.sty { @@ -379,8 +380,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Ok(()) } - fn visit_uninhabited(&mut self, _op: OpTy<'tcx, M::PointerTag>) - -> EvalResult<'tcx> + fn visit_uninhabited(&mut self) -> EvalResult<'tcx> { validation_failure!("a value of an uninhabited type", self.path) } @@ -390,8 +390,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> op: OpTy<'tcx, M::PointerTag>, layout: &layout::Scalar, ) -> EvalResult<'tcx> { - let value = try_validation!(self.ecx.read_scalar(op), - "uninitialized or unrepresentable data", self.path); + let value = self.ecx.read_scalar(op)?; // Determine the allowed range let (lo, hi) = layout.valid_range.clone().into_inner(); // `max_hi` is as big as the size fits diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 7ae747a735790..3211601400a70 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -8,7 +8,7 @@ use rustc::mir::interpret::{ }; use super::{ - Machine, EvalContext, MPlaceTy, PlaceTy, OpTy, + Machine, EvalContext, MPlaceTy, PlaceTy, OpTy, ImmTy, }; // A thing that we can project into, and that has a layout. @@ -205,9 +205,11 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz /// Called whenever we reach a value with uninhabited layout. /// Recursing to fields will continue after this! #[inline(always)] - fn visit_uninhabited(&mut self, _v: Self::V) -> EvalResult<'tcx> + fn visit_uninhabited(&mut self) -> EvalResult<'tcx> { Ok(()) } /// Called whenever we reach a value with scalar layout. + /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory + /// if the visitor is not even interested in scalars. /// Recursing to fields will continue after this! #[inline(always)] fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> EvalResult<'tcx> @@ -215,7 +217,7 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz /// Called whenever we reach a value of primitive type. There can be no recursion /// below such a value. #[inline(always)] - fn visit_primitive(&mut self, _v: Self::V) -> EvalResult<'tcx> + fn visit_primitive(&mut self, _val: ImmTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { Ok(()) } // Default recursors. Not meant to be overloaded. @@ -275,7 +277,7 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz // MyNewtype and then the scalar in there). match v.layout().abi { layout::Abi::Uninhabited => { - self.visit_uninhabited(v)?; + self.visit_uninhabited()?; } layout::Abi::Scalar(ref layout) => { self.visit_scalar(v, layout)?; @@ -295,7 +297,9 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz _ => v.layout().ty.builtin_deref(true).is_some(), }; if primitive { - return self.visit_primitive(v); + let op = v.to_op(self.ecx())?; + let val = self.ecx().read_immediate(op)?; + return self.visit_primitive(val); } // Proceed into the fields. diff --git a/src/test/ui/consts/const-eval/ub-ref.stderr b/src/test/ui/consts/const-eval/ub-ref.stderr index 359d1abb5008e..c3f5f4a26f555 100644 --- a/src/test/ui/consts/const-eval/ub-ref.stderr +++ b/src/test/ui/consts/const-eval/ub-ref.stderr @@ -26,7 +26,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-ref.rs:25:1 | LL | const REF_AS_USIZE_SLICE: &[usize] = &[unsafe { mem::transmute(&0) }]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a raw memory access tried to access part of a pointer value as raw bytes + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer at ., but expected plain bytes | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior From 91cad3961484407eef2c89fd1329d8139013f669 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 10:52:07 +0100 Subject: [PATCH 15/21] visit_aggregate with an iterator; fix some comment typos --- src/librustc_mir/interpret/validity.rs | 9 ++- src/librustc_mir/interpret/visitor.rs | 96 ++++++++++++++++---------- 2 files changed, 65 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 5ba15f4bb99af..819a966c0a6c4 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -470,8 +470,11 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } - fn visit_array(&mut self, op: OpTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> - { + fn visit_aggregate( + &mut self, + op: OpTy<'tcx, M::PointerTag>, + fields: impl Iterator>, + ) -> EvalResult<'tcx> { match op.layout.ty.sty { ty::Str => { let mplace = op.to_mem_place(); // strings are never immediate @@ -538,7 +541,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } } _ => { - self.walk_array(op)? // default handler + self.walk_aggregate(op, fields)? // default handler } } Ok(()) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 3211601400a70..80c7e075bd7f9 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -13,29 +13,29 @@ use super::{ // A thing that we can project into, and that has a layout. // This wouldn't have to depend on `Machine` but with the current type inference, -// that's just more convenient to work with (avoids repeading all the `Machine` bounds). +// that's just more convenient to work with (avoids repeating all the `Machine` bounds). pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy { - // Get this value's layout. + /// Get this value's layout. fn layout(&self) -> TyLayout<'tcx>; - // Make this into an `OpTy`. + /// Make this into an `OpTy`. fn to_op( self, ecx: &EvalContext<'a, 'mir, 'tcx, M>, ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>>; - // Create this from an `MPlaceTy`. + /// Create this from an `MPlaceTy`. fn from_mem_place(MPlaceTy<'tcx, M::PointerTag>) -> Self; - // Project to the given enum variant. + /// Project to the given enum variant. fn project_downcast( self, ecx: &EvalContext<'a, 'mir, 'tcx, M>, variant: usize, ) -> EvalResult<'tcx, Self>; - // Project to the n-th field. + /// Project to the n-th field. fn project_field( self, ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, @@ -166,27 +166,32 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Sized { type V: Value<'a, 'mir, 'tcx, M>; - // The visitor must have an `EvalContext` in it. + /// The visitor must have an `EvalContext` in it. fn ecx(&mut self) -> &mut EvalContext<'a, 'mir, 'tcx, M>; - // Recursie actions, ready to be overloaded. - /// Visit the given value, dispatching as appropriate to more speicalized visitors. + // Recursive actions, ready to be overloaded. + /// Visit the given value, dispatching as appropriate to more specialized visitors. #[inline(always)] fn visit_value(&mut self, v: Self::V) -> EvalResult<'tcx> { self.walk_value(v) } - /// Visit the given value as a union. + /// Visit the given value as a union. No automatic recursion can happen here. #[inline(always)] fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> { Ok(()) } - /// Visit the given value as an array. + /// Visit this vale as an aggregate, you are even getting an iterator yielding + /// all the fields (still in an `EvalResult`, you have to do error handling yourself). + /// Recurses into the fields. #[inline(always)] - fn visit_array(&mut self, v: Self::V) -> EvalResult<'tcx> - { - self.walk_array(v) + fn visit_aggregate( + &mut self, + v: Self::V, + fields: impl Iterator>, + ) -> EvalResult<'tcx> { + self.walk_aggregate(v, fields) } /// Called each time we recurse down to a field, passing in old and new value. /// This gives the visitor the chance to track the stack of nested fields that @@ -201,39 +206,39 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz self.visit_value(new_val) } - // Actions on the leaves, ready to be overloaded. /// Called whenever we reach a value with uninhabited layout. - /// Recursing to fields will continue after this! + /// Recursing to fields will *always* continue after this! This is not meant to control + /// whether and how we descend recursively/ into the scalar's fields if there are any, it is + /// meant to provide the chance for additional checks when a value of uninhabited layout is + /// detected. #[inline(always)] fn visit_uninhabited(&mut self) -> EvalResult<'tcx> { Ok(()) } /// Called whenever we reach a value with scalar layout. - /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory - /// if the visitor is not even interested in scalars. - /// Recursing to fields will continue after this! + /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory if the visitor is not + /// even interested in scalars. + /// Recursing to fields will *always* continue after this! This is not meant to control + /// whether and how we descend recursively/ into the scalar's fields if there are any, it is + /// meant to provide the chance for additional checks when a value of scalar layout is detected. #[inline(always)] fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> EvalResult<'tcx> { Ok(()) } + /// Called whenever we reach a value of primitive type. There can be no recursion - /// below such a value. + /// below such a value. This is the leave function. #[inline(always)] fn visit_primitive(&mut self, _val: ImmTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { Ok(()) } // Default recursors. Not meant to be overloaded. - fn walk_array(&mut self, v: Self::V) -> EvalResult<'tcx> - { - // Let's get an mplace first. - let mplace = if v.layout().is_zst() { - // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(v.layout(), self.ecx()) - } else { - // non-ZST array/slice/str cannot be immediate - v.to_op(self.ecx())?.to_mem_place() - }; + fn walk_aggregate( + &mut self, + v: Self::V, + fields: impl Iterator>, + ) -> EvalResult<'tcx> { // Now iterate over it. - for (i, field) in self.ecx().mplace_array_fields(mplace)?.enumerate() { - self.visit_field(v, i, Value::from_mem_place(field?))?; + for (idx, field_val) in fields.enumerate() { + self.visit_field(v, idx, field_val?)?; } Ok(()) } @@ -312,13 +317,30 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz self.visit_union(v)?; }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - for i in 0..offsets.len() { - let val = v.project_field(self.ecx(), i as u64)?; - self.visit_field(v, i, val)?; - } + // We collect in a vec because otherwise there are lifetime errors: + // Projecting to a field needs (mutable!) access to `ecx`. + let fields: Vec> = + (0..offsets.len()).map(|i| { + v.project_field(self.ecx(), i as u64) + }) + .collect(); + self.visit_aggregate(v, fields.into_iter())?; }, layout::FieldPlacement::Array { .. } => { - self.visit_array(v)?; + // Let's get an mplace first. + let mplace = if v.layout().is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(v.layout(), self.ecx()) + } else { + // non-ZST array/slice/str cannot be immediate + v.to_op(self.ecx())?.to_mem_place() + }; + // Now we can go over all the fields. + let iter = self.ecx().mplace_array_fields(mplace)? + .map(|f| f.and_then(|f| { + Ok(Value::from_mem_place(f)) + })); + self.visit_aggregate(v, iter)?; } } Ok(()) From 0d596f29d9cf105833db5dc5cca1eccade08ce4e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 11:24:36 +0100 Subject: [PATCH 16/21] FIXME --- src/librustc_mir/interpret/visitor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 80c7e075bd7f9..12455e9572773 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -317,7 +317,7 @@ pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Siz self.visit_union(v)?; }, layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - // We collect in a vec because otherwise there are lifetime errors: + // FIXME: We collect in a vec because otherwise there are lifetime errors: // Projecting to a field needs (mutable!) access to `ecx`. let fields: Vec> = (0..offsets.len()).map(|i| { From 7565b5ac6760e16295e1f16dc2af5232278977dc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 13:17:06 +0100 Subject: [PATCH 17/21] machine hooks for ptr (de)ref also need layout, and then they do not need the size --- src/librustc_mir/interpret/machine.rs | 22 +++++++++------------ src/librustc_mir/interpret/place.rs | 28 ++++++++++----------------- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index e9d181479e52e..27cf28ef41e8a 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -17,11 +17,11 @@ use std::hash::Hash; use rustc::hir::{self, def_id::DefId}; use rustc::mir; -use rustc::ty::{self, Ty, layout::{Size, TyLayout}, query::TyCtxtAt}; +use rustc::ty::{self, layout::{Size, TyLayout}, query::TyCtxtAt}; use super::{ Allocation, AllocId, EvalResult, Scalar, - EvalContext, PlaceTy, OpTy, Pointer, MemPlace, MemoryKind, + EvalContext, PlaceTy, MPlaceTy, OpTy, Pointer, MemoryKind, }; /// Whether this kind of memory is allowed to leak @@ -217,26 +217,22 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { #[inline] fn tag_reference( _ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, - place: MemPlace, - _ty: Ty<'tcx>, - _size: Size, + place: MPlaceTy<'tcx, Self::PointerTag>, _mutability: Option, - ) -> EvalResult<'tcx, MemPlace> { - Ok(place) + ) -> EvalResult<'tcx, Scalar> { + Ok(place.ptr) } /// Executed when evaluating the `*` operator: Following a reference. - /// This has the change to adjust the tag. It should not change anything else! + /// This has the chance to adjust the tag. It should not change anything else! /// `mutability` can be `None` in case a raw ptr is being dereferenced. #[inline] fn tag_dereference( _ecx: &EvalContext<'a, 'mir, 'tcx, Self>, - place: MemPlace, - _ty: Ty<'tcx>, - _size: Size, + place: MPlaceTy<'tcx, Self::PointerTag>, _mutability: Option, - ) -> EvalResult<'tcx, MemPlace> { - Ok(place) + ) -> EvalResult<'tcx, Scalar> { + Ok(place.ptr) } /// Execute a validation operation diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 35276fa6265d2..19430c85cf73c 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -278,11 +278,9 @@ where let meta = val.to_meta()?; let ptr = val.to_scalar_ptr()?; let mplace = MemPlace { ptr, align, meta }; + let mut mplace = MPlaceTy { mplace, layout }; // Pointer tag tracking might want to adjust the tag. - let mplace = if M::ENABLE_PTR_TRACKING_HOOKS { - let (size, _) = self.size_and_align_of(meta, layout)? - // for extern types, just cover what we can - .unwrap_or_else(|| layout.size_and_align()); + if M::ENABLE_PTR_TRACKING_HOOKS { let mutbl = match val.layout.ty.sty { // `builtin_deref` considers boxes immutable, that's useless for our purposes ty::Ref(_, _, mutbl) => Some(mutbl), @@ -290,11 +288,10 @@ where ty::RawPtr(_) => None, _ => bug!("Unexpected pointer type {}", val.layout.ty.sty), }; - M::tag_dereference(self, mplace, pointee_type, size, mutbl)? - } else { - mplace - }; - Ok(MPlaceTy { mplace, layout }) + mplace.mplace.ptr = M::tag_dereference(self, mplace, mutbl)?; + } + // Done + Ok(mplace) } /// Turn a mplace into a (thin or fat) pointer, as a reference, pointing to the same space. @@ -302,18 +299,13 @@ where /// `mutbl` indicates whether we are create a shared or mutable ref, or a raw pointer (`None`). pub fn create_ref( &mut self, - place: MPlaceTy<'tcx, M::PointerTag>, + mut place: MPlaceTy<'tcx, M::PointerTag>, mutbl: Option, ) -> EvalResult<'tcx, Immediate> { // Pointer tag tracking might want to adjust the tag - let place = if M::ENABLE_PTR_TRACKING_HOOKS { - let (size, _) = self.size_and_align_of_mplace(place)? - // for extern types, just cover what we can - .unwrap_or_else(|| place.layout.size_and_align()); - M::tag_reference(self, *place, place.layout.ty, size, mutbl)? - } else { - *place - }; + if M::ENABLE_PTR_TRACKING_HOOKS { + place.mplace.ptr = M::tag_reference(self, place, mutbl)? + } Ok(match place.meta { None => Immediate::Scalar(place.ptr.into()), Some(meta) => Immediate::ScalarPair(place.ptr.into(), meta.into()), From 873041009d1c2f5e2034b021133d2f93e901e745 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 14:06:43 +0100 Subject: [PATCH 18/21] make ValueVisitor mut-polymorphic --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/validity.rs | 8 +- src/librustc_mir/interpret/visitor.rs | 399 ++++++++++++------------- 3 files changed, 190 insertions(+), 219 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 34684587db207..011887090eefe 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -535,7 +535,7 @@ fn validate_const<'a, 'tcx>( key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>, ) -> ::rustc::mir::interpret::ConstEvalResult<'tcx> { let cid = key.value; - let mut ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap(); + let ecx = mk_eval_cx(tcx, cid.instance, key.param_env).unwrap(); let val = (|| { let op = ecx.const_to_op(constant)?; let mut ref_tracking = RefTracking::new(op); diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 819a966c0a6c4..8c8b3e2ca77c4 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -129,7 +129,7 @@ struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a path: Vec, ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, const_mode: bool, - ecx: &'rt mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &'rt EvalContext<'a, 'mir, 'tcx, M>, } impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> ValidityVisitor<'rt, 'a, 'mir, 'tcx, M> { @@ -188,8 +188,8 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> type V = OpTy<'tcx, M::PointerTag>; #[inline(always)] - fn ecx(&mut self) -> &mut EvalContext<'a, 'mir, 'tcx, M> { - &mut self.ecx + fn ecx(&self) -> &EvalContext<'a, 'mir, 'tcx, M> { + &self.ecx } #[inline] @@ -557,7 +557,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion) /// validation (e.g., pointer values are fine in integers at runtime). pub fn validate_operand( - &mut self, + &self, op: OpTy<'tcx, M::PointerTag>, path: Vec, ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 12455e9572773..24789ee1ebfe5 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -8,7 +8,7 @@ use rustc::mir::interpret::{ }; use super::{ - Machine, EvalContext, MPlaceTy, PlaceTy, OpTy, ImmTy, + Machine, EvalContext, MPlaceTy, OpTy, ImmTy, }; // A thing that we can project into, and that has a layout. @@ -38,12 +38,13 @@ pub trait Value<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>: Copy /// Project to the n-th field. fn project_field( self, - ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self>; } -// Operands and places are both values +// Operands and memory-places are both values. +// Places in general are not due to `place_field` having to do `force_allocation`. impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> for OpTy<'tcx, M::PointerTag> { @@ -77,7 +78,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> #[inline(always)] fn project_field( self, - ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, + ecx: &EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self> { ecx.operand_field(self, field) @@ -115,234 +116,204 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> #[inline(always)] fn project_field( - self, - ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, - field: u64, - ) -> EvalResult<'tcx, Self> { - ecx.mplace_field(self, field) - } -} -impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Value<'a, 'mir, 'tcx, M> - for PlaceTy<'tcx, M::PointerTag> -{ - #[inline(always)] - fn layout(&self) -> TyLayout<'tcx> { - self.layout - } - - #[inline(always)] - fn to_op( - self, - ecx: &EvalContext<'a, 'mir, 'tcx, M>, - ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { - ecx.place_to_op(self) - } - - #[inline(always)] - fn from_mem_place(mplace: MPlaceTy<'tcx, M::PointerTag>) -> Self { - mplace.into() - } - - #[inline(always)] - fn project_downcast( self, ecx: &EvalContext<'a, 'mir, 'tcx, M>, - variant: usize, - ) -> EvalResult<'tcx, Self> { - ecx.place_downcast(self, variant) - } - - #[inline(always)] - fn project_field( - self, - ecx: &mut EvalContext<'a, 'mir, 'tcx, M>, field: u64, ) -> EvalResult<'tcx, Self> { - ecx.place_field(self, field) + ecx.mplace_field(self, field) } } -// How to traverse a value and what to do when we are at the leaves. -pub trait ValueVisitor<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Sized { - type V: Value<'a, 'mir, 'tcx, M>; +macro_rules! make_value_visitor { + ($visitor_trait_name:ident, $($mutability:ident)*) => { + // How to traverse a value and what to do when we are at the leaves. + pub trait $visitor_trait_name<'a, 'mir, 'tcx: 'mir+'a, M: Machine<'a, 'mir, 'tcx>>: Sized { + type V: Value<'a, 'mir, 'tcx, M>; - /// The visitor must have an `EvalContext` in it. - fn ecx(&mut self) -> &mut EvalContext<'a, 'mir, 'tcx, M>; + /// The visitor must have an `EvalContext` in it. + fn ecx(&$($mutability)* self) + -> &$($mutability)* EvalContext<'a, 'mir, 'tcx, M>; - // Recursive actions, ready to be overloaded. - /// Visit the given value, dispatching as appropriate to more specialized visitors. - #[inline(always)] - fn visit_value(&mut self, v: Self::V) -> EvalResult<'tcx> - { - self.walk_value(v) - } - /// Visit the given value as a union. No automatic recursion can happen here. - #[inline(always)] - fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> - { - Ok(()) - } - /// Visit this vale as an aggregate, you are even getting an iterator yielding - /// all the fields (still in an `EvalResult`, you have to do error handling yourself). - /// Recurses into the fields. - #[inline(always)] - fn visit_aggregate( - &mut self, - v: Self::V, - fields: impl Iterator>, - ) -> EvalResult<'tcx> { - self.walk_aggregate(v, fields) - } - /// Called each time we recurse down to a field, passing in old and new value. - /// This gives the visitor the chance to track the stack of nested fields that - /// we are descending through. - #[inline(always)] - fn visit_field( - &mut self, - _old_val: Self::V, - _field: usize, - new_val: Self::V, - ) -> EvalResult<'tcx> { - self.visit_value(new_val) - } + // Recursive actions, ready to be overloaded. + /// Visit the given value, dispatching as appropriate to more specialized visitors. + #[inline(always)] + fn visit_value(&mut self, v: Self::V) -> EvalResult<'tcx> + { + self.walk_value(v) + } + /// Visit the given value as a union. No automatic recursion can happen here. + #[inline(always)] + fn visit_union(&mut self, _v: Self::V) -> EvalResult<'tcx> + { + Ok(()) + } + /// Visit this vale as an aggregate, you are even getting an iterator yielding + /// all the fields (still in an `EvalResult`, you have to do error handling yourself). + /// Recurses into the fields. + #[inline(always)] + fn visit_aggregate( + &mut self, + v: Self::V, + fields: impl Iterator>, + ) -> EvalResult<'tcx> { + self.walk_aggregate(v, fields) + } + /// Called each time we recurse down to a field, passing in old and new value. + /// This gives the visitor the chance to track the stack of nested fields that + /// we are descending through. + #[inline(always)] + fn visit_field( + &mut self, + _old_val: Self::V, + _field: usize, + new_val: Self::V, + ) -> EvalResult<'tcx> { + self.visit_value(new_val) + } - /// Called whenever we reach a value with uninhabited layout. - /// Recursing to fields will *always* continue after this! This is not meant to control - /// whether and how we descend recursively/ into the scalar's fields if there are any, it is - /// meant to provide the chance for additional checks when a value of uninhabited layout is - /// detected. - #[inline(always)] - fn visit_uninhabited(&mut self) -> EvalResult<'tcx> - { Ok(()) } - /// Called whenever we reach a value with scalar layout. - /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory if the visitor is not - /// even interested in scalars. - /// Recursing to fields will *always* continue after this! This is not meant to control - /// whether and how we descend recursively/ into the scalar's fields if there are any, it is - /// meant to provide the chance for additional checks when a value of scalar layout is detected. - #[inline(always)] - fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> EvalResult<'tcx> - { Ok(()) } + /// Called whenever we reach a value with uninhabited layout. + /// Recursing to fields will *always* continue after this! This is not meant to control + /// whether and how we descend recursively/ into the scalar's fields if there are any, + /// it is meant to provide the chance for additional checks when a value of uninhabited + /// layout is detected. + #[inline(always)] + fn visit_uninhabited(&mut self) -> EvalResult<'tcx> + { Ok(()) } + /// Called whenever we reach a value with scalar layout. + /// We do NOT provide a `ScalarMaybeUndef` here to avoid accessing memory if the + /// visitor is not even interested in scalars. + /// Recursing to fields will *always* continue after this! This is not meant to control + /// whether and how we descend recursively/ into the scalar's fields if there are any, + /// it is meant to provide the chance for additional checks when a value of scalar + /// layout is detected. + #[inline(always)] + fn visit_scalar(&mut self, _v: Self::V, _layout: &layout::Scalar) -> EvalResult<'tcx> + { Ok(()) } - /// Called whenever we reach a value of primitive type. There can be no recursion - /// below such a value. This is the leave function. - #[inline(always)] - fn visit_primitive(&mut self, _val: ImmTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> - { Ok(()) } + /// Called whenever we reach a value of primitive type. There can be no recursion + /// below such a value. This is the leave function. + #[inline(always)] + fn visit_primitive(&mut self, _val: ImmTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> + { Ok(()) } - // Default recursors. Not meant to be overloaded. - fn walk_aggregate( - &mut self, - v: Self::V, - fields: impl Iterator>, - ) -> EvalResult<'tcx> { - // Now iterate over it. - for (idx, field_val) in fields.enumerate() { - self.visit_field(v, idx, field_val?)?; - } - Ok(()) - } - fn walk_value(&mut self, v: Self::V) -> EvalResult<'tcx> - { - // If this is a multi-variant layout, we have find the right one and proceed with that. - // (No benefit from making this recursion, but it is equivalent to that.) - match v.layout().variants { - layout::Variants::NicheFilling { .. } | - layout::Variants::Tagged { .. } => { - let op = v.to_op(self.ecx())?; - let idx = self.ecx().read_discriminant(op)?.1; - let inner = v.project_downcast(self.ecx(), idx)?; - trace!("walk_value: variant layout: {:#?}", inner.layout()); - // recurse with the inner type - return self.visit_field(v, idx, inner); + // Default recursors. Not meant to be overloaded. + fn walk_aggregate( + &mut self, + v: Self::V, + fields: impl Iterator>, + ) -> EvalResult<'tcx> { + // Now iterate over it. + for (idx, field_val) in fields.enumerate() { + self.visit_field(v, idx, field_val?)?; + } + Ok(()) } - layout::Variants::Single { .. } => {} - } - - // Even for single variants, we might be able to get a more refined type: - // If it is a trait object, switch to the actual type that was used to create it. - match v.layout().ty.sty { - ty::Dynamic(..) => { - // immediate trait objects are not a thing - let dest = v.to_op(self.ecx())?.to_mem_place(); - let inner = self.ecx().unpack_dyn_trait(dest)?.1; - trace!("walk_value: dyn object layout: {:#?}", inner.layout); - // recurse with the inner type - return self.visit_field(v, 0, Value::from_mem_place(inner)); - }, - _ => {}, - }; + fn walk_value(&mut self, v: Self::V) -> EvalResult<'tcx> + { + // If this is a multi-variant layout, we have find the right one and proceed with + // that. + match v.layout().variants { + layout::Variants::NicheFilling { .. } | + layout::Variants::Tagged { .. } => { + let op = v.to_op(self.ecx())?; + let idx = self.ecx().read_discriminant(op)?.1; + let inner = v.project_downcast(self.ecx(), idx)?; + trace!("walk_value: variant layout: {:#?}", inner.layout()); + // recurse with the inner type + return self.visit_field(v, idx, inner); + } + layout::Variants::Single { .. } => {} + } - // If this is a scalar, visit it as such. - // Things can be aggregates and have scalar layout at the same time, and that - // is very relevant for `NonNull` and similar structs: We need to visit them - // at their scalar layout *before* descending into their fields. - // FIXME: We could avoid some redundant checks here. For newtypes wrapping - // scalars, we do the same check on every "level" (e.g. first we check - // MyNewtype and then the scalar in there). - match v.layout().abi { - layout::Abi::Uninhabited => { - self.visit_uninhabited()?; - } - layout::Abi::Scalar(ref layout) => { - self.visit_scalar(v, layout)?; - } - // FIXME: Should we do something for ScalarPair? Vector? - _ => {} - } + // Even for single variants, we might be able to get a more refined type: + // If it is a trait object, switch to the actual type that was used to create it. + match v.layout().ty.sty { + ty::Dynamic(..) => { + // immediate trait objects are not a thing + let dest = v.to_op(self.ecx())?.to_mem_place(); + let inner = self.ecx().unpack_dyn_trait(dest)?.1; + trace!("walk_value: dyn object layout: {:#?}", inner.layout); + // recurse with the inner type + return self.visit_field(v, 0, Value::from_mem_place(inner)); + }, + _ => {}, + }; - // Check primitive types. We do this after checking the scalar layout, - // just to have that done as well. Primitives can have varying layout, - // so we check them separately and before aggregate handling. - // It is CRITICAL that we get this check right, or we might be - // validating the wrong thing! - let primitive = match v.layout().fields { - // Primitives appear as Union with 0 fields -- except for Boxes and fat pointers. - layout::FieldPlacement::Union(0) => true, - _ => v.layout().ty.builtin_deref(true).is_some(), - }; - if primitive { - let op = v.to_op(self.ecx())?; - let val = self.ecx().read_immediate(op)?; - return self.visit_primitive(val); - } + // If this is a scalar, visit it as such. + // Things can be aggregates and have scalar layout at the same time, and that + // is very relevant for `NonNull` and similar structs: We need to visit them + // at their scalar layout *before* descending into their fields. + // FIXME: We could avoid some redundant checks here. For newtypes wrapping + // scalars, we do the same check on every "level" (e.g. first we check + // MyNewtype and then the scalar in there). + match v.layout().abi { + layout::Abi::Uninhabited => { + self.visit_uninhabited()?; + } + layout::Abi::Scalar(ref layout) => { + self.visit_scalar(v, layout)?; + } + // FIXME: Should we do something for ScalarPair? Vector? + _ => {} + } - // Proceed into the fields. - match v.layout().fields { - layout::FieldPlacement::Union(fields) => { - // Empty unions are not accepted by rustc. That's great, it means we can - // use that as an unambiguous signal for detecting primitives. Make sure - // we did not miss any primitive. - debug_assert!(fields > 0); - self.visit_union(v)?; - }, - layout::FieldPlacement::Arbitrary { ref offsets, .. } => { - // FIXME: We collect in a vec because otherwise there are lifetime errors: - // Projecting to a field needs (mutable!) access to `ecx`. - let fields: Vec> = - (0..offsets.len()).map(|i| { - v.project_field(self.ecx(), i as u64) - }) - .collect(); - self.visit_aggregate(v, fields.into_iter())?; - }, - layout::FieldPlacement::Array { .. } => { - // Let's get an mplace first. - let mplace = if v.layout().is_zst() { - // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(v.layout(), self.ecx()) - } else { - // non-ZST array/slice/str cannot be immediate - v.to_op(self.ecx())?.to_mem_place() + // Check primitive types. We do this after checking the scalar layout, + // just to have that done as well. Primitives can have varying layout, + // so we check them separately and before aggregate handling. + // It is CRITICAL that we get this check right, or we might be + // validating the wrong thing! + let primitive = match v.layout().fields { + // Primitives appear as Union with 0 fields - except for Boxes and fat pointers. + layout::FieldPlacement::Union(0) => true, + _ => v.layout().ty.builtin_deref(true).is_some(), }; - // Now we can go over all the fields. - let iter = self.ecx().mplace_array_fields(mplace)? - .map(|f| f.and_then(|f| { - Ok(Value::from_mem_place(f)) - })); - self.visit_aggregate(v, iter)?; + if primitive { + let op = v.to_op(self.ecx())?; + let val = self.ecx().read_immediate(op)?; + return self.visit_primitive(val); + } + + // Proceed into the fields. + match v.layout().fields { + layout::FieldPlacement::Union(fields) => { + // Empty unions are not accepted by rustc. That's great, it means we can + // use that as an unambiguous signal for detecting primitives. Make sure + // we did not miss any primitive. + debug_assert!(fields > 0); + self.visit_union(v)?; + }, + layout::FieldPlacement::Arbitrary { ref offsets, .. } => { + // FIXME: We collect in a vec because otherwise there are lifetime errors: + // Projecting to a field needs (mutable!) access to `ecx`. + let fields: Vec> = + (0..offsets.len()).map(|i| { + v.project_field(self.ecx(), i as u64) + }) + .collect(); + self.visit_aggregate(v, fields.into_iter())?; + }, + layout::FieldPlacement::Array { .. } => { + // Let's get an mplace first. + let mplace = if v.layout().is_zst() { + // it's a ZST, the memory content cannot matter + MPlaceTy::dangling(v.layout(), self.ecx()) + } else { + // non-ZST array/slice/str cannot be immediate + v.to_op(self.ecx())?.to_mem_place() + }; + // Now we can go over all the fields. + let iter = self.ecx().mplace_array_fields(mplace)? + .map(|f| f.and_then(|f| { + Ok(Value::from_mem_place(f)) + })); + self.visit_aggregate(v, iter)?; + } + } + Ok(()) } } - Ok(()) } } + +make_value_visitor!(ValueVisitor,); +make_value_visitor!(MutValueVisitor,mut); From 0529dc818f4f603c5419e25f44bd1a0b7526c6ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 4 Nov 2018 12:45:21 +0100 Subject: [PATCH 19/21] proide ptr_wrapping_offset on Scalars --- src/librustc/mir/interpret/mod.rs | 79 ++++++++++++++++------------- src/librustc/mir/interpret/value.rs | 29 ++++++++--- 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index f8a5dbc6905ca..e2abf7970d6d8 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -91,42 +91,43 @@ pub trait PointerArithmetic: layout::HasDataLayout { } //// Trunace the given value to the pointer size; also return whether there was an overflow + #[inline] fn truncate_to_ptr(&self, val: u128) -> (u64, bool) { let max_ptr_plus_1 = 1u128 << self.pointer_size().bits(); ((val % max_ptr_plus_1) as u64, val >= max_ptr_plus_1) } - // Overflow checking only works properly on the range from -u64 to +u64. - fn overflowing_signed_offset(&self, val: u64, i: i128) -> (u64, bool) { - // FIXME: is it possible to over/underflow here? - if i < 0 { - // trickery to ensure that i64::min_value() works fine - // this formula only works for true negative values, it panics for zero! - let n = u64::max_value() - (i as u64) + 1; - val.overflowing_sub(n) - } else { - self.overflowing_offset(val, i as u64) - } + #[inline] + fn offset<'tcx>(&self, val: u64, i: u64) -> EvalResult<'tcx, u64> { + let (res, over) = self.overflowing_offset(val, i); + if over { err!(Overflow(mir::BinOp::Add)) } else { Ok(res) } } + #[inline] fn overflowing_offset(&self, val: u64, i: u64) -> (u64, bool) { let (res, over1) = val.overflowing_add(i); - let (res, over2) = self.truncate_to_ptr(res as u128); + let (res, over2) = self.truncate_to_ptr(u128::from(res)); (res, over1 || over2) } + #[inline] fn signed_offset<'tcx>(&self, val: u64, i: i64) -> EvalResult<'tcx, u64> { - let (res, over) = self.overflowing_signed_offset(val, i as i128); + let (res, over) = self.overflowing_signed_offset(val, i128::from(i)); if over { err!(Overflow(mir::BinOp::Add)) } else { Ok(res) } } - fn offset<'tcx>(&self, val: u64, i: u64) -> EvalResult<'tcx, u64> { - let (res, over) = self.overflowing_offset(val, i); - if over { err!(Overflow(mir::BinOp::Add)) } else { Ok(res) } - } - - fn wrapping_signed_offset(&self, val: u64, i: i64) -> u64 { - self.overflowing_signed_offset(val, i as i128).0 + // Overflow checking only works properly on the range from -u64 to +u64. + #[inline] + fn overflowing_signed_offset(&self, val: u64, i: i128) -> (u64, bool) { + // FIXME: is it possible to over/underflow here? + if i < 0 { + // trickery to ensure that i64::min_value() works fine + // this formula only works for true negative values, it panics for zero! + let n = u64::max_value() - (i as u64) + 1; + val.overflowing_sub(n) + } else { + self.overflowing_offset(val, i as u64) + } } } @@ -176,19 +177,27 @@ impl<'tcx, Tag> Pointer { Pointer { alloc_id, offset, tag } } - pub fn wrapping_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> Self { - Pointer::new_with_tag( + #[inline] + pub fn offset(self, i: Size, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { + Ok(Pointer::new_with_tag( self.alloc_id, - Size::from_bytes(cx.data_layout().wrapping_signed_offset(self.offset.bytes(), i)), - self.tag, - ) + Size::from_bytes(cx.data_layout().offset(self.offset.bytes(), i.bytes())?), + self.tag + )) } - pub fn overflowing_signed_offset(self, i: i128, cx: &impl HasDataLayout) -> (Self, bool) { - let (res, over) = cx.data_layout().overflowing_signed_offset(self.offset.bytes(), i); + #[inline] + pub fn overflowing_offset(self, i: Size, cx: &impl HasDataLayout) -> (Self, bool) { + let (res, over) = cx.data_layout().overflowing_offset(self.offset.bytes(), i.bytes()); (Pointer::new_with_tag(self.alloc_id, Size::from_bytes(res), self.tag), over) } + #[inline(always)] + pub fn wrapping_offset(self, i: Size, cx: &impl HasDataLayout) -> Self { + self.overflowing_offset(i, cx).0 + } + + #[inline] pub fn signed_offset(self, i: i64, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { Ok(Pointer::new_with_tag( self.alloc_id, @@ -197,20 +206,18 @@ impl<'tcx, Tag> Pointer { )) } - pub fn overflowing_offset(self, i: Size, cx: &impl HasDataLayout) -> (Self, bool) { - let (res, over) = cx.data_layout().overflowing_offset(self.offset.bytes(), i.bytes()); + #[inline] + pub fn overflowing_signed_offset(self, i: i128, cx: &impl HasDataLayout) -> (Self, bool) { + let (res, over) = cx.data_layout().overflowing_signed_offset(self.offset.bytes(), i); (Pointer::new_with_tag(self.alloc_id, Size::from_bytes(res), self.tag), over) } - pub fn offset(self, i: Size, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { - Ok(Pointer::new_with_tag( - self.alloc_id, - Size::from_bytes(cx.data_layout().offset(self.offset.bytes(), i.bytes())?), - self.tag - )) + #[inline(always)] + pub fn wrapping_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> Self { + self.overflowing_signed_offset(i128::from(i), cx).0 } - #[inline] + #[inline(always)] pub fn erase_tag(self) -> Pointer { Pointer { alloc_id: self.alloc_id, offset: self.offset, tag: () } } diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 64723405b0358..66faebb8c03ed 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -143,32 +143,47 @@ impl<'tcx, Tag> Scalar { } #[inline] - pub fn ptr_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { + pub fn ptr_offset(self, i: Size, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { let dl = cx.data_layout(); match self { Scalar::Bits { bits, size } => { assert_eq!(size as u64, dl.pointer_size.bytes()); Ok(Scalar::Bits { - bits: dl.signed_offset(bits as u64, i)? as u128, + bits: dl.offset(bits as u64, i.bytes())? as u128, size, }) } - Scalar::Ptr(ptr) => ptr.signed_offset(i, dl).map(Scalar::Ptr), + Scalar::Ptr(ptr) => ptr.offset(i, dl).map(Scalar::Ptr), } } #[inline] - pub fn ptr_offset(self, i: Size, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { + pub fn ptr_wrapping_offset(self, i: Size, cx: &impl HasDataLayout) -> Self { let dl = cx.data_layout(); match self { Scalar::Bits { bits, size } => { assert_eq!(size as u64, dl.pointer_size.bytes()); + Scalar::Bits { + bits: dl.overflowing_offset(bits as u64, i.bytes()).0 as u128, + size, + } + } + Scalar::Ptr(ptr) => Scalar::Ptr(ptr.wrapping_offset(i, dl)), + } + } + + #[inline] + pub fn ptr_signed_offset(self, i: i64, cx: &impl HasDataLayout) -> EvalResult<'tcx, Self> { + let dl = cx.data_layout(); + match self { + Scalar::Bits { bits, size } => { + assert_eq!(size as u64, dl.pointer_size().bytes()); Ok(Scalar::Bits { - bits: dl.offset(bits as u64, i.bytes())? as u128, + bits: dl.signed_offset(bits as u64, i)? as u128, size, }) } - Scalar::Ptr(ptr) => ptr.offset(i, dl).map(Scalar::Ptr), + Scalar::Ptr(ptr) => ptr.signed_offset(i, dl).map(Scalar::Ptr), } } @@ -179,7 +194,7 @@ impl<'tcx, Tag> Scalar { Scalar::Bits { bits, size } => { assert_eq!(size as u64, dl.pointer_size.bytes()); Scalar::Bits { - bits: dl.wrapping_signed_offset(bits as u64, i) as u128, + bits: dl.overflowing_signed_offset(bits as u64, i128::from(i)).0 as u128, size, } } From a0074cafb10567e76bbac508355059a12112d1ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 14:34:43 +0100 Subject: [PATCH 20/21] walk_value: more tracing --- src/librustc_mir/interpret/visitor.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/interpret/visitor.rs b/src/librustc_mir/interpret/visitor.rs index 24789ee1ebfe5..392e27918092f 100644 --- a/src/librustc_mir/interpret/visitor.rs +++ b/src/librustc_mir/interpret/visitor.rs @@ -210,6 +210,7 @@ macro_rules! make_value_visitor { } fn walk_value(&mut self, v: Self::V) -> EvalResult<'tcx> { + trace!("walk_value: type: {}", v.layout().ty); // If this is a multi-variant layout, we have find the right one and proceed with // that. match v.layout().variants { From 7b7c6ceb7595eec801d18c62b7c45c5503452803 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Nov 2018 15:11:28 +0100 Subject: [PATCH 21/21] add method to obtain the ptr offset of a Scalar --- src/librustc/mir/interpret/value.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 66faebb8c03ed..3f5399396abcf 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -202,6 +202,19 @@ impl<'tcx, Tag> Scalar { } } + /// Returns this pointers offset from the allocation base, or from NULL (for + /// integer pointers). + #[inline] + pub fn get_ptr_offset(self, cx: &impl HasDataLayout) -> Size { + match self { + Scalar::Bits { bits, size } => { + assert_eq!(size as u64, cx.pointer_size().bytes()); + Size::from_bytes(bits as u64) + } + Scalar::Ptr(ptr) => ptr.offset, + } + } + #[inline] pub fn is_null_ptr(self, cx: &impl HasDataLayout) -> bool { match self {