Skip to content

Commit 39f7b35

Browse files
committed
Stacked Borrows: print affected memory location on errors
1 parent ecf330f commit 39f7b35

File tree

2 files changed

+43
-43
lines changed

2 files changed

+43
-43
lines changed

src/range_map.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ impl<T> RangeMap<T> {
6161
/// Provides read-only iteration over everything in the given range. This does
6262
/// *not* split items if they overlap with the edges. Do not use this to mutate
6363
/// through interior mutability.
64-
pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator<Item = &'a T> + 'a {
64+
///
65+
/// The iterator also provides the offset of the given element.
66+
pub fn iter<'a>(&'a self, offset: Size, len: Size) -> impl Iterator<Item = (Size, &'a T)> + 'a {
6567
let offset = offset.bytes();
6668
let len = len.bytes();
6769
// Compute a slice starting with the elements we care about.
@@ -75,7 +77,7 @@ impl<T> RangeMap<T> {
7577
};
7678
// The first offset that is not included any more.
7779
let end = offset + len;
78-
slice.iter().take_while(move |elem| elem.range.start < end).map(|elem| &elem.data)
80+
slice.iter().take_while(move |elem| elem.range.start < end).map(|elem| (Size::from_bytes(elem.range.start), &elem.data))
7981
}
8082

8183
pub fn iter_mut_all<'a>(&'a mut self) -> impl Iterator<Item = &'a mut T> + 'a {
@@ -112,11 +114,13 @@ impl<T> RangeMap<T> {
112114
/// this will split entries in the map that are only partially hit by the given range,
113115
/// to make sure that when they are mutated, the effect is constrained to the given range.
114116
/// Moreover, this will opportunistically merge neighbouring equal blocks.
117+
///
118+
/// The iterator also provides the offset of the given element.
115119
pub fn iter_mut<'a>(
116120
&'a mut self,
117121
offset: Size,
118122
len: Size,
119-
) -> impl Iterator<Item = &'a mut T> + 'a
123+
) -> impl Iterator<Item = (Size, &'a mut T)> + 'a
120124
where
121125
T: Clone + PartialEq,
122126
{
@@ -197,7 +201,7 @@ impl<T> RangeMap<T> {
197201
// Now we yield the slice. `end` is inclusive.
198202
&mut self.v[first_idx..=end_idx]
199203
};
200-
slice.iter_mut().map(|elem| &mut elem.data)
204+
slice.iter_mut().map(|elem| (Size::from_bytes(elem.range.start), &mut elem.data))
201205
}
202206
}
203207

@@ -209,26 +213,26 @@ mod tests {
209213
fn to_vec<T: Copy>(map: &RangeMap<T>, offset: u64, len: u64) -> Vec<T> {
210214
(offset..offset + len)
211215
.into_iter()
212-
.map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|&t| t).unwrap())
216+
.map(|i| map.iter(Size::from_bytes(i), Size::from_bytes(1)).next().map(|(_, &t)| t).unwrap())
213217
.collect()
214218
}
215219

216220
#[test]
217221
fn basic_insert() {
218222
let mut map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
219223
// Insert.
220-
for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) {
224+
for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(1)) {
221225
*x = 42;
222226
}
223227
// Check.
224228
assert_eq!(to_vec(&map, 10, 1), vec![42]);
225229
assert_eq!(map.v.len(), 3);
226230

