diff options
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_const_eval/src/transform/validate.rs | 46 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/dest_prop.rs | 7 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/lint.rs | 43 |
3 files changed, 43 insertions, 53 deletions
diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index b249ffb84b3..0b73691204d 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -74,7 +74,6 @@ impl<'tcx> MirPass<'tcx> for Validator { mir_phase, unwind_edge_count: 0, reachable_blocks: traversal::reachable_as_bitset(body), - place_cache: FxHashSet::default(), value_cache: FxHashSet::default(), can_unwind, }; @@ -106,7 +105,6 @@ struct CfgChecker<'a, 'tcx> { mir_phase: MirPhase, unwind_edge_count: usize, reachable_blocks: BitSet<BasicBlock>, - place_cache: FxHashSet<PlaceRef<'tcx>>, value_cache: FxHashSet<u128>, // If `false`, then the MIR must not contain `UnwindAction::Continue` or // `TerminatorKind::Resume`. @@ -294,19 +292,6 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match &statement.kind { - StatementKind::Assign(box (dest, rvalue)) => { - // FIXME(JakobDegen): Check this for all rvalues, not just this one. - if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue { - // The sides of an assignment must not alias. Currently this just checks whether - // the places are identical. - if dest == src { - self.fail( - location, - "encountered `Assign` statement with overlapping memory", - ); - } - } - } StatementKind::AscribeUserType(..) => { if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { self.fail( @@ -341,7 +326,8 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { self.fail(location, format!("explicit `{kind:?}` is forbidden")); } } - StatementKind::StorageLive(_) + StatementKind::Assign(..) + | StatementKind::StorageLive(_) | StatementKind::StorageDead(_) | StatementKind::Intrinsic(_) | StatementKind::Coverage(_) @@ -404,10 +390,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } // The call destination place and Operand::Move place used as an argument might be - // passed by a reference to the callee. Consequently they must be non-overlapping - // and cannot be packed. Currently this simply checks for duplicate places. - self.place_cache.clear(); - self.place_cache.insert(destination.as_ref()); + // passed by a reference to the callee. Consequently they cannot be packed. if is_within_packed(self.tcx, &self.body.local_decls, *destination).is_some() { // This is bad! The callee will expect the memory to be aligned. self.fail( @@ -418,10 +401,8 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { ), ); } - let mut has_duplicates = false; for arg in args { if let Operand::Move(place) = arg { - has_duplicates |= !self.place_cache.insert(place.as_ref()); if is_within_packed(self.tcx, &self.body.local_decls, *place).is_some() { // This is bad! The callee will expect the memory to be aligned. self.fail( @@ -434,16 +415,6 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> { } } } - - if has_duplicates { - self.fail( - location, - format!( - "encountered overlapping memory in `Move` arguments to `Call` terminator: {:?}", - terminator.kind, - ), - ); - } } TerminatorKind::Assert { target, unwind, .. } => { self.check_edge(location, *target, EdgeKind::Normal); @@ -1112,17 +1083,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ) } } - // FIXME(JakobDegen): Check this for all rvalues, not just this one. - if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue { - // The sides of an assignment must not alias. Currently this just checks whether - // the places are identical. - if dest == src { - self.fail( - location, - "encountered `Assign` statement with overlapping memory", - ); - } - } } StatementKind::AscribeUserType(..) => { if self.mir_phase >= MirPhase::Runtime(RuntimePhase::Initial) { diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 15502adfb5a..cd80f423466 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -133,7 +133,6 @@ use std::collections::hash_map::{Entry, OccupiedEntry}; -use crate::simplify::remove_dead_blocks; use crate::MirPass; use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; @@ -241,12 +240,6 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { apply_merges(body, tcx, &merges, &merged_locals); } - if round_count != 0 { - // Merging can introduce overlap between moved arguments and/or call destination in an - // unreachable code, which validator considers to be ill-formed. - remove_dead_blocks(body); - } - trace!(round_count); } } diff --git a/compiler/rustc_mir_transform/src/lint.rs b/compiler/rustc_mir_transform/src/lint.rs index e5d607e39b2..c0c0a3f5ee6 100644 --- a/compiler/rustc_mir_transform/src/lint.rs +++ b/compiler/rustc_mir_transform/src/lint.rs @@ -1,6 +1,7 @@ //! This pass statically detects code which has undefined behaviour or is likely to be erroneous. //! It can be used to locate problems in MIR building or optimizations. It assumes that all code //! can be executed, so it has false positives. +use rustc_data_structures::fx::FxHashSet; use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::*; @@ -31,6 +32,7 @@ pub fn lint_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, when: String) { always_live_locals, maybe_storage_live, maybe_storage_dead, + places: Default::default(), }; for (bb, data) in traversal::reachable(body) { lint.visit_basic_block_data(bb, data); @@ -45,6 +47,7 @@ struct Lint<'a, 'tcx> { always_live_locals: &'a BitSet<Local>, maybe_storage_live: ResultsCursor<'a, 'tcx, MaybeStorageLive<'a>>, maybe_storage_dead: ResultsCursor<'a, 'tcx, MaybeStorageDead<'a>>, + places: FxHashSet<PlaceRef<'tcx>>, } impl<'a, 'tcx> Lint<'a, 'tcx> { @@ -75,10 +78,22 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> { } fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - match statement.kind { + match &statement.kind { + StatementKind::Assign(box (dest, rvalue)) => { + if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue { + // The sides of an assignment must not alias. Currently this just checks whether + // the places are identical. + if dest == src { + self.fail( + location, + "encountered `Assign` statement with overlapping memory", + ); + } + } + } StatementKind::StorageLive(local) => { self.maybe_storage_live.seek_before_primary_effect(location); - if self.maybe_storage_live.get().contains(local) { + if self.maybe_storage_live.get().contains(*local) { self.fail( location, format!("StorageLive({local:?}) which already has storage here"), @@ -92,7 +107,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> { } fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { - match terminator.kind { + match &terminator.kind { TerminatorKind::Return => { if self.is_fn_like { self.maybe_storage_live.seek_after_primary_effect(location); @@ -108,6 +123,28 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> { } } } + TerminatorKind::Call { args, destination, .. } => { + // The call destination place and Operand::Move place used as an argument might be + // passed by a reference to the callee. Consequently they must be non-overlapping. + // Currently this simply checks for duplicate places. + self.places.clear(); + self.places.insert(destination.as_ref()); + let mut has_duplicates = false; + for arg in args { + if let Operand::Move(place) = arg { + has_duplicates |= !self.places.insert(place.as_ref()); + } + } + if has_duplicates { + self.fail( + location, + format!( + "encountered overlapping memory in `Move` arguments to `Call` terminator: {:?}", + terminator.kind, + ), + ); + } + } _ => {} } |