Skip to content

Commit 5b885ad

Browse files
author
Xinglu Chen
committed
Use "accessed" instead of "initialized" in LocationState
1 parent f0e1a58 commit 5b885ad

File tree

4 files changed

+68
-65
lines changed

4 files changed

+68
-65
lines changed

src/borrow_tracker/tree_borrows/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ impl DisplayFmt {
504504
if let Some(perm) = perm {
505505
format!(
506506
"{ac}{st}",
507-
ac = if perm.is_initialized() { self.accessed.yes } else { self.accessed.no },
507+
ac = if perm.is_accessed() { self.accessed.yes } else { self.accessed.no },
508508
st = perm.permission().short_name(),
509509
)
510510
} else {

src/borrow_tracker/tree_borrows/mod.rs

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

src/borrow_tracker/tree_borrows/tree.rs

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,18 @@ mod tests;
3333
/// Data for a single *location*.
3434
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
3535
pub(super) struct LocationState {
36-
/// A location is initialized when it is child-accessed for the first time (and the initial
36+
/// A location is "accessed" when it is child-accessed for the first time (and the initial
3737
/// retag initializes the location for the range covered by the type), and it then stays
38-
/// initialized forever.
39-
/// For initialized locations, "permission" is the current permission. However, for
40-
/// uninitialized locations, we still need to track the "future initial permission": this will
38+
/// accessed forever.
39+
/// For accessed locations, "permission" is the current permission. However, for
40+
/// non-accessed locations, we still need to track the "future initial permission": this will
4141
/// start out to be `default_initial_perm`, but foreign accesses need to be taken into account.
4242
/// Crucially however, while transitions to `Disabled` would usually be UB if this location is
43-
/// protected, that is *not* the case for uninitialized locations. Instead we just have a latent
43+
/// protected, that is *not* the case for non-accessed locations. Instead we just have a latent
4444
/// "future initial permission" of `Disabled`, causing UB only if an access is ever actually
4545
/// performed.
46-
/// Note that the tree root is also always initialized, as if the allocation was a write access.
47-
initialized: bool,
46+
/// Note that the tree root is also always accessed, as if the allocation was a write access.
47+
accessed: bool,
4848
/// This pointer's current permission / future initial permission.
4949
permission: Permission,
5050
/// See `foreign_access_skipping.rs`.
@@ -59,30 +59,30 @@ impl LocationState {
5959
/// to any foreign access yet.
6060
/// The permission is not allowed to be `Active`.
6161
/// `sifa` is the (strongest) idempotent foreign access, see `foreign_access_skipping.rs`
62-
pub fn new_uninit(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
62+
pub fn new_non_accessed(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
6363
assert!(permission.is_initial() || permission.is_disabled());
64-
Self { permission, initialized: false, idempotent_foreign_access: sifa }
64+
Self { permission, accessed: false, idempotent_foreign_access: sifa }
6565
}
6666

6767
/// Constructs a new initial state. It has not yet been subjected
6868
/// to any foreign access. However, it is already marked as having been accessed.
6969
/// `sifa` is the (strongest) idempotent foreign access, see `foreign_access_skipping.rs`
70-
pub fn new_init(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
71-
Self { permission, initialized: true, idempotent_foreign_access: sifa }
70+
pub fn new_accessed(permission: Permission, sifa: IdempotentForeignAccess) -> Self {
71+
Self { permission, accessed: true, idempotent_foreign_access: sifa }
7272
}
7373

74-
/// Check if the location has been initialized, i.e. if it has
74+
/// Check if the location has been accessed, i.e. if it has
7575
/// ever been accessed through a child pointer.
76-
pub fn is_initialized(&self) -> bool {
77-
self.initialized
76+
pub fn is_accessed(&self) -> bool {
77+
self.accessed
7878
}
7979

8080
/// Check if the state can exist as the initial permission of a pointer.
8181
///
82-
/// Do not confuse with `is_initialized`, the two are almost orthogonal
83-
/// as apart from `Active` which is not initial and must be initialized,
82+
/// Do not confuse with `is_accessed`, the two are almost orthogonal
83+
/// as apart from `Active` which is not initial and must be accessed,
8484
/// any other permission can have an arbitrary combination of being
85-
/// initial/initialized.
85+
/// initial/accessed.
8686
/// FIXME: when the corresponding `assert` in `tree_borrows/mod.rs` finally
8787
/// passes and can be uncommented, remove this `#[allow(dead_code)]`.
8888
#[cfg_attr(not(test), allow(dead_code))]
@@ -96,8 +96,8 @@ impl LocationState {
9696

9797
/// Apply the effect of an access to one location, including
9898
/// - applying `Permission::perform_access` to the inner `Permission`,
99-
/// - emitting protector UB if the location is initialized,
100-
/// - updating the initialized status (child accesses produce initialized locations).
99+
/// - emitting protector UB if the location is accessed,
100+
/// - updating the accessed status (child accesses produce accessed locations).
101101
fn perform_access(
102102
&mut self,
103103
access_kind: AccessKind,
@@ -107,23 +107,23 @@ impl LocationState {
107107
let old_perm = self.permission;
108108
let transition = Permission::perform_access(access_kind, rel_pos, old_perm, protected)
109109
.ok_or(TransitionError::ChildAccessForbidden(old_perm))?;
110-
self.initialized |= !rel_pos.is_foreign();
110+
self.accessed |= !rel_pos.is_foreign();
111111
self.permission = transition.applied(old_perm).unwrap();
112-
// Why do only initialized locations cause protector errors?
112+
// Why do only accessed locations cause protector errors?
113113
// Consider two mutable references `x`, `y` into disjoint parts of
114114
// the same allocation. A priori, these may actually both be used to
115115
// access the entire allocation, as long as only reads occur. However,
116116
// a write to `y` needs to somehow record that `x` can no longer be used
117-
// on that location at all. For these uninitialized locations (i.e., locations
117+
// on that location at all. For these non-accessed locations (i.e., locations
118118
// that haven't been accessed with `x` yet), we track the "future initial state":
119119
// it defaults to whatever the initial state of the tag is,
120120
// but the access to `y` moves that "future initial state" of `x` to `Disabled`.
121121
// However, usually a `Reserved -> Disabled` transition would be UB due to the protector!
122122
// So clearly protectors shouldn't fire for such "future initial state" transitions.
123123
//
124124
// See the test `two_mut_protected_same_alloc` in `tests/pass/tree_borrows/tree-borrows.rs`
125-
// for an example of safe code that would be UB if we forgot to check `self.initialized`.
126-
if protected && self.initialized && transition.produces_disabled() {
125+
// for an example of safe code that would be UB if we forgot to check `self.accessed`.
126+
if protected && self.accessed && transition.produces_disabled() {
127127
return Err(TransitionError::ProtectedDisabled(old_perm));
128128
}
129129
Ok(transition)
@@ -158,11 +158,11 @@ impl LocationState {
158158
self.idempotent_foreign_access.can_skip_foreign_access(happening_now);
159159
if self.permission.is_disabled() {
160160
// A foreign access to a `Disabled` tag will have almost no observable effect.
161-
// It's a theorem that `Disabled` node have no protected initialized children,
161+
// It's a theorem that `Disabled` node have no protected accessed children,
162162
// and so this foreign access will never trigger any protector.
163-
// (Intuition: You're either protected initialized, and thus can't become Disabled
164-
// or you're already Disabled protected, but not initialized, and then can't
165-
// become initialized since that requires a child access, which Disabled blocks.)
163+
// (Intuition: You're either protected accessed, and thus can't become Disabled
164+
// or you're already Disabled protected, but not accessed, and then can't
165+
// become accessed since that requires a child access, which Disabled blocks.)
166166
// Further, the children will never be able to read or write again, since they
167167
// have a `Disabled` parent. So this only affects diagnostics, such that the
168168
// blocking write will still be identified directly, just at a different tag.
@@ -218,7 +218,7 @@ impl LocationState {
218218
impl fmt::Display for LocationState {
219219
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
220220
write!(f, "{}", self.permission)?;
221-
if !self.initialized {
221+
if !self.accessed {
222222
write!(f, "?")?;
223223
}
224224
Ok(())
@@ -599,12 +599,15 @@ impl Tree {
599599
let rperms = {
600600
let mut perms = UniValMap::default();
601601
// We manually set it to `Active` on all in-bounds positions.
602-
// We also ensure that it is initialized, so that no `Active` but
603-
// not yet initialized nodes exist. Essentially, we pretend there
602+
// We also ensure that it is accessed, so that no `Active` but
603+
// not yet accessed nodes exist. Essentially, we pretend there
604604
// was a write that initialized these to `Active`.
605605
perms.insert(
606606
root_idx,
607-
LocationState::new_init(Permission::new_active(), IdempotentForeignAccess::None),
607+
LocationState::new_accessed(
608+
Permission::new_active(),
609+
IdempotentForeignAccess::None,
610+
),
608611
);
609612
RangeMap::new(size, perms)
610613
};
@@ -782,14 +785,14 @@ impl<'tcx> Tree {
782785
///
783786
/// If `access_range_and_kind` is `None`, this is interpreted as the special
784787
/// access that is applied on protector release:
785-
/// - the access will be applied only to initialized locations of the allocation,
788+
/// - the access will be applied only to accessed locations of the allocation,
786789
/// - it will not be visible to children,
787790
/// - it will be recorded as a `FnExit` diagnostic access
788791
/// - and it will be a read except if the location is `Active`, i.e. has been written to,
789792
/// in which case it will be a write.
790793
///
791794
/// `LocationState::perform_access` will take care of raising transition
792-
/// errors and updating the `initialized` status of each location,
795+
/// errors and updating the `accessed` status of each location,
793796
/// this traversal adds to that:
794797
/// - inserting into the map locations that do not exist yet,
795798
/// - trimming the traversal,
@@ -882,7 +885,7 @@ impl<'tcx> Tree {
882885
}
883886
} else {
884887
// This is a special access through the entire allocation.
885-
// It actually only affects `initialized` locations, so we need
888+
// It actually only affects `accessed` locations, so we need
886889
// to filter on those before initiating the traversal.
887890
//
888891
// In addition this implicit access should not be visible to children,
@@ -892,10 +895,10 @@ impl<'tcx> Tree {
892895
// why this is important.
893896
for (perms_range, perms) in self.rperms.iter_mut_all() {
894897
let idx = self.tag_mapping.get(&tag).unwrap();
895-
// Only visit initialized permissions
898+
// Only visit accessed permissions
896899
if let Some(p) = perms.get(idx)
897900
&& let Some(access_kind) = p.permission.protector_end_access()
898-
&& p.initialized
901+
&& p.accessed
899902
{
900903
let access_cause = diagnostics::AccessCause::FnExit(access_kind);
901904
TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
@@ -1059,7 +1062,7 @@ impl Tree {
10591062

10601063
impl Node {
10611064
pub fn default_location_state(&self) -> LocationState {
1062-
LocationState::new_uninit(
1065+
LocationState::new_non_accessed(
10631066
self.default_initial_perm,
10641067
self.default_initial_idempotent_foreign_access,
10651068
)

0 commit comments

Comments
 (0)