Skip to content

Commit

Permalink
Include dereferences in slices of pointers, add special-case precisio…
Browse files Browse the repository at this point in the history
…n improvement for abstract regions
  • Loading branch information
willcrichton committed Oct 14, 2021
1 parent d83de42 commit 4ed56b7
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 32 deletions.
29 changes: 26 additions & 3 deletions src/core/aliases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ impl Aliases<'tcx> {
body_with_facts: &BodyWithBorrowckFacts<'tcx>,
region_to_pointers: &HashMap<RegionVid, Vec<(Place<'tcx>, Mutability)>>,
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> IndexVec<RegionVid, BitSet<RegionVid>> {
let outlives_constraints = body_with_facts
.input_facts
Expand All @@ -167,12 +168,33 @@ impl Aliases<'tcx> {
.unwrap_or(0)
+ 1;

let body = &body_with_facts.body;

// All regions for references in function arguments
let abstract_regions = body
.args_iter()
.map(|local| {
let arg = utils::local_to_place(local, tcx);
utils::interior_pointers(arg, tcx, body, def_id).into_keys()
})
.flatten()
.collect::<HashSet<_>>();

let static_region = RegionVid::from_usize(0);
let mut processed_constraints = outlives_constraints
.clone()
.into_iter()
// static region outlives everything
//
// Static region outlives everything, so add static :> r for all r
.chain((1..max_region).map(|i| (static_region, RegionVid::from_usize(i))))
//
// Outlives-constraints on abstract regions are useful for borrow checking but aren't
// useful for alias-analysis. Eg if self : &'a mut (i32, i32) and x = &'b mut *self.0,
// then knowing 'a : 'b would naively add self to the loan set of 'b. So for increased
// precision, we can safely filter any constraints 'a : _ where 'a is abstract.
// See the interprocedural_field_independence test for an example of where this works
// and also how it breaks.
.filter(|(sup, _)| !abstract_regions.contains(sup))
.collect::<Vec<_>>();

if is_extension_active(|mode| mode.pointer_mode == PointerMode::Conservative) {
Expand Down Expand Up @@ -428,10 +450,11 @@ impl Aliases<'tcx> {
body_with_facts: &BodyWithBorrowckFacts<'tcx>,
region_to_pointers: HashMap<RegionVid, Vec<(Place<'tcx>, Mutability)>>,
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> IndexVec<RegionVid, HashSet<Place<'tcx>>> {
// Get the graph of which regions outlive which other ones
let region_ancestors =
Self::compute_region_ancestors(body_with_facts, &region_to_pointers, tcx);
Self::compute_region_ancestors(body_with_facts, &region_to_pointers, tcx, def_id);

// Initialize the loan set where loan['a] = {*x} if x: &'a T
let mut loans = IndexVec::from_elem_n(HashSet::default(), region_ancestors.len());
Expand Down Expand Up @@ -496,7 +519,7 @@ impl Aliases<'tcx> {
}

// Use outlives-constraints to get the loan set for each region
let loans = Self::compute_loans(body_with_facts, region_to_pointers, tcx);
let loans = Self::compute_loans(body_with_facts, region_to_pointers, tcx, def_id);
debug!("Loans: {:?}", {
let mut v = loans.iter_enumerated().collect::<Vec<_>>();
v.sort_by_key(|(r, _)| *r);
Expand Down
68 changes: 47 additions & 21 deletions src/flow/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub enum Direction {
}

struct ForwardVisitor<'tcx> {
targets: Vec<(Place<'tcx>, Location)>,
expanded_targets: Vec<(PlaceSet<'tcx>, Location)>,
target_deps: Vec<(LocationSet, PlaceSet<'tcx>)>,
outputs: Vec<(LocationSet, PlaceSet<'tcx>)>,
}
Expand All @@ -29,8 +29,8 @@ impl ResultsVisitor<'mir, 'tcx> for ForwardVisitor<'tcx> {
_statement: &'mir Statement<'tcx>,
location: Location,
) {
for ((target_place, _), ((locs, places), (out_locs, out_places))) in self
.targets
for ((target_places, _), ((locs, places), (out_locs, out_places))) in self
.expanded_targets
.iter()
.zip(self.target_deps.iter().zip(self.outputs.iter_mut()))
{
Expand All @@ -43,7 +43,12 @@ impl ResultsVisitor<'mir, 'tcx> for ForwardVisitor<'tcx> {
}

if let Some(place_deps) = state.places.row_set(place) {
if place_deps.contains(*target_place) && place_deps.is_superset(places) {
let contains_target = {
let mut place_deps = place_deps.to_owned();
place_deps.intersect(&target_places);
place_deps.len() > 0
};
if contains_target && place_deps.is_superset(places) {
out_places.insert(place);
}
}
Expand All @@ -63,30 +68,51 @@ pub fn compute_dependencies(
targets: Vec<(Place<'tcx>, Location)>,
direction: Direction,
) -> Vec<(LocationSet, PlaceSet<'tcx>)> {
let tcx = results.analysis.tcx;
let body = results.analysis.body;
let aliases = &results.analysis.aliases;

let new_location_set = || LocationSet::new(results.analysis.location_domain().clone());
let new_place_set = || PlaceSet::new(results.analysis.place_domain().clone());

let expanded_targets = targets
.into_iter()
.map(|(place, location)| {
let mut places = new_place_set();
places.insert(place);

for (_, ptrs) in utils::interior_pointers(place, tcx, body, results.analysis.def_id) {
for (place, _) in ptrs {
places.union(&aliases.aliases.row_set(tcx.mk_place_deref(place)).unwrap());
}
}

(places, location)
})
.collect::<Vec<_>>();

let target_deps = {
let mut cursor = ResultsRefCursor::new(body, results);
let get_deps = |(place, location): &(Place<'tcx>, Location)| {
let get_deps = |(targets, location): &(PlaceSet<'tcx>, Location)| {
cursor.seek_after_primary_effect(*location);
let state = cursor.get();
(
state
.locations
.row_set(*place)
.map(|s| s.to_owned())
.unwrap_or_else(new_location_set),
state
.places
.row_set(*place)
.map(|s| s.to_owned())
.unwrap_or_else(new_place_set),
)

let mut locations = new_location_set();
let mut places = new_place_set();

for target in targets.indices() {
if let Some(dep_locations) = state.locations.row_set(target) {
locations.union(&dep_locations);
}

if let Some(dep_places) = state.places.row_set(target) {
places.union(&dep_places);
}
}

(locations, places)
};
targets.iter().map(get_deps).collect::<Vec<_>>()
expanded_targets.iter().map(get_deps).collect::<Vec<_>>()
};

match direction {
Expand All @@ -96,12 +122,12 @@ pub fn compute_dependencies(
.iter()
.map(|_| (new_location_set(), new_place_set()))
.collect::<Vec<_>>();
for ((target_place, _), (_, places)) in targets.iter().zip(outputs.iter_mut()) {
places.insert(*target_place);
for ((target_places, _), (_, places)) in expanded_targets.iter().zip(outputs.iter_mut()) {
places.union(target_places);
}

let mut visitor = ForwardVisitor {
targets,
expanded_targets,
target_deps,
outputs,
};
Expand Down
79 changes: 71 additions & 8 deletions tests/backward_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,20 @@ fn main() {
backward_slice(src);
}

#[test]
fn pointer_slice_includes_deref() {
let src = r#"
fn main() {
`[let `[mut x]` = `[1]`;]`
`[let `[y]` = `[&mut x]`;]`
`[`[*y = 1]`;]`
`[`(y)`;]`
}
"#;

backward_slice(src);
}

#[test]
fn pointer_read() {
let src = r#"
Expand Down Expand Up @@ -594,6 +608,23 @@ fn main() {
backward_slice(src);
}

#[test]
fn pointer_mutate_pointer() {
let src = r#"
fn main() {
`[let `[mut x]` = `[1]`;]`
`[let `[mut y]` = `[1]`;]`
`[let `[mut a]` = `[&mut x]`;]`
`[let `[b]` = `[&mut a]`;]`
`[`[*b = `[&mut y]`]`;]`
`[`[**b = 2]`;]`
`[`(y)`;]`
}
"#;

backward_slice(src);
}

#[test]
fn interprocedural_output() {
let src = r#"
Expand Down Expand Up @@ -787,6 +818,40 @@ fn main() {
backward_slice(src);
}

#[test]
fn interprocedural_field_independence() {
let src = r#"
use std::ops::AddAssign;
struct Foo(i32, i32);
impl Foo {
fn bar(`[&mut self]`) {
self.0.add_assign(0);
`[`(self.1)`;]`
}
}
fn main() {}
"#;

backward_slice(src);

let src = r#"
use std::ops::AddAssign;
struct Foo(i32, i32);
impl Foo {
fn bar(`[&mut self]`) {
`[let `[a]` = `[&mut *self]`;]`
`[`[a.0.add_assign(0)]`;]`
`[`(a.1)`;]`
}
}
fn main() {}
"#;

backward_slice(src);
}

#[test]
fn interprocedural_ref_output() {
let src = r#"
Expand Down Expand Up @@ -878,9 +943,9 @@ fn main() {}
#[test]
fn function_lifetime_alias_equal() {
let src = r#"
fn foo<'a>(`[x]`: &'a mut i32, `[y]`: &'a mut i32) {
`[let `[z]` = `[x]`;]`
`[`[*z = 1]`;]`
fn foo<'a>(x: &'a mut i32, `[y]`: &'a mut i32) {
let z = x;
*z = 1;
`[`(*y)`;]`
}
Expand All @@ -892,12 +957,10 @@ fn main() {}

#[test]
fn function_lifetime_alias_outlives() {
// given our algorithm of estimating aliases from lifetimes, `w` and `x` are considered
// to aliases `y` and `z` given the constraint `'b: 'a`
let src = r#"
fn foo<'a, 'b: 'a>(`[x]`: &'a mut i32, `[y]`: &'b mut i32) -> &'a mut i32 {
`[let `[z]` = `[x]`;]`
`[`[*z = 1]`;]`
fn foo<'a, 'b: 'a>(x: &'a mut i32, `[y]`: &'b mut i32) -> &'a mut i32 {
let z = x;
*z = 1;
`[`(y)`]`
}
Expand Down

0 comments on commit 4ed56b7

Please sign in to comment.