diff options
8 files changed, 64 insertions, 58 deletions
diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 2da9123b846..d37eaeed095 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -187,9 +187,6 @@ pub enum LocalValue<Prov: Provenance = AllocId> { impl<'tcx, Prov: Provenance + 'static> LocalState<'tcx, Prov> { /// Read the local's value or error if the local is not yet live or not live anymore. - /// - /// Note: This may only be invoked from the `Machine::access_local` hook and not from - /// anywhere else. You may be invalidating machine invariants if you do! #[inline] pub fn access(&self) -> InterpResult<'tcx, &Operand<Prov>> { match &self.value { diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 6bed8a7a007..b151d03681f 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -215,23 +215,12 @@ pub trait Machine<'mir, 'tcx>: Sized { right: &ImmTy<'tcx, Self::Provenance>, ) -> InterpResult<'tcx, (Scalar<Self::Provenance>, bool, Ty<'tcx>)>; - /// Called to read the specified `local` from the `frame`. - /// Since reading a ZST is not actually accessing memory or locals, this is never invoked - /// for ZST reads. - #[inline] - fn access_local<'a>( - frame: &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>, - local: mir::Local, - ) -> InterpResult<'tcx, &'a Operand<Self::Provenance>> - where - 'tcx: 'mir, - { - frame.locals[local].access() - } - /// Called to write the specified `local` from the `frame`. /// Since writing a ZST is not actually accessing memory or locals, this is never invoked /// for ZST reads. + /// + /// Due to borrow checker trouble, we indicate the `frame` as an index rather than an `&mut + /// Frame`. #[inline] fn access_local_mut<'a>( ecx: &'a mut InterpCx<'mir, 'tcx, Self>, diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index e80a82acd58..91a97fe4d4d 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -444,7 +444,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } - /// Read from a local. Will not actually access the local if reading from a ZST. + /// Read from a local. /// Will not access memory, instead an indirect `Operand` is returned. /// /// This is public because it is used by [priroda](https://github.com/oli-obk/priroda) to get an @@ -456,12 +456,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { layout: Option<TyAndLayout<'tcx>>, ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> { let layout = self.layout_of_local(frame, local, layout)?; - let op = if layout.is_zst() { - // Bypass `access_local` (helps in ConstProp) - Operand::Immediate(Immediate::Uninit) - } else { - *M::access_local(frame, local)? - }; + let op = *frame.locals[local].access()?; Ok(OpTy { op, layout, align: Some(layout.align.abi) }) } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 7aa76fe1dae..d56323448ce 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -642,7 +642,7 @@ where // avoid force_allocation. let src = match self.read_immediate_raw(src)? { Ok(src_val) => { - assert!(!src.layout.is_unsized(), "cannot have unsized immediates"); + assert!(!src.layout.is_unsized(), "cannot copy unsized immediates"); assert!( !dest.layout.is_unsized(), "the src is sized, so the dest must also be sized" diff --git a/compiler/rustc_const_eval/src/interpret/projection.rs b/compiler/rustc_const_eval/src/interpret/projection.rs index 742339f2b0a..16ce5bc7175 100644 --- a/compiler/rustc_const_eval/src/interpret/projection.rs +++ b/compiler/rustc_const_eval/src/interpret/projection.rs @@ -100,6 +100,8 @@ where // This makes several assumptions about what layouts we will encounter; we match what // codegen does as good as we can (see `extract_field` in `rustc_codegen_ssa/src/mir/operand.rs`). let field_val: Immediate<_> = match (*base, base.layout.abi) { + // if the entire value is uninit, then so is the field (can happen in ConstProp) + (Immediate::Uninit, _) => Immediate::Uninit, // the field contains no information, can be left uninit _ if field_layout.is_zst() => Immediate::Uninit, // the field covers the entire type @@ -124,6 +126,7 @@ where b_val }) } + // everything else is a bug _ => span_bug!( self.cur_span(), "invalid field access on immediate {}, layout {:#?}", diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 5c9f4f57243..53f33a7a0ba 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -243,24 +243,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> throw_machine_stop_str!("pointer arithmetic or comparisons aren't supported in ConstProp") } - fn access_local<'a>( - frame: &'a Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>, - local: Local, - ) -> InterpResult<'tcx, &'a interpret::Operand<Self::Provenance>> { - let l = &frame.locals[local]; - - if matches!( - l.value, - LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit)) - ) { - // For us "uninit" means "we don't know its value, might be initiailized or not". - // So stop here. - throw_machine_stop_str!("tried to access alocal with unknown value ") - } - - l.access() - } - fn access_local_mut<'a>( ecx: &'a mut InterpCx<'mir, 'tcx, Self>, frame: usize, @@ -431,7 +413,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { fn get_const(&self, place: Place<'tcx>) -> Option<OpTy<'tcx>> { let op = match self.ecx.eval_place_to_op(place, None) { - Ok(op) => op, + Ok(op) => { + if matches!(*op, interpret::Operand::Immediate(Immediate::Uninit)) { + // Make sure nobody accidentally uses this value. + return None; + } + op + } Err(e) => { trace!("get_const failed: {}", e); return None; @@ -643,6 +631,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { if rvalue.needs_subst() { return None; } + if !rvalue + .ty(&self.ecx.frame().body.local_decls, *self.ecx.tcx) + .is_sized(self.ecx.tcx, self.param_env) + { + // the interpreter doesn't support unsized locals (only unsized arguments), + // but rustc does (in a kinda broken way), so we have to skip them here + return None; + } if self.tcx.sess.mir_opt_level() >= 4 { self.eval_rvalue_with_identities(rvalue, place) @@ -660,18 +656,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { self.use_ecx(|this| match rvalue { Rvalue::BinaryOp(op, box (left, right)) | Rvalue::CheckedBinaryOp(op, box (left, right)) => { - let l = this.ecx.eval_operand(left, None); - let r = this.ecx.eval_operand(right, None); + let l = this.ecx.eval_operand(left, None).and_then(|x| this.ecx.read_immediate(&x)); + let r = + this.ecx.eval_operand(right, None).and_then(|x| this.ecx.read_immediate(&x)); let const_arg = match (l, r) { - (Ok(ref x), Err(_)) | (Err(_), Ok(ref x)) => this.ecx.read_immediate(x)?, - (Err(e), Err(_)) => return Err(e), - (Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place), + (Ok(x), Err(_)) | (Err(_), Ok(x)) => x, // exactly one side is known + (Err(e), Err(_)) => return Err(e), // neither side is known + (Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place), // both sides are known }; if !matches!(const_arg.layout.abi, abi::Abi::Scalar(..)) { // We cannot handle Scalar Pair stuff. - return this.ecx.eval_rvalue_into_place(rvalue, place); + // No point in calling `eval_rvalue_into_place`, since only one side is known + throw_machine_stop_str!("cannot optimize this") } let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size)?; @@ -696,7 +694,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { this.ecx.write_immediate(*const_arg, &dest) } } - _ => this.ecx.eval_rvalue_into_place(rvalue, place), + _ => throw_machine_stop_str!("cannot optimize this"), } } _ => this.ecx.eval_rvalue_into_place(rvalue, place), @@ -1073,7 +1071,11 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { if let Some(ref value) = self.eval_operand(&cond) { trace!("assertion on {:?} should be {:?}", value, expected); let expected = Scalar::from_bool(*expected); - let value_const = self.ecx.read_scalar(&value).unwrap(); + let Ok(value_const) = self.ecx.read_scalar(&value) else { + // FIXME should be used use_ecx rather than a local match... but we have + // quite a few of these read_scalar/read_immediate that need fixing. + return + }; if expected != value_const { // Poison all places this operand references so that further code // doesn't use the invalid value diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 74b146833d0..082d6c9f07e 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -6,6 +6,7 @@ use crate::const_prop::ConstPropMachine; use crate::const_prop::ConstPropMode; use crate::MirLint; use rustc_const_eval::const_eval::ConstEvalErr; +use rustc_const_eval::interpret::Immediate; use rustc_const_eval::interpret::{ self, InterpCx, InterpResult, LocalState, LocalValue, MemoryKind, OpTy, Scalar, StackPopCleanup, }; @@ -229,7 +230,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { fn get_const(&self, place: Place<'tcx>) -> Option<OpTy<'tcx>> { let op = match self.ecx.eval_place_to_op(place, None) { - Ok(op) => op, + Ok(op) => { + if matches!(*op, interpret::Operand::Immediate(Immediate::Uninit)) { + // Make sure nobody accidentally uses this value. + return None; + } + op + } Err(e) => { trace!("get_const failed: {}", e); return None; @@ -515,6 +522,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { if rvalue.needs_subst() { return None; } + if !rvalue + .ty(&self.ecx.frame().body.local_decls, *self.ecx.tcx) + .is_sized(self.ecx.tcx, self.param_env) + { + // the interpreter doesn't support unsized locals (only unsized arguments), + // but rustc does (in a kinda broken way), so we have to skip them here + return None; + } self.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, place)) } @@ -624,7 +639,11 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { if let Some(ref value) = self.eval_operand(&cond, source_info) { trace!("assertion on {:?} should be {:?}", value, expected); let expected = Scalar::from_bool(*expected); - let value_const = self.ecx.read_scalar(&value).unwrap(); + let Ok(value_const) = self.ecx.read_scalar(&value) else { + // FIXME should be used use_ecx rather than a local match... but we have + // quite a few of these read_scalar/read_immediate that need fixing. + return + }; if expected != value_const { enum DbgVal<T> { Val(T), @@ -641,9 +660,9 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { let mut eval_to_int = |op| { // This can be `None` if the lhs wasn't const propagated and we just // triggered the assert on the value of the rhs. - self.eval_operand(op, source_info).map_or(DbgVal::Underscore, |op| { - DbgVal::Val(self.ecx.read_immediate(&op).unwrap().to_const_int()) - }) + self.eval_operand(op, source_info) + .and_then(|op| self.ecx.read_immediate(&op).ok()) + .map_or(DbgVal::Underscore, |op| DbgVal::Val(op.to_const_int())) }; let msg = match msg { AssertKind::DivisionByZero(op) => { diff --git a/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.diff index 4f205667be0..186a9537356 100644 --- a/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.diff @@ -41,7 +41,8 @@ StorageLive(_4); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+4:9: +4:10 _4 = (_2.1: i32); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+4:13: +4:16 StorageLive(_5); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:9: +5:10 - _5 = (_2.0: i32); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16 +- _5 = (_2.0: i32); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16 ++ _5 = const 1_i32; // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16 nop; // scope 0 at $DIR/mutable_variable_unprop_assign.rs:+0:11: +6:2 StorageDead(_5); // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+6:1: +6:2 StorageDead(_4); // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+6:1: +6:2 |