Rollup merge of #119077 - tmiasko:lint, r=cjgillot

Separate MIR lints from validation

Add a MIR lint pass, enabled with -Zlint-mir, which identifies undefined or
likely erroneous behaviour.

The initial implementation mostly migrates existing checks of this nature from
MIR validator, where they did not belong (those checks have false positives and
there is nothing inherently invalid about MIR with undefined behaviour).

Fixes #104736
Fixes #104843
Fixes #116079
Fixes #116736
Fixes #118990

compiler/rustc_mir_transform/src/lib.rs

@@ -86,6 +86,7 @@ pub mod inline;
 mod instsimplify;
 mod jump_threading;
 mod large_enums;
+mod lint;
 mod lower_intrinsics;
 mod lower_slice_len;
 mod match_branches;

compiler/rustc_mir_transform/src/lint.rs

@@ -0,0 +1,119 @@
+//! 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_index::bit_set::BitSet;
+use rustc_middle::mir::visit::{PlaceContext, Visitor};
+use rustc_middle::mir::*;
+use rustc_middle::ty::TyCtxt;
+use rustc_mir_dataflow::impls::{MaybeStorageDead, MaybeStorageLive};
+use rustc_mir_dataflow::storage::always_storage_live_locals;
+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))
+        .into_engine(tcx, body)
+        .iterate_to_fixpoint()
+        .into_results_cursor(body);
+
+    let maybe_storage_dead = MaybeStorageDead::new(Cow::Borrowed(always_live_locals))
+        .into_engine(tcx, body)
+        .iterate_to_fixpoint()
+        .into_results_cursor(body);
+
+    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,
+    }
+    .visit_body(body);
+}
+
+struct Lint<'a, 'tcx> {
+    tcx: TyCtxt<'tcx>,
+    when: String,
+    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>>,
+}
+
+impl<'a, 'tcx> Lint<'a, 'tcx> {
+    #[track_caller]
+    fn fail(&self, location: Location, msg: impl AsRef<str>) {
+        let span = self.body.source_info(location).span;
+        self.tcx.sess.dcx().span_delayed_bug(
+            span,
+            format!(
+                "broken MIR in {:?} ({}) at {:?}:\n{}",
+                self.body.source.instance,
+                self.when,
+                location,
+                msg.as_ref()
+            ),
+        );
+    }
+}
+
+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() {
+            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"));
+            }
+        }
+    }
+
+    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) {
+                        self.fail(
+                            location,
+                            format!("StorageLive({local:?}) which already has storage here"),
+                        );
+                    }
+                }
+            }
+            _ => {}
+        }
+
+        self.super_statement(statement, location);
+    }
+
+    fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
+        match terminator.kind {
+            TerminatorKind::Return => {
+                if self.is_fn_like && self.reachable_blocks.contains(location.block) {
+                    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) {
+                            self.fail(
+                                location,
+                                format!(
+                                    "local {local:?} still has storage when returning from function"
+                                ),
+                            );
+                        }
+                    }
+                }
+            }
+            _ => {}
+        }
+
+        self.super_terminator(terminator, location);
+    }
+}

compiler/rustc_mir_transform/src/pass_manager.rs

@@ -2,7 +2,7 @@ use rustc_middle::mir::{self, Body, MirPhase, RuntimePhase};
 use rustc_middle::ty::TyCtxt;
 use rustc_session::Session;
 
-use crate::{validate, MirPass};
+use crate::{lint::lint_body, validate, MirPass};
 
 /// Just like `MirPass`, except it cannot mutate `Body`.
 pub trait MirLint<'tcx> {
@@ -109,6 +109,7 @@ 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);
@@ -131,6 +132,9 @@ fn run_passes_inner<'tcx>(
             if validate {
                 validate_body(tcx, body, format!("before pass {name}"));
             }
+            if lint {
+                lint_body(tcx, body, format!("before pass {name}"));
+            }
 
             if let Some(prof_arg) = &prof_arg {
                 tcx.sess
@@ -147,6 +151,9 @@ fn run_passes_inner<'tcx>(
             if validate {
                 validate_body(tcx, body, format!("after pass {name}"));
             }
+            if lint {
+                lint_body(tcx, body, format!("after pass {name}"));
+            }
 
             body.pass_count += 1;
         }
@@ -164,6 +171,9 @@ fn run_passes_inner<'tcx>(
         if validate || new_phase == MirPhase::Runtime(RuntimePhase::Optimized) {
             validate_body(tcx, body, format!("after phase change to {}", new_phase.name()));
         }
+        if lint {
+            lint_body(tcx, body, format!("after phase change to {}", new_phase.name()));
+        }
 
         body.pass_count = 1;
     }

compiler/rustc_mir_transform/src/ref_prop.rs

@@ -7,6 +7,7 @@ use rustc_middle::ty::TyCtxt;
 use rustc_mir_dataflow::impls::MaybeStorageDead;
 use rustc_mir_dataflow::storage::always_storage_live_locals;
 use rustc_mir_dataflow::Analysis;
+use std::borrow::Cow;
 
 use crate::ssa::{SsaLocals, StorageLiveLocals};
 
@@ -120,7 +121,7 @@ fn compute_replacement<'tcx>(
 
     // Compute `MaybeStorageDead` dataflow to check that we only replace when the pointee is
     // definitely live.
-    let mut maybe_dead = MaybeStorageDead::new(always_live_locals)
+    let mut maybe_dead = MaybeStorageDead::new(Cow::Owned(always_live_locals))
         .into_engine(tcx, body)
         .iterate_to_fixpoint()
         .into_results_cursor(body);