Skip to content

Commit 76a750a

Browse files
committed
miri/const eval: support MaybeDangling
1 parent 98e7077 commit 76a750a

9 files changed

Lines changed: 167 additions & 26 deletions

File tree

compiler/rustc_const_eval/src/interpret/validity.rs

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use std::borrow::Cow;
88
use std::fmt::Write;
99
use std::hash::Hash;
10+
use std::mem;
1011
use std::num::NonZero;
1112

1213
use either::{Left, Right};
@@ -288,6 +289,8 @@ struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> {
288289
/// If this is `Some`, then `reset_provenance_and_padding` must be true (but not vice versa:
289290
/// we might not track data vs padding bytes if the operand isn't stored in memory anyway).
290291
data_bytes: Option<RangeSet>,
292+
/// True if we are inside of `MaybeDangling`. This disables pointer access checks.
293+
may_dangle: bool,
291294
}
292295

293296
impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
@@ -489,7 +492,8 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
489492
if place.layout.is_unsized() {
490493
self.check_wide_ptr_meta(place.meta(), place.layout)?;
491494
}
492-
// Make sure this is dereferenceable and all.
495+
496+
// Determine size and alignment of pointee.
493497
let size_and_align = try_validation!(
494498
self.ecx.size_and_align_of_val(&place),
495499
self.path,
@@ -503,27 +507,33 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
503507
// alignment and size determined by the layout (size will be 0,
504508
// alignment should take attributes into account).
505509
.unwrap_or_else(|| (place.layout.size, place.layout.align.abi));
506-
// Direct call to `check_ptr_access_align` checks alignment even on CTFE machines.
507-
try_validation!(
508-
self.ecx.check_ptr_access(
509-
place.ptr(),
510-
size,
511-
CheckInAllocMsg::Dereferenceable, // will anyway be replaced by validity message
512-
),
513-
self.path,
514-
Ub(DanglingIntPointer { addr: 0, .. }) => NullPtr { ptr_kind, maybe: false },
515-
Ub(DanglingIntPointer { addr: i, .. }) => DanglingPtrNoProvenance {
516-
ptr_kind,
517-
// FIXME this says "null pointer" when null but we need translate
518-
pointer: format!("{}", Pointer::<Option<AllocId>>::without_provenance(i))
519-
},
520-
Ub(PointerOutOfBounds { .. }) => DanglingPtrOutOfBounds {
521-
ptr_kind
522-
},
523-
Ub(PointerUseAfterFree(..)) => DanglingPtrUseAfterFree {
524-
ptr_kind,
525-
},
526-
);
510+
511+
if !self.may_dangle {
512+
// Make sure this is dereferenceable and all.
513+
514+
// Direct call to `check_ptr_access_align` checks alignment even on CTFE machines.
515+
// Call `check_ptr_access` to avoid checking alignment here.
516+
try_validation!(
517+
self.ecx.check_ptr_access(
518+
place.ptr(),
519+
size,
520+
CheckInAllocMsg::Dereferenceable, // will anyway be replaced by validity message
521+
),
522+
self.path,
523+
Ub(DanglingIntPointer { addr: 0, .. }) => NullPtr { ptr_kind, maybe: false },
524+
Ub(DanglingIntPointer { addr: i, .. }) => DanglingPtrNoProvenance {
525+
ptr_kind,
526+
pointer: format!("{}", Pointer::<Option<AllocId>>::without_provenance(i))
527+
},
528+
Ub(PointerOutOfBounds { .. }) => DanglingPtrOutOfBounds {
529+
ptr_kind
530+
},
531+
Ub(PointerUseAfterFree(..)) => DanglingPtrUseAfterFree {
532+
ptr_kind,
533+
},
534+
);
535+
}
536+
527537
try_validation!(
528538
self.ecx.check_ptr_align(
529539
place.ptr(),
@@ -536,8 +546,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
536546
found_bytes: has.bytes()
537547
},
538548
);
539-
// Make sure this is non-null. We checked dereferenceability above, but if `size` is zero
540-
// that does not imply non-null.
549+
550+
// Make sure this is non-null. This is obviously needed when `may_dangle` is set,
551+
// but even if we did check dereferenceability above that would still allow null
552+
// pointers if `size` is zero.
541553
let scalar = Scalar::from_maybe_pointer(place.ptr(), self.ecx);
542554
if self.ecx.scalar_may_be_null(scalar)? {
543555
let maybe = !M::Provenance::OFFSET_IS_ADDR && matches!(scalar, Scalar::Ptr(..));
@@ -1265,6 +1277,14 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
12651277
ty::PatternKind::Or(_patterns) => {}
12661278
}
12671279
}
1280+
ty::Adt(adt, _) if adt.is_maybe_dangling() => {
1281+
let old_may_dangle = mem::replace(&mut self.may_dangle, true);
1282+
1283+
let inner = self.ecx.project_field(val, FieldIdx::ZERO)?;
1284+
self.visit_value(&inner)?;
1285+
1286+
self.may_dangle = old_may_dangle;
1287+
}
12681288
_ => {
12691289
// default handler
12701290
try_validation!(
@@ -1350,6 +1370,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
13501370
ecx,
13511371
reset_provenance_and_padding,
13521372
data_bytes: reset_padding.then_some(RangeSet(Vec::new())),
1373+
may_dangle: false,
13531374
};
13541375
v.visit_value(val)?;
13551376
v.reset_padding(val)?;

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
917917
RetagInfo { cause: self.retag_cause, in_field: self.in_field },
918918
)?;
919919
self.ecx.write_immediate(*val, place)?;
920+
920921
interp_ok(())
921922
}
922923
}
@@ -964,6 +965,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
964965
// even if field retagging is not enabled. *shrug*)
965966
self.walk_value(place)?;
966967
}
968+
ty::Adt(adt, _) if adt.is_maybe_dangling() => {
969+
// Skip traversing for everything inside of `MaybeDangling`
970+
}
967971
_ => {
968972
// Not a reference/pointer/box. Recurse.
969973
let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
523523
// even if field retagging is not enabled. *shrug*)
524524
self.walk_value(place)?;
525525
}
526+
ty::Adt(adt, _) if adt.is_maybe_dangling() => {
527+
// Skip traversing for everything inside of `MaybeDangling`
528+
}
526529
_ => {
527530
// Not a reference/pointer/box. Recurse.
528531
self.walk_value(place)?;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Test that an unaligned `MaybeDangling<&u8>` is still detected as UB.
2+
//
3+
//@compile-flags: -Zmiri-disable-stacked-borrows
4+
#![feature(maybe_dangling)]
5+
6+
use std::mem::{MaybeDangling, transmute};
7+
8+
fn main() {
9+
let a = [1u16, 0u16];
10+
unsafe {
11+
let unaligned = MaybeDangling::new(a.as_ptr().byte_add(1));
12+
transmute::<MaybeDangling<*const u16>, MaybeDangling<&u16>>(unaligned)
13+
//~^ ERROR: Undefined Behavior: constructing invalid value: encountered an unaligned reference
14+
};
15+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: Undefined Behavior: constructing invalid value: encountered an unaligned reference (required ALIGN byte alignment but found ALIGN)
2+
--> tests/fail/unaligned_pointers/maybe_dangling_unalighed.rs:LL:CC
3+
|
4+
LL | transmute::<MaybeDangling<*const u16>, MaybeDangling<&u16>>(unaligned)
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
10+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
11+
12+
error: aborting due to 1 previous error
13+
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Test that a null `MaybeDangling<&u8>` is still detected as UB.
2+
//
3+
//@compile-flags: -Zmiri-disable-stacked-borrows
4+
#![feature(maybe_dangling)]
5+
6+
use std::mem::{MaybeDangling, transmute};
7+
use std::ptr::null;
8+
9+
fn main() {
10+
let null = MaybeDangling::new(null());
11+
unsafe { transmute::<MaybeDangling<*const u8>, MaybeDangling<&u8>>(null) };
12+
//~^ ERROR: Undefined Behavior: constructing invalid value: encountered a null reference
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: Undefined Behavior: constructing invalid value: encountered a null reference
2+
--> tests/fail/validity/maybe_dangling_null.rs:LL:CC
3+
|
4+
LL | unsafe { transmute::<MaybeDangling<*const u8>, MaybeDangling<&u8>>(null) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
10+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
11+
12+
error: aborting due to 1 previous error
13+
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Check that `MaybeDangling` actually prevents UB when it wraps dangling
2+
// boxes and references
3+
//
4+
//@revisions: stack tree
5+
//@[tree]compile-flags: -Zmiri-tree-borrows
6+
#![feature(maybe_dangling)]
7+
8+
use std::mem::{self, MaybeDangling};
9+
use std::ptr::drop_in_place;
10+
11+
fn main() {
12+
boxy();
13+
reference();
14+
write_through_shared_ref();
15+
}
16+
17+
fn boxy() {
18+
let mut x = MaybeDangling::new(Box::new(1));
19+
20+
// make the box dangle
21+
unsafe { drop_in_place(x.as_mut()) };
22+
23+
// move the dangling box (without `MaybeDangling` this causes UB)
24+
let x: MaybeDangling<Box<u32>> = x;
25+
26+
mem::forget(x);
27+
}
28+
29+
fn reference() {
30+
let x = {
31+
let local = 0;
32+
33+
// erase the lifetime to make a dangling reference
34+
unsafe {
35+
mem::transmute::<MaybeDangling<&u32>, MaybeDangling<&u32>>(MaybeDangling::new(&local))
36+
}
37+
};
38+
39+
// move the dangling reference (without `MaybeDangling` this causes UB)
40+
let _x: MaybeDangling<&u32> = x;
41+
}
42+
43+
fn write_through_shared_ref() {
44+
// Under the current models, we do not forbid writing through
45+
// `MaybeDangling<&i32>`. That's not yet finally decided, but meanwhile
46+
// ensure we document this and notice when it changes.
47+
48+
unsafe {
49+
let mutref = &mut 0;
50+
write_through_shr(mem::transmute(mutref));
51+
}
52+
53+
fn write_through_shr(x: MaybeDangling<&i32>) {
54+
unsafe {
55+
let y: *mut i32 = mem::transmute(x);
56+
y.write(1);
57+
}
58+
}
59+
}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
0..1: [ SharedReadWrite<TAG> ]
22
0..1: [ SharedReadWrite<TAG> ]
33
0..1: [ SharedReadWrite<TAG> ]
4-
0..1: [ SharedReadWrite<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> ]
5-
0..1: [ SharedReadWrite<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> SharedReadOnly<TAG> ]
4+
0..1: [ SharedReadWrite<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> ]
5+
0..1: [ SharedReadWrite<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> SharedReadOnly<TAG> ]
66
0..1: [ unknown-bottom(..<TAG>) ]

0 commit comments

Comments
 (0)