From 9103f8f812563ca9a02d60f57e28e1401f4e0c05 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 16:46:33 +0100 Subject: [PATCH 01/18] 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 | 12 +- src/librustc_mir/interpret/validity.rs | 583 ++++++++++----------- src/librustc_mir/interpret/visitor.rs | 125 +++++ 6 files changed, 425 insertions(+), 306 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 38ab21b7c8c85..5b06eb5352536 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 bc7ad16dc97bc..fdb91a44cdb4b 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -551,7 +551,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 5620ea4cee254..5fa30fec58346 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, Value, ValTy, 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 3b104e2284fe2..effe44bc7e1ec 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>> { @@ -510,7 +512,7 @@ where Place::Ptr(mplace) => self.mplace_downcast(MPlaceTy { mplace, layout: base.layout }, variant)?.into(), Place::Local { .. } => { - let layout = base.layout.for_variant(&self, variant); + let layout = base.layout.for_variant(self, variant); PlaceTy { layout, ..base } } }) @@ -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 15d0a9166a678..1e9b43b4888d3 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::{ - ValTy, 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: ValTy<'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_value(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_value(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 03d97e55539238c60b0c6e44e8338f772532357c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 18:44:00 +0100 Subject: [PATCH 02/18] reduce the amount of traversal/projection code that the visitor has to implement itself --- src/librustc_mir/interpret/validity.rs | 251 +++++++++--------- 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, 238 insertions(+), 161 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 1e9b43b4888d3..ec75bd8bc2a95 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,8 +513,15 @@ 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)?; + 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. @@ -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..31dc801d300cc 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 c1c28a36feb25c58a199e894220c9d463c361363 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 19:52:10 +0100 Subject: [PATCH 03/18] 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 31dc801d300cc..9569bcfac8810 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 cf853d5a45b49680b3d50c29e4a7e19afe1e73ec Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 31 Oct 2018 21:07:17 +0100 Subject: [PATCH 04/18] 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 ec75bd8bc2a95..c699f286cfeff 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 d01610efe87bd66158072dbe143290a9eb160073 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:11:23 +0100 Subject: [PATCH 05/18] 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 c699f286cfeff..67ed221a4902f 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 9569bcfac8810..790cc0b8067b8 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 8cc34fd9aee5f9a14d26cc054d4788d351a82d22 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:23:25 +0100 Subject: [PATCH 06/18] 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 790cc0b8067b8..d03124df6be66 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 e025a6dff460b56b821f83d9a67a80883488e616 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 13:53:21 +0100 Subject: [PATCH 07/18] 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 67ed221a4902f..4bc30529e9801 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 d03124df6be66..48f1510c6a84a 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 0d09533e01034a9591e8db4e7e3265448c9e9323 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 14:14:51 +0100 Subject: [PATCH 08/18] 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 4304f08a78f0c..8ac49508fa770 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 d0a32161485b4..8767b65b9b248 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 @@ -729,8 +739,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(); @@ -745,7 +759,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 @@ -753,7 +767,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 4bc30529e9801..2b686e1370a10 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 e5c34700d3eb3c350bd48abd6fca99fb7f9c7076 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 14:41:36 +0100 Subject: [PATCH 09/18] 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 2b435b92d77e21be142f064226500c93547136af Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 1 Nov 2018 22:27:55 +0100 Subject: [PATCH 10/18] 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 48f1510c6a84a..9305b1e57976f 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 19c3a177af30aa64c601fb504b43cde9de6beeaf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 08:17:40 +0100 Subject: [PATCH 11/18] use more traditional walk_array/visit_array instead of the handle_array hook --- src/librustc_mir/interpret/validity.rs | 28 +++--- src/librustc_mir/interpret/visitor.rs | 120 ++++++++++++------------- 2 files changed, 75 insertions(+), 73 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 2b686e1370a10..c3f570a241842 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 @@ -499,13 +498,13 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> } => { let mplace = if self.op.layout.is_zst() { // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(self.op.layout, ectx) + 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)?; + 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. @@ -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 9305b1e57976f..b0b29b0047329 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 9e32b6f55bd112f8ca65cc8006c5de722caee8d4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 09:33:26 +0100 Subject: [PATCH 12/18] 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 c3f570a241842..fc0aea85bb1e5 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_value(self.op), + let value = try_validation!(self.ecx.read_value(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 b0b29b0047329..9a53b51b633b1 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 d0e8a9267ff308e140df7e4c5cce040804d47c06 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 09:52:17 +0100 Subject: [PATCH 13/18] 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 9a53b51b633b1..51a4ffd65d4b7 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 25f5afacb0de0a5a86b876c61b32f72e3564131d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 10:01:22 +0100 Subject: [PATCH 14/18] 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 fc0aea85bb1e5..7b21d98b3c578 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, ValTy, 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: ValTy<'tcx, M::PointerTag>) -> EvalResult<'tcx> { - let value = try_validation!(self.ecx.read_value(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 51a4ffd65d4b7..7139225b012e7 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, ValTy, }; // 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: ValTy<'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_value(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 edf27f034cb2580e00698acbf7e7eb712c8ada5d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 10:52:07 +0100 Subject: [PATCH 15/18] 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 7b21d98b3c578..982fd4ef7702b 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 7139225b012e7..806972719234a 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: ValTy<'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 babd7a4c723e0f61f1a470a0b1bb847f6484bc8c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 11:24:36 +0100 Subject: [PATCH 16/18] 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 806972719234a..deaf216bf9b35 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 dfbfd2c8614dd6e0b3cfdeead64d2fd09ab005c1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 13:17:06 +0100 Subject: [PATCH 17/18] 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 effe44bc7e1ec..521c9ec3708c9 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, Value> { // 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 => Value::Scalar(place.ptr.into()), Some(meta) => Value::ScalarPair(place.ptr.into(), meta.into()), From 7f93557c311fd35ffbf141d9c4b3e175fa6ddb5d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 2 Nov 2018 14:06:43 +0100 Subject: [PATCH 18/18] make ValueVisitor mut-polymorphic --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/eval_context.rs | 31 +- src/librustc_mir/interpret/validity.rs | 8 +- src/librustc_mir/interpret/visitor.rs | 399 ++++++++++----------- 4 files changed, 210 insertions(+), 230 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 5b06eb5352536..5357450907d7f 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/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index fdb91a44cdb4b..60edf7789d2b1 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -139,8 +139,8 @@ impl<'tcx, Tag> LocalValue { } } -impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout - for &'b EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout + for &EvalContext<'a, 'mir, 'tcx, M> { #[inline] fn data_layout(&self) -> &layout::TargetDataLayout { @@ -148,8 +148,8 @@ impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout } } -impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout - for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout + for &&EvalContext<'a, 'mir, 'tcx, M> { #[inline] fn data_layout(&self) -> &layout::TargetDataLayout { @@ -157,7 +157,16 @@ impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout } } -impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> HasDataLayout + for & &mut EvalContext<'a, 'mir, 'tcx, M> +{ + #[inline] + fn data_layout(&self) -> &layout::TargetDataLayout { + &self.tcx.data_layout + } +} + +impl<'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &EvalContext<'a, 'mir, 'tcx, M> where M: Machine<'a, 'mir, 'tcx> { #[inline] @@ -166,8 +175,8 @@ impl<'b, 'a, 'mir, 'tcx, M> layout::HasTyCtxt<'tcx> for &'b EvalContext<'a, 'mir } } -impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx> - for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx> + for & &mut EvalContext<'a, 'mir, 'tcx, M> { #[inline] fn tcx<'d>(&'d self) -> TyCtxt<'d, 'tcx, 'tcx> { @@ -175,8 +184,8 @@ impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> layout::HasTyCtxt<'tcx> } } -impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf - for &'b EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf + for &EvalContext<'a, 'mir, 'tcx, M> { type Ty = Ty<'tcx>; type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>; @@ -188,8 +197,8 @@ impl<'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf } } -impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf - for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M> +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> LayoutOf + for & &mut EvalContext<'a, 'mir, 'tcx, M> { type Ty = Ty<'tcx>; type TyLayout = EvalResult<'tcx, TyLayout<'tcx>>; diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 982fd4ef7702b..d412878733b5b 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 deaf216bf9b35..443f45deb61c3 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, ValTy, + Machine, EvalContext, MPlaceTy, OpTy, ValTy, }; // 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: ValTy<'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: ValTy<'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_value(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_value(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);