227231
// Insert with size 0.
228-
for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) {
232+
for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(0)) {
229233
*x = 19;
230234
}
231-
for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) {
235+
for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(0)) {
232236
*x = 19;
233237
}
234238
assert_eq!(to_vec(&map, 10, 2), vec![42, -1]);
@@ -238,16 +242,16 @@ mod tests {
238242
#[test]
239243
fn gaps() {
240244
let mut map = RangeMap::<i32>::new(Size::from_bytes(20), -1);
241-
for x in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) {
245+
for (_, x) in map.iter_mut(Size::from_bytes(11), Size::from_bytes(1)) {
242246
*x = 42;
243247
}
244-
for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) {
248+
for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(1)) {
245249
*x = 43;
246250
}
247251
assert_eq!(map.v.len(), 5);
248252
assert_eq!(to_vec(&map, 10, 10), vec![-1, 42, -1, -1, -1, 43, -1, -1, -1, -1]);
249253

250-
for x in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) {
254+
for (_, x) in map.iter_mut(Size::from_bytes(10), Size::from_bytes(10)) {
251255
if *x < 42 {
252256
*x = 23;
253257
}
@@ -256,14 +260,14 @@ mod tests {
256260
assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 43, 23, 23, 23, 23]);
257261
assert_eq!(to_vec(&map, 13, 5), vec![23, 23, 43, 23, 23]);
258262

259-
for x in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) {
263+
for (_, x) in map.iter_mut(Size::from_bytes(15), Size::from_bytes(5)) {
260264
*x = 19;
261265
}
262266
assert_eq!(map.v.len(), 6);
263267
assert_eq!(to_vec(&map, 10, 10), vec![23, 42, 23, 23, 23, 19, 19, 19, 19, 19]);
264268
// Should be seeing two blocks with 19.
265269
assert_eq!(
266-
map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|&t| t).collect::<Vec<_>>(),
270+
map.iter(Size::from_bytes(15), Size::from_bytes(2)).map(|(_, &t)| t).collect::<Vec<_>>(),
267271
vec![19, 19]
268272
);
269273

src/stacked_borrows.rs

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -309,14 +309,14 @@ impl<'tcx> Stack {
309309

