From bc17936c8a466e56d472b811e0aca0cf8ffdf9a1 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 3 Sep 2019 21:56:15 -0400 Subject: [PATCH 01/15] [const-prop] Replace `eval_place()` with use of `InterpCx` --- src/librustc/mir/interpret/error.rs | 12 ++++ src/librustc_mir/interpret/eval_context.rs | 3 +- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/place.rs | 9 +++ src/librustc_mir/transform/const_prop.rs | 56 ++----------------- src/test/mir-opt/const_prop/slice_len.rs | 2 +- src/test/ui/consts/const-eval/issue-50814.rs | 1 + .../ui/consts/const-eval/issue-50814.stderr | 12 +++- src/test/ui/issues/issue-52060.stderr | 3 +- 9 files changed, 43 insertions(+), 57 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index ac99ccd45eafe..0328f93859bb7 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -389,6 +389,14 @@ pub enum UnsupportedOpInfo<'tcx> { /// Free-form case. Only for errors that are never caught! Unsupported(String), + /// Error used by the `ConstProp` pass when an attempt is made + /// to read an uninitialized local. + UninitializedLocal, + + /// Error used by the `ConstProp` pass to prevent reading statics + /// while evaluating `const` items. + ReadOfStaticInConst, + // -- Everything below is not categorized yet -- FunctionAbiMismatch(Abi, Abi), FunctionArgMismatch(Ty<'tcx>, Ty<'tcx>), @@ -511,6 +519,8 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { addresses, e.g., comparing pointers into different allocations"), DeadLocal => write!(f, "tried to access a dead local variable"), + UninitializedLocal => + write!(f, "tried to access an uninitialized local variable"), DerefFunctionPointer => write!(f, "tried to dereference a function pointer"), ExecuteMemory => @@ -552,6 +562,8 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { not a power of two"), Unsupported(ref msg) => write!(f, "{}", msg), + ReadOfStaticInConst => + write!(f, "tried to read from a static during const evaluation"), } } } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index fdf85260c3d96..7ca14b51740a9 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -134,8 +134,7 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> { pub fn access(&self) -> InterpResult<'tcx, Operand> { match self.value { LocalValue::Dead => throw_unsup!(DeadLocal), - LocalValue::Uninitialized => - bug!("The type checker should prevent reading from a never-written local"), + LocalValue::Uninitialized => throw_unsup!(UninitializedLocal), LocalValue::Live(val) => Ok(val), } } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index f5d1ec3eb7556..89af0031bfd26 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -481,7 +481,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. - pub(super) fn eval_place_to_op( + pub fn eval_place_to_op( &self, place: &mir::Place<'tcx>, layout: Option>, diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 2f1b35757fe9c..f9ba1452d64fa 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -597,6 +597,15 @@ where } StaticKind::Static => { + //if the first frame on the stack isn't a static item, then we shouldn't + //eval any static places (unless -Z unleash-the-miri-inside-of-you is on) + if let ty::InstanceDef::Item(item_def_id) = self.stack[0].instance.def { + if !self.tcx.is_static(item_def_id) && + !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { + trace!("eval_static_to_mplace: can't eval static in constant"); + throw_unsup!(ReadOfStaticInConst); + } + } let ty = place_static.ty; assert!(!ty.needs_subst()); let layout = self.layout_of(ty)?; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 857757ada53b7..ff1a7c2c48fd9 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -6,14 +6,14 @@ use std::cell::Cell; use rustc::hir::def::DefKind; use rustc::mir::{ AggregateKind, Constant, Location, Place, PlaceBase, Body, Operand, Rvalue, - Local, NullOp, UnOp, StatementKind, Statement, LocalKind, Static, StaticKind, - TerminatorKind, Terminator, ClearCrossCrate, SourceInfo, BinOp, ProjectionElem, + Local, NullOp, UnOp, StatementKind, Statement, LocalKind, + TerminatorKind, Terminator, ClearCrossCrate, SourceInfo, BinOp, SourceScope, SourceScopeLocalData, LocalDecl, }; use rustc::mir::visit::{ Visitor, PlaceContext, MutatingUseContext, MutVisitor, NonMutatingUseContext, }; -use rustc::mir::interpret::{Scalar, GlobalId, InterpResult, PanicInfo}; +use rustc::mir::interpret::{Scalar, InterpResult, PanicInfo}; use rustc::ty::{self, Instance, ParamEnv, Ty, TyCtxt}; use syntax_pos::{Span, DUMMY_SP}; use rustc::ty::subst::InternalSubsts; @@ -282,53 +282,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { fn eval_place(&mut self, place: &Place<'tcx>, source_info: SourceInfo) -> Option> { trace!("eval_place(place={:?})", place); - let mut eval = match place.base { - PlaceBase::Local(loc) => self.get_const(loc).clone()?, - PlaceBase::Static(box Static {kind: StaticKind::Promoted(promoted, _), ..}) => { - let generics = self.tcx.generics_of(self.source.def_id()); - if generics.requires_monomorphization(self.tcx) { - // FIXME: can't handle code with generics - return None; - } - let substs = InternalSubsts::identity_for_item(self.tcx, self.source.def_id()); - let instance = Instance::new(self.source.def_id(), substs); - let cid = GlobalId { - instance, - promoted: Some(promoted), - }; - let res = self.use_ecx(source_info, |this| { - this.ecx.const_eval_raw(cid) - })?; - trace!("evaluated promoted {:?} to {:?}", promoted, res); - res.into() - } - _ => return None, - }; - - for (i, elem) in place.projection.iter().enumerate() { - let proj_base = &place.projection[..i]; - - match elem { - ProjectionElem::Field(field, _) => { - trace!("field proj on {:?}", proj_base); - eval = self.use_ecx(source_info, |this| { - this.ecx.operand_field(eval, field.index() as u64) - })?; - }, - ProjectionElem::Deref => { - trace!("processing deref"); - eval = self.use_ecx(source_info, |this| { - this.ecx.deref_operand(eval) - })?.into(); - } - // We could get more projections by using e.g., `operand_projection`, - // but we do not even have the stack frame set up properly so - // an `Index` projection would throw us off-track. - _ => return None, - } - } - - Some(eval) + self.use_ecx(source_info, |this| { + this.ecx.eval_place_to_op(place, None) + }) } fn eval_operand(&mut self, op: &Operand<'tcx>, source_info: SourceInfo) -> Option> { diff --git a/src/test/mir-opt/const_prop/slice_len.rs b/src/test/mir-opt/const_prop/slice_len.rs index 5babeb195a826..05595ce147c96 100644 --- a/src/test/mir-opt/const_prop/slice_len.rs +++ b/src/test/mir-opt/const_prop/slice_len.rs @@ -34,7 +34,7 @@ fn main() { // assert(const true, "index out of bounds: the len is move _7 but the index is _6") -> bb1; // } // bb1: { -// _1 = (*_2)[_6]; +// _1 = const 2u32; // ... // return; // } diff --git a/src/test/ui/consts/const-eval/issue-50814.rs b/src/test/ui/consts/const-eval/issue-50814.rs index b85cecda16e95..274967ef60de5 100644 --- a/src/test/ui/consts/const-eval/issue-50814.rs +++ b/src/test/ui/consts/const-eval/issue-50814.rs @@ -11,6 +11,7 @@ struct Sum(A,B); impl Unsigned for Sum { const MAX: u8 = A::MAX + B::MAX; //~ ERROR any use of this value will cause an error + //~| ERROR any use of this value will cause an error } fn foo(_: T) -> &'static u8 { diff --git a/src/test/ui/consts/const-eval/issue-50814.stderr b/src/test/ui/consts/const-eval/issue-50814.stderr index 707dfee7cd5b8..de3459c72dd2b 100644 --- a/src/test/ui/consts/const-eval/issue-50814.stderr +++ b/src/test/ui/consts/const-eval/issue-50814.stderr @@ -9,13 +9,21 @@ LL | const MAX: u8 = A::MAX + B::MAX; = note: `#[deny(const_err)]` on by default error[E0080]: evaluation of constant expression failed - --> $DIR/issue-50814.rs:17:5 + --> $DIR/issue-50814.rs:18:5 | LL | &Sum::::MAX | ^----------------- | | | referenced constant has errors -error: aborting due to 2 previous errors +error: any use of this value will cause an error + --> $DIR/issue-50814.rs:13:21 + | +LL | const MAX: u8 = A::MAX + B::MAX; + | ----------------^^^^^^^^^^^^^^^- + | | + | attempt to add with overflow + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/issues/issue-52060.stderr b/src/test/ui/issues/issue-52060.stderr index 2f90f7f9e035b..6fbc1ed52c478 100644 --- a/src/test/ui/issues/issue-52060.stderr +++ b/src/test/ui/issues/issue-52060.stderr @@ -6,4 +6,5 @@ LL | static B: [u32; 1] = [0; A.len()]; error: aborting due to previous error -For more information about this error, try `rustc --explain E0013`. +Some errors have detailed explanations: E0013, E0080. +For more information about an error, try `rustc --explain E0013`. From 86c7c4d7beccc0862e4ee0d4813b0364fccdc722 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 5 Sep 2019 21:06:57 -0400 Subject: [PATCH 02/15] [const-prop] Replace `Use` handling with use of `InterpCx` --- src/librustc_mir/interpret/step.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 49 ++++++++++++------------ 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index affca10bf5265..daca7a25787ca 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -132,7 +132,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue /// type writes its results directly into the memory specified by the place. - fn eval_rvalue_into_place( + pub fn eval_rvalue_into_place( &mut self, rvalue: &mir::Rvalue<'tcx>, place: &mir::Place<'tcx>, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index ff1a7c2c48fd9..d92eb4706bb28 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -300,11 +300,16 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { rvalue: &Rvalue<'tcx>, place_layout: TyLayout<'tcx>, source_info: SourceInfo, + place: &Place<'tcx>, ) -> Option> { let span = source_info.span; match *rvalue { - Rvalue::Use(ref op) => { - self.eval_operand(op, source_info) + Rvalue::Use(_) | + Rvalue::Len(_) => { + self.use_ecx(source_info, |this| { + this.ecx.eval_rvalue_into_place(rvalue, place)?; + this.ecx.eval_place_to_op(place, Some(place_layout)) + }) }, Rvalue::Ref(_, _, ref place) => { let src = self.eval_place(place, source_info)?; @@ -324,22 +329,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Ok(dest.into()) }) }, - Rvalue::Len(ref place) => { - let place = self.eval_place(&place, source_info)?; - let mplace = place.try_as_mplace().ok()?; - - if let ty::Slice(_) = mplace.layout.ty.kind { - let len = mplace.meta.unwrap().to_usize(&self.ecx).unwrap(); - - Some(ImmTy::from_uint( - len, - self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?, - ).into()) - } else { - trace!("not slice: {:?}", mplace.layout.ty.kind); - None - } - }, Rvalue::NullaryOp(NullOp::SizeOf, ty) => { type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some( ImmTy::from_uint( @@ -626,15 +615,15 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { .ty(&self.local_decls, self.tcx) .ty; if let Ok(place_layout) = self.tcx.layout_of(self.param_env.and(place_ty)) { - if let Some(value) = self.const_prop(rval, place_layout, statement.source_info) { - if let Place { - base: PlaceBase::Local(local), - projection: box [], - } = *place { + if let Place { + base: PlaceBase::Local(local), + projection: box [], + } = *place { + if let Some(value) = self.const_prop(rval, place_layout, statement.source_info, place) { trace!("checking whether {:?} can be stored to {:?}", value, local); if self.can_const_prop[local] { trace!("storing {:?} to {:?}", value, local); - assert!(self.get_const(local).is_none()); + assert!(self.get_const(local).is_none() || self.get_const(local) == Some(value)); self.set_const(local, value); if self.should_const_prop() { @@ -648,6 +637,18 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { } } } + } else if let StatementKind::StorageLive(local) = statement.kind { + if self.can_const_prop[local] { + let frame = self.ecx.frame_mut(); + + frame.locals[local].value = LocalValue::Uninitialized; + } + } else if let StatementKind::StorageDead(local) = statement.kind { + if self.can_const_prop[local] { + let frame = self.ecx.frame_mut(); + + frame.locals[local].value = LocalValue::Dead; + } } self.super_statement(statement, location); } From ecc4cc2fc4079dba2141a79d4b9a636cbf5c9bf5 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 5 Sep 2019 21:42:30 -0400 Subject: [PATCH 03/15] [const-prop] Replace `Cast` handling with use of `InterpCx` --- src/librustc_mir/transform/const_prop.rs | 13 +++---------- src/test/mir-opt/const_prop/reify_fn_ptr.rs | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d92eb4706bb28..e1a2d16931998 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -24,7 +24,7 @@ use rustc::ty::layout::{ use crate::interpret::{ self, InterpCx, ScalarMaybeUndef, Immediate, OpTy, - ImmTy, MemoryKind, StackPopCleanup, LocalValue, LocalState, + ImmTy, StackPopCleanup, LocalValue, LocalState, }; use crate::const_eval::{ CompileTimeInterpreter, error_to_const_error, mk_eval_cx, @@ -305,7 +305,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let span = source_info.span; match *rvalue { Rvalue::Use(_) | - Rvalue::Len(_) => { + Rvalue::Len(_) | + Rvalue::Cast(..) => { self.use_ecx(source_info, |this| { this.ecx.eval_rvalue_into_place(rvalue, place)?; this.ecx.eval_place_to_op(place, Some(place_layout)) @@ -321,14 +322,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Rvalue::NullaryOp(NullOp::Box, _) | Rvalue::Discriminant(..) => None, - Rvalue::Cast(kind, ref operand, _) => { - let op = self.eval_operand(operand, source_info)?; - self.use_ecx(source_info, |this| { - let dest = this.ecx.allocate(place_layout, MemoryKind::Stack); - this.ecx.cast(op, kind, dest.into())?; - Ok(dest.into()) - }) - }, Rvalue::NullaryOp(NullOp::SizeOf, ty) => { type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some( ImmTy::from_uint( diff --git a/src/test/mir-opt/const_prop/reify_fn_ptr.rs b/src/test/mir-opt/const_prop/reify_fn_ptr.rs index 7e36b2a6b1b39..e9b61690cf89e 100644 --- a/src/test/mir-opt/const_prop/reify_fn_ptr.rs +++ b/src/test/mir-opt/const_prop/reify_fn_ptr.rs @@ -16,7 +16,7 @@ fn main() { // START rustc.main.ConstProp.after.mir // bb0: { // ... -// _3 = const Scalar(AllocId(1).0x0) : fn(); +// _3 = const Scalar(AllocId(0).0x0) : fn(); // _2 = move _3 as usize (Misc); // ... // _1 = move _2 as *const fn() (Misc); From 1c219bb34bead8b603da164dfcbb7c808ae70384 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 6 Sep 2019 05:49:18 -0400 Subject: [PATCH 04/15] [const-prop] Replace `NullaryOp` handling with use of `InterpCx` --- src/librustc_mir/transform/const_prop.rs | 28 ++++++------------------ 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index e1a2d16931998..c4633d57c2310 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -304,9 +304,15 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) -> Option> { let span = source_info.span; match *rvalue { + Rvalue::Repeat(..) | + Rvalue::Aggregate(..) | + Rvalue::NullaryOp(NullOp::Box, _) | + Rvalue::Discriminant(..) => None, + Rvalue::Use(_) | Rvalue::Len(_) | - Rvalue::Cast(..) => { + Rvalue::Cast(..) | + Rvalue::NullaryOp(..) => { self.use_ecx(source_info, |this| { this.ecx.eval_rvalue_into_place(rvalue, place)?; this.ecx.eval_place_to_op(place, Some(place_layout)) @@ -317,19 +323,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let mplace = src.try_as_mplace().ok()?; Some(ImmTy::from_scalar(mplace.ptr.into(), place_layout).into()) }, - Rvalue::Repeat(..) | - Rvalue::Aggregate(..) | - Rvalue::NullaryOp(NullOp::Box, _) | - Rvalue::Discriminant(..) => None, - Rvalue::NullaryOp(NullOp::SizeOf, ty) => { - type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some( - ImmTy::from_uint( - n, - self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?, - ).into() - )) - } Rvalue::UnaryOp(op, ref arg) => { let def_id = if self.tcx.is_closure(self.source.def_id()) { self.tcx.closure_base_def_id(self.source.def_id()) @@ -515,14 +509,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } } -fn type_size_of<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ty::ParamEnv<'tcx>, - ty: Ty<'tcx>, -) -> Option { - tcx.layout_of(param_env.and(ty)).ok().map(|layout| layout.size.bytes()) -} - struct CanConstProp { can_const_prop: IndexVec, // false at the beginning, once set, there are not allowed to be any more assignments From 644d4f3ee93b49ee773b667a970a552983d5e8fa Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 6 Sep 2019 07:48:52 -0400 Subject: [PATCH 05/15] [const-prop] Replace most `UnaryOp` handling with use of `InterpCx` --- src/librustc_mir/transform/const_prop.rs | 45 +++++++++--------------- 1 file changed, 17 insertions(+), 28 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index c4633d57c2310..f172071a8fb9c 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -325,39 +325,28 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { }, Rvalue::UnaryOp(op, ref arg) => { - let def_id = if self.tcx.is_closure(self.source.def_id()) { - self.tcx.closure_base_def_id(self.source.def_id()) - } else { - self.source.def_id() - }; - let generics = self.tcx.generics_of(def_id); - if generics.requires_monomorphization(self.tcx) { - // FIXME: can't handle code with generics - return None; - } + let overflow_check = self.tcx.sess.overflow_checks(); - let arg = self.eval_operand(arg, source_info)?; - let oflo_check = self.tcx.sess.overflow_checks(); - let val = self.use_ecx(source_info, |this| { - let prim = this.ecx.read_immediate(arg)?; - match op { - UnOp::Neg => { - // We check overflow in debug mode already - // so should only check in release mode. - if !oflo_check - && prim.layout.ty.is_signed() - && prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) { + self.use_ecx(source_info, |this| { + // We check overflow in debug mode already + // so should only check in release mode. + if op == UnOp::Neg && !overflow_check { + let ty = arg.ty(&this.local_decls, this.tcx); + + if ty.is_integral() { + let arg = this.ecx.eval_operand(arg, None)?; + let prim = this.ecx.read_immediate(arg)?; + // Need to do overflow check here: For actual CTFE, MIR + // generation emits code that does this before calling the op. + if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) { throw_panic!(OverflowNeg) } } - UnOp::Not => { - // Cannot overflow - } } - // Now run the actual operation. - this.ecx.unary_op(op, prim) - })?; - Some(val.into()) + + this.ecx.eval_rvalue_into_place(rvalue, place)?; + this.ecx.eval_place_to_op(place, Some(place_layout)) + }) } Rvalue::CheckedBinaryOp(op, ref left, ref right) | Rvalue::BinaryOp(op, ref left, ref right) => { From 11eb91f412b2693c970216099b9d910498b0b789 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sun, 8 Sep 2019 21:02:54 -0400 Subject: [PATCH 06/15] [const-prop] Replace `CheckedBinaryOp` handling with use of `InterpCx` --- src/librustc_mir/transform/const_prop.rs | 28 +++++++++--------------- src/test/run-fail/overflowing-lsh-1.rs | 1 + src/test/run-fail/overflowing-lsh-2.rs | 1 + src/test/run-fail/overflowing-lsh-3.rs | 1 + src/test/run-fail/overflowing-lsh-4.rs | 1 + src/test/run-fail/overflowing-rsh-1.rs | 1 + src/test/run-fail/overflowing-rsh-2.rs | 1 + src/test/run-fail/overflowing-rsh-3.rs | 1 + src/test/run-fail/overflowing-rsh-4.rs | 1 + 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index f172071a8fb9c..865afefa649b5 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -312,7 +312,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Rvalue::Use(_) | Rvalue::Len(_) | Rvalue::Cast(..) | - Rvalue::NullaryOp(..) => { + Rvalue::NullaryOp(..) | + Rvalue::CheckedBinaryOp(..) => { self.use_ecx(source_info, |this| { this.ecx.eval_rvalue_into_place(rvalue, place)?; this.ecx.eval_place_to_op(place, Some(place_layout)) @@ -348,7 +349,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { this.ecx.eval_place_to_op(place, Some(place_layout)) }) } - Rvalue::CheckedBinaryOp(op, ref left, ref right) | Rvalue::BinaryOp(op, ref left, ref right) => { trace!("rvalue binop {:?} for {:?} and {:?}", op, left, right); let right = self.eval_operand(right, source_info)?; @@ -403,23 +403,15 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let (val, overflow, _ty) = self.use_ecx(source_info, |this| { this.ecx.overflowing_binary_op(op, l, r) })?; - let val = if let Rvalue::CheckedBinaryOp(..) = *rvalue { - Immediate::ScalarPair( - val.into(), - Scalar::from_bool(overflow).into(), - ) - } else { - // We check overflow in debug mode already - // so should only check in release mode. - if !self.tcx.sess.overflow_checks() && overflow { - let err = err_panic!(Overflow(op)).into(); - let _: Option<()> = self.use_ecx(source_info, |_| Err(err)); - return None; - } - Immediate::Scalar(val.into()) - }; + // We check overflow in debug mode already + // so should only check in release mode. + if !self.tcx.sess.overflow_checks() && overflow { + let err = err_panic!(Overflow(op)).into(); + let _: Option<()> = self.use_ecx(source_info, |_| Err(err)); + return None; + } let res = ImmTy { - imm: val, + imm: Immediate::Scalar(val.into()), layout: place_layout, }; Some(res.into()) diff --git a/src/test/run-fail/overflowing-lsh-1.rs b/src/test/run-fail/overflowing-lsh-1.rs index c69da0f49adf1..37fbf01e487dc 100644 --- a/src/test/run-fail/overflowing-lsh-1.rs +++ b/src/test/run-fail/overflowing-lsh-1.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = 1_i32 << 32; diff --git a/src/test/run-fail/overflowing-lsh-2.rs b/src/test/run-fail/overflowing-lsh-2.rs index 21659bd52393a..7b0b37dfed043 100644 --- a/src/test/run-fail/overflowing-lsh-2.rs +++ b/src/test/run-fail/overflowing-lsh-2.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = 1 << -1; diff --git a/src/test/run-fail/overflowing-lsh-3.rs b/src/test/run-fail/overflowing-lsh-3.rs index 8f06cd1153354..1768a8e6d3138 100644 --- a/src/test/run-fail/overflowing-lsh-3.rs +++ b/src/test/run-fail/overflowing-lsh-3.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = 1_u64 << 64; diff --git a/src/test/run-fail/overflowing-lsh-4.rs b/src/test/run-fail/overflowing-lsh-4.rs index 084c147094dee..ec304b4144e0f 100644 --- a/src/test/run-fail/overflowing-lsh-4.rs +++ b/src/test/run-fail/overflowing-lsh-4.rs @@ -5,6 +5,7 @@ // sidestep the overflow checking. #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { // this signals overflow when checking is on diff --git a/src/test/run-fail/overflowing-rsh-1.rs b/src/test/run-fail/overflowing-rsh-1.rs index 24a77da896d9e..14514540c06e1 100644 --- a/src/test/run-fail/overflowing-rsh-1.rs +++ b/src/test/run-fail/overflowing-rsh-1.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = -1_i32 >> 32; diff --git a/src/test/run-fail/overflowing-rsh-2.rs b/src/test/run-fail/overflowing-rsh-2.rs index b5615cacc2055..83dcbd13d2ad8 100644 --- a/src/test/run-fail/overflowing-rsh-2.rs +++ b/src/test/run-fail/overflowing-rsh-2.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = -1_i32 >> -1; diff --git a/src/test/run-fail/overflowing-rsh-3.rs b/src/test/run-fail/overflowing-rsh-3.rs index 7598e026d8139..3521c0506591c 100644 --- a/src/test/run-fail/overflowing-rsh-3.rs +++ b/src/test/run-fail/overflowing-rsh-3.rs @@ -2,6 +2,7 @@ // compile-flags: -C debug-assertions #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { let _x = -1_i64 >> 64; diff --git a/src/test/run-fail/overflowing-rsh-4.rs b/src/test/run-fail/overflowing-rsh-4.rs index a8d3b69392a97..ed1191849e57c 100644 --- a/src/test/run-fail/overflowing-rsh-4.rs +++ b/src/test/run-fail/overflowing-rsh-4.rs @@ -5,6 +5,7 @@ // truncation does not sidestep the overflow checking. #![warn(exceeding_bitshifts)] +#![warn(const_err)] fn main() { // this signals overflow when checking is on From 9ec928ca06877611f60fbf652775ae8e4aaf8cfc Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 9 Sep 2019 22:28:02 -0400 Subject: [PATCH 07/15] [const-prop] Replace some `Binaryp` handling with use of `InterpCx` --- src/librustc_mir/transform/const_prop.rs | 62 +++++++++--------------- 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 865afefa649b5..06117dc6a7ee4 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -351,30 +351,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } Rvalue::BinaryOp(op, ref left, ref right) => { trace!("rvalue binop {:?} for {:?} and {:?}", op, left, right); - let right = self.eval_operand(right, source_info)?; - let def_id = if self.tcx.is_closure(self.source.def_id()) { - self.tcx.closure_base_def_id(self.source.def_id()) - } else { - self.source.def_id() - }; - let generics = self.tcx.generics_of(def_id); - if generics.requires_monomorphization(self.tcx) { - // FIXME: can't handle code with generics - return None; - } let r = self.use_ecx(source_info, |this| { - this.ecx.read_immediate(right) + this.ecx.read_immediate(this.ecx.eval_operand(right, None)?) })?; if op == BinOp::Shr || op == BinOp::Shl { - let left_ty = left.ty(&self.local_decls, self.tcx); - let left_bits = self - .tcx - .layout_of(self.param_env.and(left_ty)) - .unwrap() - .size - .bits(); - let right_size = right.layout.size; + let left_bits = place_layout.size.bits(); + let right_size = r.layout.size; let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size)); if r_bits.ok().map_or(false, |b| b >= left_bits as u128) { let source_scope_local_data = match self.source_scope_local_data { @@ -395,26 +378,29 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { return None; } } - let left = self.eval_operand(left, source_info)?; - let l = self.use_ecx(source_info, |this| { - this.ecx.read_immediate(left) - })?; trace!("const evaluating {:?} for {:?} and {:?}", op, left, right); - let (val, overflow, _ty) = self.use_ecx(source_info, |this| { - this.ecx.overflowing_binary_op(op, l, r) + let val = self.use_ecx(source_info, |this| { + let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?; + let (val, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?; + + // We check overflow in debug mode already + // so should only check in release mode. + if !this.tcx.sess.overflow_checks() && overflow { + let err = err_panic!(Overflow(op)).into(); + return Err(err); + } + + let val = ImmTy { + imm: Immediate::Scalar(val.into()), + layout: place_layout, + }; + + let dest = this.ecx.eval_place(place)?; + this.ecx.write_immediate(*val, dest)?; + + Ok(val) })?; - // We check overflow in debug mode already - // so should only check in release mode. - if !self.tcx.sess.overflow_checks() && overflow { - let err = err_panic!(Overflow(op)).into(); - let _: Option<()> = self.use_ecx(source_info, |_| Err(err)); - return None; - } - let res = ImmTy { - imm: Immediate::Scalar(val.into()), - layout: place_layout, - }; - Some(res.into()) + Some(val.into()) }, } } From c0c8ce80b345dee4f306519c2262c4af687ef818 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 11 Sep 2019 07:46:17 -0400 Subject: [PATCH 08/15] [const-prop] Replace `Ref` handling with use of `InterpCx` --- src/librustc_mir/interpret/step.rs | 21 +++++++++++++++++++-- src/librustc_mir/transform/const_prop.rs | 8 ++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index daca7a25787ca..0aa6f5174c111 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -2,11 +2,11 @@ //! //! The main entry point is the `step` method. -use rustc::mir; +use rustc::mir::{self, Place, PlaceBase}; use rustc::ty::layout::LayoutOf; use rustc::mir::interpret::{InterpResult, Scalar, PointerArithmetic}; -use super::{InterpCx, Machine}; +use super::{InterpCx, LocalValue, Machine}; /// Classify whether an operator is "left-homogeneous", i.e., the LHS has the /// same type as the result. @@ -240,6 +240,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Ref(_, _, ref place) => { + // FIXME(wesleywiser) we don't currently handle the case where we try to make a ref + // from a function argument that hasn't been assigned to in this function. So just + // report those as uninitialized for now. + if let Place { + base: PlaceBase::Local(local), + projection: None + } = place { + let alive = + if let LocalValue::Live(_) = self.frame().locals[*local].value { + true + } else { false }; + + if local.as_usize() <= self.frame().body.arg_count && !alive { + trace!("skipping Ref({:?})", place); + throw_unsup!(UninitializedLocal); + } + } let src = self.eval_place(place)?; let place = self.force_allocation(src)?; if place.layout.size.bytes() > 0 { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 06117dc6a7ee4..0653598bc8600 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -313,17 +313,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Rvalue::Len(_) | Rvalue::Cast(..) | Rvalue::NullaryOp(..) | - Rvalue::CheckedBinaryOp(..) => { + Rvalue::CheckedBinaryOp(..) | + Rvalue::Ref(..) => { self.use_ecx(source_info, |this| { this.ecx.eval_rvalue_into_place(rvalue, place)?; this.ecx.eval_place_to_op(place, Some(place_layout)) }) }, - Rvalue::Ref(_, _, ref place) => { - let src = self.eval_place(place, source_info)?; - let mplace = src.try_as_mplace().ok()?; - Some(ImmTy::from_scalar(mplace.ptr.into(), place_layout).into()) - }, Rvalue::UnaryOp(op, ref arg) => { let overflow_check = self.tcx.sess.overflow_checks(); From fadfd92c2f27fbaf3473f952fb9a35b8fc20693a Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 12 Sep 2019 22:03:20 -0400 Subject: [PATCH 09/15] Don't run the ConstProp MIR pass on generators This can cause cycles as `ConstProp` uses `layout_of` which, for generators, uses `optimized_mir` which runs `ConstProp`. --- src/librustc_mir/transform/const_prop.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 0653598bc8600..7049fee5f8da5 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -57,6 +57,14 @@ impl<'tcx> MirPass<'tcx> for ConstProp { return } + let is_generator = tcx.type_of(source.def_id()).is_generator(); + // FIXME(welseywiser) const prop doesn't work on generators because of query cycles + // computing their layout. + if is_generator { + trace!("ConstProp skipped for generator {:?}", source.def_id()); + return + } + trace!("ConstProp starting for {:?}", source.def_id()); // Steal some data we need from `body`. From 15d2b7ae8122c19c4c4133a6206f90f676a60dff Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 13 Sep 2019 08:40:43 -0400 Subject: [PATCH 10/15] Respond to code review feedback and fix tidy --- src/librustc_mir/interpret/eval_context.rs | 4 ++- src/librustc_mir/interpret/step.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 33 +++++++++++-------- .../consts/const-prop-read-static-in-const.rs | 12 +++++++ .../const-prop-read-static-in-const.stderr | 6 ++++ 5 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/consts/const-prop-read-static-in-const.rs create mode 100644 src/test/ui/consts/const-prop-read-static-in-const.stderr diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 7ca14b51740a9..bc7a5a1a7c374 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -134,7 +134,9 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> { pub fn access(&self) -> InterpResult<'tcx, Operand> { match self.value { LocalValue::Dead => throw_unsup!(DeadLocal), - LocalValue::Uninitialized => throw_unsup!(UninitializedLocal), + LocalValue::Uninitialized => + // this is reachable from ConstProp + throw_unsup!(UninitializedLocal), LocalValue::Live(val) => Ok(val), } } diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 0aa6f5174c111..bdc44471b64e1 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -245,7 +245,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // report those as uninitialized for now. if let Place { base: PlaceBase::Local(local), - projection: None + projection: box [] } = place { let alive = if let LocalValue::Live(_) = self.frame().locals[*local].value { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7049fee5f8da5..b00c15ca50143 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -569,11 +569,15 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { base: PlaceBase::Local(local), projection: box [], } = *place { - if let Some(value) = self.const_prop(rval, place_layout, statement.source_info, place) { + if let Some(value) = self.const_prop(rval, + place_layout, + statement.source_info, + place) { trace!("checking whether {:?} can be stored to {:?}", value, local); if self.can_const_prop[local] { trace!("storing {:?} to {:?}", value, local); - assert!(self.get_const(local).is_none() || self.get_const(local) == Some(value)); + assert!(self.get_const(local).is_none() || + self.get_const(local) == Some(value)); self.set_const(local, value); if self.should_const_prop() { @@ -587,19 +591,22 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { } } } - } else if let StatementKind::StorageLive(local) = statement.kind { - if self.can_const_prop[local] { - let frame = self.ecx.frame_mut(); - - frame.locals[local].value = LocalValue::Uninitialized; - } - } else if let StatementKind::StorageDead(local) = statement.kind { - if self.can_const_prop[local] { - let frame = self.ecx.frame_mut(); - - frame.locals[local].value = LocalValue::Dead; + } else { + match statement.kind { + StatementKind::StorageLive(local) | + StatementKind::StorageDead(local) if self.can_const_prop[local] => { + let frame = self.ecx.frame_mut(); + frame.locals[local].value = + if let StatementKind::StorageLive(_) = statement.kind { + LocalValue::Uninitialized + } else { + LocalValue::Dead + }; + } + _ => {} } } + self.super_statement(statement, location); } diff --git a/src/test/ui/consts/const-prop-read-static-in-const.rs b/src/test/ui/consts/const-prop-read-static-in-const.rs new file mode 100644 index 0000000000000..7504fd525955a --- /dev/null +++ b/src/test/ui/consts/const-prop-read-static-in-const.rs @@ -0,0 +1,12 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you +// run-pass + +#![allow(dead_code)] + +const TEST: u8 = MY_STATIC; +//~^ skipping const checks + +static MY_STATIC: u8 = 4; + +fn main() { +} diff --git a/src/test/ui/consts/const-prop-read-static-in-const.stderr b/src/test/ui/consts/const-prop-read-static-in-const.stderr new file mode 100644 index 0000000000000..bbd5b12ed7dfc --- /dev/null +++ b/src/test/ui/consts/const-prop-read-static-in-const.stderr @@ -0,0 +1,6 @@ +warning: skipping const checks + --> $DIR/const-prop-read-static-in-const.rs:6:18 + | +LL | const TEST: u8 = MY_STATIC; + | ^^^^^^^^^ + From 93335141021df7d53ba597f6315a8014fe21195f Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Sat, 14 Sep 2019 07:00:16 -0400 Subject: [PATCH 11/15] Move Ref-from-arg checking from `step.rs` to `const_prop.rs` --- src/librustc_mir/interpret/step.rs | 21 +-- src/librustc_mir/transform/const_prop.rs | 173 ++++++++++++----------- 2 files changed, 95 insertions(+), 99 deletions(-) diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index bdc44471b64e1..daca7a25787ca 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -2,11 +2,11 @@ //! //! The main entry point is the `step` method. -use rustc::mir::{self, Place, PlaceBase}; +use rustc::mir; use rustc::ty::layout::LayoutOf; use rustc::mir::interpret::{InterpResult, Scalar, PointerArithmetic}; -use super::{InterpCx, LocalValue, Machine}; +use super::{InterpCx, Machine}; /// Classify whether an operator is "left-homogeneous", i.e., the LHS has the /// same type as the result. @@ -240,23 +240,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Ref(_, _, ref place) => { - // FIXME(wesleywiser) we don't currently handle the case where we try to make a ref - // from a function argument that hasn't been assigned to in this function. So just - // report those as uninitialized for now. - if let Place { - base: PlaceBase::Local(local), - projection: box [] - } = place { - let alive = - if let LocalValue::Live(_) = self.frame().locals[*local].value { - true - } else { false }; - - if local.as_usize() <= self.frame().body.arg_count && !alive { - trace!("skipping Ref({:?})", place); - throw_unsup!(UninitializedLocal); - } - } let src = self.eval_place(place)?; let place = self.force_allocation(src)?; if place.layout.size.bytes() > 0 { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index b00c15ca50143..d7d58327865b9 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -24,7 +24,7 @@ use rustc::ty::layout::{ use crate::interpret::{ self, InterpCx, ScalarMaybeUndef, Immediate, OpTy, - ImmTy, StackPopCleanup, LocalValue, LocalState, + StackPopCleanup, LocalValue, LocalState, }; use crate::const_eval::{ CompileTimeInterpreter, error_to_const_error, mk_eval_cx, @@ -311,102 +311,115 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { place: &Place<'tcx>, ) -> Option> { let span = source_info.span; - match *rvalue { + + // if this isn't a supported operation, then return None + match rvalue { Rvalue::Repeat(..) | Rvalue::Aggregate(..) | Rvalue::NullaryOp(NullOp::Box, _) | - Rvalue::Discriminant(..) => None, + Rvalue::Discriminant(..) => return None, Rvalue::Use(_) | Rvalue::Len(_) | Rvalue::Cast(..) | Rvalue::NullaryOp(..) | Rvalue::CheckedBinaryOp(..) | - Rvalue::Ref(..) => { - self.use_ecx(source_info, |this| { - this.ecx.eval_rvalue_into_place(rvalue, place)?; - this.ecx.eval_place_to_op(place, Some(place_layout)) - }) - }, + Rvalue::Ref(..) | + Rvalue::UnaryOp(..) | + Rvalue::BinaryOp(..) => { } + } - Rvalue::UnaryOp(op, ref arg) => { - let overflow_check = self.tcx.sess.overflow_checks(); - - self.use_ecx(source_info, |this| { - // We check overflow in debug mode already - // so should only check in release mode. - if op == UnOp::Neg && !overflow_check { - let ty = arg.ty(&this.local_decls, this.tcx); - - if ty.is_integral() { - let arg = this.ecx.eval_operand(arg, None)?; - let prim = this.ecx.read_immediate(arg)?; - // Need to do overflow check here: For actual CTFE, MIR - // generation emits code that does this before calling the op. - if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) { - throw_panic!(OverflowNeg) - } + // perform any special checking for specific Rvalue types + if let Rvalue::UnaryOp(op, arg) = rvalue { + trace!("checking UnaryOp(op = {:?}, arg = {:?})", op, arg); + let overflow_check = self.tcx.sess.overflow_checks(); + + self.use_ecx(source_info, |this| { + // We check overflow in debug mode already + // so should only check in release mode. + if *op == UnOp::Neg && !overflow_check { + let ty = arg.ty(&this.local_decls, this.tcx); + + if ty.is_integral() { + let arg = this.ecx.eval_operand(arg, None)?; + let prim = this.ecx.read_immediate(arg)?; + // Need to do overflow check here: For actual CTFE, MIR + // generation emits code that does this before calling the op. + if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) { + throw_panic!(OverflowNeg) } } - - this.ecx.eval_rvalue_into_place(rvalue, place)?; - this.ecx.eval_place_to_op(place, Some(place_layout)) - }) - } - Rvalue::BinaryOp(op, ref left, ref right) => { - trace!("rvalue binop {:?} for {:?} and {:?}", op, left, right); - - let r = self.use_ecx(source_info, |this| { - this.ecx.read_immediate(this.ecx.eval_operand(right, None)?) - })?; - if op == BinOp::Shr || op == BinOp::Shl { - let left_bits = place_layout.size.bits(); - let right_size = r.layout.size; - let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size)); - if r_bits.ok().map_or(false, |b| b >= left_bits as u128) { - let source_scope_local_data = match self.source_scope_local_data { - ClearCrossCrate::Set(ref data) => data, - ClearCrossCrate::Clear => return None, - }; - let dir = if op == BinOp::Shr { - "right" - } else { - "left" - }; - let hir_id = source_scope_local_data[source_info.scope].lint_root; - self.tcx.lint_hir( - ::rustc::lint::builtin::EXCEEDING_BITSHIFTS, - hir_id, - span, - &format!("attempt to shift {} with overflow", dir)); - return None; - } } - trace!("const evaluating {:?} for {:?} and {:?}", op, left, right); - let val = self.use_ecx(source_info, |this| { - let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?; - let (val, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?; - - // We check overflow in debug mode already - // so should only check in release mode. - if !this.tcx.sess.overflow_checks() && overflow { - let err = err_panic!(Overflow(op)).into(); - return Err(err); - } - let val = ImmTy { - imm: Immediate::Scalar(val.into()), - layout: place_layout, + Ok(()) + })?; + } else if let Rvalue::BinaryOp(op, left, right) = rvalue { + trace!("checking BinaryOp(op = {:?}, left = {:?}, right = {:?})", op, left, right); + + let r = self.use_ecx(source_info, |this| { + this.ecx.read_immediate(this.ecx.eval_operand(right, None)?) + })?; + if *op == BinOp::Shr || *op == BinOp::Shl { + let left_bits = place_layout.size.bits(); + let right_size = r.layout.size; + let r_bits = r.to_scalar().and_then(|r| r.to_bits(right_size)); + if r_bits.ok().map_or(false, |b| b >= left_bits as u128) { + let source_scope_local_data = match self.source_scope_local_data { + ClearCrossCrate::Set(ref data) => data, + ClearCrossCrate::Clear => return None, }; + let dir = if *op == BinOp::Shr { + "right" + } else { + "left" + }; + let hir_id = source_scope_local_data[source_info.scope].lint_root; + self.tcx.lint_hir( + ::rustc::lint::builtin::EXCEEDING_BITSHIFTS, + hir_id, + span, + &format!("attempt to shift {} with overflow", dir)); + return None; + } + } + self.use_ecx(source_info, |this| { + let l = this.ecx.read_immediate(this.ecx.eval_operand(left, None)?)?; + let (_, overflow, _ty) = this.ecx.overflowing_binary_op(*op, l, r)?; + + // We check overflow in debug mode already + // so should only check in release mode. + if !this.tcx.sess.overflow_checks() && overflow { + let err = err_panic!(Overflow(*op)).into(); + return Err(err); + } - let dest = this.ecx.eval_place(place)?; - this.ecx.write_immediate(*val, dest)?; - - Ok(val) - })?; - Some(val.into()) - }, + Ok(()) + })?; + } else if let Rvalue::Ref(_, _, place) = rvalue { + trace!("checking Ref({:?})", place); + // FIXME(wesleywiser) we don't currently handle the case where we try to make a ref + // from a function argument that hasn't been assigned to in this function. + if let Place { + base: PlaceBase::Local(local), + projection: box [] + } = place { + let alive = + if let LocalValue::Live(_) = self.ecx.frame().locals[*local].value { + true + } else { false }; + + if local.as_usize() <= self.ecx.frame().body.arg_count && !alive { + trace!("skipping Ref({:?})", place); + return None; + } + } } + + self.use_ecx(source_info, |this| { + trace!("calling eval_rvalue_into_place(rvalue = {:?}, place = {:?})", rvalue, place); + this.ecx.eval_rvalue_into_place(rvalue, place)?; + this.ecx.eval_place_to_op(place, Some(place_layout)) + }) } fn operand_from_scalar(&self, scalar: Scalar, ty: Ty<'tcx>, span: Span) -> Operand<'tcx> { From 4e58e2e3a3f3cda15bad6cbb87242eaf555dda85 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 17 Sep 2019 05:48:38 -0400 Subject: [PATCH 12/15] Work around for #64506 --- src/librustc/mir/interpret/error.rs | 6 ++++++ src/librustc_mir/interpret/place.rs | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 0328f93859bb7..4da5d979cc3e8 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -397,6 +397,10 @@ pub enum UnsupportedOpInfo<'tcx> { /// while evaluating `const` items. ReadOfStaticInConst, + /// FIXME(#64506) Error used to work around accessing projections of + /// uninhabited types. + UninhabitedValue, + // -- Everything below is not categorized yet -- FunctionAbiMismatch(Abi, Abi), FunctionArgMismatch(Ty<'tcx>, Ty<'tcx>), @@ -564,6 +568,8 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { write!(f, "{}", msg), ReadOfStaticInConst => write!(f, "tried to read from a static during const evaluation"), + UninhabitedValue => + write!(f, "tried to use an uninhabited value"), } } } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index f9ba1452d64fa..1ece913eb59a4 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -9,7 +9,7 @@ use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; use rustc::ty::layout::{ - self, Size, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt + self, Size, Abi, Align, LayoutOf, TyLayout, HasDataLayout, VariantIdx, PrimitiveExt }; use rustc::ty::TypeFoldable; @@ -385,6 +385,10 @@ where stride * field } layout::FieldPlacement::Union(count) => { + // FIXME(#64506) `UninhabitedValue` can be removed when this issue is resolved + if base.layout.abi == Abi::Uninhabited { + throw_unsup!(UninhabitedValue); + } assert!(field < count as u64, "Tried to access field {} of union with {} fields", field, count); // Offset is always 0 From dcc6c28c53e6f59a6e966b47ec872a7d26e2b23b Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 24 Sep 2019 21:12:59 -0400 Subject: [PATCH 13/15] Introduce a `ConstPropMachine` This allows us to avoid changing things directly in the miri engine just for const prop. --- src/librustc/mir/interpret/error.rs | 12 -- src/librustc_mir/const_eval.rs | 10 +- src/librustc_mir/interpret/eval_context.rs | 3 +- src/librustc_mir/interpret/machine.rs | 18 +++ src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/place.rs | 11 +- src/librustc_mir/transform/const_prop.rs | 157 +++++++++++++++++++-- src/test/ui/issues/issue-52060.stderr | 3 +- 8 files changed, 177 insertions(+), 39 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 4da5d979cc3e8..71967b513a049 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -389,14 +389,6 @@ pub enum UnsupportedOpInfo<'tcx> { /// Free-form case. Only for errors that are never caught! Unsupported(String), - /// Error used by the `ConstProp` pass when an attempt is made - /// to read an uninitialized local. - UninitializedLocal, - - /// Error used by the `ConstProp` pass to prevent reading statics - /// while evaluating `const` items. - ReadOfStaticInConst, - /// FIXME(#64506) Error used to work around accessing projections of /// uninhabited types. UninhabitedValue, @@ -523,8 +515,6 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { addresses, e.g., comparing pointers into different allocations"), DeadLocal => write!(f, "tried to access a dead local variable"), - UninitializedLocal => - write!(f, "tried to access an uninitialized local variable"), DerefFunctionPointer => write!(f, "tried to dereference a function pointer"), ExecuteMemory => @@ -566,8 +556,6 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { not a power of two"), Unsupported(ref msg) => write!(f, "{}", msg), - ReadOfStaticInConst => - write!(f, "tried to read from a static during const evaluation"), UninhabitedValue => write!(f, "tried to use an uninhabited value"), } diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index ca238867421ab..bb02b99dd8d87 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -21,7 +21,7 @@ use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, PlaceTy, MPlaceTy, OpTy, ImmTy, Immediate, Scalar, Pointer, - RawConst, ConstValue, + RawConst, ConstValue, Machine, InterpResult, InterpErrorInfo, GlobalId, InterpCx, StackPopCleanup, Allocation, AllocId, MemoryKind, Memory, snapshot, RefTracking, intern_const_alloc_recursive, @@ -41,7 +41,7 @@ const DETECTOR_SNAPSHOT_PERIOD: isize = 256; /// that inform us about the generic bounds of the constant. E.g., using an associated constant /// of a function's generic parameter will require knowledge about the bounds on the generic /// parameter. These bounds are passed to `mk_eval_cx` via the `ParamEnv` argument. -pub(crate) fn mk_eval_cx<'mir, 'tcx>( +fn mk_eval_cx<'mir, 'tcx>( tcx: TyCtxt<'tcx>, span: Span, param_env: ty::ParamEnv<'tcx>, @@ -169,7 +169,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( } #[derive(Clone, Debug)] -enum ConstEvalError { +pub enum ConstEvalError { NeedsRfc(String), } @@ -521,8 +521,8 @@ pub fn const_variant_index<'tcx>( /// Turn an interpreter error into something to report to the user. /// As a side-effect, if RUSTC_CTFE_BACKTRACE is set, this prints the backtrace. /// Should be called only if the error is actually going to to be reported! -pub fn error_to_const_error<'mir, 'tcx>( - ecx: &InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, +pub fn error_to_const_error<'mir, 'tcx, M: Machine<'mir, 'tcx>>( + ecx: &InterpCx<'mir, 'tcx, M>, mut error: InterpErrorInfo<'tcx>, ) -> ConstEvalErr<'tcx> { error.print_backtrace(); diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index bc7a5a1a7c374..fdf85260c3d96 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -135,8 +135,7 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> { match self.value { LocalValue::Dead => throw_unsup!(DeadLocal), LocalValue::Uninitialized => - // this is reachable from ConstProp - throw_unsup!(UninitializedLocal), + bug!("The type checker should prevent reading from a never-written local"), LocalValue::Live(val) => Ok(val), } } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index bb74a50156e56..e0be53b80d734 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -12,6 +12,7 @@ use rustc::ty::{self, Ty, TyCtxt}; use super::{ Allocation, AllocId, InterpResult, Scalar, AllocationExtra, InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, + Frame, Operand, }; /// Whether this kind of memory is allowed to leak @@ -184,6 +185,23 @@ pub trait Machine<'mir, 'tcx>: Sized { dest: PlaceTy<'tcx, Self::PointerTag>, ) -> InterpResult<'tcx>; + /// Called to read the specified `local` from the `frame`. + fn access_local( + _ecx: &InterpCx<'mir, 'tcx, Self>, + frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>, + local: mir::Local, + ) -> InterpResult<'tcx, Operand> { + frame.locals[local].access() + } + + /// Called before a `StaticKind::Static` value is read. + fn before_eval_static( + _ecx: &InterpCx<'mir, 'tcx, Self>, + _place_static: &mir::Static<'tcx>, + ) -> InterpResult<'tcx> { + Ok(()) + } + /// Called to initialize the "extra" state of an allocation and make the pointers /// it contains (in relocations) tagged. The way we construct allocations is /// to always first construct it without extra and then add the extra. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 89af0031bfd26..861e5ebef877d 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -458,7 +458,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Do not read from ZST, they might not be initialized Operand::Immediate(Scalar::zst().into()) } else { - frame.locals[local].access()? + M::access_local(&self, frame, local)? }; Ok(OpTy { op, layout }) } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 1ece913eb59a4..c882f4ef4ddf3 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -601,15 +601,8 @@ where } StaticKind::Static => { - //if the first frame on the stack isn't a static item, then we shouldn't - //eval any static places (unless -Z unleash-the-miri-inside-of-you is on) - if let ty::InstanceDef::Item(item_def_id) = self.stack[0].instance.def { - if !self.tcx.is_static(item_def_id) && - !self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - trace!("eval_static_to_mplace: can't eval static in constant"); - throw_unsup!(ReadOfStaticInConst); - } - } + M::before_eval_static(self, place_static)?; + let ty = place_static.ty; assert!(!ty.needs_subst()); let layout = self.layout_of(ty)?; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d7d58327865b9..ce1acc3b7edf9 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -1,14 +1,16 @@ //! Propagates constants for early reporting of statically known //! assertion failures +use std::borrow::Cow; use std::cell::Cell; use rustc::hir::def::DefKind; +use rustc::hir::def_id::DefId; use rustc::mir::{ AggregateKind, Constant, Location, Place, PlaceBase, Body, Operand, Rvalue, - Local, NullOp, UnOp, StatementKind, Statement, LocalKind, + Local, NullOp, UnOp, StatementKind, Statement, LocalKind, Static, TerminatorKind, Terminator, ClearCrossCrate, SourceInfo, BinOp, - SourceScope, SourceScopeLocalData, LocalDecl, + SourceScope, SourceScopeLocalData, LocalDecl, BasicBlock, }; use rustc::mir::visit::{ Visitor, PlaceContext, MutatingUseContext, MutVisitor, NonMutatingUseContext, @@ -17,6 +19,7 @@ use rustc::mir::interpret::{Scalar, InterpResult, PanicInfo}; use rustc::ty::{self, Instance, ParamEnv, Ty, TyCtxt}; use syntax_pos::{Span, DUMMY_SP}; use rustc::ty::subst::InternalSubsts; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::indexed_vec::IndexVec; use rustc::ty::layout::{ LayoutOf, TyLayout, LayoutError, HasTyCtxt, TargetDataLayout, HasDataLayout, @@ -24,11 +27,11 @@ use rustc::ty::layout::{ use crate::interpret::{ self, InterpCx, ScalarMaybeUndef, Immediate, OpTy, - StackPopCleanup, LocalValue, LocalState, -}; -use crate::const_eval::{ - CompileTimeInterpreter, error_to_const_error, mk_eval_cx, + StackPopCleanup, LocalValue, LocalState, AllocId, Frame, + Allocation, MemoryKind, ImmTy, Pointer, Memory, PlaceTy, + Operand as InterpOperand, }; +use crate::const_eval::error_to_const_error; use crate::transform::{MirPass, MirSource}; pub struct ConstProp; @@ -111,11 +114,149 @@ impl<'tcx> MirPass<'tcx> for ConstProp { } } +struct ConstPropMachine; + +impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { + type MemoryKinds= !; + type PointerTag = (); + type ExtraFnVal = !; + + type FrameExtra = (); + type MemoryExtra = (); + type AllocExtra = (); + + type MemoryMap = FxHashMap, Allocation)>; + + const STATIC_KIND: Option = None; + + const CHECK_ALIGN: bool = false; + + #[inline(always)] + fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool { + false + } + + fn find_fn( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + _instance: ty::Instance<'tcx>, + _args: &[OpTy<'tcx>], + _dest: Option>, + _ret: Option, + ) -> InterpResult<'tcx, Option<&'mir Body<'tcx>>> { + Ok(None) + } + + fn call_extra_fn( + _ecx: &mut InterpCx<'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>, + _args: &[OpTy<'tcx>], + _dest: PlaceTy<'tcx>, + ) -> InterpResult<'tcx> { + throw_unsup_format!("calling intrinsics isn't supported in ConstProp"); + } + + fn ptr_to_int( + _mem: &Memory<'mir, 'tcx, Self>, + _ptr: Pointer, + ) -> InterpResult<'tcx, u64> { + throw_unsup_format!("ptr-to-int casts aren't supported in ConstProp"); + } + + fn binary_ptr_op( + _ecx: &InterpCx<'mir, 'tcx, Self>, + _bin_op: BinOp, + _left: ImmTy<'tcx>, + _right: ImmTy<'tcx>, + ) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> { + // We can't do this because aliasing of memory can differ between const eval and llvm + throw_unsup_format!("pointer arithmetic or comparisons aren't supported in ConstProp"); + } + + fn find_foreign_static( + _tcx: TyCtxt<'tcx>, + _def_id: DefId, + ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { + throw_unsup!(ReadForeignStatic) + } + + #[inline(always)] + fn tag_allocation<'b>( + _memory_extra: &(), + _id: AllocId, + alloc: Cow<'b, Allocation>, + _kind: Option>, + ) -> (Cow<'b, Allocation>, Self::PointerTag) { + // We do not use a tag so we can just cheaply forward the allocation + (alloc, ()) + } + + #[inline(always)] + fn tag_static_base_pointer( + _memory_extra: &(), + _id: AllocId, + ) -> Self::PointerTag { + () + } + + fn box_alloc( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + _dest: PlaceTy<'tcx>, + ) -> InterpResult<'tcx> { + throw_unsup_format!("can't const prop `box` keyword"); + } + + fn access_local( + _ecx: &InterpCx<'mir, 'tcx, Self>, + frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>, + local: Local, + ) -> InterpResult<'tcx, InterpOperand> { + let l = &frame.locals[local]; + + if l.value == LocalValue::Uninitialized { + throw_unsup_format!("tried to access an uninitialized local"); + } + + l.access() + } + + fn before_eval_static( + _ecx: &InterpCx<'mir, 'tcx, Self>, + _place_static: &Static<'tcx>, + ) -> InterpResult<'tcx> { + throw_unsup_format!("can't eval statics in ConstProp"); + } + + fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + Ok(()) + } + + #[inline(always)] + fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + Ok(()) + } + + /// Called immediately before a stack frame gets popped. + #[inline(always)] + fn stack_pop(_ecx: &mut InterpCx<'mir, 'tcx, Self>, _extra: ()) -> InterpResult<'tcx> { + Ok(()) + } +} + type Const<'tcx> = OpTy<'tcx>; /// Finds optimization opportunities on the MIR. struct ConstPropagator<'mir, 'tcx> { - ecx: InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>>, + ecx: InterpCx<'mir, 'tcx, ConstPropMachine>, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, can_const_prop: IndexVec, @@ -158,7 +299,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let def_id = source.def_id(); let param_env = tcx.param_env(def_id); let span = tcx.def_span(def_id); - let mut ecx = mk_eval_cx(tcx, span, param_env); + let mut ecx = InterpCx::new(tcx.at(span), param_env, ConstPropMachine, ()); let can_const_prop = CanConstProp::check(body); ecx.push_stack_frame( diff --git a/src/test/ui/issues/issue-52060.stderr b/src/test/ui/issues/issue-52060.stderr index 6fbc1ed52c478..2f90f7f9e035b 100644 --- a/src/test/ui/issues/issue-52060.stderr +++ b/src/test/ui/issues/issue-52060.stderr @@ -6,5 +6,4 @@ LL | static B: [u32; 1] = [0; A.len()]; error: aborting due to previous error -Some errors have detailed explanations: E0013, E0080. -For more information about an error, try `rustc --explain E0013`. +For more information about this error, try `rustc --explain E0013`. From a99f255015b613015a37b5c244064d6b8eb0136b Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 24 Sep 2019 22:08:22 -0400 Subject: [PATCH 14/15] Allow reading non-mutable statics in const prop --- src/librustc_mir/interpret/machine.rs | 7 ++--- src/librustc_mir/interpret/memory.rs | 2 ++ src/librustc_mir/interpret/place.rs | 2 -- src/librustc_mir/transform/const_prop.rs | 16 ++++++---- .../const_prop/read_immutable_static.rs | 29 +++++++++++++++++++ 5 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 src/test/mir-opt/const_prop/read_immutable_static.rs diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index e0be53b80d734..c30c59bbf10c8 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -194,10 +194,9 @@ pub trait Machine<'mir, 'tcx>: Sized { frame.locals[local].access() } - /// Called before a `StaticKind::Static` value is read. - fn before_eval_static( - _ecx: &InterpCx<'mir, 'tcx, Self>, - _place_static: &mir::Static<'tcx>, + /// Called before a `StaticKind::Static` value is accessed. + fn before_access_static( + _allocation: &Allocation, ) -> InterpResult<'tcx> { Ok(()) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 62b1760508b4c..924474c53175c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -462,6 +462,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // Make sure we use the ID of the resolved memory, not the lazy one! let id = raw_const.alloc_id; let allocation = tcx.alloc_map.lock().unwrap_memory(id); + + M::before_access_static(allocation)?; Cow::Borrowed(allocation) } } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index c882f4ef4ddf3..1166ca9bf2444 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -601,8 +601,6 @@ where } StaticKind::Static => { - M::before_eval_static(self, place_static)?; - let ty = place_static.ty; assert!(!ty.needs_subst()); let layout = self.layout_of(ty)?; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index ce1acc3b7edf9..612822b6d9d34 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -8,7 +8,7 @@ use rustc::hir::def::DefKind; use rustc::hir::def_id::DefId; use rustc::mir::{ AggregateKind, Constant, Location, Place, PlaceBase, Body, Operand, Rvalue, - Local, NullOp, UnOp, StatementKind, Statement, LocalKind, Static, + Local, NullOp, UnOp, StatementKind, Statement, LocalKind, TerminatorKind, Terminator, ClearCrossCrate, SourceInfo, BinOp, SourceScope, SourceScopeLocalData, LocalDecl, BasicBlock, }; @@ -17,6 +17,7 @@ use rustc::mir::visit::{ }; use rustc::mir::interpret::{Scalar, InterpResult, PanicInfo}; use rustc::ty::{self, Instance, ParamEnv, Ty, TyCtxt}; +use syntax::ast::Mutability; use syntax_pos::{Span, DUMMY_SP}; use rustc::ty::subst::InternalSubsts; use rustc_data_structures::fx::FxHashMap; @@ -229,11 +230,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { l.access() } - fn before_eval_static( - _ecx: &InterpCx<'mir, 'tcx, Self>, - _place_static: &Static<'tcx>, + fn before_access_static( + allocation: &Allocation, ) -> InterpResult<'tcx> { - throw_unsup_format!("can't eval statics in ConstProp"); + // 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::Mutable || allocation.relocations().len() > 0 { + throw_unsup_format!("can't eval mutable statics in ConstProp"); + } + + Ok(()) } fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { diff --git a/src/test/mir-opt/const_prop/read_immutable_static.rs b/src/test/mir-opt/const_prop/read_immutable_static.rs new file mode 100644 index 0000000000000..c2902dbd7c129 --- /dev/null +++ b/src/test/mir-opt/const_prop/read_immutable_static.rs @@ -0,0 +1,29 @@ +// compile-flags: -O + +static FOO: u8 = 2; + +fn main() { + let x = FOO + FOO; +} + +// END RUST SOURCE +// START rustc.main.ConstProp.before.mir +// bb0: { +// ... +// _2 = (FOO: u8); +// ... +// _3 = (FOO: u8); +// _1 = Add(move _2, move _3); +// ... +// } +// END rustc.main.ConstProp.before.mir +// START rustc.main.ConstProp.after.mir +// bb0: { +// ... +// _2 = const 2u8; +// ... +// _3 = const 2u8; +// _1 = Add(move _2, move _3); +// ... +// } +// END rustc.main.ConstProp.after.mir From ba2d6c42fc4ca5c4e590c133ef91412507b7a640 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Fri, 27 Sep 2019 20:09:52 -0400 Subject: [PATCH 15/15] Fix lint-exceeding-bitshifts ui tests --- src/test/ui/lint/lint-exceeding-bitshifts.rs | 2 + .../ui/lint/lint-exceeding-bitshifts.stderr | 38 +++++++++---------- src/test/ui/lint/lint-exceeding-bitshifts2.rs | 2 + .../ui/lint/lint-exceeding-bitshifts2.stderr | 8 ++-- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/test/ui/lint/lint-exceeding-bitshifts.rs b/src/test/ui/lint/lint-exceeding-bitshifts.rs index 0f397a7efe81f..dd3b839342911 100644 --- a/src/test/ui/lint/lint-exceeding-bitshifts.rs +++ b/src/test/ui/lint/lint-exceeding-bitshifts.rs @@ -1,3 +1,5 @@ +// compile-flags: -O + #![deny(exceeding_bitshifts, const_err)] #![allow(unused_variables)] #![allow(dead_code)] diff --git a/src/test/ui/lint/lint-exceeding-bitshifts.stderr b/src/test/ui/lint/lint-exceeding-bitshifts.stderr index f9f168c14dee3..25e079b6d814f 100644 --- a/src/test/ui/lint/lint-exceeding-bitshifts.stderr +++ b/src/test/ui/lint/lint-exceeding-bitshifts.stderr @@ -1,113 +1,113 @@ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:7:15 + --> $DIR/lint-exceeding-bitshifts.rs:9:15 | LL | let n = 1u8 << 8; | ^^^^^^^^ | note: lint level defined here - --> $DIR/lint-exceeding-bitshifts.rs:1:9 + --> $DIR/lint-exceeding-bitshifts.rs:3:9 | LL | #![deny(exceeding_bitshifts, const_err)] | ^^^^^^^^^^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:9:15 + --> $DIR/lint-exceeding-bitshifts.rs:11:15 | LL | let n = 1u16 << 16; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:11:15 + --> $DIR/lint-exceeding-bitshifts.rs:13:15 | LL | let n = 1u32 << 32; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:13:15 + --> $DIR/lint-exceeding-bitshifts.rs:15:15 | LL | let n = 1u64 << 64; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:15:15 + --> $DIR/lint-exceeding-bitshifts.rs:17:15 | LL | let n = 1i8 << 8; | ^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:17:15 + --> $DIR/lint-exceeding-bitshifts.rs:19:15 | LL | let n = 1i16 << 16; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:19:15 + --> $DIR/lint-exceeding-bitshifts.rs:21:15 | LL | let n = 1i32 << 32; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:21:15 + --> $DIR/lint-exceeding-bitshifts.rs:23:15 | LL | let n = 1i64 << 64; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:24:15 + --> $DIR/lint-exceeding-bitshifts.rs:26:15 | LL | let n = 1u8 >> 8; | ^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:26:15 + --> $DIR/lint-exceeding-bitshifts.rs:28:15 | LL | let n = 1u16 >> 16; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:28:15 + --> $DIR/lint-exceeding-bitshifts.rs:30:15 | LL | let n = 1u32 >> 32; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:30:15 + --> $DIR/lint-exceeding-bitshifts.rs:32:15 | LL | let n = 1u64 >> 64; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:32:15 + --> $DIR/lint-exceeding-bitshifts.rs:34:15 | LL | let n = 1i8 >> 8; | ^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:34:15 + --> $DIR/lint-exceeding-bitshifts.rs:36:15 | LL | let n = 1i16 >> 16; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:36:15 + --> $DIR/lint-exceeding-bitshifts.rs:38:15 | LL | let n = 1i32 >> 32; | ^^^^^^^^^^ error: attempt to shift right with overflow - --> $DIR/lint-exceeding-bitshifts.rs:38:15 + --> $DIR/lint-exceeding-bitshifts.rs:40:15 | LL | let n = 1i64 >> 64; | ^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:42:15 + --> $DIR/lint-exceeding-bitshifts.rs:44:15 | LL | let n = n << 8; | ^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts.rs:44:15 + --> $DIR/lint-exceeding-bitshifts.rs:46:15 | LL | let n = 1u8 << -8; | ^^^^^^^^^ diff --git a/src/test/ui/lint/lint-exceeding-bitshifts2.rs b/src/test/ui/lint/lint-exceeding-bitshifts2.rs index bde4865aa3f9e..69b627355b801 100644 --- a/src/test/ui/lint/lint-exceeding-bitshifts2.rs +++ b/src/test/ui/lint/lint-exceeding-bitshifts2.rs @@ -1,3 +1,5 @@ +// compile-flags: -O + #![deny(exceeding_bitshifts, const_err)] #![allow(unused_variables)] #![allow(dead_code)] diff --git a/src/test/ui/lint/lint-exceeding-bitshifts2.stderr b/src/test/ui/lint/lint-exceeding-bitshifts2.stderr index 8a6d2a89c7063..cb96982a78930 100644 --- a/src/test/ui/lint/lint-exceeding-bitshifts2.stderr +++ b/src/test/ui/lint/lint-exceeding-bitshifts2.stderr @@ -1,23 +1,23 @@ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts2.rs:7:15 + --> $DIR/lint-exceeding-bitshifts2.rs:9:15 | LL | let n = 1u8 << (4+4); | ^^^^^^^^^^^^ | note: lint level defined here - --> $DIR/lint-exceeding-bitshifts2.rs:1:9 + --> $DIR/lint-exceeding-bitshifts2.rs:3:9 | LL | #![deny(exceeding_bitshifts, const_err)] | ^^^^^^^^^^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts2.rs:15:15 + --> $DIR/lint-exceeding-bitshifts2.rs:17:15 | LL | let n = 1_isize << BITS; | ^^^^^^^^^^^^^^^ error: attempt to shift left with overflow - --> $DIR/lint-exceeding-bitshifts2.rs:16:15 + --> $DIR/lint-exceeding-bitshifts2.rs:18:15 | LL | let n = 1_usize << BITS; | ^^^^^^^^^^^^^^^