user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use "accessed" instead of "initialized" in LocationState

Browse source
This commit is contained in:
Xinglu Chen 2025-05-30 14:01:19 +02:00
parent 089acfaeb4
commit 2107793ebe
4 changed files with 68 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs
View file

@ -504,7 +504,7 @@ impl DisplayFmt {
if let Some(perm) = perm {
format!(
"{ac}{st}",
ac = if perm.is_initialized() { self.accessed.yes } else { self.accessed.no },
ac = if perm.is_accessed() { self.accessed.yes } else { self.accessed.no },
st = perm.permission().short_name(),
)
} else {

10
src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs
View file

@ -97,7 +97,7 @@ impl<'tcx> Tree {
/// A tag just lost its protector.
///
/// This emits a special kind of access that is only applied
/// to initialized locations, as a protection against other
/// to accessed locations, as a protection against other
/// tags not having been made aware of the existence of this
/// protector.
pub fn release_protector(
@ -318,7 +318,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// Store initial permissions and their corresponding range.
let mut perms_map: RangeMap<LocationState> = RangeMap::new(
ptr_size,
LocationState::new_init(Permission::new_disabled(), IdempotentForeignAccess::None), // this will be overwritten
LocationState::new_accessed(Permission::new_disabled(), IdempotentForeignAccess::None), // this will be overwritten
);
// Keep track of whether the node has any part that allows for interior mutability.
// FIXME: This misses `PhantomData<UnsafeCell<T>>` which could be considered a marker
@ -357,13 +357,13 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let sifa = perm.strongest_idempotent_foreign_access(protected);
// NOTE: Currently, `access` is false if and only if `perm` is Cell, so this `if`
// doesn't not change whether any code is UB or not. We could just always use
// `new_init` and everything would stay the same. But that seems conceptually
// `new_accessed` and everything would stay the same. But that seems conceptually
// odd, so we keep the initial "accessed" bit of the `LocationState` in sync with whether
// a read access is performed below.
if access {
*loc = LocationState::new_init(perm, sifa);
*loc = LocationState::new_accessed(perm, sifa);
} else {
*loc = LocationState::new_uninit(perm, sifa);
*loc = LocationState::new_non_accessed(perm, sifa);
}
}

79
src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs
View file

@ -33,18 +33,18 @@ mod tests;
/// Data for a single *location*.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(super) struct LocationState {
/// A location is initialized when it is child-accessed for the first time (and the initial
/// A location is "accessed" when it is child-accessed for the first time (and the initial
/// retag initializes the location for the range covered by the type), and it then stays
/// initialized forever.
/// For initialized locations, "permission" is the current permission. However, for
/// uninitialized locations, we still need to track the "future initial permission": this will
/// accessed forever.
/// For accessed locations, "permission" is the current permission. However, for
/// non-accessed locations, we still need to track the "future initial permission": this will
/// start out to be `default_initial_perm`, but foreign accesses need to be taken into account.
/// Crucially however, while transitions to `Disabled` would usually be UB if this location is
/// protected, that is *not* the case for uninitialized locations. Instead we just have a latent
/// protected, that is *not* the case for non-accessed locations. Instead we just have a latent
/// "future initial permission" of `Disabled`, causing UB only if an access is ever actually
/// performed.
/// Note that the tree root is also always initialized, as if the allocation was a write access.
initialized: bool,
/// Note that the tree root is also always accessed, as if the allocation was a write access.
accessed: bool,
/// This pointer's current permission / future initial permission.
permission: Permission,
/// See `foreign_access_skipping.rs`.
@ -59,30 +59,30 @@ impl LocationState {
/// to any foreign access yet.
/// The permission is not allowed to be `Active`.
/// `sifa` is the (strongest) idempotent foreign access, see `foreign_access_skipping.rs`
pub fn new_uninit(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
pub fn new_non_accessed(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
assert!(permission.is_initial() || permission.is_disabled());
Self { permission, initialized: false, idempotent_foreign_access: sifa }
Self { permission, accessed: false, idempotent_foreign_access: sifa }
}
/// Constructs a new initial state. It has not yet been subjected
/// to any foreign access. However, it is already marked as having been accessed.
/// `sifa` is the (strongest) idempotent foreign access, see `foreign_access_skipping.rs`
pub fn new_init(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
Self { permission, initialized: true, idempotent_foreign_access: sifa }
pub fn new_accessed(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
Self { permission, accessed: true, idempotent_foreign_access: sifa }
}
/// Check if the location has been initialized, i.e. if it has
/// Check if the location has been accessed, i.e. if it has
/// ever been accessed through a child pointer.
pub fn is_initialized(&self) -> bool {
self.initialized
pub fn is_accessed(&self) -> bool {
self.accessed
}
/// Check if the state can exist as the initial permission of a pointer.
///
/// Do not confuse with `is_initialized`, the two are almost orthogonal
/// as apart from `Active` which is not initial and must be initialized,
/// Do not confuse with `is_accessed`, the two are almost orthogonal
/// as apart from `Active` which is not initial and must be accessed,
/// any other permission can have an arbitrary combination of being
/// initial/initialized.
/// initial/accessed.
/// FIXME: when the corresponding `assert` in `tree_borrows/mod.rs` finally
/// passes and can be uncommented, remove this `#[allow(dead_code)]`.
#[cfg_attr(not(test), allow(dead_code))]
@ -96,8 +96,8 @@ impl LocationState {
/// Apply the effect of an access to one location, including
/// - applying `Permission::perform_access` to the inner `Permission`,
/// - emitting protector UB if the location is initialized,
/// - updating the initialized status (child accesses produce initialized locations).
/// - emitting protector UB if the location is accessed,
/// - updating the accessed status (child accesses produce accessed locations).
fn perform_access(
&mut self,
access_kind: AccessKind,
@ -107,14 +107,14 @@ impl LocationState {
let old_perm = self.permission;
let transition = Permission::perform_access(access_kind, rel_pos, old_perm, protected)
.ok_or(TransitionError::ChildAccessForbidden(old_perm))?;
self.initialized |= !rel_pos.is_foreign();
self.accessed |= !rel_pos.is_foreign();
self.permission = transition.applied(old_perm).unwrap();
// Why do only initialized locations cause protector errors?
// Why do only accessed locations cause protector errors?
// Consider two mutable references `x`, `y` into disjoint parts of
// the same allocation. A priori, these may actually both be used to
// access the entire allocation, as long as only reads occur. However,
// a write to `y` needs to somehow record that `x` can no longer be used
// on that location at all. For these uninitialized locations (i.e., locations
// on that location at all. For these non-accessed locations (i.e., locations
// that haven't been accessed with `x` yet), we track the "future initial state":
// it defaults to whatever the initial state of the tag is,
// but the access to `y` moves that "future initial state" of `x` to `Disabled`.
@ -122,8 +122,8 @@ impl LocationState {
// So clearly protectors shouldn't fire for such "future initial state" transitions.
//
// See the test `two_mut_protected_same_alloc` in `tests/pass/tree_borrows/tree-borrows.rs`
// for an example of safe code that would be UB if we forgot to check `self.initialized`.
if protected && self.initialized && transition.produces_disabled() {
// for an example of safe code that would be UB if we forgot to check `self.accessed`.
if protected && self.accessed && transition.produces_disabled() {
return Err(TransitionError::ProtectedDisabled(old_perm));
}
Ok(transition)
@ -158,11 +158,11 @@ impl LocationState {
self.idempotent_foreign_access.can_skip_foreign_access(happening_now);
if self.permission.is_disabled() {
// A foreign access to a `Disabled` tag will have almost no observable effect.
// It's a theorem that `Disabled` node have no protected initialized children,
// It's a theorem that `Disabled` node have no protected accessed children,
// and so this foreign access will never trigger any protector.
// (Intuition: You're either protected initialized, and thus can't become Disabled
// or you're already Disabled protected, but not initialized, and then can't
// become initialized since that requires a child access, which Disabled blocks.)
// (Intuition: You're either protected accessed, and thus can't become Disabled
// or you're already Disabled protected, but not accessed, and then can't
// become accessed since that requires a child access, which Disabled blocks.)
// Further, the children will never be able to read or write again, since they
// have a `Disabled` parent. So this only affects diagnostics, such that the
// blocking write will still be identified directly, just at a different tag.
@ -218,7 +218,7 @@ impl LocationState {
impl fmt::Display for LocationState {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.permission)?;
if !self.initialized {
if !self.accessed {
write!(f, "?")?;
}
Ok(())
@ -599,12 +599,15 @@ impl Tree {
let rperms = {
let mut perms = UniValMap::default();
// We manually set it to `Active` on all in-bounds positions.
// We also ensure that it is initialized, so that no `Active` but
// not yet initialized nodes exist. Essentially, we pretend there
// We also ensure that it is accessed, so that no `Active` but
// not yet accessed nodes exist. Essentially, we pretend there
// was a write that initialized these to `Active`.
perms.insert(
root_idx,
LocationState::new_init(Permission::new_active(), IdempotentForeignAccess::None),
LocationState::new_accessed(
Permission::new_active(),
IdempotentForeignAccess::None,
),
);
RangeMap::new(size, perms)
};
@ -782,14 +785,14 @@ impl<'tcx> Tree {
///
/// If `access_range_and_kind` is `None`, this is interpreted as the special
/// access that is applied on protector release:
/// - the access will be applied only to initialized locations of the allocation,
/// - the access will be applied only to accessed locations of the allocation,
/// - it will not be visible to children,
/// - it will be recorded as a `FnExit` diagnostic access
/// - and it will be a read except if the location is `Active`, i.e. has been written to,
/// in which case it will be a write.
///
/// `LocationState::perform_access` will take care of raising transition
/// errors and updating the `initialized` status of each location,
/// errors and updating the `accessed` status of each location,
/// this traversal adds to that:
/// - inserting into the map locations that do not exist yet,
/// - trimming the traversal,
@ -882,7 +885,7 @@ impl<'tcx> Tree {
}
} else {
// This is a special access through the entire allocation.
// It actually only affects `initialized` locations, so we need
// It actually only affects `accessed` locations, so we need
// to filter on those before initiating the traversal.
//
// In addition this implicit access should not be visible to children,
@ -892,10 +895,10 @@ impl<'tcx> Tree {
// why this is important.
for (perms_range, perms) in self.rperms.iter_mut_all() {
let idx = self.tag_mapping.get(&tag).unwrap();
// Only visit initialized permissions
// Only visit accessed permissions
if let Some(p) = perms.get(idx)
&& let Some(access_kind) = p.permission.protector_end_access()
&& p.initialized
&& p.accessed
{
let access_cause = diagnostics::AccessCause::FnExit(access_kind);
TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
@ -1059,7 +1062,7 @@ impl Tree {
impl Node {
pub fn default_location_state(&self) -> LocationState {
LocationState::new_uninit(
LocationState::new_non_accessed(
self.default_initial_perm,
self.default_initial_idempotent_foreign_access,
)

42
src/tools/miri/src/borrow_tracker/tree_borrows/tree/tests.rs
View file

@ -9,10 +9,10 @@ use crate::borrow_tracker::tree_borrows::exhaustive::{Exhaustive, precondition};
impl Exhaustive for LocationState {
fn exhaustive() -> Box<dyn Iterator<Item = Self>> {
// We keep `latest_foreign_access` at `None` as that's just a cache.
Box::new(<(Permission, bool)>::exhaustive().map(|(permission, initialized)| {
Box::new(<(Permission, bool)>::exhaustive().map(|(permission, accessed)| {
Self {
permission,
initialized,
accessed,
idempotent_foreign_access: IdempotentForeignAccess::default(),
}
}))
@ -76,8 +76,8 @@ fn as_protected(b: bool) -> &'static str {
if b { " (protected)" } else { "" }
}
fn as_lazy_or_init(b: bool) -> &'static str {
if b { "initialized" } else { "lazy" }
fn as_lazy_or_accessed(b: bool) -> &'static str {
if b { "accessed" } else { "lazy" }
}
/// Test that tree compacting (as performed by the GC) is sound.
@ -106,7 +106,7 @@ fn tree_compacting_is_sound() {
as_foreign_or_child(rel),
kind,
parent.permission(),
as_lazy_or_init(child.is_initialized()),
as_lazy_or_accessed(child.is_accessed()),
child.permission(),
as_protected(child_protected),
np.permission(),
@ -122,7 +122,7 @@ fn tree_compacting_is_sound() {
as_foreign_or_child(rel),
kind,
parent.permission(),
as_lazy_or_init(child.is_initialized()),
as_lazy_or_accessed(child.is_accessed()),
child.permission(),
as_protected(child_protected),
nc.permission()
@ -435,19 +435,19 @@ mod spurious_read {
Ok(Self { x, y, ..self })
}
/// Perform a read on the given pointer if its state is `initialized`.
/// Perform a read on the given pointer if its state is `accessed`.
/// Must be called just after reborrowing a pointer, and just after
/// removing a protector.
fn read_if_initialized(self, ptr: PtrSelector) -> Result<Self, ()> {
let initialized = match ptr {
PtrSelector::X => self.x.state.initialized,
PtrSelector::Y => self.y.state.initialized,
fn read_if_accessed(self, ptr: PtrSelector) -> Result<Self, ()> {
let accessed = match ptr {
PtrSelector::X => self.x.state.accessed,
PtrSelector::Y => self.y.state.accessed,
PtrSelector::Other =>
panic!(
"the `initialized` status of `PtrSelector::Other` is unknown, do not pass it to `read_if_initialized`"
"the `accessed` status of `PtrSelector::Other` is unknown, do not pass it to `read_if_accessed`"
),
};
if initialized {
if accessed {
self.perform_test_access(&TestAccess { ptr, kind: AccessKind::Read })
} else {
Ok(self)
@ -457,13 +457,13 @@ mod spurious_read {
/// Remove the protector of `x`, including the implicit read on function exit.
fn end_protector_x(self) -> Result<Self, ()> {
let x = self.x.end_protector();
Self { x, ..self }.read_if_initialized(PtrSelector::X)
Self { x, ..self }.read_if_accessed(PtrSelector::X)
}
/// Remove the protector of `y`, including the implicit read on function exit.
fn end_protector_y(self) -> Result<Self, ()> {
let y = self.y.end_protector();
Self { y, ..self }.read_if_initialized(PtrSelector::Y)
Self { y, ..self }.read_if_accessed(PtrSelector::Y)
}
fn retag_y(self, new_y: LocStateProt) -> Result<Self, ()> {
@ -473,7 +473,7 @@ mod spurious_read {
}
// `xy_rel` changes to "mutually foreign" now: `y` can no longer be a parent of `x`.
Self { y: new_y, xy_rel: RelPosXY::MutuallyForeign, ..self }
.read_if_initialized(PtrSelector::Y)
.read_if_accessed(PtrSelector::Y)
}
fn perform_test_event<RetX, RetY>(self, evt: &TestEvent<RetX, RetY>) -> Result<Self, ()> {
@ -602,14 +602,14 @@ mod spurious_read {
xy_rel: RelPosXY::MutuallyForeign,
x: LocStateProt {
// For the tests, the strongest idempotent foreign access does not matter, so we use `Default::default`
state: LocationState::new_init(
state: LocationState::new_accessed(
Permission::new_frozen(),
IdempotentForeignAccess::default(),
),
prot: true,
},
y: LocStateProt {
state: LocationState::new_uninit(
state: LocationState::new_non_accessed(
Permission::new_reserved(/* freeze */ true, /* protected */ true),
IdempotentForeignAccess::default(),
),
@ -650,8 +650,8 @@ mod spurious_read {
for xy_rel in RelPosXY::exhaustive() {
for (x_retag_perm, y_current_perm) in <(LocationState, LocationState)>::exhaustive()
{
// We can only do spurious reads for initialized locations anyway.
precondition!(x_retag_perm.initialized);
// We can only do spurious reads for accessed locations anyway.
precondition!(x_retag_perm.accessed);
// And `x` just got retagged, so it must be initial.
precondition!(x_retag_perm.permission.is_initial());
// As stated earlier, `x` is always protected in the patterns we consider here.
@ -696,7 +696,7 @@ mod spurious_read {
fn initial_state(&self) -> Result<LocStateProtPair, ()> {
let (x, y) = self.retag_permissions();
let state = LocStateProtPair { xy_rel: self.xy_rel, x, y };
state.read_if_initialized(PtrSelector::X)
state.read_if_accessed(PtrSelector::X)
}
}