From 39f7b35327cd4747da1a20a187fbaf220ee4a09c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Oct 2020 12:04:39 +0100 Subject: [PATCH] Stacked Borrows: print affected memory location on errors --- src/range_map.rs | 30 ++++++++++++---------- src/stacked_borrows.rs | 56 ++++++++++++++++++++---------------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/range_map.rs b/src/range_map.rs index 16ad5fd7c2b..607c830530e 100644 --- a/src/range_map.rs +++ b/src/range_map.rs @@ -61,7 +61,9 @@ fn find_offset(&self, offset: u64) -> usize { /// Provides read-only iteration over everything in the given range. This does /// *not* split items if they overlap with the edges. Do not use this to mutate /// through interior mutability. - pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator + 'a { + /// + /// The iterator also provides the offset of the given element. + pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator + 'a { let offset = offset.bytes(); let len = len.bytes(); // Compute a slice starting with the elements we care about. @@ -75,7 +77,7 @@ pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator(&'a mut self) -> impl Iterator + 'a { @@ -112,11 +114,13 @@ fn split_index(&mut self, index: usize, split_offset: u64) -> bool /// this will split entries in the map that are only partially hit by the given range, /// to make sure that when they are mutated, the effect is constrained to the given range. /// Moreover, this will opportunistically merge neighbouring equal blocks. + /// + /// The iterator also provides the offset of the given element. pub fn iter_mut<'a>( &'a mut self, offset: Size, len: Size, - ) -> impl Iterator + 'a + ) -> impl Iterator + 'a where T: Clone + PartialEq, { @@ -197,7 +201,7 @@ pub fn iter_mut<'a>( // Now we yield the slice. `end` is inclusive. &mut self.v[first_idx..=end_idx] }; - slice.iter_mut().map(|elem| &mut elem.data) + slice.iter_mut().map(|elem| (Size::from_bytes(elem.range.start), &mut elem.data)) } } @@ -209,7 +213,7 @@ mod tests { fn to_vec(map: &RangeMap, offset: u64, len: u64) -> Vec { (offset..offset + len) .into_iter() - .map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|&t| t).unwrap()) + .map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|(_, &t)| t).unwrap()) .collect() } @@ -217,7 +221,7 @@ fn to_vec(map: &RangeMap, offset: u64, len: u64) -> Vec { fn basic_insert() { let mut map = RangeMap::::new(Size::from_bytes(20), -1); // Insert. - for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) { + for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) { *x = 42; } // Check. @@ -225,10 +229,10 @@ fn basic_insert() { assert_eq!(map.v.len(), 3); // Insert with size 0. - for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) { + for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) { *x = 19; } - for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) { + for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) { *x = 19; } assert_eq!(to_vec(&map, 10, 2), vec![42, -1]); @@ -238,16 +242,16 @@ fn basic_insert() { #[test] fn gaps() { let mut map = RangeMap::::new(Size::from_bytes(20), -1); - for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) { + for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) { *x = 42; } - for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) { + for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) { *x = 43; } assert_eq!(map.v.len(), 5); assert_eq!(to_vec(&map, 10, 10), vec![-1, 42, -1, -1, -1, 43, -1, -1, -1, -1]); - for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) { + for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) { if *x < 42 { *x = 23; } @@ -256,14 +260,14 @@ fn gaps() { assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 43, 23, 23, 23, 23]); assert_eq!(to_vec(&map, 13, 5), vec![23, 23, 43, 23, 23]); - for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) { + for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) { *x = 19; } assert_eq!(map.v.len(), 6); assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 19, 19, 19, 19, 19]); // Should be seeing two blocks with 19. assert_eq!( - map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|&t| t).collect::>(), + map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|(_, &t)| t).collect::>(), vec![19, 19] ); diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 257208056d7..cf5b31597da 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -309,14 +309,14 @@ fn check_protector(item: &Item, tag: Option, global: &GlobalState) -> Inter /// Test if a memory `access` using pointer tagged `tag` is granted. /// If yes, return the index of the item that granted it. - fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> { + fn access(&mut self, access: AccessKind, ptr: Pointer, global: &GlobalState) -> InterpResult<'tcx> { // Two main steps: Find granting item, remove incompatible items above. // Step 1: Find granting item. - let granting_idx = self.find_granting(access, tag).ok_or_else(|| { + let granting_idx = self.find_granting(access, ptr.tag).ok_or_else(|| { err_sb_ub(format!( - "no item granting {} to tag {:?} found in borrow stack.", - access, tag + "no item granting {} to tag {:?} at {} found in borrow stack.", + access, ptr.tag, ptr.erase_tag(), )) })?; @@ -328,7 +328,7 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte let first_incompatible_idx = self.find_first_write_incompatible(granting_idx); for item in self.borrows.drain(first_incompatible_idx..).rev() { trace!("access: popping item {:?}", item); - Stack::check_protector(&item, Some(tag), global)?; + Stack::check_protector(&item, Some(ptr.tag), global)?; } } else { // On a read, *disable* all `Unique` above the granting item. This ensures U2 for read accesses. @@ -343,7 +343,7 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte let item = &mut self.borrows[idx]; if item.perm == Permission::Unique { trace!("access: disabling item {:?}", item); - Stack::check_protector(item, Some(tag), global)?; + Stack::check_protector(item, Some(ptr.tag), global)?; item.perm = Permission::Disabled; } } @@ -355,12 +355,12 @@ fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> Inte /// Deallocate a location: Like a write access, but also there must be no /// active protectors at all because we will remove all items. - fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> { + fn dealloc(&mut self, ptr: Pointer, global: &GlobalState) -> InterpResult<'tcx> { // Step 1: Find granting item. - self.find_granting(AccessKind::Write, tag).ok_or_else(|| { + self.find_granting(AccessKind::Write, ptr.tag).ok_or_else(|| { err_sb_ub(format!( - "no item granting write access for deallocation to tag {:?} found in borrow stack", - tag, + "no item granting write access for deallocation to tag {:?} at {} found in borrow stack", + ptr.tag, ptr.erase_tag(), )) })?; @@ -372,20 +372,20 @@ fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> { Ok(()) } - /// Derived a new pointer from one with the given tag. + /// Derive a new pointer from one with the given tag. /// `weak` controls whether this operation is weak or strong: weak granting does not act as /// an access, and they add the new item directly on top of the one it is derived /// from instead of all the way at the top of the stack. - fn grant(&mut self, derived_from: Tag, new: Item, global: &GlobalState) -> InterpResult<'tcx> { + fn grant(&mut self, derived_from: Pointer, new: Item, global: &GlobalState) -> InterpResult<'tcx> { // Figure out which access `perm` corresponds to. let access = if new.perm.grants(AccessKind::Write) { AccessKind::Write } else { AccessKind::Read }; // Now we figure out which item grants our parent (`derived_from`) this kind of access. // We use that to determine where to put the new item. - let granting_idx = self.find_granting(access, derived_from) + let granting_idx = self.find_granting(access, derived_from.tag) .ok_or_else(|| err_sb_ub(format!( - "trying to reborrow for {:?}, but parent tag {:?} does not have an appropriate item in the borrow stack", - new.perm, derived_from, + "trying to reborrow for {:?} at {}, but parent tag {:?} does not have an appropriate item in the borrow stack", + new.perm, derived_from.erase_tag(), derived_from.tag, )))?; // Compute where to put the new item. @@ -443,12 +443,14 @@ fn for_each( &self, ptr: Pointer, size: Size, - f: impl Fn(&mut Stack, &GlobalState) -> InterpResult<'tcx>, + f: impl Fn(Pointer, &mut Stack, &GlobalState) -> InterpResult<'tcx>, ) -> InterpResult<'tcx> { let global = self.global.borrow(); let mut stacks = self.stacks.borrow_mut(); - for stack in stacks.iter_mut(ptr.offset, size) { - f(stack, &*global)?; + for (offset, stack) in stacks.iter_mut(ptr.offset, size) { + let mut cur_ptr = ptr; + cur_ptr.offset = offset; + f(cur_ptr, stack, &*global)?; } Ok(()) } @@ -487,19 +489,13 @@ pub fn new_allocation( #[inline(always)] pub fn memory_read<'tcx>(&self, ptr: Pointer, size: Size) -> InterpResult<'tcx> { trace!("read access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes()); - self.for_each(ptr, size, |stack, global| { - stack.access(AccessKind::Read, ptr.tag, global)?; - Ok(()) - }) + self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Read, ptr, global)) } #[inline(always)] pub fn memory_written<'tcx>(&mut self, ptr: Pointer, size: Size) -> InterpResult<'tcx> { trace!("write access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes()); - self.for_each(ptr, size, |stack, global| { - stack.access(AccessKind::Write, ptr.tag, global)?; - Ok(()) - }) + self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Write, ptr, global)) } #[inline(always)] @@ -509,7 +505,7 @@ pub fn memory_deallocated<'tcx>( size: Size, ) -> InterpResult<'tcx> { trace!("deallocation with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes()); - self.for_each(ptr, size, |stack, global| stack.dealloc(ptr.tag, global)) + self.for_each(ptr, size, |ptr, stack, global| stack.dealloc(ptr, global)) } } @@ -561,14 +557,14 @@ fn reborrow( Permission::SharedReadWrite }; let item = Item { perm, tag: new_tag, protector }; - stacked_borrows.for_each(cur_ptr, size, |stack, global| { - stack.grant(cur_ptr.tag, item, global) + stacked_borrows.for_each(cur_ptr, size, |cur_ptr, stack, global| { + stack.grant(cur_ptr, item, global) }) }); } }; let item = Item { perm, tag: new_tag, protector }; - stacked_borrows.for_each(ptr, size, |stack, global| stack.grant(ptr.tag, item, global)) + stacked_borrows.for_each(ptr, size, |ptr, stack, global| stack.grant(ptr, item, global)) } /// Retags an indidual pointer, returning the retagged version. -- 2.44.0