about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-08-26 08:44:10 +0000
committerbors <bors@rust-lang.org>2024-08-26 08:44:10 +0000
commit22572d0994593197593e2a1b7b18d720a9a349a7 (patch)
tree127548e1d05f81029098dcb0af783d0815211c13
parentf48062e7d040579ac5705e1b36b34675000da5dd (diff)
parent74a21425cca6742f08a6df25bb153a3b7a43f56a (diff)
downloadrust-22572d0994593197593e2a1b7b18d720a9a349a7.tar.gz
rust-22572d0994593197593e2a1b7b18d720a9a349a7.zip
Auto merge of #129508 - RalfJung:transient-locals, r=cjgillot
const checking: properly compute the set of transient locals

For const-checking the MIR of a const/static initializer, we have to know the set of "transient" locals. The reason for this is that creating a mutable (or interior mutable) reference to a transient local is "safe" in the sense that this reference cannot possibly end up in the final value of the const -- even if it is turned into a raw pointer and stored in a union, we will see that pointer during interning and reliably reject it as dangling.

So far, we determined the set of transient locals as "locals that have a `StorageDead` somewhere". But that's not quite right -- if we had MIR like
```rust
StorageLive(_5);
StorageDead(_5);
StorageLive(_5);
// no further storage annotations for _5
```
Then we'd consider `_5` to be "transient", but it is not actually transient.

We do not currently generate such MIR, but I feel uneasy relying on subtle invariants like this. So this PR implements a proper analysis for computing the set of "transient" locals: a local is "transient" if it is guaranteed dead at all `Return` terminators.

Cc `@cjgillot`
-rw-r--r--compiler/rustc_const_eval/src/check_consts/check.rs50
1 files changed, 32 insertions, 18 deletions
diff --git a/compiler/rustc_const_eval/src/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs
index c80b3e673b0..6a086a3a7e5 100644
--- a/compiler/rustc_const_eval/src/check_consts/check.rs
+++ b/compiler/rustc_const_eval/src/check_consts/check.rs
@@ -1,6 +1,7 @@
 //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations.
 
 use std::assert_matches::assert_matches;
+use std::borrow::Cow;
 use std::mem;
 use std::ops::Deref;
 
@@ -15,6 +16,8 @@ use rustc_middle::mir::*;
 use rustc_middle::span_bug;
 use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt, TypeVisitableExt};
+use rustc_mir_dataflow::impls::MaybeStorageLive;
+use rustc_mir_dataflow::storage::always_storage_live_locals;
 use rustc_mir_dataflow::Analysis;
 use rustc_span::{sym, Span, Symbol, DUMMY_SP};
 use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
@@ -188,8 +191,9 @@ pub struct Checker<'mir, 'tcx> {
     /// The span of the current statement.
     span: Span,
 
-    /// A set that stores for each local whether it has a `StorageDead` for it somewhere.
-    local_has_storage_dead: Option<BitSet<Local>>,
+    /// A set that stores for each local whether it is "transient", i.e. guaranteed to be dead
+    /// when this MIR body returns.
+    transient_locals: Option<BitSet<Local>>,
 
     error_emitted: Option<ErrorGuaranteed>,
     secondary_errors: Vec<Diag<'tcx>>,
@@ -209,7 +213,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
             span: ccx.body.span,
             ccx,
             qualifs: Default::default(),
-            local_has_storage_dead: None,
+            transient_locals: None,
             error_emitted: None,
             secondary_errors: Vec::new(),
         }
@@ -264,23 +268,33 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
         }
     }
 
-    fn local_has_storage_dead(&mut self, local: Local) -> bool {
+    fn local_is_transient(&mut self, local: Local) -> bool {
         let ccx = self.ccx;
-        self.local_has_storage_dead
+        self.transient_locals
             .get_or_insert_with(|| {
-                struct StorageDeads {
-                    locals: BitSet<Local>,
-                }
-                impl<'tcx> Visitor<'tcx> for StorageDeads {
-                    fn visit_statement(&mut self, stmt: &Statement<'tcx>, _: Location) {
-                        if let StatementKind::StorageDead(l) = stmt.kind {
-                            self.locals.insert(l);
-                        }
+                // A local is "transient" if it is guaranteed dead at all `Return`.
+                // So first compute the say of "maybe live" locals at each program point.
+                let always_live_locals = &always_storage_live_locals(&ccx.body);
+                let mut maybe_storage_live =
+                    MaybeStorageLive::new(Cow::Borrowed(always_live_locals))
+                        .into_engine(ccx.tcx, &ccx.body)
+                        .iterate_to_fixpoint()
+                        .into_results_cursor(&ccx.body);
+
+                // And then check all `Return` in the MIR, and if a local is "maybe live" at a
+                // `Return` then it is definitely not transient.
+                let mut transient = BitSet::new_filled(ccx.body.local_decls.len());
+                // Make sure to only visit reachable blocks, the dataflow engine can ICE otherwise.
+                for (bb, data) in traversal::reachable(&ccx.body) {
+                    if matches!(data.terminator().kind, TerminatorKind::Return) {
+                        let location = ccx.body.terminator_loc(bb);
+                        maybe_storage_live.seek_after_primary_effect(location);
+                        // If a local may be live here, it is definitely not transient.
+                        transient.subtract(maybe_storage_live.get());
                     }
                 }
-                let mut v = StorageDeads { locals: BitSet::new_empty(ccx.body.local_decls.len()) };
-                v.visit_body(ccx.body);
-                v.locals
+
+                transient
             })
             .contains(local)
     }
@@ -375,7 +389,7 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
                 // `StorageDead` in every control flow path leading to a `return` terminator.
                 // The good news is that interning will detect if any unexpected mutable
                 // pointer slips through.
-                if place.is_indirect() || self.local_has_storage_dead(place.local) {
+                if place.is_indirect() || self.local_is_transient(place.local) {
                     self.check_op(ops::TransientMutBorrow(kind));
                 } else {
                     self.check_op(ops::MutBorrow(kind));
@@ -526,7 +540,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                             // `StorageDead` in every control flow path leading to a `return` terminator.
                             // The good news is that interning will detect if any unexpected mutable
                             // pointer slips through.
-                            if self.local_has_storage_dead(place.local) {
+                            if self.local_is_transient(place.local) {
                                 self.check_op(ops::TransientCellBorrow);
                             } else {
                                 self.check_op(ops::CellBorrow);