Skip to content

Commit

Permalink
Auto merge of #931 - RalfJung:retag-makes-my-head-hurt, r=RalfJung
Browse files Browse the repository at this point in the history
Stacked Borrows: don't read from memory during retagging

Currently, retagging of a shared reference tries real hard to find the `UnsafeCell` and mark everything else as frozen. We even read enum discriminants to figure out the variant and determine if there is an `UnsafeCell` in there or not.

Unfortunately, that leads to some very hard to analyze behavior: during retagging, we do read accesses, which are subject to the rules of Stacked Borrows and the existing tags! My head hurts when I try to think about this. It's just too recursive.

This PR simplifies the semantics by treating enums like unions: if any variant has an `UnsafeCell`, the entire thing behaves like an `UnsafeCell`. This means retagging no longer has to read from memory, the way it affects the stack is entirely determined by the type.
  • Loading branch information
bors committed Aug 29, 2019
2 parents 2661580 + f3ff100 commit e3b87f6
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// This is `Freeze`, there cannot be an `UnsafeCell`
Ok(())
} else {
// Proceed further
self.walk_value(v)
// We want to not actually read from memory for this visit. So, before
// walking this value, we have to make sure it is not a
// `Variants::Multiple`.
match v.layout.variants {
layout::Variants::Multiple { .. } => {
// A multi-variant enum, or generator, or so.
// Treat this like a union: without reading from memory,
// we cannot determine the variant we are in. Reading from
// memory would be subject to Stacked Borrows rules, leading
// to all sorts of "funny" recursion.
// We only end up here if the type is *not* freeze, so we just call the
// `UnsafeCell` action.
(self.unsafe_cell_action)(v)
}
layout::Variants::Single { .. } => {
// Proceed further, try to find where exactly that `UnsafeCell`
// is hiding.
self.walk_value(v)
}
}
}
}

Expand Down

0 comments on commit e3b87f6

Please sign in to comment.