diff options
| author | bors <bors@rust-lang.org> | 2024-08-26 08:44:10 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-08-26 08:44:10 +0000 |
| commit | 22572d0994593197593e2a1b7b18d720a9a349a7 (patch) | |
| tree | 127548e1d05f81029098dcb0af783d0815211c13 | |
| parent | f48062e7d040579ac5705e1b36b34675000da5dd (diff) | |
| parent | 74a21425cca6742f08a6df25bb153a3b7a43f56a (diff) | |
| download | rust-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.rs | 50 |
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); |
