diff options
Diffstat (limited to 'compiler/rustc_mir_transform/src')
| -rw-r--r-- | compiler/rustc_mir_transform/src/check_unsafety.rs | 11 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/dest_prop.rs | 7 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/errors.rs | 24 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/lib.rs | 4 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/lint.rs | 62 | ||||
| -rw-r--r-- | compiler/rustc_mir_transform/src/pass_manager.rs | 12 |
6 files changed, 77 insertions, 43 deletions
diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs index d94d96c1115..582c2c0c6b6 100644 --- a/compiler/rustc_mir_transform/src/check_unsafety.rs +++ b/compiler/rustc_mir_transform/src/check_unsafety.rs @@ -131,7 +131,7 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> { &AggregateKind::Closure(def_id, _) | &AggregateKind::Coroutine(def_id, _) => { let def_id = def_id.expect_local(); let UnsafetyCheckResult { violations, used_unsafe_blocks, .. } = - self.tcx.unsafety_check_result(def_id); + self.tcx.mir_unsafety_check_result(def_id); self.register_violations(violations, used_unsafe_blocks.items().copied()); } }, @@ -153,7 +153,7 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> { if self.tcx.def_kind(def_id) == DefKind::InlineConst { let local_def_id = def_id.expect_local(); let UnsafetyCheckResult { violations, used_unsafe_blocks, .. } = - self.tcx.unsafety_check_result(local_def_id); + self.tcx.mir_unsafety_check_result(local_def_id); self.register_violations(violations, used_unsafe_blocks.items().copied()); } } @@ -390,7 +390,7 @@ impl<'tcx> UnsafetyChecker<'_, 'tcx> { } pub(crate) fn provide(providers: &mut Providers) { - *providers = Providers { unsafety_check_result, ..*providers }; + *providers = Providers { mir_unsafety_check_result, ..*providers }; } /// Context information for [`UnusedUnsafeVisitor`] traversal, @@ -490,7 +490,7 @@ fn check_unused_unsafe( unused_unsafes } -fn unsafety_check_result(tcx: TyCtxt<'_>, def: LocalDefId) -> &UnsafetyCheckResult { +fn mir_unsafety_check_result(tcx: TyCtxt<'_>, def: LocalDefId) -> &UnsafetyCheckResult { debug!("unsafety_violations({:?})", def); // N.B., this borrow is valid because all the consumers of @@ -538,7 +538,8 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) { return; } - let UnsafetyCheckResult { violations, unused_unsafes, .. } = tcx.unsafety_check_result(def_id); + let UnsafetyCheckResult { violations, unused_unsafes, .. } = + tcx.mir_unsafety_check_result(def_id); // Only suggest wrapping the entire function body in an unsafe block once let mut suggest_unsafe_block = true; 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/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 17916e16daf..bde442049b1 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/compiler/rustc_mir_transform/src/errors.rs @@ -67,11 +67,11 @@ impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for RequiresUnsafe { fn into_diagnostic(self, dcx: &'a DiagCtxt, level: Level) -> DiagnosticBuilder<'a, G> { let mut diag = DiagnosticBuilder::new(dcx, level, fluent::mir_transform_requires_unsafe); diag.code(rustc_errors::DiagnosticId::Error("E0133".to_string())); - diag.set_span(self.span); + diag.span(self.span); diag.span_label(self.span, self.details.label()); let desc = dcx.eagerly_translate_to_string(self.details.label(), [].into_iter()); - diag.set_arg("details", desc); - diag.set_arg("op_in_unsafe_fn_allowed", self.op_in_unsafe_fn_allowed); + diag.arg("details", desc); + diag.arg("op_in_unsafe_fn_allowed", self.op_in_unsafe_fn_allowed); self.details.add_subdiagnostics(&mut diag); if let Some(sp) = self.enclosing { diag.span_label(sp, fluent::mir_transform_not_inherited); @@ -122,16 +122,16 @@ impl RequiresUnsafeDetail { } CallToFunctionWith { ref missing, ref build_enabled } => { diag.help(fluent::mir_transform_target_feature_call_help); - diag.set_arg( + diag.arg( "missing_target_features", DiagnosticArgValue::StrListSepByAnd( missing.iter().map(|feature| Cow::from(feature.as_str())).collect(), ), ); - diag.set_arg("missing_target_features_count", missing.len()); + diag.arg("missing_target_features_count", missing.len()); if !build_enabled.is_empty() { diag.note(fluent::mir_transform_target_feature_call_note); - diag.set_arg( + diag.arg( "build_target_features", DiagnosticArgValue::StrListSepByAnd( build_enabled @@ -140,7 +140,7 @@ impl RequiresUnsafeDetail { .collect(), ), ); - diag.set_arg("build_target_features_count", build_enabled.len()); + diag.arg("build_target_features_count", build_enabled.len()); } } } @@ -183,7 +183,7 @@ impl<'a> DecorateLint<'a, ()> for UnsafeOpInUnsafeFn { fn decorate_lint<'b>(self, diag: &'b mut DiagnosticBuilder<'a, ()>) { let dcx = diag.dcx().expect("lint should not yet be emitted"); let desc = dcx.eagerly_translate_to_string(self.details.label(), [].into_iter()); - diag.set_arg("details", desc); + diag.arg("details", desc); diag.span_label(self.details.span, self.details.label()); self.details.add_subdiagnostics(diag); @@ -213,7 +213,7 @@ impl<'a, P: std::fmt::Debug> DecorateLint<'a, ()> for AssertLint<P> { let assert_kind = self.panic(); let message = assert_kind.diagnostic_message(); assert_kind.add_args(&mut |name, value| { - diag.set_arg(name, value); + diag.arg(name, value); }); diag.span_label(span, message); } @@ -280,9 +280,9 @@ impl<'a> DecorateLint<'a, ()> for MustNotSupend<'_, '_> { diag.subdiagnostic(reason); } diag.span_help(self.src_sp, fluent::_subdiag::help); - diag.set_arg("pre", self.pre); - diag.set_arg("def_path", self.tcx.def_path_str(self.def_id)); - diag.set_arg("post", self.post); + diag.arg("pre", self.pre); + diag.arg("def_path", self.tcx.def_path_str(self.def_id)); + diag.arg("post", self.post); } fn msg(&self) -> rustc_errors::DiagnosticMessage { diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 5562ae7f3bd..164b6b9c4f5 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -285,9 +285,9 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs { /// FIXME(oli-obk): it's unclear whether we still need this phase (and its corresponding query). /// We used to have this for pre-miri MIR based const eval. fn mir_const(tcx: TyCtxt<'_>, def: LocalDefId) -> &Steal<Body<'_>> { - // Unsafety check uses the raw mir, so make sure it is run. + // MIR unsafety check uses the raw mir, so make sure it is run. if !tcx.sess.opts.unstable_opts.thir_unsafeck { - tcx.ensure_with_value().unsafety_check_result(def); + tcx.ensure_with_value().mir_unsafety_check_result(def); } // has_ffi_unwind_calls query uses the raw mir, so make sure it is run. diff --git a/compiler/rustc_mir_transform/src/lint.rs b/compiler/rustc_mir_transform/src/lint.rs index 3940d0ddbf3..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::*; @@ -11,7 +12,6 @@ use rustc_mir_dataflow::{Analysis, ResultsCursor}; use std::borrow::Cow; pub fn lint_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, when: String) { - let reachable_blocks = traversal::reachable_as_bitset(body); let always_live_locals = &always_storage_live_locals(body); let maybe_storage_live = MaybeStorageLive::new(Cow::Borrowed(always_live_locals)) @@ -24,17 +24,19 @@ pub fn lint_body<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, when: String) { .iterate_to_fixpoint() .into_results_cursor(body); - Lint { + let mut lint = Lint { tcx, when, body, is_fn_like: tcx.def_kind(body.source.def_id()).is_fn_like(), always_live_locals, - reachable_blocks, maybe_storage_live, maybe_storage_dead, + places: Default::default(), + }; + for (bb, data) in traversal::reachable(body) { + lint.visit_basic_block_data(bb, data); } - .visit_body(body); } struct Lint<'a, 'tcx> { @@ -43,9 +45,9 @@ struct Lint<'a, 'tcx> { body: &'a Body<'tcx>, is_fn_like: bool, always_live_locals: &'a BitSet<Local>, - reachable_blocks: BitSet<BasicBlock>, 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> { @@ -67,7 +69,7 @@ impl<'a, 'tcx> Lint<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> { fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) { - if self.reachable_blocks.contains(location.block) && context.is_use() { + if context.is_use() { self.maybe_storage_dead.seek_after_primary_effect(location); if self.maybe_storage_dead.get().contains(local) { self.fail(location, format!("use of local {local:?}, which has no storage here")); @@ -76,18 +78,28 @@ impl<'a, 'tcx> Visitor<'tcx> for Lint<'a, 'tcx> { } fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { - match statement.kind { - StatementKind::StorageLive(local) => { - if self.reachable_blocks.contains(location.block) { - self.maybe_storage_live.seek_before_primary_effect(location); - if self.maybe_storage_live.get().contains(local) { + 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, - format!("StorageLive({local:?}) which already has storage here"), + "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) { + self.fail( + location, + format!("StorageLive({local:?}) which already has storage here"), + ); + } + } _ => {} } @@ -95,9 +107,9 @@ 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.reachable_blocks.contains(location.block) { + if self.is_fn_like { self.maybe_storage_live.seek_after_primary_effect(location); for local in self.maybe_storage_live.get().iter() { if !self.always_live_locals.contains(local) { @@ -111,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, + ), + ); + } + } _ => {} } diff --git a/compiler/rustc_mir_transform/src/pass_manager.rs b/compiler/rustc_mir_transform/src/pass_manager.rs index 82074f1960d..f4c572aec12 100644 --- a/compiler/rustc_mir_transform/src/pass_manager.rs +++ b/compiler/rustc_mir_transform/src/pass_manager.rs @@ -109,14 +109,15 @@ fn run_passes_inner<'tcx>( phase_change: Option<MirPhase>, validate_each: bool, ) { - let lint = tcx.sess.opts.unstable_opts.lint_mir & !body.should_skip(); - let validate = validate_each & tcx.sess.opts.unstable_opts.validate_mir & !body.should_skip(); let overridden_passes = &tcx.sess.opts.unstable_opts.mir_enable_passes; trace!(?overridden_passes); let prof_arg = tcx.sess.prof.enabled().then(|| format!("{:?}", body.source.def_id())); if !body.should_skip() { + let validate = validate_each & tcx.sess.opts.unstable_opts.validate_mir; + let lint = tcx.sess.opts.unstable_opts.lint_mir; + for pass in passes { let name = pass.name(); @@ -162,7 +163,12 @@ fn run_passes_inner<'tcx>( body.pass_count = 0; dump_mir_for_phase_change(tcx, body); - if validate || new_phase == MirPhase::Runtime(RuntimePhase::Optimized) { + + let validate = + (validate_each & tcx.sess.opts.unstable_opts.validate_mir & !body.should_skip()) + || new_phase == MirPhase::Runtime(RuntimePhase::Optimized); + let lint = tcx.sess.opts.unstable_opts.lint_mir & !body.should_skip(); + if validate { validate_body(tcx, body, format!("after phase change to {}", new_phase.name())); } if lint { |