Skip to content

Commit f8ff427

Browse files
RalfJungRalfJung
committed
Stacked Borrows: raw pointers inherit the tag from their parent pointer
1 parent 321dde1 commit f8ff427

Some content is hidden

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

44 files changed

+268
-200
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 retaggs 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)

‎compiler/rustc_mir_transform/src/lib.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,7 @@ fn run_runtime_lowering_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
628628
fn run_runtime_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
629629
let passes: &[&dyn MirPass<'tcx>] = &[
630630
&lower_intrinsics::LowerIntrinsics,
631+
&lower_slice_len::LowerSliceLenCalls,
631632
&remove_place_mention::RemovePlaceMention,
632633
&simplify::SimplifyCfg::PreOptimizations,
633634
];
@@ -671,9 +672,6 @@ pub(crate) fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'
671672
&check_null::CheckNull,
672673
// Before inlining: trim down MIR with passes to reduce inlining work.
673674

674-
// Has to be done before inlining, otherwise actual call will be almost always inlined.
675-
// Also simple, so can just do first.
676-
&lower_slice_len::LowerSliceLenCalls,
677675
// Perform instsimplify before inline to eliminate some trivial calls (like clone
678676
// shims).
679677
&instsimplify::InstSimplify::BeforeInline,

‎compiler/rustc_mir_transform/src/lower_slice_len.rs

Lines changed: 86 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,18 @@
22
//! It should run before inlining!
33
44
use rustc_hir::def_id::DefId;
5+
use rustc_index::IndexVec;
56
use rustc_middle::mir::*;
6-
use rustc_middle::ty::TyCtxt;
7+
use rustc_middle::ty::adjustment::PointerCoercion;
8+
use rustc_middle::ty::{Ty, TyCtxt};
79

810
pub(super) struct LowerSliceLenCalls;
911

1012
impl<'tcx> crate::MirPass<'tcx> for LowerSliceLenCalls {
1113
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
12-
sess.mir_opt_level() > 0
14+
// This pass also removes some otherwise-problematic retags, so we
15+
// always enable it when retags matter.
16+
sess.mir_opt_level() > 0 || sess.opts.unstable_opts.mir_emit_retag
1317
}
1418

1519
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
@@ -23,7 +27,7 @@ impl<'tcx> crate::MirPass<'tcx> for LowerSliceLenCalls {
2327
let basic_blocks = body.basic_blocks.as_mut_preserves_cfg();
2428
for block in basic_blocks {
2529
// lower `<[_]>::len` calls
26-
lower_slice_len_call(block, slice_len_fn_item_def_id);
30+
lower_slice_len_call(tcx, &mut body.local_decls, block, slice_len_fn_item_def_id);
2731
}
2832
}
2933

@@ -32,7 +36,12 @@ impl<'tcx> crate::MirPass<'tcx> for LowerSliceLenCalls {
3236
}
3337
}
3438

