From c3454c000706176b61ef089107203766735348f7 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 27 Jul 2020 13:52:40 +0200 Subject: [PATCH 1/7] Check whether locals are too large instead of whether accesses into them are too large --- src/librustc_mir/transform/const_prop.rs | 116 ++++++++++++----------- 1 file changed, 60 insertions(+), 56 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index a1db06e6aa3de..9184173e402ea 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -34,7 +34,9 @@ use crate::interpret::{ }; use crate::transform::{MirPass, MirSource}; -/// The maximum number of bytes that we'll allocate space for a return value. +/// The maximum number of bytes that we'll allocate space for a local or the return value. +/// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just +/// Severely regress performance. const MAX_ALLOC_LIMIT: u64 = 1024; /// Macro for machine-specific `InterpError` without allocation. @@ -331,7 +333,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let param_env = tcx.param_env(def_id).with_reveal_all(); let span = tcx.def_span(def_id); - let can_const_prop = CanConstProp::check(body); + // FIXME: `CanConstProp::check` computes the layout of all locals, return those layouts + // so we can write them to `ecx.frame_mut().locals.layout, reducing the duplication in + // `layout_of` query invocations. + let can_const_prop = CanConstProp::check(tcx, param_env, body); let mut only_propagate_inside_block_locals = BitSet::new_empty(can_const_prop.len()); for (l, mode) in can_const_prop.iter_enumerated() { if *mode == ConstPropMode::OnlyInsideOwnBlock { @@ -612,15 +617,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { fn const_prop( &mut self, rvalue: &Rvalue<'tcx>, - place_layout: TyAndLayout<'tcx>, source_info: SourceInfo, place: Place<'tcx>, ) -> Option<()> { - // #66397: Don't try to eval into large places as that can cause an OOM - if place_layout.size >= Size::from_bytes(MAX_ALLOC_LIMIT) { - return None; - } - // Perform any special handling for specific Rvalue types. // Generally, checks here fall into one of two categories: // 1. Additional checking to provide useful lints to the user @@ -893,7 +892,11 @@ struct CanConstProp { impl CanConstProp { /// Returns true if `local` can be propagated - fn check(body: &Body<'_>) -> IndexVec { + fn check( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + body: &Body<'tcx>, + ) -> IndexVec { let mut cpv = CanConstProp { can_const_prop: IndexVec::from_elem(ConstPropMode::FullConstProp, &body.local_decls), found_assignment: BitSet::new_empty(body.local_decls.len()), @@ -903,6 +906,16 @@ impl CanConstProp { ), }; for (local, val) in cpv.can_const_prop.iter_enumerated_mut() { + let ty = body.local_decls[local].ty; + match tcx.layout_of(param_env.and(ty)) { + Ok(layout) if layout.size < Size::from_bytes(MAX_ALLOC_LIMIT) => {} + // Either the layout fails to compute, then we can't use this local anyway + // or the local is too large, then we don't want to. + _ => { + *val = ConstPropMode::NoPropagation; + continue; + } + } // Cannot use args at all // Cannot use locals because if x < y { y - x } else { x - y } would // lint for x != y @@ -1018,61 +1031,52 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { let source_info = statement.source_info; self.source_info = Some(source_info); if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind { - let place_ty: Ty<'tcx> = place.ty(&self.local_decls, self.tcx).ty; - if let Ok(place_layout) = self.tcx.layout_of(self.param_env.and(place_ty)) { - let can_const_prop = self.can_const_prop[place.local]; - if let Some(()) = self.const_prop(rval, place_layout, source_info, place) { - // This will return None if the above `const_prop` invocation only "wrote" a - // type whose creation requires no write. E.g. a generator whose initial state - // consists solely of uninitialized memory (so it doesn't capture any locals). - if let Some(value) = self.get_const(place) { - if self.should_const_prop(value) { - trace!("replacing {:?} with {:?}", rval, value); - self.replace_with_const(rval, value, source_info); - if can_const_prop == ConstPropMode::FullConstProp - || can_const_prop == ConstPropMode::OnlyInsideOwnBlock - { - trace!("propagated into {:?}", place); - } + let can_const_prop = self.can_const_prop[place.local]; + if let Some(()) = self.const_prop(rval, source_info, place) { + // This will return None if the above `const_prop` invocation only "wrote" a + // type whose creation requires no write. E.g. a generator whose initial state + // consists solely of uninitialized memory (so it doesn't capture any locals). + if let Some(value) = self.get_const(place) { + if self.should_const_prop(value) { + trace!("replacing {:?} with {:?}", rval, value); + self.replace_with_const(rval, value, source_info); + if can_const_prop == ConstPropMode::FullConstProp + || can_const_prop == ConstPropMode::OnlyInsideOwnBlock + { + trace!("propagated into {:?}", place); } } - match can_const_prop { - ConstPropMode::OnlyInsideOwnBlock => { - trace!( - "found local restricted to its block. \ + } + match can_const_prop { + ConstPropMode::OnlyInsideOwnBlock => { + trace!( + "found local restricted to its block. \ Will remove it from const-prop after block is finished. Local: {:?}", - place.local - ); - } - ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { - trace!("can't propagate into {:?}", place); - if place.local != RETURN_PLACE { - Self::remove_const(&mut self.ecx, place.local); - } + place.local + ); + } + ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { + trace!("can't propagate into {:?}", place); + if place.local != RETURN_PLACE { + Self::remove_const(&mut self.ecx, place.local); } - ConstPropMode::FullConstProp => {} } - } else { - // Const prop failed, so erase the destination, ensuring that whatever happens - // from here on, does not know about the previous value. - // This is important in case we have - // ```rust - // let mut x = 42; - // x = SOME_MUTABLE_STATIC; - // // x must now be undefined - // ``` - // FIXME: we overzealously erase the entire local, because that's easier to - // implement. - trace!( - "propagation into {:?} failed. - Nuking the entire site from orbit, it's the only way to be sure", - place, - ); - Self::remove_const(&mut self.ecx, place.local); + ConstPropMode::FullConstProp => {} } } else { + // Const prop failed, so erase the destination, ensuring that whatever happens + // from here on, does not know about the previous value. + // This is important in case we have + // ```rust + // let mut x = 42; + // x = SOME_MUTABLE_STATIC; + // // x must now be undefined + // ``` + // FIXME: we overzealously erase the entire local, because that's easier to + // implement. trace!( - "cannot propagate into {:?}, because the type of the local is generic.", + "propagation into {:?} failed. + Nuking the entire site from orbit, it's the only way to be sure", place, ); Self::remove_const(&mut self.ecx, place.local); From b26a7d5cd9d9c9ec84eba90b806a453135d20b99 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 27 Jul 2020 15:01:25 +0200 Subject: [PATCH 2/7] Stop propagating to locals that were marks as unpropagatable. We used to erase these values immediately after propagation, but some things slipped through and it caused us to still initialize huge locals. --- src/librustc_mir/transform/const_prop.rs | 16 +++++++++++----- ...ariable_aggregate_mut_ref.main.ConstProp.diff | 6 ++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 9184173e402ea..a87b00263e5c4 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -157,14 +157,19 @@ struct ConstPropMachine<'mir, 'tcx> { written_only_inside_own_block_locals: FxHashSet, /// Locals that need to be cleared after every block terminates. only_propagate_inside_block_locals: BitSet, + can_const_prop: IndexVec, } impl<'mir, 'tcx> ConstPropMachine<'mir, 'tcx> { - fn new(only_propagate_inside_block_locals: BitSet) -> Self { + fn new( + only_propagate_inside_block_locals: BitSet, + can_const_prop: IndexVec, + ) -> Self { Self { stack: Vec::new(), written_only_inside_own_block_locals: Default::default(), only_propagate_inside_block_locals, + can_const_prop, } } } @@ -243,6 +248,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> local: Local, ) -> InterpResult<'tcx, Result<&'a mut LocalValue, MemPlace>> { + if ecx.machine.can_const_prop[local] == ConstPropMode::NoPropagation { + throw_machine_stop_str!("tried to write to a local that is marked as not propagatable") + } if frame == 0 && ecx.machine.only_propagate_inside_block_locals.contains(local) { ecx.machine.written_only_inside_own_block_locals.insert(local); } @@ -287,7 +295,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> struct ConstPropagator<'mir, 'tcx> { ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, tcx: TyCtxt<'tcx>, - can_const_prop: IndexVec, param_env: ParamEnv<'tcx>, // FIXME(eddyb) avoid cloning these two fields more than once, // by accessing them through `ecx` instead. @@ -347,7 +354,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { tcx, span, param_env, - ConstPropMachine::new(only_propagate_inside_block_locals), + ConstPropMachine::new(only_propagate_inside_block_locals, can_const_prop), (), ); @@ -373,7 +380,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ecx, tcx, param_env, - can_const_prop, // FIXME(eddyb) avoid cloning these two fields more than once, // by accessing them through `ecx` instead. source_scopes: body.source_scopes.clone(), @@ -1031,7 +1037,7 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { let source_info = statement.source_info; self.source_info = Some(source_info); if let StatementKind::Assign(box (place, ref mut rval)) = statement.kind { - let can_const_prop = self.can_const_prop[place.local]; + let can_const_prop = self.ecx.machine.can_const_prop[place.local]; if let Some(()) = self.const_prop(rval, source_info, place) { // This will return None if the above `const_prop` invocation only "wrote" a // type whose creation requires no write. E.g. a generator whose initial state diff --git a/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref.main.ConstProp.diff b/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref.main.ConstProp.diff index e78bc31b77480..6e2ee0957ab33 100644 --- a/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/mutable_variable_aggregate_mut_ref.main.ConstProp.diff @@ -23,15 +23,13 @@ // + ty: i32 // + val: Value(Scalar(0x0000002a)) // mir::Constant -- // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:18: 5:20 -+ // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:17: 5:25 + // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:18: 5:20 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) } // ty::Const // + ty: i32 // + val: Value(Scalar(0x0000002b)) // mir::Constant -- // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:22: 5:24 -+ // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:17: 5:25 + // + span: $DIR/mutable_variable_aggregate_mut_ref.rs:5:22: 5:24 // + literal: Const { ty: i32, val: Value(Scalar(0x0000002b)) } StorageLive(_2); // scope 1 at $DIR/mutable_variable_aggregate_mut_ref.rs:6:9: 6:10 _2 = &mut _1; // scope 1 at $DIR/mutable_variable_aggregate_mut_ref.rs:6:13: 6:19 From 9e21004c74b8749686c0e5b9195e6822be6280d0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 27 Jul 2020 15:43:06 +0200 Subject: [PATCH 3/7] Update ui tests --- .../ui/recursion/issue-26548-recursion-via-normalize.stderr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr b/src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr index be55890c08c88..7f197a238e5a0 100644 --- a/src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr +++ b/src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr @@ -1,7 +1,7 @@ -error[E0391]: cycle detected when computing layout of `std::option::Option` +error[E0391]: cycle detected when computing layout of `S` | - = note: ...which requires computing layout of `S`... - = note: ...which again requires computing layout of `std::option::Option`, completing the cycle + = note: ...which requires computing layout of `std::option::Option`... + = note: ...which again requires computing layout of `S`, completing the cycle note: cycle used when optimizing MIR for `main` --> $DIR/issue-26548-recursion-via-normalize.rs:15:1 | From 8d5f2bdcd17f3139965b9ef6da8cb13183324485 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 28 Jul 2020 14:39:45 +0200 Subject: [PATCH 4/7] Add test ensuring that we don't propagate large arrays --- .../mir-opt/const_prop/large_array_index.rs | 7 ++ .../32bit/rustc.main.ConstProp.diff | 85 +++++++++++++++++++ .../64bit/rustc.main.ConstProp.diff | 85 +++++++++++++++++++ 3 files changed, 177 insertions(+) create mode 100644 src/test/mir-opt/const_prop/large_array_index.rs create mode 100644 src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff create mode 100644 src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff diff --git a/src/test/mir-opt/const_prop/large_array_index.rs b/src/test/mir-opt/const_prop/large_array_index.rs new file mode 100644 index 0000000000000..e6a12f3364aea --- /dev/null +++ b/src/test/mir-opt/const_prop/large_array_index.rs @@ -0,0 +1,7 @@ +// EMIT_MIR_FOR_EACH_BIT_WIDTH + +// EMIT_MIR rustc.main.ConstProp.diff +fn main() { + // check that we don't propagate this, because it's too large + let x: u8 = [0_u8; 5000][2]; +} diff --git a/src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff new file mode 100644 index 0000000000000..721766f984971 --- /dev/null +++ b/src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff @@ -0,0 +1,85 @@ +- // MIR for `main` before ConstProp ++ // MIR for `main` after ConstProp + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/large_array_index.rs:4:11: 4:11 + let _1: u8; // in scope 0 at $DIR/large_array_index.rs:6:9: 6:10 + let mut _2: [u8; 5000]; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + let _3: usize; // in scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + let mut _4: usize; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + let mut _5: bool; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + scope 1 { + debug x => _1; // in scope 1 at $DIR/large_array_index.rs:6:9: 6:10 + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/large_array_index.rs:6:9: 6:10 + StorageLive(_2); // scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + _2 = [const 0_u8; 5000]; // scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + // ty::Const + // + ty: u8 + // + val: Value(Scalar(0x00)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:18: 6:22 + // + literal: Const { ty: u8, val: Value(Scalar(0x00)) } + StorageLive(_3); // scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + _3 = const 2_usize; // scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + // ty::Const + // + ty: usize + // + val: Value(Scalar(0x00000002)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:30: 6:31 + // + literal: Const { ty: usize, val: Value(Scalar(0x00000002)) } + _4 = const 5000_usize; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + // ty::Const + // + ty: usize + // + val: Value(Scalar(0x00001388)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:17: 6:32 + // + literal: Const { ty: usize, val: Value(Scalar(0x00001388)) } +- _5 = Lt(_3, _4); // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 +- assert(move _5, "index out of bounds: the len is {} but the index is {}", move _4, _3) -> bb1; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ _5 = const true; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ // ty::Const ++ // + ty: bool ++ // + val: Value(Scalar(0x01)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) } ++ assert(const true, "index out of bounds: the len is {} but the index is {}", const 5000_usize, const 2_usize) -> bb1; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ // ty::Const ++ // + ty: bool ++ // + val: Value(Scalar(0x01)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) } ++ // ty::Const ++ // + ty: usize ++ // + val: Value(Scalar(0x00001388)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: usize, val: Value(Scalar(0x00001388)) } ++ // ty::Const ++ // + ty: usize ++ // + val: Value(Scalar(0x00000002)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: usize, val: Value(Scalar(0x00000002)) } + } + + bb1: { + _1 = _2[_3]; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + StorageDead(_3); // scope 0 at $DIR/large_array_index.rs:6:32: 6:33 + StorageDead(_2); // scope 0 at $DIR/large_array_index.rs:6:32: 6:33 + _0 = const (); // scope 0 at $DIR/large_array_index.rs:4:11: 7:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/large_array_index.rs:4:11: 7:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_1); // scope 0 at $DIR/large_array_index.rs:7:1: 7:2 + return; // scope 0 at $DIR/large_array_index.rs:7:2: 7:2 + } + } + diff --git a/src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff new file mode 100644 index 0000000000000..eae2ce6671cc1 --- /dev/null +++ b/src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff @@ -0,0 +1,85 @@ +- // MIR for `main` before ConstProp ++ // MIR for `main` after ConstProp + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/large_array_index.rs:4:11: 4:11 + let _1: u8; // in scope 0 at $DIR/large_array_index.rs:6:9: 6:10 + let mut _2: [u8; 5000]; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + let _3: usize; // in scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + let mut _4: usize; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + let mut _5: bool; // in scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + scope 1 { + debug x => _1; // in scope 1 at $DIR/large_array_index.rs:6:9: 6:10 + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/large_array_index.rs:6:9: 6:10 + StorageLive(_2); // scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + _2 = [const 0_u8; 5000]; // scope 0 at $DIR/large_array_index.rs:6:17: 6:29 + // ty::Const + // + ty: u8 + // + val: Value(Scalar(0x00)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:18: 6:22 + // + literal: Const { ty: u8, val: Value(Scalar(0x00)) } + StorageLive(_3); // scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + _3 = const 2_usize; // scope 0 at $DIR/large_array_index.rs:6:30: 6:31 + // ty::Const + // + ty: usize + // + val: Value(Scalar(0x0000000000000002)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:30: 6:31 + // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000002)) } + _4 = const 5000_usize; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + // ty::Const + // + ty: usize + // + val: Value(Scalar(0x0000000000001388)) + // mir::Constant + // + span: $DIR/large_array_index.rs:6:17: 6:32 + // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000001388)) } +- _5 = Lt(_3, _4); // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 +- assert(move _5, "index out of bounds: the len is {} but the index is {}", move _4, _3) -> bb1; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ _5 = const true; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ // ty::Const ++ // + ty: bool ++ // + val: Value(Scalar(0x01)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) } ++ assert(const true, "index out of bounds: the len is {} but the index is {}", const 5000_usize, const 2_usize) -> bb1; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 ++ // ty::Const ++ // + ty: bool ++ // + val: Value(Scalar(0x01)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) } ++ // ty::Const ++ // + ty: usize ++ // + val: Value(Scalar(0x0000000000001388)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000001388)) } ++ // ty::Const ++ // + ty: usize ++ // + val: Value(Scalar(0x0000000000000002)) ++ // mir::Constant ++ // + span: $DIR/large_array_index.rs:6:17: 6:32 ++ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000002)) } + } + + bb1: { + _1 = _2[_3]; // scope 0 at $DIR/large_array_index.rs:6:17: 6:32 + StorageDead(_3); // scope 0 at $DIR/large_array_index.rs:6:32: 6:33 + StorageDead(_2); // scope 0 at $DIR/large_array_index.rs:6:32: 6:33 + _0 = const (); // scope 0 at $DIR/large_array_index.rs:4:11: 7:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/large_array_index.rs:4:11: 7:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_1); // scope 0 at $DIR/large_array_index.rs:7:1: 7:2 + return; // scope 0 at $DIR/large_array_index.rs:7:2: 7:2 + } + } + From dc0408ec43fa6b63b5cbffbbc07823577c97ea24 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 29 Jul 2020 13:45:20 +0200 Subject: [PATCH 5/7] Update clippy ui test. The reason we do not trigger these lints anymore is that clippy sets the mir-opt-level to 0, and the recent changes subtly changed how the const propagator works. --- .../tests/ui/indexing_slicing_index.stderr | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/tools/clippy/tests/ui/indexing_slicing_index.stderr b/src/tools/clippy/tests/ui/indexing_slicing_index.stderr index ac5f0d0a39e89..2b3f9be2dfb9b 100644 --- a/src/tools/clippy/tests/ui/indexing_slicing_index.stderr +++ b/src/tools/clippy/tests/ui/indexing_slicing_index.stderr @@ -1,23 +1,3 @@ -error: this operation will panic at runtime - --> $DIR/indexing_slicing_index.rs:11:5 - | -LL | x[4]; // Ok, let rustc's `const_err` lint handle `usize` indexing on arrays. - | ^^^^ index out of bounds: the len is 4 but the index is 4 - | - = note: `#[deny(unconditional_panic)]` on by default - -error: this operation will panic at runtime - --> $DIR/indexing_slicing_index.rs:12:5 - | -LL | x[1 << 3]; // Ok, let rustc's `const_err` lint handle `usize` indexing on arrays. - | ^^^^^^^^^ index out of bounds: the len is 4 but the index is 8 - -error: this operation will panic at runtime - --> $DIR/indexing_slicing_index.rs:27:5 - | -LL | x[N]; // Ok, let rustc's `const_err` lint handle `usize` indexing on arrays. - | ^^^^ index out of bounds: the len is 4 but the index is 15 - error: indexing may panic. --> $DIR/indexing_slicing_index.rs:10:5 | @@ -75,5 +55,5 @@ LL | v[M]; | = help: Consider using `.get(n)` or `.get_mut(n)` instead -error: aborting due to 10 previous errors +error: aborting due to 7 previous errors From f7a1e64fdb209951e6e77369364feb530a60b04c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 29 Jul 2020 22:39:09 +0200 Subject: [PATCH 6/7] Update tests after rebase --- ...nstProp.diff => large_array_index.main.ConstProp.diff.32bit} | 0 ...nstProp.diff => large_array_index.main.ConstProp.diff.64bit} | 0 src/test/mir-opt/const_prop/large_array_index.rs | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename src/test/mir-opt/const_prop/{large_array_index/32bit/rustc.main.ConstProp.diff => large_array_index.main.ConstProp.diff.32bit} (100%) rename src/test/mir-opt/const_prop/{large_array_index/64bit/rustc.main.ConstProp.diff => large_array_index.main.ConstProp.diff.64bit} (100%) diff --git a/src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/large_array_index.main.ConstProp.diff.32bit similarity index 100% rename from src/test/mir-opt/const_prop/large_array_index/32bit/rustc.main.ConstProp.diff rename to src/test/mir-opt/const_prop/large_array_index.main.ConstProp.diff.32bit diff --git a/src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/large_array_index.main.ConstProp.diff.64bit similarity index 100% rename from src/test/mir-opt/const_prop/large_array_index/64bit/rustc.main.ConstProp.diff rename to src/test/mir-opt/const_prop/large_array_index.main.ConstProp.diff.64bit diff --git a/src/test/mir-opt/const_prop/large_array_index.rs b/src/test/mir-opt/const_prop/large_array_index.rs index e6a12f3364aea..48d134376db62 100644 --- a/src/test/mir-opt/const_prop/large_array_index.rs +++ b/src/test/mir-opt/const_prop/large_array_index.rs @@ -1,6 +1,6 @@ // EMIT_MIR_FOR_EACH_BIT_WIDTH -// EMIT_MIR rustc.main.ConstProp.diff +// EMIT_MIR large_array_index.main.ConstProp.diff fn main() { // check that we don't propagate this, because it's too large let x: u8 = [0_u8; 5000][2]; From 1864a973b3a0a5a6c5a7c71d7d7cd052732e5c02 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 29 Jul 2020 22:39:50 +0200 Subject: [PATCH 7/7] Improve the diagnostics around misspelled mir dump filenames --- src/tools/compiletest/src/runtest.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9354cc16a9ae9..b97ca9e3a0f27 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3239,8 +3239,19 @@ impl<'test> TestCx<'test> { } fn diff_mir_files(&self, before: PathBuf, after: PathBuf) -> String { - let before = self.get_mir_dump_dir().join(before); - let after = self.get_mir_dump_dir().join(after); + let to_full_path = |path: PathBuf| { + let full = self.get_mir_dump_dir().join(&path); + if !full.exists() { + panic!( + "the mir dump file for {} does not exist (requested in {})", + path.display(), + self.testpaths.file.display(), + ); + } + full + }; + let before = to_full_path(before); + let after = to_full_path(after); debug!("comparing the contents of: {} with {}", before.display(), after.display()); let before = fs::read_to_string(before).unwrap(); let after = fs::read_to_string(after).unwrap();