diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 06e015cc4e6842bad7dc032486a075def075509e..82ff36d65acf0b0dce4b16597d661e5d9c2b559e 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -10,7 +10,7 @@ self, Size, Align, HasDataLayout, LayoutOf, TyLayout, Primitive }; use rustc::ty::subst::{Subst, Substs}; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::query::TyCtxtAt; use rustc_data_structures::fx::{FxHashSet, FxHasher}; use rustc_data_structures::indexed_vec::IndexVec; @@ -24,7 +24,7 @@ use syntax::ast::Mutability; use super::{ - Value, ValTy, Operand, MemPlace, MPlaceTy, Place, + Value, ValTy, Operand, MemPlace, MPlaceTy, Place, PlaceExtra, Memory, Machine }; @@ -442,10 +442,14 @@ pub fn load_mir( } } - pub fn monomorphize(&self, ty: Ty<'tcx>, substs: &'tcx Substs<'tcx>) -> Ty<'tcx> { + pub fn monomorphize + Subst<'tcx>>( + &self, + t: T, + substs: &'tcx Substs<'tcx> + ) -> T { // miri doesn't care about lifetimes, and will choke on some crazy ones // let's simply get rid of them - let substituted = ty.subst(*self.tcx, substs); + let substituted = t.subst(*self.tcx, substs); self.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substituted) } @@ -844,35 +848,41 @@ pub fn validate_mplace( match dest.layout.ty.builtin_deref(false).map(|tam| &tam.ty.sty) { | Some(ty::TyStr) | Some(ty::TySlice(_)) => { - // check the length + // check the length (for nicer error messages) let len_mplace = self.mplace_field(dest, 1)?; let len = self.read_scalar(len_mplace.into())?; let len = match len.to_bits(len_mplace.layout.size) { Err(_) => return validation_failure!("length is not a valid integer", path), Ok(len) => len as u64, }; - // get the fat ptr + // get the fat ptr, and recursively check it let ptr = self.ref_to_mplace(self.read_value(dest.into())?)?; - let mut path = path.clone(); - self.dump_field_name(&mut path, dest.layout.ty, 0, variant).unwrap(); - // check all fields - for i in 0..len { + assert_eq!(ptr.extra, PlaceExtra::Length(len)); + let unpacked_ptr = self.unpack_unsized_mplace(ptr)?; + if seen.insert(unpacked_ptr) { let mut path = path.clone(); - self.dump_field_name(&mut path, ptr.layout.ty, i as usize, 0).unwrap(); - let field = self.mplace_field(ptr, i)?; - self.validate_mplace(field, path, seen, todo)?; + self.dump_field_name(&mut path, dest.layout.ty, 0, 0).unwrap(); + todo.push((unpacked_ptr, path)) } - // FIXME: For a TyStr, check that this is valid UTF-8 }, Some(ty::TyDynamic(..)) => { - let vtable_mplace = self.mplace_field(dest, 1)?; - let vtable = self.read_scalar(vtable_mplace.into())?; - if vtable.to_ptr().is_err() { - return validation_failure!("vtable address is not a pointer", path); + // check the vtable (for nicer error messages) + let vtable = self.read_scalar(self.mplace_field(dest, 1)?.into())?; + let vtable = match vtable.to_ptr() { + Err(_) => return validation_failure!("vtable address is not a pointer", path), + Ok(vtable) => vtable, + }; + // get the fat ptr, and recursively check it + let ptr = self.ref_to_mplace(self.read_value(dest.into())?)?; + assert_eq!(ptr.extra, PlaceExtra::Vtable(vtable)); + let unpacked_ptr = self.unpack_unsized_mplace(ptr)?; + if seen.insert(unpacked_ptr) { + let mut path = path.clone(); + self.dump_field_name(&mut path, dest.layout.ty, 0, 0).unwrap(); + todo.push((unpacked_ptr, path)) } - // get the fat ptr - let _ptr = self.ref_to_mplace(self.read_value(dest.into())?)?; - // FIXME: What can we verify about this? + // FIXME: More checks for the vtable... making sure it is exactly + // the one one would expect for this type. }, Some(ty) => bug!("Unexpected fat pointer target type {:?}", ty), @@ -884,6 +894,7 @@ pub fn validate_mplace( let field = self.mplace_field(dest, i as u64)?; self.validate_mplace(field, path, seen, todo)?; } + // FIXME: For a TyStr, check that this is valid UTF-8. }, } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 555bc797f6c50a8581c97d104643ba2f54c6e2d8..b53aa4e2e65b8269bfbedec7ee5bfeae11d8879d 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -721,4 +721,39 @@ pub fn place_to_op(&self, place: PlaceTy<'tcx>) -> EvalResult<'tcx, OpTy<'tcx>> }; Ok(OpTy { op, layout: place.layout }) } + + /// Turn a place that is a dyn trait (i.e., PlaceExtra::Vtable and the appropriate layout) + /// or a slice into the specific fixed-size place and layout that is given by the vtable/len. + /// This "unpacks" the existential quantifier, so to speak. + pub fn unpack_unsized_mplace(&self, mplace: MPlaceTy<'tcx>) -> EvalResult<'tcx, MPlaceTy<'tcx>> { + trace!("Unpacking {:?} ({:?})", *mplace, mplace.layout.ty); + let layout = match mplace.extra { + PlaceExtra::Vtable(vtable) => { + // the drop function signature + let drop_instance = self.read_drop_type_from_vtable(vtable)?; + trace!("Found drop fn: {:?}", drop_instance); + let fn_sig = drop_instance.ty(*self.tcx).fn_sig(*self.tcx); + let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig); + // the drop function takes *mut T where T is the type being dropped, so get that + let ty = fn_sig.inputs()[0].builtin_deref(true).unwrap().ty; + let layout = self.layout_of(ty)?; + // Sanity checks + let (size, align) = self.read_size_and_align_from_vtable(vtable)?; + assert_eq!(size, layout.size); + assert_eq!(align.abi(), layout.align.abi()); // only ABI alignment is preserved + // Done! + layout + }, + PlaceExtra::Length(len) => { + let ty = self.tcx.mk_array(mplace.layout.field(self, 0)?.ty, len); + self.layout_of(ty)? + } + PlaceExtra::None => bug!("Expected a fat pointer"), + }; + trace!("Unpacked type: {:?}", layout.ty); + Ok(MPlaceTy { + mplace: MemPlace { extra: PlaceExtra::None, ..*mplace }, + layout + }) + } } diff --git a/src/librustc_mir/interpret/terminator/drop.rs b/src/librustc_mir/interpret/terminator/drop.rs index 11fd7373284d1f210c9514ec3bacc4ef93c450de..598ee74fbfc46f04db76ddc37844364344a63ae5 100644 --- a/src/librustc_mir/interpret/terminator/drop.rs +++ b/src/librustc_mir/interpret/terminator/drop.rs @@ -1,57 +1,45 @@ use rustc::mir::BasicBlock; -use rustc::ty::{self, Ty, layout::LayoutOf}; +use rustc::ty::{self, layout::LayoutOf}; use syntax::source_map::Span; use rustc::mir::interpret::{EvalResult}; -use interpret::{Machine, EvalContext, PlaceTy, Value, OpTy, Operand}; +use interpret::{Machine, EvalContext, PlaceTy, PlaceExtra, OpTy, Operand}; impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - pub(crate) fn drop_place( + pub(crate) fn drop_in_place( &mut self, place: PlaceTy<'tcx>, instance: ty::Instance<'tcx>, span: Span, target: BasicBlock, ) -> EvalResult<'tcx> { + trace!("drop_in_place: {:?},\n {:?}, {:?}", *place, place.layout.ty, instance); // We take the address of the object. This may well be unaligned, which is fine for us here. // However, unaligned accesses will probably make the actual drop implementation fail -- a problem shared // by rustc. - let val = self.force_allocation(place)?.to_ref(&self); - self.drop(val, instance, place.layout.ty, span, target) - } - - fn drop( - &mut self, - arg: Value, - instance: ty::Instance<'tcx>, - ty: Ty<'tcx>, - span: Span, - target: BasicBlock, - ) -> EvalResult<'tcx> { - trace!("drop: {:?},\n {:?}, {:?}", arg, ty.sty, instance.def); + let place = self.force_allocation(place)?; - let (instance, arg) = match ty.sty { + let (instance, place) = match place.layout.ty.sty { ty::TyDynamic(..) => { - if let Value::ScalarPair(ptr, vtable) = arg { - // Figure out the specific drop function to call, and just pass along - // the thin part of the pointer. - let instance = self.read_drop_type_from_vtable(vtable.to_ptr()?)?; - trace!("Dropping via vtable: {:?}", instance.def); - (instance, Value::Scalar(ptr)) - } else { - bug!("expected fat ptr, got {:?}", arg); - } + // Dropping a trait object. + let vtable = match place.extra { + PlaceExtra::Vtable(vtable) => vtable, + _ => bug!("Expected vtable when dropping {:#?}", place), + }; + let place = self.unpack_unsized_mplace(place)?; + let instance = self.read_drop_type_from_vtable(vtable)?; + (instance, place) } - _ => (instance, arg), + _ => (instance, place), }; - // the drop function expects a reference to the value - let fn_sig = self.tcx.fn_sig(instance.def_id()).skip_binder().clone(); + let fn_sig = instance.ty(*self.tcx).fn_sig(*self.tcx); + let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig); + let arg = OpTy { - op: Operand::Immediate(arg), - layout: self.layout_of(fn_sig.output())?, + op: Operand::Immediate(place.to_ref(&self)), + layout: self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, }; - trace!("Dropped type: {:?}", fn_sig.output()); // This should always be (), but getting it from the sig seems // easier than creating a layout of (). diff --git a/src/librustc_mir/interpret/terminator/mod.rs b/src/librustc_mir/interpret/terminator/mod.rs index e0980a4657e6475e0aeb4a3717ca16377dc285b0..5ceff57bfee47b3532f11911fe3af4069079467e 100644 --- a/src/librustc_mir/interpret/terminator/mod.rs +++ b/src/librustc_mir/interpret/terminator/mod.rs @@ -129,7 +129,7 @@ pub(super) fn eval_terminator( trace!("TerminatorKind::drop: {:?}, type {}", location, ty); let instance = ::monomorphize::resolve_drop_in_place(*self.tcx, ty); - self.drop_place( + self.drop_in_place( place, instance, terminator.source_info.span, diff --git a/src/test/ui/union-ub-fat-ptr.rs b/src/test/ui/union-ub-fat-ptr.rs index cfce92ece7a921da496263721a927bd80a472f79..45e70a9dbc96c999fa24768f0c4bb3f0d26e86dc 100644 --- a/src/test/ui/union-ub-fat-ptr.rs +++ b/src/test/ui/union-ub-fat-ptr.rs @@ -66,16 +66,18 @@ trait Trait {} // OK const A: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 1 } }.str}; -// should lint +// bad const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str}; +//~^ ERROR this constant likely exhibits undefined behavior // bad const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.str}; //~^ ERROR this constant likely exhibits undefined behavior // OK const A2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 1 } }.slice}; -// should lint +// bad const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice}; +//~^ ERROR this constant likely exhibits undefined behavior // bad const C2: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice}; //~^ ERROR this constant likely exhibits undefined behavior diff --git a/src/test/ui/union-ub-fat-ptr.stderr b/src/test/ui/union-ub-fat-ptr.stderr index f0298d9013c933b6ff03de3318a9c553dbc60810..924d0d667f27122b124c06672f38d4c1d5f24d1b 100644 --- a/src/test/ui/union-ub-fat-ptr.stderr +++ b/src/test/ui/union-ub-fat-ptr.stderr @@ -1,5 +1,13 @@ error[E0080]: this constant likely exhibits undefined behavior - --> $DIR/union-ub-fat-ptr.rs:72:1 + --> $DIR/union-ub-fat-ptr.rs:70:1 + | +LL | const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access at offset N, outside bounds of allocation N which has size N + | + = 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]: this constant likely exhibits undefined behavior + --> $DIR/union-ub-fat-ptr.rs:73:1 | LL | const C: &str = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.str}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered length is not a valid integer @@ -7,7 +15,15 @@ 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]: this constant likely exhibits undefined behavior - --> $DIR/union-ub-fat-ptr.rs:80:1 + --> $DIR/union-ub-fat-ptr.rs:79:1 + | +LL | const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access at offset N, outside bounds of allocation N which has size N + | + = 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]: this constant likely exhibits undefined behavior + --> $DIR/union-ub-fat-ptr.rs:82:1 | LL | const C2: &[u8] = unsafe { SliceTransmute { bad: BadSliceRepr { ptr: &42, len: &3 } }.slice}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered length is not a valid integer @@ -15,7 +31,7 @@ LL | const C2: &[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]: this constant likely exhibits undefined behavior - --> $DIR/union-ub-fat-ptr.rs:84:1 + --> $DIR/union-ub-fat-ptr.rs:86:1 | LL | const D: &Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: &3 } }.rust}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tried to access memory with alignment N, but alignment N is required @@ -23,21 +39,21 @@ 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]: this constant likely exhibits undefined behavior - --> $DIR/union-ub-fat-ptr.rs:87:1 + --> $DIR/union-ub-fat-ptr.rs:89:1 | LL | const E: &Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtable: &3 } }.rust}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access at offset N, outside bounds of allocation N which has size N + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a memory access tried to interpret some bytes as a pointer | = 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]: this constant likely exhibits undefined behavior - --> $DIR/union-ub-fat-ptr.rs:90:1 + --> $DIR/union-ub-fat-ptr.rs:92:1 | LL | const F: &Trait = unsafe { DynTransmute { bad: BadDynRepr { ptr: &92, vtable: 3 } }.rust}; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered vtable address is not a pointer | = 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 5 previous errors +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0080`.