Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 54bf046

Browse files
authoredJun 8, 2025··
Auto merge of #142170 - RalfJung:sb-raw-retag, r=<try>
Stacked Borrows: raw pointers inherit the tag from their parent pointer The biggest design mistake of SB in my opinion was the decision to have raw pointers be distinct from the reference they are derived from. This has multiple undesirable consequences: - Within a function, a `let mut x` and `&raw mut x` are not allowed to be used in an interleaving way, which is quite surprising and requires dedicated work-arounds e.g. in c2rust. - Casting a `&mut T` to a `*const T` results in a read-only pointer, which regularly startles people. - On the implementation side, this means we have to recognize all places where "safe" pointers turn into raw pointers, which turns out to be tricky for `Box`. TB fully mitigates this by having a raw pointer inherit the tag from its parent pointer. I think we should do the same for SB. However, this requires a bit of hackery to prevent code like this from becoming UB: ```rust fn foo(x: &mut [i32]) { let ptr = x.as_mut_ptr(); let len = x.len(); assert!(len > 0); ptr.write(0); } ``` Under SB, `x.len()` causes a read of the entire slice which invalidates `ptr` since that is derived from a child of `x` (created as part of the implicit retag when calling `as_mut_ptr`). So this experiment adds some special magic hackery to `len` to avoid `x.len()` from retagging anything. `len` is already special in invisible ways by having the function call be replaced by a MIR op; this extends that magic for the purposes of our alias tracking. We also do one further change to the SB access logic: writing to a `Unique` does not invalidate SRW above this item. This, too, matches Tree Borrows. It does not affect local reasoning since owners of a `Unique` know whether any SRW have been pushed on top. SRW are now only created for `UnsafeCell`, so this allows aliasing of an `&UnsafeCell` with the `&mut UnsafeCell` from which it was derived (making it equivalent to a `*mut UnsafeCell` derived from that mutable reference). This is definitely not the final answer, but it is a minimal step that lets us make SB slightly cleaner, thus unlocking some patterns in the ecosystem that Miri has so far rejected, and some cleanup in the compiler. Longer-term plans include a new attribute one can put on functions to avoid retags -- putting that attribute on `len` *or* `as_mut_ptr` would fix the above example. The main potential downside of this PR is that it will lead Miri to accept code like `let x = 5; (&raw const x).cast_mut().write(0);` by default. This is discussed in rust-lang/unsafe-code-guidelines#400. Depending on how likely we are to make this UB in the future, we may want Miri to keep people away from those patterns. However, rejecting such code in an operational way is non-trivial; the only proposal we have is [arguably a hack](rust-lang/unsafe-code-guidelines#400 (comment)). Also, note that destructors will receive an `&mut self` pointing to such an immutable local variable, so operationally, it seems a bit futile to pretend that they are immutable. WIP: This also gets rid of the "quirk" in Stacked Borrows, where reading from some pointer disabled the `Unique` above that pointer, but did *not* touch SRW/SRO derived from that `Unique` pointer. This quirk was always quite unfortunate since it violates the stack discipline and can be used to write code that is accepted by SB but rejected by TB. The changes described above make the quirk a lot less effective since it no longer applies to raw pointers (as those no longer get a separate item in the stack); rather than having a quirk that only applies to particular situations involving `UnsafeCell`, IMO we should get rid of it entirely. TODO: - Check which further TB tests should be moved to "both borrows" now. - Entirely remove `RetagKind::Raw`. - Add mir-opt tests checking the MIR around `x.len()` looks the way we want it to. - If we can indeed get rid of the quirk: remove leftovers of "disable `Unique`" infrastructure (including removing `Permission::Disabled`). try-job: `*gnu*aux` try-job: `dist-x86_64-linux`
2 parents 244bbfc + add928f commit 54bf046

File tree

62 files changed

+372
-373
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+372
-373
lines changed
 

‎compiler/rustc_middle/src/mir/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1108,7 +1108,7 @@ pub enum LocalInfo<'tcx> {
11081108
/// A temporary created during evaluating `if` predicate, possibly for pattern matching for `let`s,
11091109
/// and subject to Edition 2024 temporary lifetime rules
11101110
IfThenRescopeTemp { if_then: HirId },
1111-
/// A temporary created during the pass `Derefer` to avoid it's retagging
1111+
/// A temporary created during the pass `Derefer` to avoid its retagging
11121112
DerefTemp,
11131113
/// A temporary created for borrow checking.
11141114
FakeBorrow,

‎compiler/rustc_mir_transform/src/add_retag.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
//! of MIR building, and only after this pass we think of the program has having the
55
//! normal MIR semantics.
66
7-
use rustc_hir::LangItem;
87
use rustc_middle::mir::*;
98
use rustc_middle::ty::{self, Ty, TyCtxt};
109

@@ -28,7 +27,6 @@ fn may_contain_reference<'tcx>(ty: Ty<'tcx>, depth: u32, tcx: TyCtxt<'tcx>) -> b
2827
// References and Boxes (`noalias` sources)
2928
ty::Ref(..) => true,
3029
ty::Adt(..) if ty.is_box() => true,
31-
ty::Adt(adt, _) if tcx.is_lang_item(adt.did(), LangItem::PtrUnique) => true,
3230
// Compound types: recurse
3331
ty::Array(ty, _) | ty::Slice(ty) => {
3432
// This does not branch so we keep the depth the same.
@@ -153,6 +151,13 @@ impl<'tcx> crate::MirPass<'tcx> for AddRetag {
153151
}
154152
}
155153
Rvalue::Ref(..) => None,
154+
// Also skip coercions whose source is already a local. This is crucial
155+
// to avoid unnecessary retags in `my_array.len()` calls.
156+
Rvalue::Cast(
157+
CastKind::PointerCoercion(_, CoercionSource::Implicit),
158+
Operand::Copy(p) | Operand::Move(p),
159+
_,
160+
) if p.as_local().is_some() => None,
156161
_ => {
157162
if needs_retag(place) {
158163
Some(RetagKind::Default)

0 commit comments

Comments
 (0)
Please sign in to comment.