From 82f4a1a9b98ef7f16b25de44150d004c3b1b528a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 22 Mar 2020 11:41:07 +0100 Subject: [PATCH 1/4] get rid of ConstPropUnsupported; use ZST marker structs instead --- src/libcore/any.rs | 2 +- src/librustc/mir/interpret/error.rs | 49 ++++++++++++++++++------ src/librustc/mir/interpret/mod.rs | 4 +- src/librustc_mir/transform/const_prop.rs | 32 +++++++++++----- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/libcore/any.rs b/src/libcore/any.rs index 97ef513cbcc63..39df803bbea30 100644 --- a/src/libcore/any.rs +++ b/src/libcore/any.rs @@ -164,7 +164,7 @@ impl dyn Any { // Get `TypeId` of the type this function is instantiated with. let t = TypeId::of::(); - // Get `TypeId` of the type in the trait object. + // Get `TypeId` of the type in the trait object (`self`). let concrete = self.type_id(); // Compare both `TypeId`s on equality. diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index ff107a5f1e268..fd7f5361214f4 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -14,7 +14,10 @@ use rustc_hir as hir; use rustc_macros::HashStable; use rustc_session::CtfeBacktrace; use rustc_span::{def_id::DefId, Pos, Span}; -use std::{any::Any, fmt}; +use std::{ + any::{Any, TypeId}, + fmt, mem, +}; #[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable, RustcEncodable, RustcDecodable)] pub enum ErrorHandled { @@ -451,9 +454,6 @@ impl fmt::Debug for UndefinedBehaviorInfo { pub enum UnsupportedOpInfo { /// Free-form case. Only for errors that are never caught! Unsupported(String), - /// When const-prop encounters a situation it does not support, it raises this error. - /// This must not allocate for performance reasons (hence `str`, not `String`). - ConstPropUnsupported(&'static str), /// Accessing an unsupported foreign static. ReadForeignStatic(DefId), /// Could not find MIR for a function. @@ -472,9 +472,6 @@ impl fmt::Debug for UnsupportedOpInfo { use UnsupportedOpInfo::*; match self { Unsupported(ref msg) => write!(f, "{}", msg), - ConstPropUnsupported(ref msg) => { - write!(f, "Constant propagation encountered an unsupported situation: {}", msg) - } ReadForeignStatic(did) => { write!(f, "tried to read from foreign (extern) static {:?}", did) } @@ -516,6 +513,35 @@ impl fmt::Debug for ResourceExhaustionInfo { } } +/// A trait for machine-specific errors (or other "machine stop" conditions). +pub trait MachineStopType: Any + fmt::Debug + Send {} +impl MachineStopType for String {} + +// Copy-pasted from `any.rs`; there does not seem to be a way to re-use that. +impl dyn MachineStopType { + pub fn is(&self) -> bool { + // Get `TypeId` of the type this function is instantiated with. + let t = TypeId::of::(); + + // Get `TypeId` of the type in the trait object (`self`). + let concrete = self.type_id(); + + // Compare both `TypeId`s on equality. + t == concrete + } + + pub fn downcast_ref(&self) -> Option<&T> { + if self.is::() { + // SAFETY: just checked whether we are pointing to the correct type, and we can rely on + // that check for memory safety because `Any` is implemented for all types; no other + // impls can exist as they would conflict with our impl. + unsafe { Some(&*(self as *const dyn MachineStopType as *const T)) } + } else { + None + } + } +} + pub enum InterpError<'tcx> { /// The program caused undefined behavior. UndefinedBehavior(UndefinedBehaviorInfo), @@ -529,7 +555,7 @@ pub enum InterpError<'tcx> { ResourceExhaustion(ResourceExhaustionInfo), /// Stop execution for a machine-controlled reason. This is never raised by /// the core engine itself. - MachineStop(Box), + MachineStop(Box), } pub type InterpResult<'tcx, T = ()> = Result>; @@ -549,7 +575,7 @@ impl fmt::Debug for InterpError<'_> { InvalidProgram(ref msg) => write!(f, "{:?}", msg), UndefinedBehavior(ref msg) => write!(f, "{:?}", msg), ResourceExhaustion(ref msg) => write!(f, "{:?}", msg), - MachineStop(_) => bug!("unhandled MachineStop"), + MachineStop(ref msg) => write!(f, "{:?}", msg), } } } @@ -560,8 +586,9 @@ impl InterpError<'_> { /// waste of resources. pub fn allocates(&self) -> bool { match self { - InterpError::MachineStop(_) - | InterpError::Unsupported(UnsupportedOpInfo::Unsupported(_)) + // Zero-sized boxes to not allocate. + InterpError::MachineStop(b) => mem::size_of_val(&**b) > 0, + InterpError::Unsupported(UnsupportedOpInfo::Unsupported(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ValidationFailure(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::Ub(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::UbExperimental(_)) => true, diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index dfe5adb1bbff0..35876c8e95ab5 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -92,8 +92,8 @@ mod value; pub use self::error::{ struct_error, ConstEvalErr, ConstEvalRawResult, ConstEvalResult, ErrorHandled, FrameInfo, - InterpError, InterpErrorInfo, InterpResult, InvalidProgramInfo, ResourceExhaustionInfo, - UndefinedBehaviorInfo, UnsupportedOpInfo, + InterpError, InterpErrorInfo, InterpResult, InvalidProgramInfo, MachineStopType, + ResourceExhaustionInfo, UndefinedBehaviorInfo, UnsupportedOpInfo, }; pub use self::value::{get_slice_bytes, ConstValue, RawConst, Scalar, ScalarMaybeUndef}; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8d7cafc34b356..687bacfdc1b83 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -4,7 +4,7 @@ use std::borrow::Cow; use std::cell::Cell; -use rustc::mir::interpret::{InterpResult, Scalar}; +use rustc::mir::interpret::{InterpResult, MachineStopType, Scalar}; use rustc::mir::visit::{ MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor, }; @@ -192,7 +192,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, _unwind: Option, ) -> InterpResult<'tcx> { - throw_unsup!(ConstPropUnsupported("calling intrinsics isn't supported in ConstProp")) + #[derive(Debug)] + struct ConstPropIntrinsic; + impl MachineStopType for ConstPropIntrinsic {} + throw_machine_stop!(ConstPropIntrinsic) } fn assert_panic( @@ -204,7 +207,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { } fn ptr_to_int(_mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer) -> InterpResult<'tcx, u64> { - throw_unsup!(ConstPropUnsupported("ptr-to-int casts aren't supported in ConstProp")) + throw_unsup!(ReadPointerAsBytes) } fn binary_ptr_op( @@ -213,11 +216,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _left: ImmTy<'tcx>, _right: ImmTy<'tcx>, ) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> { + #[derive(Debug)] + struct ConstPropPtrOp; + impl MachineStopType for ConstPropPtrOp {} // We can't do this because aliasing of memory can differ between const eval and llvm - throw_unsup!(ConstPropUnsupported( - "pointer arithmetic or comparisons aren't supported \ - in ConstProp" - )) + throw_machine_stop!(ConstPropPtrOp) } #[inline(always)] @@ -240,7 +243,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ecx: &mut InterpCx<'mir, 'tcx, Self>, _dest: PlaceTy<'tcx>, ) -> InterpResult<'tcx> { - throw_unsup!(ConstPropUnsupported("can't const prop `box` keyword")) + #[derive(Debug)] + struct ConstPropBox; + impl MachineStopType for ConstPropBox {} + throw_machine_stop!(ConstPropBox) } fn access_local( @@ -251,7 +257,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { let l = &frame.locals[local]; if l.value == LocalValue::Uninitialized { - throw_unsup!(ConstPropUnsupported("tried to access an uninitialized local")); + #[derive(Debug)] + struct ConstPropUninitLocal; + impl MachineStopType for ConstPropUninitLocal {} + throw_machine_stop!(ConstPropUninitLocal) } l.access() @@ -261,10 +270,13 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _memory_extra: &(), allocation: &Allocation, ) -> InterpResult<'tcx> { + #[derive(Debug)] + struct ConstPropGlobalMem; + impl MachineStopType for ConstPropGlobalMem {} // if the static allocation is mutable or if it has relocations (it may be legal to mutate // the memory behind that in the future), then we can't const prop it if allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0 { - throw_unsup!(ConstPropUnsupported("can't eval mutable statics in ConstProp")); + throw_machine_stop!(ConstPropGlobalMem) } Ok(()) From cda81da6ea4c037ef036067d9cb98e80208ee525 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 22 Mar 2020 20:24:22 +0100 Subject: [PATCH 2/4] avoid unsafe code, use upcasting-trait instead (trick by oli) --- src/librustc/mir/interpret/error.rs | 41 +++++++++++------------------ 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index fd7f5361214f4..d00eb7921a04b 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -14,10 +14,7 @@ use rustc_hir as hir; use rustc_macros::HashStable; use rustc_session::CtfeBacktrace; use rustc_span::{def_id::DefId, Pos, Span}; -use std::{ - any::{Any, TypeId}, - fmt, mem, -}; +use std::{any::Any, fmt, mem}; #[derive(Debug, Copy, Clone, PartialEq, Eq, HashStable, RustcEncodable, RustcDecodable)] pub enum ErrorHandled { @@ -513,32 +510,26 @@ impl fmt::Debug for ResourceExhaustionInfo { } } +/// A trait to work around not having trait object upcasting. +pub trait AsAny: Any { + fn as_any(&self) -> &dyn Any; +} + +impl AsAny for T { + #[inline(always)] + fn as_any(&self) -> &dyn Any { + self + } +} + /// A trait for machine-specific errors (or other "machine stop" conditions). -pub trait MachineStopType: Any + fmt::Debug + Send {} +pub trait MachineStopType: AsAny + fmt::Debug + Send {} impl MachineStopType for String {} -// Copy-pasted from `any.rs`; there does not seem to be a way to re-use that. impl dyn MachineStopType { - pub fn is(&self) -> bool { - // Get `TypeId` of the type this function is instantiated with. - let t = TypeId::of::(); - - // Get `TypeId` of the type in the trait object (`self`). - let concrete = self.type_id(); - - // Compare both `TypeId`s on equality. - t == concrete - } - + #[inline(always)] pub fn downcast_ref(&self) -> Option<&T> { - if self.is::() { - // SAFETY: just checked whether we are pointing to the correct type, and we can rely on - // that check for memory safety because `Any` is implemented for all types; no other - // impls can exist as they would conflict with our impl. - unsafe { Some(&*(self as *const dyn MachineStopType as *const T)) } - } else { - None - } + self.as_any().downcast_ref() } } From 410385dfd0532f3e8867afaaa4b89c315b0e84b1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 10:58:43 +0100 Subject: [PATCH 3/4] add macro to reduce boilerplate and keep readable messages --- src/librustc/mir/interpret/error.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 45 +++++++++++++----------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index d00eb7921a04b..010b73db9ac20 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -577,7 +577,7 @@ impl InterpError<'_> { /// waste of resources. pub fn allocates(&self) -> bool { match self { - // Zero-sized boxes to not allocate. + // Zero-sized boxes do not allocate. InterpError::MachineStop(b) => mem::size_of_val(&**b) > 0, InterpError::Unsupported(UnsupportedOpInfo::Unsupported(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ValidationFailure(_)) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 687bacfdc1b83..74140a1fc6daf 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -4,7 +4,7 @@ use std::borrow::Cow; use std::cell::Cell; -use rustc::mir::interpret::{InterpResult, MachineStopType, Scalar}; +use rustc::mir::interpret::{InterpResult, Scalar}; use rustc::mir::visit::{ MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor, }; @@ -39,6 +39,24 @@ use crate::transform::{MirPass, MirSource}; /// The maximum number of bytes that we'll allocate space for a return value. const MAX_ALLOC_LIMIT: u64 = 1024; +/// Macro for machine-specific `InterpError` without allocation. +/// (These will never be shown to the user, but they help diagnose ICEs.) +macro_rules! throw_machine_stop_str { + ($($tt:tt)*) => {{ + // We make a new local type for it. The type itself does not carry any information, + // but its vtable (for the `MachineStopType` trait) does. + struct Zst; + // Debug-printing this type shows the desired string. + impl std::fmt::Debug for Zst { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, $($tt)*) + } + } + impl rustc::mir::interpret::MachineStopType for Zst {} + throw_machine_stop!(Zst) + }}; +} + pub struct ConstProp; impl<'tcx> MirPass<'tcx> for ConstProp { @@ -192,10 +210,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, _unwind: Option, ) -> InterpResult<'tcx> { - #[derive(Debug)] - struct ConstPropIntrinsic; - impl MachineStopType for ConstPropIntrinsic {} - throw_machine_stop!(ConstPropIntrinsic) + throw_machine_stop_str!("calling intrinsics isn't supported in ConstProp") } fn assert_panic( @@ -216,11 +231,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _left: ImmTy<'tcx>, _right: ImmTy<'tcx>, ) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> { - #[derive(Debug)] - struct ConstPropPtrOp; - impl MachineStopType for ConstPropPtrOp {} // We can't do this because aliasing of memory can differ between const eval and llvm - throw_machine_stop!(ConstPropPtrOp) + throw_machine_stop_str!("pointer arithmetic or comparisons aren't supported in ConstProp") } #[inline(always)] @@ -243,10 +255,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _ecx: &mut InterpCx<'mir, 'tcx, Self>, _dest: PlaceTy<'tcx>, ) -> InterpResult<'tcx> { - #[derive(Debug)] - struct ConstPropBox; - impl MachineStopType for ConstPropBox {} - throw_machine_stop!(ConstPropBox) + throw_machine_stop_str!("can't const prop heap allocations") } fn access_local( @@ -257,10 +266,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { let l = &frame.locals[local]; if l.value == LocalValue::Uninitialized { - #[derive(Debug)] - struct ConstPropUninitLocal; - impl MachineStopType for ConstPropUninitLocal {} - throw_machine_stop!(ConstPropUninitLocal) + throw_machine_stop_str!("tried to access an uninitialized local") } l.access() @@ -270,13 +276,10 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _memory_extra: &(), allocation: &Allocation, ) -> InterpResult<'tcx> { - #[derive(Debug)] - struct ConstPropGlobalMem; - impl MachineStopType for ConstPropGlobalMem {} // if the static allocation is mutable or if it has relocations (it may be legal to mutate // the memory behind that in the future), then we can't const prop it if allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0 { - throw_machine_stop!(ConstPropGlobalMem) + throw_machine_stop_str!("can't eval mutable statics in ConstProp") } Ok(()) From e619b85776feca7ae484c42dff1e37e0844aa06c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 11:03:39 +0100 Subject: [PATCH 4/4] make sure we are checking the size of the right thing --- src/librustc/mir/interpret/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 010b73db9ac20..bd7d2c57509b3 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -578,7 +578,7 @@ impl InterpError<'_> { pub fn allocates(&self) -> bool { match self { // Zero-sized boxes do not allocate. - InterpError::MachineStop(b) => mem::size_of_val(&**b) > 0, + InterpError::MachineStop(b) => mem::size_of_val::(&**b) > 0, InterpError::Unsupported(UnsupportedOpInfo::Unsupported(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ValidationFailure(_)) | InterpError::UndefinedBehavior(UndefinedBehaviorInfo::Ub(_))