310310
/// Test if a memory `access` using pointer tagged `tag` is granted.
311311
/// If yes, return the index of the item that granted it.
312-
fn access(&mut self, access: AccessKind, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
312+
fn access(&mut self, access: AccessKind, ptr: Pointer<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
313313
// Two main steps: Find granting item, remove incompatible items above.
314314

315315
// Step 1: Find granting item.
316-
let granting_idx = self.find_granting(access, tag).ok_or_else(|| {
316+
let granting_idx = self.find_granting(access, ptr.tag).ok_or_else(|| {
317317
err_sb_ub(format!(
318-
"no item granting {} to tag {:?} found in borrow stack.",
319-
access, tag
318+
"no item granting {} to tag {:?} at {} found in borrow stack.",
319+
access, ptr.tag, ptr.erase_tag(),
320320
))
321321
})?;
322322

@@ -328,7 +328,7 @@ impl<'tcx> Stack {
328328
let first_incompatible_idx = self.find_first_write_incompatible(granting_idx);
329329
for item in self.borrows.drain(first_incompatible_idx..).rev() {
330330
trace!("access: popping item {:?}", item);
331-
Stack::check_protector(&item, Some(tag), global)?;
331+
Stack::check_protector(&item, Some(ptr.tag), global)?;
332332
}
333333
} else {
334334
// On a read, *disable* all `Unique` above the granting item. This ensures U2 for read accesses.
@@ -343,7 +343,7 @@ impl<'tcx> Stack {
343343
let item = &mut self.borrows[idx];
344344
if item.perm == Permission::Unique {
345345
trace!("access: disabling item {:?}", item);
346-
Stack::check_protector(item, Some(tag), global)?;
346+
Stack::check_protector(item, Some(ptr.tag), global)?;
347347
item.perm = Permission::Disabled;
348348
}
349349
}
@@ -355,12 +355,12 @@ impl<'tcx> Stack {
355355

356356
/// Deallocate a location: Like a write access, but also there must be no
357357
/// active protectors at all because we will remove all items.
358-
fn dealloc(&mut self, tag: Tag, global: &GlobalState) -> InterpResult<'tcx> {
358+
fn dealloc(&mut self, ptr: Pointer<Tag>, global: &GlobalState) -> InterpResult<'tcx> {
359359
// Step 1: Find granting item.
360-
self.find_granting(AccessKind::Write, tag).ok_or_else(|| {
360+
self.find_granting(AccessKind::Write, ptr.tag).ok_or_else(|| {
361361
err_sb_ub(format!(
362-
"no item granting write access for deallocation to tag {:?} found in borrow stack",
363-
tag,
362+
"no item granting write access for deallocation to tag {:?} at {} found in borrow stack",
363+
ptr.tag, ptr.erase_tag(),
364364
))
365365
})?;
366366

@@ -372,20 +372,20 @@ impl<'tcx> Stack {
372372
Ok(())
373373
}
374374

375-
/// Derived a new pointer from one with the given tag.
375+
/// Derive a new pointer from one with the given tag.
376376
/// `weak` controls whether this operation is weak or strong: weak granting does not act as
377377
/// an access, and they add the new item directly on top of the one it is derived
378378
/// from instead of all the way at the top of the stack.
379-
fn grant(&mut self, derived_from: Tag, new: Item, global: &GlobalState) -> InterpResult<'tcx> {
379+
fn grant(&mut self, derived_from: Pointer<Tag>, new: Item, global: &GlobalState) -> InterpResult<'tcx> {
380380
// Figure out which access `perm` corresponds to.
381381
let access =
382382
if new.perm.grants(AccessKind::Write) { AccessKind::Write } else { AccessKind::Read };
383383
// Now we figure out which item grants our parent (`derived_from`) this kind of access.
384384
// We use that to determine where to put the new item.
385-
let granting_idx = self.find_granting(access, derived_from)
385+
let granting_idx = self.find_granting(access, derived_from.tag)
386386
.ok_or_else(|| err_sb_ub(format!(
387-
"trying to reborrow for {:?}, but parent tag {:?} does not have an appropriate item in the borrow stack",
388-
new.perm, derived_from,
387+
"trying to reborrow for {:?} at {}, but parent tag {:?} does not have an appropriate item in the borrow stack",
388+
new.perm, derived_from.erase_tag(), derived_from.tag,
389389
)))?;
390390

391391
// Compute where to put the new item.
@@ -443,12 +443,14 @@ impl<'tcx> Stacks {
443443
&self,
444444
ptr: Pointer<Tag>,
445445
size: Size,
446-
f: impl Fn(&mut Stack, &GlobalState) -> InterpResult<'tcx>,
446+
f: impl Fn(Pointer<Tag>, &mut Stack, &GlobalState) -> InterpResult<'tcx>,
447447
) -> InterpResult<'tcx> {
448448
let global = self.global.borrow();
449449
let mut stacks = self.stacks.borrow_mut();
450-
for stack in stacks.iter_mut(ptr.offset, size) {
451-
f(stack, &*global)?;
450+
for (offset, stack) in stacks.iter_mut(ptr.offset, size) {
451+
let mut cur_ptr = ptr;
452+
cur_ptr.offset = offset;
453+
f(cur_ptr, stack, &*global)?;
452454
}
453455
Ok(())
454456
}
@@ -487,19 +489,13 @@ impl Stacks {
487489
#[inline(always)]
488490
pub fn memory_read<'tcx>(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
489491
trace!("read access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
490-
self.for_each(ptr, size, |stack, global| {
491-
stack.access(AccessKind::Read, ptr.tag, global)?;
492-
Ok(())
493-
})
492+
self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Read, ptr, global))
494493
}
495494

496495
#[inline(always)]
497496
pub fn memory_written<'tcx>(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
498497
trace!("write access with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
499-
self.for_each(ptr, size, |stack, global| {
500-
stack.access(AccessKind::Write, ptr.tag, global)?;
501-
Ok(())
502-
})
498+
self.for_each(ptr, size, |ptr, stack, global| stack.access(AccessKind::Write, ptr, global))
503499
}
504500

505501
#[inline(always)]
@@ -509,7 +505,7 @@ impl Stacks {
509505
size: Size,
510506
) -> InterpResult<'tcx> {
511507
trace!("deallocation with tag {:?}: {:?}, size {}", ptr.tag, ptr.erase_tag(), size.bytes());
512-
self.for_each(ptr, size, |stack, global| stack.dealloc(ptr.tag, global))
508+
self.for_each(ptr, size, |ptr, stack, global| stack.dealloc(ptr, global))
513509
}
514510
}
515511

@@ -561,14 +557,14 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
561557
Permission::SharedReadWrite
562558
};
563559
let item = Item { perm, tag: new_tag, protector };
564-
stacked_borrows.for_each(cur_ptr, size, |stack, global| {
565-
stack.grant(cur_ptr.tag, item, global)
560+
stacked_borrows.for_each(cur_ptr, size, |cur_ptr, stack, global| {
561+
stack.grant(cur_ptr, item, global)
566562
})
567563
});
568564
}
569565
};
570566
let item = Item { perm, tag: new_tag, protector };
571-
stacked_borrows.for_each(ptr, size, |stack, global| stack.grant(ptr.tag, item, global))
567+
stacked_borrows.for_each(ptr, size, |ptr, stack, global| stack.grant(ptr, item, global))
572568
}
573569

574570
/// Retags an indidual pointer, returning the retagged version.

0 commit comments

Comments
 (0)