From 1297a274a37f6e7c48ad8cc8a402c28d83cb98b5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 13:51:18 +0200 Subject: [PATCH 1/8] Add basic support for "other" kinds of values for function pointers, determined by the machine instance. So far, however, calling such a function will fail. --- src/librustc_mir/const_eval.rs | 1 + src/librustc_mir/interpret/cast.rs | 6 +- src/librustc_mir/interpret/machine.rs | 5 + src/librustc_mir/interpret/memory.rs | 121 ++++++++++++++--------- src/librustc_mir/interpret/mod.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 26 ++--- src/librustc_mir/interpret/traits.rs | 10 +- 7 files changed, 107 insertions(+), 64 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index aa264bbd4bb5c..a5c52324c89c2 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -316,6 +316,7 @@ impl interpret::MayLeak for ! { impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> { type MemoryKinds = !; type PointerTag = (); + type ExtraFnVal = !; type FrameExtra = (); type MemoryExtra = (); diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index d61fb87336ccf..3ef525979f8c9 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use rustc::mir::CastKind; -use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate}; +use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate, FnVal}; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool { @@ -86,7 +86,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { def_id, substs, ).ok_or_else(|| InterpError::TooGeneric.into()); - let fn_ptr = self.memory.create_fn_alloc(instance?); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance?)); self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?; } _ => bug!("reify fn pointer on {:?}", src.layout.ty), @@ -115,7 +115,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { substs, ty::ClosureKind::FnOnce, ); - let fn_ptr = self.memory.create_fn_alloc(instance); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance)); let val = Immediate::Scalar(Scalar::Ptr(fn_ptr.into()).into()); self.write_immediate(val, dest)?; } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index f16c21857b987..02ba00508441c 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -67,6 +67,11 @@ pub trait Machine<'mir, 'tcx>: Sized { /// The `default()` is used for pointers to consts, statics, vtables and functions. type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static; + /// Machines can define extra (non-instance) things that represent values of function pointers. + /// For example, Miri uses this to return a fucntion pointer from `dlsym` + /// that can later be called to execute the right thing. + type ExtraFnVal: ::std::fmt::Debug + Copy; + /// Extra data stored in every call frame. type FrameExtra; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 5b177d05bb862..fab559bf39db4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::ptr; use std::borrow::Cow; -use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; +use rustc::ty::{self, Instance, query::TyCtxtAt}; use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; @@ -54,6 +54,26 @@ pub enum AllocCheck { MaybeDead, } +/// The value of a function pointer. +#[derive(Debug, Copy, Clone)] +pub enum FnVal<'tcx, Other> { + Instance(Instance<'tcx>), + Other(Other), +} + +impl<'tcx, Other> FnVal<'tcx, Other> { + pub fn as_instance(self) -> InterpResult<'tcx, Instance<'tcx>> { + match self { + FnVal::Instance(instance) => + Ok(instance), + FnVal::Other(_) => + err!(MachineError( + format!("Expected instance function pointer, got 'other' pointer") + )), + } + } +} + // `Memory` has to depend on the `Machine` because some of its operations // (e.g., `get`) call a `Machine` hook. pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -69,16 +89,20 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { // FIXME: this should not be public, but interning currently needs access to it pub(super) alloc_map: M::MemoryMap, + /// Map for "extra" function pointers. + extra_fn_ptr_map: FxHashMap, + /// To be able to compare pointers with NULL, and to check alignment for accesses /// to ZSTs (where pointers may dangle), we keep track of the size even for allocations /// that do not exist any more. + // FIXME: this should not be public, but interning currently needs access to it pub(super) dead_alloc_map: FxHashMap, /// Extra data added by the machine. pub extra: M::MemoryExtra, /// Lets us implement `HasDataLayout`, which is awfully convenient. - pub(super) tcx: TyCtxtAt<'tcx>, + pub tcx: TyCtxtAt<'tcx>, } impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> { @@ -98,6 +122,7 @@ where fn clone(&self) -> Self { Memory { alloc_map: self.alloc_map.clone(), + extra_fn_ptr_map: self.extra_fn_ptr_map.clone(), dead_alloc_map: self.dead_alloc_map.clone(), extra: (), tcx: self.tcx, @@ -109,6 +134,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn new(tcx: TyCtxtAt<'tcx>) -> Self { Memory { alloc_map: M::MemoryMap::default(), + extra_fn_ptr_map: FxHashMap::default(), dead_alloc_map: FxHashMap::default(), extra: M::MemoryExtra::default(), tcx, @@ -120,8 +146,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self)) } - pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { - let id = self.tcx.alloc_map.lock().create_fn_alloc(instance); + pub fn create_fn_alloc( + &mut self, + fn_val: FnVal<'tcx, M::ExtraFnVal>, + ) -> Pointer + { + let id = match fn_val { + FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance), + FnVal::Other(extra) => { + // TODO: Should we have a cache here? + let id = self.tcx.alloc_map.lock().reserve(); + let old = self.extra_fn_ptr_map.insert(id, extra); + assert!(old.is_none()); + id + } + }; self.tag_static_base_pointer(Pointer::from(id)) } @@ -495,56 +534,50 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, liveness: AllocCheck, ) -> InterpResult<'static, (Size, Align)> { + // Regular allocations. if let Ok(alloc) = self.get(id) { - return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); + Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) } - // can't do this in the match argument, we may get cycle errors since the lock would get - // dropped after the match. - let alloc = self.tcx.alloc_map.lock().get(id); - // Could also be a fn ptr or extern static - match alloc { - Some(GlobalAlloc::Function(..)) => { - if let AllocCheck::Dereferencable = liveness { - // The caller requested no function pointers. - err!(DerefFunctionPointer) - } else { - Ok((Size::ZERO, Align::from_bytes(1).unwrap())) - } + // Function pointers. + else if let Ok(_) = self.get_fn_alloc(id) { + if let AllocCheck::Dereferencable = liveness { + // The caller requested no function pointers. + err!(DerefFunctionPointer) + } else { + Ok((Size::ZERO, Align::from_bytes(1).unwrap())) } - // `self.get` would also work, but can cause cycles if a static refers to itself - Some(GlobalAlloc::Static(did)) => { - // The only way `get` couldn't have worked here is if this is an extern static - assert!(self.tcx.is_foreign_item(did)); - // Use size and align of the type - let ty = self.tcx.type_of(did); - let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); - Ok((layout.size, layout.align.abi)) + } + // The rest must be dead. + else if let AllocCheck::MaybeDead = liveness { + // Deallocated pointers are allowed, we should be able to find + // them in the map. + Ok(*self.dead_alloc_map.get(&id) + .expect("deallocated pointers should all be recorded in `dead_alloc_map`")) + } else { + err!(DanglingPointerDeref) + } + } + + fn get_fn_alloc(&self, id: AllocId) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { + trace!("reading fn ptr: {}", id); + if let Some(extra) = self.extra_fn_ptr_map.get(&id) { + Ok(FnVal::Other(*extra)) + } else { + match self.tcx.alloc_map.lock().get(id) { + Some(GlobalAlloc::Function(instance)) => Ok(FnVal::Instance(instance)), + _ => Err(InterpError::ExecuteMemory.into()), } - _ => { - if let Ok(alloc) = self.get(id) { - Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) - } - else if let AllocCheck::MaybeDead = liveness { - // Deallocated pointers are allowed, we should be able to find - // them in the map. - Ok(*self.dead_alloc_map.get(&id) - .expect("deallocated pointers should all be recorded in `dead_alloc_map`")) - } else { - err!(DanglingPointerDeref) - } - }, } } - pub fn get_fn(&self, ptr: Pointer) -> InterpResult<'tcx, Instance<'tcx>> { + pub fn get_fn( + &self, + ptr: Pointer, + ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { if ptr.offset.bytes() != 0 { return err!(InvalidFunctionPointer); } - trace!("reading fn ptr: {}", ptr.alloc_id); - match self.tcx.alloc_map.lock().get(ptr.alloc_id) { - Some(GlobalAlloc::Function(instance)) => Ok(instance), - _ => Err(InterpError::ExecuteMemory.into()), - } + self.get_fn_alloc(ptr.alloc_id) } pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 2b20f9df53837..45d24347e4efd 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -24,7 +24,7 @@ pub use self::eval_context::{ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; -pub use self::memory::{Memory, MemoryKind, AllocCheck}; +pub use self::memory::{Memory, MemoryKind, AllocCheck, FnVal}; pub use self::machine::{Machine, AllocMap, MayLeak}; diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index d6f3de02ec918..4440d677a41d4 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -6,9 +6,9 @@ use rustc::ty::layout::{self, TyLayout, LayoutOf}; use syntax::source_map::Span; use rustc_target::spec::abi::Abi; -use rustc::mir::interpret::{InterpResult, PointerArithmetic, InterpError, Scalar}; use super::{ - InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup + InterpResult, PointerArithmetic, InterpError, Scalar, + InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup, FnVal, }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { @@ -76,16 +76,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let func = self.eval_operand(func, None)?; - let (fn_def, abi) = match func.layout.ty.sty { + let (fn_val, abi) = match func.layout.ty.sty { ty::FnPtr(sig) => { let caller_abi = sig.abi(); let fn_ptr = self.force_ptr(self.read_scalar(func)?.not_undef()?)?; - let instance = self.memory.get_fn(fn_ptr)?; - (instance, caller_abi) + let fn_val = self.memory.get_fn(fn_ptr)?; + (fn_val, caller_abi) } ty::FnDef(def_id, substs) => { let sig = func.layout.ty.fn_sig(*self.tcx); - (self.resolve(def_id, substs)?, sig.abi()) + (FnVal::Instance(self.resolve(def_id, substs)?), sig.abi()) }, _ => { let msg = format!("can't handle callee of type {:?}", func.layout.ty); @@ -94,7 +94,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let args = self.eval_operands(args)?; self.eval_fn_call( - fn_def, + fn_val, terminator.source_info.span, abi, &args[..], @@ -228,14 +228,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Call this function -- pushing the stack frame and initializing the arguments. fn eval_fn_call( &mut self, - instance: ty::Instance<'tcx>, + fn_val: FnVal<'tcx, M::ExtraFnVal>, span: Span, caller_abi: Abi, args: &[OpTy<'tcx, M::PointerTag>], dest: Option>, ret: Option, ) -> InterpResult<'tcx> { - trace!("eval_fn_call: {:#?}", instance); + trace!("eval_fn_call: {:#?}", fn_val); + + let instance = fn_val.as_instance()?; match instance.def { ty::InstanceDef::Intrinsic(..) => { @@ -432,7 +434,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { )?.expect("cannot be a ZST"); let fn_ptr = self.memory.get(vtable_slot.alloc_id)? .read_ptr_sized(self, vtable_slot)?.to_ptr()?; - let instance = self.memory.get_fn(fn_ptr)?; + let drop_fn = self.memory.get_fn(fn_ptr)?; // `*mut receiver_place.layout.ty` is almost the layout that we // want for args[0]: We have to project to field 0 because we want @@ -447,7 +449,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function - self.eval_fn_call(instance, span, caller_abi, &args, dest, ret) + self.eval_fn_call(drop_fn, span, caller_abi, &args, dest, ret) } } } @@ -482,7 +484,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = MPlaceTy::dangling(self.layout_of(ty)?, self); self.eval_fn_call( - instance, + FnVal::Instance(instance), span, Abi::Rust, &[arg.into()], diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 27d127514229c..35eb01392f4d5 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -2,7 +2,7 @@ use rustc::ty::{self, Ty, Instance}; use rustc::ty::layout::{Size, Align, LayoutOf}; use rustc::mir::interpret::{Scalar, Pointer, InterpResult, PointerArithmetic}; -use super::{InterpCx, InterpError, Machine, MemoryKind}; +use super::{InterpCx, InterpError, Machine, MemoryKind, FnVal}; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Creates a dynamic vtable for the given type and vtable origin. This is used only for @@ -56,7 +56,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let tcx = &*self.tcx; let drop = Instance::resolve_drop_in_place(*tcx, ty); - let drop = self.memory.create_fn_alloc(drop); + let drop = self.memory.create_fn_alloc(FnVal::Instance(drop)); // no need to do any alignment checks on the memory accesses below, because we know the // allocation is correctly aligned as we created it above. Also we're only offsetting by @@ -84,7 +84,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { def_id, substs, ).ok_or_else(|| InterpError::TooGeneric)?; - let fn_ptr = self.memory.create_fn_alloc(instance); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance)); let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?; self.memory .get_mut(method_ptr.alloc_id)? @@ -113,7 +113,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .get(vtable.alloc_id)? .read_ptr_sized(self, vtable)? .to_ptr()?; - let drop_instance = self.memory.get_fn(drop_fn)?; + // We *need* an instance here, no other kind of function value, to be able + // to determine the type. + let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?; 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); From 5612feb5133d24caaf68f18e9ae8812ecbaa0ba3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:06:13 +0200 Subject: [PATCH 2/8] add machine hook to handle calls to 'extra' function values --- src/librustc_mir/const_eval.rs | 10 ++++++++++ src/librustc_mir/interpret/machine.rs | 10 ++++++++++ src/librustc_mir/interpret/terminator.rs | 7 ++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index a5c52324c89c2..74df2898db4c5 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -371,6 +371,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, })) } + fn call_extra_fn( + _ecx: &mut InterpretCx<'mir, 'tcx, Self>, + fn_val: !, + _args: &[OpTy<'tcx>], + _dest: Option>, + _ret: Option, + ) -> InterpResult<'tcx> { + match fn_val {} + } + fn call_intrinsic( ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 02ba00508441c..b35d468c49a91 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -124,6 +124,16 @@ pub trait Machine<'mir, 'tcx>: Sized { ret: Option, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; + /// Execute `fn_val`. it is the hook's responsibility to advance the instruction + /// pointer as appropriate. + fn call_extra_fn( + ecx: &mut InterpretCx<'mir, 'tcx, Self>, + fn_val: Self::ExtraFnVal, + args: &[OpTy<'tcx, Self::PointerTag>], + dest: Option>, + ret: Option, + ) -> InterpResult<'tcx>; + /// Directly process an intrinsic without pushing a stack frame. /// If this returns successfully, the engine will take care of jumping to the next block. fn call_intrinsic( diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 4440d677a41d4..7978e8c36c772 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -237,7 +237,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx> { trace!("eval_fn_call: {:#?}", fn_val); - let instance = fn_val.as_instance()?; + let instance = match fn_val { + FnVal::Instance(instance) => instance, + FnVal::Other(extra) => { + return M::call_extra_fn(self, extra, args, dest, ret); + } + }; match instance.def { ty::InstanceDef::Intrinsic(..) => { From 486720f0800fd53cb0466f20bbba840798fe5da3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:29:04 +0200 Subject: [PATCH 3/8] fix determinig the size of foreign static allocations --- src/librustc_mir/interpret/memory.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index fab559bf39db4..bed531504d3e4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::ptr; use std::borrow::Cow; -use rustc::ty::{self, Instance, query::TyCtxtAt}; +use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; @@ -536,19 +536,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'static, (Size, Align)> { // Regular allocations. if let Ok(alloc) = self.get(id) { - Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) + return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); } // Function pointers. - else if let Ok(_) = self.get_fn_alloc(id) { - if let AllocCheck::Dereferencable = liveness { + if let Ok(_) = self.get_fn_alloc(id) { + return if let AllocCheck::Dereferencable = liveness { // The caller requested no function pointers. err!(DerefFunctionPointer) } else { Ok((Size::ZERO, Align::from_bytes(1).unwrap())) + }; + } + // Foreign statics. + // Can't do this in the match argument, we may get cycle errors since the lock would + // be held throughout the match. + let alloc = self.tcx.alloc_map.lock().get(id); + match alloc { + Some(GlobalAlloc::Static(did)) => { + assert!(self.tcx.is_foreign_item(did)); + // Use size and align of the type + let ty = self.tcx.type_of(did); + let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); + return Ok((layout.size, layout.align.abi)); } + _ => {} } // The rest must be dead. - else if let AllocCheck::MaybeDead = liveness { + if let AllocCheck::MaybeDead = liveness { // Deallocated pointers are allowed, we should be able to find // them in the map. Ok(*self.dead_alloc_map.get(&id) From b4be08a66692e5f9aa15b14b233d88e626f5c605 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:35:39 +0200 Subject: [PATCH 4/8] fix for tidy --- src/librustc_mir/interpret/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index bed531504d3e4..0bd732eb31d6d 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -154,7 +154,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let id = match fn_val { FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance), FnVal::Other(extra) => { - // TODO: Should we have a cache here? + // FIXME(RalfJung): Should we have a cache here? let id = self.tcx.alloc_map.lock().reserve(); let old = self.extra_fn_ptr_map.insert(id, extra); assert!(old.is_none()); From 842bbd2764797cbb7a8364cb17c0bb4b4a5c8432 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 11:16:18 +0200 Subject: [PATCH 5/8] make Memory::get_fn take a Scalar like most of the Memory API surface --- src/librustc_mir/interpret/memory.rs | 3 ++- src/librustc_mir/interpret/terminator.rs | 4 ++-- src/librustc_mir/interpret/traits.rs | 2 +- src/librustc_mir/interpret/validity.rs | 10 +++++----- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 0bd732eb31d6d..1cdfa4c63b475 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -586,8 +586,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn get_fn( &self, - ptr: Pointer, + ptr: Scalar, ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { + let ptr = self.force_ptr(ptr)?; // We definitely need a pointer value. if ptr.offset.bytes() != 0 { return err!(InvalidFunctionPointer); } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 7978e8c36c772..0ab428628de68 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -79,7 +79,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let (fn_val, abi) = match func.layout.ty.sty { ty::FnPtr(sig) => { let caller_abi = sig.abi(); - let fn_ptr = self.force_ptr(self.read_scalar(func)?.not_undef()?)?; + let fn_ptr = self.read_scalar(func)?.not_undef()?; let fn_val = self.memory.get_fn(fn_ptr)?; (fn_val, caller_abi) } @@ -438,7 +438,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.tcx.data_layout.pointer_align.abi, )?.expect("cannot be a ZST"); let fn_ptr = self.memory.get(vtable_slot.alloc_id)? - .read_ptr_sized(self, vtable_slot)?.to_ptr()?; + .read_ptr_sized(self, vtable_slot)?.not_undef()?; let drop_fn = self.memory.get_fn(fn_ptr)?; // `*mut receiver_place.layout.ty` is almost the layout that we diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 35eb01392f4d5..e7363f6876c28 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -112,7 +112,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let drop_fn = self.memory .get(vtable.alloc_id)? .read_ptr_sized(self, vtable)? - .to_ptr()?; + .not_undef()?; // We *need* an instance here, no other kind of function value, to be able // to determine the type. let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?; diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 374f42261bf62..5049c50004a95 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -457,10 +457,10 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } ty::FnPtr(_sig) => { let value = value.to_scalar_or_undef(); - let ptr = try_validation!(value.to_ptr(), - value, self.path, "a pointer"); - let _fn = try_validation!(self.ecx.memory.get_fn(ptr), - value, self.path, "a function pointer"); + let _fn = try_validation!( + value.not_undef().and_then(|ptr| self.ecx.memory.get_fn(ptr)), + value, self.path, "a function pointer" + ); // FIXME: Check if the signature matches } // This should be all the primitive types @@ -508,7 +508,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // differentiate between null pointers and dangling pointers if self.ref_tracking_for_consts.is_some() && self.ecx.memory.get(ptr.alloc_id).is_err() && - self.ecx.memory.get_fn(ptr).is_err() { + self.ecx.memory.get_fn(ptr.into()).is_err() { return validation_failure!( "encountered dangling pointer", self.path ); From 317c6ac12935046b04c349f6151148600d4c1512 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 11:26:28 +0200 Subject: [PATCH 6/8] use get_size_and_align to test if an allocation is live --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/machine.rs | 2 +- src/librustc_mir/interpret/validity.rs | 17 ++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 74df2898db4c5..10dbe1799a5dc 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -372,7 +372,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } fn call_extra_fn( - _ecx: &mut InterpretCx<'mir, 'tcx, Self>, + _ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: !, _args: &[OpTy<'tcx>], _dest: Option>, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index b35d468c49a91..5d88274eb968f 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -127,7 +127,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Execute `fn_val`. it is the hook's responsibility to advance the instruction /// pointer as appropriate. fn call_extra_fn( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: Self::ExtraFnVal, args: &[OpTy<'tcx, Self::PointerTag>], dest: Option>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 5049c50004a95..642227fe29c16 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -6,14 +6,12 @@ use rustc::hir; use rustc::ty::layout::{self, TyLayout, LayoutOf, VariantIdx}; use rustc::ty; use rustc_data_structures::fx::FxHashSet; -use rustc::mir::interpret::{ - GlobalAlloc, InterpResult, InterpError, -}; use std::hash::Hash; use super::{ - OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, + GlobalAlloc, InterpResult, InterpError, + OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, AllocCheck, }; macro_rules! validation_failure { @@ -505,19 +503,20 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // Only NULL is the niche. So make sure the ptr is NOT NULL. if self.ecx.memory.ptr_may_be_null(ptr) { // These conditions are just here to improve the diagnostics so we can - // differentiate between null pointers and dangling pointers + // differentiate between null pointers and dangling pointers. if self.ref_tracking_for_consts.is_some() && - self.ecx.memory.get(ptr.alloc_id).is_err() && - self.ecx.memory.get_fn(ptr.into()).is_err() { + self.ecx.memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live) + .is_err() + { return validation_failure!( - "encountered dangling pointer", self.path + "a dangling pointer", self.path ); } return validation_failure!("a potentially NULL pointer", self.path); } return Ok(()); } else { - // Conservatively, we reject, because the pointer *could* have this + // Conservatively, we reject, because the pointer *could* have a bad // value. return validation_failure!( "a pointer", From d9d6b3bb28a0a1e94486f066f38f2923dc0cd64d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 19:11:32 +0200 Subject: [PATCH 7/8] turns out that dangling pointer branch is dead code; remove it and improve the error that actually gets shown a bit --- src/librustc_mir/interpret/validity.rs | 21 +++---- src/test/ui/consts/const-eval/ub-enum.rs | 32 +++++++---- src/test/ui/consts/const-eval/ub-enum.stderr | 56 ++++++++++++------- src/test/ui/consts/const-eval/ub-nonnull.rs | 10 ++++ .../ui/consts/const-eval/ub-nonnull.stderr | 27 ++++++--- 5 files changed, 97 insertions(+), 49 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 642227fe29c16..4a8906338bc6a 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use std::hash::Hash; use super::{ GlobalAlloc, InterpResult, InterpError, - OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, AllocCheck, + OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -502,17 +502,14 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> if lo == 1 && hi == max_hi { // Only NULL is the niche. So make sure the ptr is NOT NULL. if self.ecx.memory.ptr_may_be_null(ptr) { - // These conditions are just here to improve the diagnostics so we can - // differentiate between null pointers and dangling pointers. - if self.ref_tracking_for_consts.is_some() && - self.ecx.memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live) - .is_err() - { - return validation_failure!( - "a dangling pointer", self.path - ); - } - return validation_failure!("a potentially NULL pointer", self.path); + return validation_failure!( + "a potentially NULL pointer", + self.path, + format!( + "something that cannot possibly fail to be {}", + wrapping_range_format(&layout.valid_range, max_hi) + ) + ); } return Ok(()); } else { diff --git a/src/test/ui/consts/const-eval/ub-enum.rs b/src/test/ui/consts/const-eval/ub-enum.rs index fe8675d326eee..d4b2220695102 100644 --- a/src/test/ui/consts/const-eval/ub-enum.rs +++ b/src/test/ui/consts/const-eval/ub-enum.rs @@ -1,5 +1,10 @@ #![allow(const_err)] // make sure we cannot allow away the errors tested here + +#[repr(transparent)] +#[derive(Copy, Clone)] +struct Wrap(T); + #[repr(usize)] #[derive(Copy, Clone)] enum Enum { @@ -7,11 +12,20 @@ enum Enum { } union TransmuteEnum { in1: &'static u8, + in2: usize, out1: Enum, + out2: Wrap, } -// A pointer is guaranteed non-null -const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; +const GOOD_ENUM: Enum = unsafe { TransmuteEnum { in2: 0 }.out1 }; + +const BAD_ENUM: Enum = unsafe { TransmuteEnum { in2: 1 }.out1 }; +//~^ ERROR is undefined behavior + +const BAD_ENUM_PTR: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; +//~^ ERROR is undefined behavior + +const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; //~^ ERROR is undefined behavior // (Potentially) invalid enum discriminant @@ -20,9 +34,7 @@ const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; enum Enum2 { A = 2, } -#[repr(transparent)] -#[derive(Copy, Clone)] -struct Wrap(T); + union TransmuteEnum2 { in1: usize, in2: &'static u8, @@ -33,17 +45,17 @@ union TransmuteEnum2 { } const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; //~^ ERROR is undefined behavior -const BAD_ENUM3: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; +const BAD_ENUM2_PTR: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; //~^ ERROR is undefined behavior -const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; +const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; //~^ ERROR is undefined behavior // Undef enum discriminant. -const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; +const BAD_ENUM2_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; //~^ ERROR is undefined behavior // Pointer value in an enum with a niche that is not just 0. -const BAD_ENUM_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; +const BAD_ENUM2_OPTION_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; //~^ ERROR is undefined behavior // Invalid enum field content (mostly to test printing of paths for enum tuple @@ -53,7 +65,7 @@ union TransmuteChar { b: char, } // Need to create something which does not clash with enum layout optimizations. -const BAD_ENUM_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); +const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); //~^ ERROR is undefined behavior fn main() { diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 2bf4cf9bfd65b..38374e7fbad88 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -1,13 +1,29 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:14:1 + --> $DIR/ub-enum.rs:22:1 | -LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { in2: 1 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 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 error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:34:1 + --> $DIR/ub-enum.rs:25:1 + | +LL | const BAD_ENUM_PTR: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:28:1 + | +LL | const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be less or equal to 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-enum.rs:46:1 | LL | const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected a valid enum discriminant @@ -15,45 +31,45 @@ LL | const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; = 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:36:1 + --> $DIR/ub-enum.rs:48:1 | -LL | const BAD_ENUM3: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM2_PTR: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:38:1 + --> $DIR/ub-enum.rs:50:1 | -LL | const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 +LL | const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 | = 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:42:1 + --> $DIR/ub-enum.rs:54:1 | -LL | const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid enum discriminant +LL | const BAD_ENUM2_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, 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:46:1 + --> $DIR/ub-enum.rs:58:1 | -LL | const BAD_ENUM_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM2_OPTION_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:56:1 + --> $DIR/ub-enum.rs:68:1 | -LL | const BAD_ENUM_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected something less or equal to 1114111 +LL | const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected something less or equal to 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 7 previous errors +error: aborting due to 9 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs index 3e0b0948ef3c3..bcbb4358aec03 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.rs +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -5,9 +5,19 @@ use std::mem; use std::ptr::NonNull; use std::num::{NonZeroU8, NonZeroUsize}; +const NON_NULL: NonNull = unsafe { mem::transmute(1usize) }; +const NON_NULL_PTR: NonNull = unsafe { mem::transmute(&1) }; + const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; //~^ ERROR it is undefined behavior to use this value +const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { +//~^ ERROR it is undefined behavior to use this value + let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle + let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic + mem::transmute(out_of_bounds_ptr) +} }; + const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; //~^ ERROR it is undefined behavior to use this value const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index 6230712ad6f23..2f9423fed3530 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:8:1 + --> $DIR/ub-nonnull.rs:11: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,20 @@ 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:11:1 + --> $DIR/ub-nonnull.rs:14:1 + | +LL | / const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { +LL | | +LL | | let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle +LL | | let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic +LL | | mem::transmute(out_of_bounds_ptr) +LL | | } }; + | |____^ type validation failed: encountered a potentially NULL pointer, but expected something that cannot possibly fail to be 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 + +error[E0080]: it is undefined behavior to use this value + --> $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 +28,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:13: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 @@ -23,7 +36,7 @@ LL | const NULL_USIZE: NonZeroUsize = 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:30:1 | LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected something greater or equal to 1 @@ -31,7 +44,7 @@ LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; = 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:28:1 + --> $DIR/ub-nonnull.rs:38:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 @@ -39,13 +52,13 @@ LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(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/ub-nonnull.rs:34:1 + --> $DIR/ub-nonnull.rs:44:1 | LL | const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 20, but expected something less or equal to 10, or greater or equal to 30 | = 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 6 previous errors +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0080`. From ceb496cf59ea28d98146cdcfc072cede4f7c6585 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 19:19:19 +0200 Subject: [PATCH 8/8] improve validity error range printing for singleton ranges --- src/librustc_mir/interpret/validity.rs | 17 +++++++++-------- src/test/ui/consts/const-eval/ub-enum.stderr | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 4a8906338bc6a..34892f5b8ca01 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -151,15 +151,16 @@ fn wrapping_range_format(r: &RangeInclusive, max_hi: u128) -> String { debug_assert!(hi <= max_hi); if lo > hi { format!("less or equal to {}, or greater or equal to {}", hi, lo) + } else if lo == hi { + format!("equal to {}", lo) + } else if lo == 0 { + debug_assert!(hi < max_hi, "should not be printing if the range covers everything"); + format!("less or equal to {}", hi) + } else if hi == max_hi { + debug_assert!(lo > 0, "should not be printing if the range covers everything"); + format!("greater or equal to {}", lo) } else { - if lo == 0 { - debug_assert!(hi < max_hi, "should not be printing if the range covers everything"); - format!("less or equal to {}", hi) - } else if hi == max_hi { - format!("greater or equal to {}", lo) - } else { - format!("in the range {:?}", r) - } + format!("in the range {:?}", r) } } diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 38374e7fbad88..8ecb1aabdd0f7 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -18,7 +18,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:28:1 | LL | const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be less or equal to 0 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 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 @@ -42,7 +42,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:50:1 | LL | const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 2 | = 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