35-
fn lower_slice_len_call<'tcx>(block: &mut BasicBlockData<'tcx>, slice_len_fn_item_def_id: DefId) {
39+
fn lower_slice_len_call<'tcx>(
40+
tcx: TyCtxt<'tcx>,
41+
local_decls: &mut IndexVec<Local, LocalDecl<'tcx>>,
42+
block: &mut BasicBlockData<'tcx>,
43+
slice_len_fn_item_def_id: DefId,
44+
) {
3645
let terminator = block.terminator();
3746
if let TerminatorKind::Call {
3847
func,
@@ -50,11 +59,12 @@ fn lower_slice_len_call<'tcx>(block: &mut BasicBlockData<'tcx>, slice_len_fn_ite
5059
// perform modifications from something like:
5160
// _5 = core::slice::<impl [u8]>::len(move _6) -> bb1
5261
// into:
53-
// _5 = PtrMetadata(move _6)
62+
// _5 = PtrMetadata(move _6);
5463
// goto bb1
5564

5665
// make new RValue for Len
57-
let r_value = Rvalue::UnaryOp(UnOp::PtrMetadata, arg.node.clone());
66+
let arg = arg.node.clone();
67+
let r_value = Rvalue::UnaryOp(UnOp::PtrMetadata, arg.clone());
5868
let len_statement_kind = StatementKind::Assign(Box::new((*destination, r_value)));
5969
let add_statement =
6070
Statement { kind: len_statement_kind, source_info: terminator.source_info };
@@ -64,5 +74,75 @@ fn lower_slice_len_call<'tcx>(block: &mut BasicBlockData<'tcx>, slice_len_fn_ite
6474

6575
block.statements.push(add_statement);
6676
block.terminator_mut().kind = new_terminator_kind;
77+
78+
if tcx.sess.opts.unstable_opts.mir_emit_retag {
79+
// If we care about retags, we want there to be *no retag* for this `len` call.
80+
// That means we have to clean up the MIR a little: the MIR will now often be:
81+
// _6 = &*_4;
82+
// _5 = PtrMetadata(move _6);
83+
// Change that to:
84+
// _6 = &raw const *_4;
85+
// _5 = PtrMetadata(move _6);
86+
let len = block.statements.len();
87+
if len >= 2
88+
&& let &mut StatementKind::Assign(box (rebor_dest, ref mut rebor_r)) =
89+
&mut block.statements[len - 2].kind
90+
&& let Some(retag_local) = rebor_dest.as_local()
91+
&& !local_decls[retag_local].is_user_variable()
92+
// The LHS of this previous assignment is the `arg` from above.
93+
&& matches!(arg, Operand::Copy(p) | Operand::Move(p) if p.as_local() == Some(retag_local))
94+
// The RHS is a reference-taking operation.
95+
&& let Rvalue::Ref(_, BorrowKind::Shared, rebor_orig) = *rebor_r
96+
{
97+
let orig_inner_ty = rebor_orig.ty(local_decls, tcx).ty;
98+
// Change the previous statement to use `&raw` instead of `&`.
99+
*rebor_r = Rvalue::RawPtr(RawPtrKind::FakeForPtrMetadata, rebor_orig);
100+
// Change the type of the local to match.
101+
local_decls[retag_local].ty = Ty::new_ptr(tcx, orig_inner_ty, Mutability::Not);
102+
}
103+
// There's a second pattern we need to recognize for `array.len()` calls. There,
104+
// the MIR is:
105+
// _4 = &*_3;
106+
// _6 = move _4 as &[T] (PointerCoercion(Unsize, Implicit));
107+
// StorageDead(_4);
108+
// _5 = PtrMetadata(move _6);
109+
// Change that to:
110+
// _4 = &raw const *_3;
111+
// _6 = move _4 as *const [T] (PointerCoercion(Unsize, Implicit));
112+
// StorageDead(_4);
113+
// _5 = PtrMetadata(move _6);
114+
if len >= 4
115+
&& let [reborrow_stmt, unsize_stmt, storage_dead_stmt] =
116+
block.statements.get_disjoint_mut([len - 4, len - 3, len - 2]).unwrap()
117+
// Check that the 3 statements have the right shape.
118+
&& let &mut StatementKind::Assign(box (rebor_dest, ref mut rebor_r)) = &mut reborrow_stmt.kind
119+
&& let Some(retag_local) = rebor_dest.as_local()
120+
&& !local_decls[retag_local].is_user_variable()
121+
&& let &mut StatementKind::Assign(box (unsized_dest, ref mut unsize_r)) = &mut unsize_stmt.kind
122+
&& let Some(unsized_local) = unsized_dest.as_local()
123+
&& !local_decls[unsized_local].is_user_variable()
124+
&& storage_dead_stmt.kind == StatementKind::StorageDead(retag_local)
125+
// And check that they have the right operands.
126+
&& matches!(arg, Operand::Copy(p) | Operand::Move(p) if p.as_local() == Some(unsized_local))
127+
&& let &mut Rvalue::Cast(
128+
CastKind::PointerCoercion(PointerCoercion::Unsize, CoercionSource::Implicit),
129+
ref cast_op,
130+
ref mut cast_ty,
131+
) = unsize_r
132+
&& let Operand::Copy(unsize_src) | Operand::Move(unsize_src) = cast_op
133+
&& unsize_src.as_local() == Some(retag_local)
134+
&& let Rvalue::Ref(_, BorrowKind::Shared, rebor_orig) = *rebor_r
135+
{
136+
let orig_inner_ty = rebor_orig.ty(local_decls, tcx).ty;
137+
let unsize_inner_ty = cast_ty.builtin_deref(true).unwrap();
138+
// Change the reborrow to use `&raw` instead of `&`.
139+
*rebor_r = Rvalue::RawPtr(RawPtrKind::FakeForPtrMetadata, rebor_orig);
140+
// Change the unsize coercion in the same vein.
141+
*cast_ty = Ty::new_ptr(tcx, unsize_inner_ty, Mutability::Not);
142+
// Change the type of the locals to match.
143+
local_decls[retag_local].ty = Ty::new_ptr(tcx, orig_inner_ty, Mutability::Not);
144+
local_decls[unsized_local].ty = *cast_ty;
145+
}
146+
}
67147
}
68148
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 14 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,6 @@ impl NewPermission {
8686
}
8787
}
8888
}
89-
ty::RawPtr(_, Mutability::Mut) => {
90-
assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw.
91-
// Mutable raw pointer. No access, not protected.
92-
NewPermission::Uniform {
93-
perm: Permission::SharedReadWrite,
94-
access: None,
95-
protector: None,
96-
}
97-
}
9889
ty::Ref(_, _pointee, Mutability::Not) => {
9990
// Shared references. If frozen, these get `noalias` and `dereferenceable`; otherwise neither.
10091
NewPermission::FreezeSensitive {
@@ -110,17 +101,6 @@ impl NewPermission {
110101
// This fixes https://github.com/rust-lang/rust/issues/55005.
111102
}
112103
}
113-
ty::RawPtr(_, Mutability::Not) => {
114-
assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw.
115-
// `*const T`, when freshly created, are read-only in the frozen part.
116-
NewPermission::FreezeSensitive {
117-
freeze_perm: Permission::SharedReadOnly,
118-
freeze_access: Some(AccessKind::Read),
119-
freeze_protector: None,
120-
nonfreeze_perm: Permission::SharedReadWrite,
121-
nonfreeze_access: None,
122-
}
123-
}
124104
_ => unreachable!(),
125105
}
126106
}
@@ -196,11 +176,7 @@ impl<'tcx> Stack {
196176
match perm {
197177
Permission::SharedReadOnly => bug!("Cannot use SharedReadOnly for writing"),
198178
Permission::Disabled => bug!("Cannot use Disabled for anything"),
199-
Permission::Unique => {
200-
// On a write, everything above us is incompatible.
201-
granting + 1
202-
}
203-
Permission::SharedReadWrite => {
179+
Permission::Unique | Permission::SharedReadWrite => {
204180
// The SharedReadWrite *just* above us are compatible, to skip those.
205181
let mut idx = granting + 1;
206182
while let Some(item) = self.get(idx) {
@@ -863,12 +839,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
863839
val: &ImmTy<'tcx>,
864840
) -> InterpResult<'tcx, ImmTy<'tcx>> {
865841
let this = self.eval_context_mut();
866-
let new_perm = NewPermission::from_ref_ty(val.layout.ty, kind, this);
867842
let cause = match kind {
868843
RetagKind::TwoPhase => RetagCause::TwoPhase,
869844
RetagKind::FnEntry => unreachable!(),
870-
RetagKind::Raw | RetagKind::Default => RetagCause::Normal,
845+
RetagKind::Default => RetagCause::Normal,
846+
RetagKind::Raw => return interp_ok(val.clone()), // no raw retags!
871847
};
848+
let new_perm = NewPermission::from_ref_ty(val.layout.ty, kind, this);
872849
this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false })
873850
}
874851

@@ -942,14 +919,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
942919

943920
// Check the type of this value to see what to do with it (retag, or recurse).
944921
match place.layout.ty.kind() {
945-
ty::Ref(..) | ty::RawPtr(..) => {
946-
if matches!(place.layout.ty.kind(), ty::Ref(..))
947-
|| self.kind == RetagKind::Raw
948-
{
949-
let new_perm =
950-
NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx);
951-
self.retag_ptr_inplace(place, new_perm)?;
952-
}
922+
ty::Ref(..) => {
923+
let new_perm =
924+
NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx);
925+
self.retag_ptr_inplace(place, new_perm)?;
926+
}
927+
ty::RawPtr(_, _) => {
928+
// We definitely do *not* want to recurse into raw pointers -- wide raw
929+
// pointers have fields, and for dyn Trait pointees those can have reference
930+
// type!
931+
// We also do not want to reborrow them.
953932
}
954933
ty::Adt(adt, _) if adt.is_box() => {
955934
// Recurse for boxes, they require some tricky handling and will end up in `visit_box` above.

‎src/tools/miri/tests/fail/both_borrows/aliasing_mut3.stack.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ LL | pub fn safe(x: &mut i32, y: &i32) {
1212
help: <TAG> was created by a SharedReadOnly retag at offsets [0x0..0x4]
1313
--> tests/fail/both_borrows/aliasing_mut3.rs:LL:CC
1414
|
15-
LL | safe_raw(xraw, xshr);
16-
| ^^^^
15+
LL | let xshr = &*xref;
16+
| ^^^^^^
1717
help: <TAG> was later invalidated at offsets [0x0..0x4] by a Unique function-entry retag inside this call
1818
--> tests/fail/both_borrows/aliasing_mut3.rs:LL:CC
1919
|

‎src/tools/miri/tests/fail/both_borrows/box_exclusive_violation1.stack.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ LL | *LEAK = 7;
1212
help: <TAG> was created by a SharedReadOnly retag at offsets [0x0..0x4]
1313
--> tests/fail/both_borrows/box_exclusive_violation1.rs:LL:CC
1414
|
15-
LL | LEAK = x as *const _ as *mut _;
16-
| ^
15+
LL | fn unknown_code_1(x: &i32) {
16+
| ^
1717
help: <TAG> was later invalidated at offsets [0x0..0x4] by a write access
1818
--> tests/fail/both_borrows/box_exclusive_violation1.rs:LL:CC
1919
|

‎src/tools/miri/tests/fail/both_borrows/box_noalias_violation.stack.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | *y
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9-
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
9+
help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1010
--> tests/fail/both_borrows/box_noalias_violation.rs:LL:CC
1111
|
1212
LL | let ptr = &mut v as *mut i32;

‎src/tools/miri/tests/fail/both_borrows/illegal_write1.stack.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ LL | unsafe { *x = 42 };
1212
help: <TAG> was created by a SharedReadOnly retag at offsets [0x0..0x4]
1313
--> tests/fail/both_borrows/illegal_write1.rs:LL:CC
1414
|
15-
LL | let x: *mut u32 = xref as *const _ as *mut _;
16-
| ^^^^
15+
LL | let xref = &*target;
16+
| ^^^^^^^^
1717
= note: BACKTRACE (of the first span):
1818
= note: inside `main` at tests/fail/both_borrows/illegal_write1.rs:LL:CC
1919

‎src/tools/miri/tests/fail/both_borrows/illegal_write6.stack.stderr

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ LL | unsafe { *y = 2 };
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9-
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
9+
help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1010
--> tests/fail/both_borrows/illegal_write6.rs:LL:CC
1111
|
12-
LL | let p = x as *mut u32;
13-
| ^
12+
LL | let x = &mut 0u32;
13+
| ^^^^^^^^^
1414
help: <TAG> is this argument
1515
--> tests/fail/both_borrows/illegal_write6.rs:LL:CC
1616
|

‎src/tools/miri/tests/fail/both_borrows/invalidate_against_protector2.stack.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | unsafe { *x = 0 };
66
|
77
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
88
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
9-
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
9+
help: <TAG> was created by a Unique retag at offsets [0x0..0x4]
1010
--> tests/fail/both_borrows/invalidate_against_protector2.rs:LL:CC
1111
|
1212
LL | let xraw = &mut x as *mut _;

‎src/tools/miri/tests/fail/both_borrows/mut_exclusive_violation1.stack.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ LL | *LEAK = 7;
1212
help: <TAG> was created by a SharedReadOnly retag at offsets [0x0..0x4]
1313
--> tests/fail/both_borrows/mut_exclusive_violation1.rs:LL:CC
1414
|
15-
LL | LEAK = x as *const _ as *mut _;
16-
| ^
15+
LL | fn unknown_code_1(x: &i32) {
16+
| ^
1717
help: <TAG> was later invalidated at offsets [0x0..0x4] by a write access
1818
--> tests/fail/both_borrows/mut_exclusive_violation1.rs:LL:CC
1919
|

‎src/tools/miri/tests/fail/both_borrows/mut_raw_mut2.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//@revisions: stack tree
2+
//@[tree]compile-flags: -Zmiri-tree-borrows
3+
4+
// This used to accidentally be accepted by SB.
5+
fn main() {
6+
unsafe {
7+
let mut root = 0;
8+
let to = &mut root as *mut i32;
9+
*to = 0;
10+
let _val = root;
11+
*to = 0;
12+
//~[stack]^ ERROR: tag does not exist in the borrow stack
13+
//~[tree]| ERROR: forbidden
14+
}
15+
}

0 commit comments

Comments
 (0)