Skip to content

Commit

Permalink
Auto merge of #129508 - RalfJung:transient-locals, r=cjgillot
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
bors committed Aug 26, 2024
2 parents f48062e + 74a2142 commit 22572d0
Showing 1 changed file with 32 additions and 18 deletions.
50 changes: 32 additions & 18 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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>>,
Expand All @@ -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(),
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 22572d0

Please sign in to comment.