Skip to content

Commit

Permalink
Store protectors outside Item, pack Tag and Perm
Browse files Browse the repository at this point in the history
  • Loading branch information
@saethlin
saethlin committed Jul 3, 2022
1 parent 984b46c commit 85d3a5d
Show file tree
Hide file tree
Showing 3 changed files with 139 additions and 66 deletions.
13 changes: 10 additions & 3 deletions src/machine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,18 @@ pub struct FrameData<'tcx> {
/// for the start of this frame. When we finish executing this frame,
/// we use this to register a completed event with `measureme`.
pub timing: Option<measureme::DetachedTiming>,

pub protected_tags: Vec<SbTag>,
}

impl<'tcx> std::fmt::Debug for FrameData<'tcx> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// Omitting `timing`, it does not support `Debug`.
let FrameData { call_id, catch_unwind, timing: _ } = self;
let FrameData { call_id, catch_unwind, timing: _, protected_tags } = self;
f.debug_struct("FrameData")
.field("call_id", call_id)
.field("catch_unwind", catch_unwind)
.field("protected_tags", protected_tags)
.finish()
}
}
Expand Down Expand Up @@ -887,7 +890,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
stacked_borrows.borrow_mut().new_call()
});

let extra = FrameData { call_id, catch_unwind: None, timing };
let extra = FrameData { call_id, catch_unwind: None, timing, protected_tags: Vec::new() };
Ok(frame.with_extra(extra))
}

Expand Down Expand Up @@ -931,7 +934,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
) -> InterpResult<'tcx, StackPopJump> {
let timing = frame.extra.timing.take();
if let Some(stacked_borrows) = &ecx.machine.stacked_borrows {
stacked_borrows.borrow_mut().end_call(frame.extra.call_id);
let mut sb = stacked_borrows.borrow_mut();
sb.end_call(frame.extra.call_id);
for tag in frame.extra.protected_tags.drain(..) {
sb.protected_tags.remove(&tag);
}
}
let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding);
if let Some(profiler) = ecx.machine.profiler.as_ref() {
Expand Down
73 changes: 32 additions & 41 deletions src/stacked_borrows.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,11 @@ pub struct Item {
perm: Permission,
/// The pointers the permission is granted to.
tag: SbTag,
/// An optional protector, ensuring the item cannot get popped until `CallId` is over.
protector: Option<CallId>,
}

impl fmt::Debug for Item {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "[{:?} for {:?}", self.perm, self.tag)?;
if let Some(call) = self.protector {
write!(f, " (call {})", call)?;
}
write!(f, "]")?;
Ok(())
write!(f, "[{:?} for {:?}]", self.perm, self.tag)
}
}

Expand Down Expand Up @@ -138,6 +131,8 @@ pub struct GlobalStateInner {
next_call_id: CallId,
/// Those call IDs corresponding to functions that are still running.
active_calls: FxHashSet<CallId>,
/// All tags currently protected
pub(crate) protected_tags: FxHashSet<SbTag>,
/// The pointer ids to trace
tracked_pointer_tags: HashSet<SbTag>,
/// The call ids to trace
Expand Down Expand Up @@ -202,6 +197,7 @@ impl GlobalStateInner {
base_ptr_tags: FxHashMap::default(),
next_call_id: NonZeroU64::new(1).unwrap(),
active_calls: FxHashSet::default(),
protected_tags: FxHashSet::default(),
tracked_pointer_tags,
tracked_call_ids,
retag_fields,
Expand Down Expand Up @@ -230,10 +226,6 @@ impl GlobalStateInner {
assert!(self.active_calls.remove(&id));
}

fn is_active(&self, id: CallId) -> bool {
self.active_calls.contains(&id)
}

pub fn base_ptr_tag(&mut self, id: AllocId) -> SbTag {
self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| {
let tag = self.new_ptr();
Expand Down Expand Up @@ -333,24 +325,22 @@ impl<'tcx> Stack {
));
}

if let Some(call) = item.protector {
if global.is_active(call) {
if let Some((tag, _alloc_range, _offset, _access)) = provoking_access {
Err(err_sb_ub(
format!(
"not granting access to tag {:?} because incompatible item is protected: {:?}",
tag, item
),
None,
tag.and_then(|tag| alloc_history.get_logs_relevant_to(tag, Some(item.tag))),
))?
} else {
Err(err_sb_ub(
format!("deallocating while item is protected: {:?}", item),
None,
None,
))?
}
if global.protected_tags.contains(&item.tag) {
if let Some((tag, _alloc_range, _offset, _access)) = provoking_access {
Err(err_sb_ub(
format!(
"not granting access to tag {:?} because incompatible item is protected: {:?}",
tag, item
),
None,
tag.and_then(|tag| alloc_history.get_logs_relevant_to(tag, Some(item.tag))),
))?
} else {
Err(err_sb_ub(
format!("deallocating while item is protected: {:?}", item),
None,
None,
))?
}
}
Ok(())
Expand Down Expand Up @@ -578,7 +568,7 @@ impl<'tcx> Stack {
impl<'tcx> Stacks {
/// Creates new stack with initial tag.
fn new(size: Size, perm: Permission, tag: SbTag) -> Self {
let item = Item { perm, tag, protector: None };
let item = Item { perm, tag };
let stack = Stack::new(item);

Stacks {
Expand Down Expand Up @@ -808,7 +798,6 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
});
}

let protector = if protect { Some(this.frame().extra.call_id) } else { None };
trace!(
"reborrow: {} reference {:?} derived from {:?} (pointee {}): {:?}, size {}",
kind,
Expand All @@ -819,6 +808,15 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
size.bytes()
);

// FIXME: This just protects everything, which is wrong. At the very least, we should not
// protect anything that contains an UnsafeCell.
if protect {
this.frame_mut().extra.protected_tags.push(new_tag);
this.machine.stacked_borrows.as_mut().unwrap().get_mut().protected_tags.insert(new_tag);
}
// FIXME: can't hold the current span handle across the borrows of self above
let current_span = &mut this.machine.current_span();

// Update the stacks.
// Make sure that raw pointers and mutable shared references are reborrowed "weak":
// There could be existing unique pointers reborrowed from them that should remain valid!
Expand Down Expand Up @@ -855,14 +853,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
} else {
Permission::SharedReadWrite
};
let protector = if frozen {
protector
} else {
// We do not protect inside UnsafeCell.
// This fixes https://github.com/rust-lang/rust/issues/55005.
None
};
let item = Item { perm, tag: new_tag, protector };
let item = Item { perm, tag: new_tag };
let mut global = this.machine.stacked_borrows.as_ref().unwrap().borrow_mut();
stacked_borrows.for_each(range, |offset, stack, history, exposed_tags| {
stack.grant(
Expand All @@ -888,7 +879,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
.as_mut()
.expect("we should have Stacked Borrows data")
.borrow_mut();
let item = Item { perm, tag: new_tag, protector };
let item = Item { perm, tag: new_tag };
let range = alloc_range(base_offset, size);
let mut global = machine.stacked_borrows.as_ref().unwrap().borrow_mut();
let current_span = &mut machine.current_span(); // `get_alloc_extra_mut` invalidated our old `current_span`
Expand Down
Loading

0 comments on commit 85d3a5d

Please sign in to comment.