diff --git a/creusot/src/analysis/not_final_places.rs b/creusot/src/analysis/not_final_places.rs index 4d0f021c6a..ae8b22ca42 100644 --- a/creusot/src/analysis/not_final_places.rs +++ b/creusot/src/analysis/not_final_places.rs @@ -1,9 +1,12 @@ use crate::extended_location::ExtendedLocation; +use rustc_borrowck::consumers::PlaceConflictBias; use rustc_index::bit_set::ChunkedBitSet; use rustc_middle::{ mir::{ - self, visit::Visitor, BasicBlock, Body, CallReturnPlaces, Location, Place, PlaceElem, - PlaceRef, ProjectionElem, Statement, Terminator, TerminatorEdges, + self, + visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}, + BasicBlock, CallReturnPlaces, Location, Place, PlaceRef, ProjectionElem, Statement, + Terminator, TerminatorEdges, }, ty::TyCtxt, }; @@ -72,32 +75,40 @@ pub struct NotFinalPlaces<'tcx> { impl<'tcx> NotFinalPlaces<'tcx> { pub fn new(tcx: TyCtxt<'tcx>, body: &mir::Body<'tcx>) -> Self { - struct VisitAllPlaces<'tcx>(HashMap, PlaceId>); + struct VisitAllPlaces<'tcx> { + places: HashMap, PlaceId>, + } impl<'tcx> Visitor<'tcx> for VisitAllPlaces<'tcx> { - fn visit_place( - &mut self, - place: &Place<'tcx>, - _: mir::visit::PlaceContext, - _: Location, - ) { + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _: Location) { let place_ref = place.as_ref(); + // This is both an optimization, and required for `rustc_borrowck::consumers::places_conflict` to not crash. + // This is ok to do, because those are places we will not consider anyways. + if !(place_context_gen(context) + || place_context_gen_deref(context) + || place_context_kill(context)) + { + return; + } for place in std::iter::once(place_ref).chain(place_ref.iter_projections().map(|(p, _)| p)) { - let idx = self.0.len(); - if let std::collections::hash_map::Entry::Vacant(entry) = self.0.entry(place) { + let idx = self.places.len(); + if let std::collections::hash_map::Entry::Vacant(entry) = + self.places.entry(place) + { entry.insert(idx); } } } } - let mut visitor = VisitAllPlaces(HashMap::new()); + let mut visitor = VisitAllPlaces { places: HashMap::new() }; visitor.visit_body(body); - let places = visitor.0; + let places = visitor.places; let mut has_dereference = ChunkedBitSet::new_empty(places.len()); let mut subplaces = vec![Vec::new(); places.len()]; let mut conflicting_places = vec![Vec::new(); places.len()]; + for (&place, &id) in &places { if Self::place_get_first_deref(place, body, tcx).is_some() { has_dereference.insert(id); @@ -109,7 +120,18 @@ impl<'tcx> NotFinalPlaces<'tcx> { if other_place.projection.get(..place.projection.len()) == Some(place.projection) { subplaces[id].push(other_place); } - if places_conflict(tcx, body, place, other_place) { + + // We use `PlaceConflictBias::Overlap`, because we assume that unknown index accesses _do_ overlap. + + // This function would crash if `place` is a `*x` where `x: &T`. + // But we filtered such places in the visitor. + if rustc_borrowck::consumers::places_conflict( + tcx, + body, + place.to_place(tcx), + other_place.to_place(tcx), + PlaceConflictBias::Overlap, + ) { conflicting_places[id].push(other_place); } } @@ -190,252 +212,6 @@ impl<'tcx> NotFinalPlaces<'tcx> { } } -/// Helper function for checking if two places conflict. -/// -/// Common conflicting cases include: -/// - Both places are the same: `*a.b` and `*a.b` -/// - One place is a subplace of the other: `a.b.c` and `a.b` -/// - There is a runtime indirection: `a[i]` and `a[j]` -// FIXME: this is mostly copied from `rustc_borrowck::places_conflict`: it would be nice if the corresponding function would be public. -fn places_conflict<'tcx>( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - place1: PlaceRef<'tcx>, - place2: PlaceRef<'tcx>, -) -> bool { - if place1.local != place2.local { - // We have proven the borrow disjoint - further projections will remain disjoint. - return false; - } - - place_components_conflict(tcx, body, place1, place2) -} - -enum Overlap { - /// Places components are garanteed disjoint (e.g. `a.b` and `a.c`) - Disjoint, - /// Places pass through different fields of an union: we consider this case to be a conflict. - Arbitrary, - /// Places components might lead to the same place (e.g. `a.b` and `a.b`, or `a[i]` and `a[j]`) - EqualOrDisjoint, -} - -fn place_components_conflict<'tcx>( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - place1: PlaceRef<'tcx>, - place2: PlaceRef<'tcx>, -) -> bool { - // loop invariant: borrow_c is always either equal to access_c or disjoint from it. - for ((borrow_place, borrow_c), &access_c) in - std::iter::zip(place1.iter_projections(), place2.projection) - { - // Borrow and access path both have more components. - // - // Examples: - // - // - borrow of `a.(...)`, access to `a.(...)` - // - borrow of `a.(...)`, access to `b.(...)` - // - // Here we only see the components we have checked so - // far (in our examples, just the first component). We - // check whether the components being borrowed vs - // accessed are disjoint (as in the second example, - // but not the first). - match place_projection_conflict(tcx, body, borrow_place, borrow_c, access_c) { - Overlap::Arbitrary => { - // We have encountered different fields of potentially - // the same union - the borrow now partially overlaps. - // - // There is no *easy* way of comparing the fields - // further on, because they might have different types - // (e.g., borrows of `u.a.0` and `u.b.y` where `.0` and - // `.y` come from different structs). - return true; - } - Overlap::EqualOrDisjoint => { - // This is the recursive case - proceed to the next element. - } - Overlap::Disjoint => { - // We have proven the borrow disjoint - further - // projections will remain disjoint. - return false; - } - } - } - - // One place is a subplace of the other, e.g. `a.b` and `a.b.c`. - true -} - -// Given that the bases of `elem1` and `elem2` are always either equal -// or disjoint (and have the same type!), return the overlap situation -// between `elem1` and `elem2`. -fn place_projection_conflict<'tcx>( - tcx: TyCtxt<'tcx>, - body: &Body<'tcx>, - pi1: PlaceRef<'tcx>, - pi1_elem: PlaceElem<'tcx>, - pi2_elem: PlaceElem<'tcx>, -) -> Overlap { - match (pi1_elem, pi2_elem) { - (ProjectionElem::Deref, ProjectionElem::Deref) => { - // derefs (e.g., `*x` vs. `*x`) - recur. - Overlap::EqualOrDisjoint - } - (ProjectionElem::OpaqueCast(_), ProjectionElem::OpaqueCast(_)) => { - // casts to other types may always conflict irrespective of the type being cast to. - Overlap::EqualOrDisjoint - } - (ProjectionElem::Field(f1, _), ProjectionElem::Field(f2, _)) => { - if f1 == f2 { - // same field (e.g., `a.y` vs. `a.y`) - recur. - Overlap::EqualOrDisjoint - } else { - let ty = pi1.ty(body, tcx).ty; - if ty.is_union() { - // Different fields of a union, we are basically stuck. - Overlap::Arbitrary - } else { - // Different fields of a struct (`a.x` vs. `a.y`). Disjoint! - Overlap::Disjoint - } - } - } - (ProjectionElem::Downcast(_, v1), ProjectionElem::Downcast(_, v2)) => { - if v1 == v2 { - // same variant. - Overlap::EqualOrDisjoint - } else { - // even if the two variants may occupy the same space, they are disjoint since they cannot be active at the same time. The same is _not_ true for unions. - Overlap::Disjoint - } - } - ( - ProjectionElem::Index(..), - ProjectionElem::Index(..) - | ProjectionElem::ConstantIndex { .. } - | ProjectionElem::Subslice { .. }, - ) - | ( - ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. }, - ProjectionElem::Index(..), - ) => { - // Array indexes (`a[0]` vs. `a[i]`). They might overlap. - Overlap::EqualOrDisjoint - } - ( - ProjectionElem::ConstantIndex { offset: o1, min_length: _, from_end: false }, - ProjectionElem::ConstantIndex { offset: o2, min_length: _, from_end: false }, - ) - | ( - ProjectionElem::ConstantIndex { offset: o1, min_length: _, from_end: true }, - ProjectionElem::ConstantIndex { offset: o2, min_length: _, from_end: true }, - ) => { - if o1 == o2 { - Overlap::EqualOrDisjoint - } else { - Overlap::Disjoint - } - } - ( - ProjectionElem::ConstantIndex { - offset: offset_from_begin, - min_length: min_length1, - from_end: false, - }, - ProjectionElem::ConstantIndex { - offset: offset_from_end, - min_length: min_length2, - from_end: true, - }, - ) - | ( - ProjectionElem::ConstantIndex { - offset: offset_from_end, - min_length: min_length1, - from_end: true, - }, - ProjectionElem::ConstantIndex { - offset: offset_from_begin, - min_length: min_length2, - from_end: false, - }, - ) => { - // both patterns matched so it must be at least the greater of the two - let min_length = std::cmp::max(min_length1, min_length2); - // `offset_from_end` can be in range `[1..min_length]`, 1 indicates the last - // element (like -1 in Python) and `min_length` the first. - // Therefore, `min_length - offset_from_end` gives the minimal possible - // offset from the beginning - if offset_from_begin >= min_length - offset_from_end { - Overlap::EqualOrDisjoint - } else { - Overlap::Disjoint - } - } - ( - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }, - ProjectionElem::Subslice { from, to, from_end: false }, - ) - | ( - ProjectionElem::Subslice { from, to, from_end: false }, - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }, - ) => { - if (from..to).contains(&offset) { - Overlap::EqualOrDisjoint - } else { - Overlap::Disjoint - } - } - ( - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }, - ProjectionElem::Subslice { from, .. }, - ) - | ( - ProjectionElem::Subslice { from, .. }, - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }, - ) => { - if offset >= from { - Overlap::EqualOrDisjoint - } else { - Overlap::Disjoint - } - } - ( - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true }, - ProjectionElem::Subslice { to, from_end: true, .. }, - ) - | ( - ProjectionElem::Subslice { to, from_end: true, .. }, - ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true }, - ) => { - if offset > to { - Overlap::EqualOrDisjoint - } else { - Overlap::Disjoint - } - } - ( - ProjectionElem::Subslice { from: f1, to: t1, from_end: false }, - ProjectionElem::Subslice { from: f2, to: t2, from_end: false }, - ) => { - if f2 >= t1 || f1 >= t2 { - Overlap::Disjoint - } else { - Overlap::EqualOrDisjoint - } - } - (ProjectionElem::Subslice { .. }, ProjectionElem::Subslice { .. }) => { - Overlap::EqualOrDisjoint - } - _ => panic!( - "mismatched projections in place_element_conflict: {:?} and {:?}", - pi1_elem, pi2_elem - ), - } -} - impl<'tcx> DebugWithContext> for ChunkedBitSet {} impl<'tcx> AnalysisDomain<'tcx> for NotFinalPlaces<'tcx> { @@ -508,6 +284,50 @@ impl<'tcx> GenKillAnalysis<'tcx> for NotFinalPlaces<'tcx> { } } +/// Determine when a place should be unconditionnaly 'gen'ed' in the analysis +fn place_context_gen(context: PlaceContext) -> bool { + match context { + // Although they read the borrow, most `NonMutatingUse` are fine, because they can't influence the prophecized value. + // The only non-mutating uses we need to consider are: + // - `Move`: the borrow may be used somewhere else to change its prophecy. + // - 'Copy': cannot be emitted for a mutable borrow + PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) + // Note that a borrow triggers a gen + // e.g. + // ```rust + // let bor = &mut x; + // let r1 = &mut *bor; + // // ... + // let r2 = &mut *bor; // r1 is not final ! + // ``` + | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => true, + _ => false, + } +} +/// Determine when a place should be 'gen'ed' in the analysis. The caller should then also +/// check that the write occurs under a dereference. +fn place_context_gen_deref(context: PlaceContext) -> bool { + match context { + PlaceContext::MutatingUse(MutatingUseContext::Store) + | PlaceContext::MutatingUse(MutatingUseContext::Call) + | PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant) + | PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) => true, + _ => false, + } +} +/// Determine when a place should be 'killed' in the analysis +fn place_context_kill(context: PlaceContext) -> bool { + matches!( + context, + PlaceContext::MutatingUse( + MutatingUseContext::Store + | MutatingUseContext::Call + // Technically true, but unsupported by Creusot anyways + | MutatingUseContext::AsmOutput, + ) + ) +} + struct PlaceVisitorGen<'a, 'tcx, T> { mapping: &'a NotFinalPlaces<'tcx>, trans: &'a mut T, @@ -516,36 +336,15 @@ impl<'a, 'tcx, T> Visitor<'tcx> for PlaceVisitorGen<'a, 'tcx, T> where T: GenKill, { - fn visit_place(&mut self, place: &Place<'tcx>, context: mir::visit::PlaceContext, _: Location) { - use mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext}; - match context { - // Although they read the borrow, most `NonMutatingUse` are fine, because they can't influence the prophecized value. - // The only non-mutating uses we need to consider are: - // - `Move`: the borrow may be used somewhere else to change its prophecy. - // - 'Copy': cannot be emitted for a mutable borrow - PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) - // Note that a borrow triggers a gen - // e.g. - // ```rust - // let bor = &mut x; - // let r1 = &mut *bor; - // // ... - // let r2 = &mut *bor; // r1 is not final ! - // ``` - | PlaceContext::MutatingUse(MutatingUseContext::Borrow) => { - self.trans.gen_(self.mapping.places[&place.as_ref()]) - } - PlaceContext::MutatingUse(MutatingUseContext::Store) - | PlaceContext::MutatingUse(MutatingUseContext::Call) - | PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant) - | PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) => { - let id = self.mapping.places[&place.as_ref()]; - if self.mapping.has_dereference.contains(id) { - // We are writing under a dereference, so this changes the prophecy of the underlying borrow. - self.trans.gen_(id); - } + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _: Location) { + if place_context_gen(context) { + self.trans.gen_(self.mapping.places[&place.as_ref()]) + } else if place_context_gen_deref(context) { + let id = self.mapping.places[&place.as_ref()]; + if self.mapping.has_dereference.contains(id) { + // We are writing under a dereference, so this changes the prophecy of the underlying borrow. + self.trans.gen_(id); } - _ => {} } } } @@ -558,24 +357,15 @@ impl<'a, 'tcx, T> Visitor<'tcx> for PlaceVisitorKill<'a, 'tcx, T> where T: GenKill, { - fn visit_place(&mut self, place: &Place<'tcx>, context: mir::visit::PlaceContext, _: Location) { - use mir::visit::{MutatingUseContext, PlaceContext}; - match context { - PlaceContext::MutatingUse( - MutatingUseContext::Store - | MutatingUseContext::Call - // Technically true, but unsupported by Creusot anyways - | MutatingUseContext::AsmOutput, - ) => { - let id: usize = self.mapping.places[&place.as_ref()]; - self.trans.kill(id); - // Now, kill all subplaces of this place - for subplace in self.mapping.subplaces[id].iter() { - let subplace_id: usize = self.mapping.places[subplace]; - self.trans.kill(subplace_id); - } + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _: Location) { + if place_context_kill(context) { + let id: usize = self.mapping.places[&place.as_ref()]; + self.trans.kill(id); + // Now, kill all subplaces of this place + for subplace in self.mapping.subplaces[id].iter() { + let subplace_id: usize = self.mapping.places[subplace]; + self.trans.kill(subplace_id); } - _ => {} } } }