diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index 2cf490350e907..b72927f455c76 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -7,6 +7,7 @@ use std::borrow::Cow; use std::fmt::Write; use std::hash::Hash; +use std::mem; use std::num::NonZero; use either::{Left, Right}; @@ -288,6 +289,8 @@ struct ValidityVisitor<'rt, 'tcx, M: Machine<'tcx>> { /// If this is `Some`, then `reset_provenance_and_padding` must be true (but not vice versa: /// we might not track data vs padding bytes if the operand isn't stored in memory anyway). data_bytes: Option, + /// True if we are inside of `MaybeDangling`. This disables pointer access checks. + may_dangle: bool, } impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { @@ -489,7 +492,8 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { if place.layout.is_unsized() { self.check_wide_ptr_meta(place.meta(), place.layout)?; } - // Make sure this is dereferenceable and all. + + // Determine size and alignment of pointee. let size_and_align = try_validation!( self.ecx.size_and_align_of_val(&place), self.path, @@ -503,27 +507,33 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { // alignment and size determined by the layout (size will be 0, // alignment should take attributes into account). .unwrap_or_else(|| (place.layout.size, place.layout.align.abi)); - // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines. - try_validation!( - self.ecx.check_ptr_access( - place.ptr(), - size, - CheckInAllocMsg::Dereferenceable, // will anyway be replaced by validity message - ), - self.path, - Ub(DanglingIntPointer { addr: 0, .. }) => NullPtr { ptr_kind, maybe: false }, - Ub(DanglingIntPointer { addr: i, .. }) => DanglingPtrNoProvenance { - ptr_kind, - // FIXME this says "null pointer" when null but we need translate - pointer: format!("{}", Pointer::>::without_provenance(i)) - }, - Ub(PointerOutOfBounds { .. }) => DanglingPtrOutOfBounds { - ptr_kind - }, - Ub(PointerUseAfterFree(..)) => DanglingPtrUseAfterFree { - ptr_kind, - }, - ); + + if !self.may_dangle { + // Make sure this is dereferenceable and all. + + // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines. + // Call `check_ptr_access` to avoid checking alignment here. + try_validation!( + self.ecx.check_ptr_access( + place.ptr(), + size, + CheckInAllocMsg::Dereferenceable, // will anyway be replaced by validity message + ), + self.path, + Ub(DanglingIntPointer { addr: 0, .. }) => NullPtr { ptr_kind, maybe: false }, + Ub(DanglingIntPointer { addr: i, .. }) => DanglingPtrNoProvenance { + ptr_kind, + pointer: format!("{}", Pointer::>::without_provenance(i)) + }, + Ub(PointerOutOfBounds { .. }) => DanglingPtrOutOfBounds { + ptr_kind + }, + Ub(PointerUseAfterFree(..)) => DanglingPtrUseAfterFree { + ptr_kind, + }, + ); + } + try_validation!( self.ecx.check_ptr_align( place.ptr(), @@ -536,8 +546,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> { found_bytes: has.bytes() }, ); - // Make sure this is non-null. We checked dereferenceability above, but if `size` is zero - // that does not imply non-null. + + // Make sure this is non-null. This is obviously needed when `may_dangle` is set, + // but even if we did check dereferenceability above that would still allow null + // pointers if `size` is zero. let scalar = Scalar::from_maybe_pointer(place.ptr(), self.ecx); if self.ecx.scalar_may_be_null(scalar)? { 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, ty::PatternKind::Or(_patterns) => {} } } + ty::Adt(adt, _) if adt.is_maybe_dangling() => { + let old_may_dangle = mem::replace(&mut self.may_dangle, true); + + let inner = self.ecx.project_field(val, FieldIdx::ZERO)?; + self.visit_value(&inner)?; + + self.may_dangle = old_may_dangle; + } _ => { // default handler try_validation!( @@ -1350,6 +1370,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { ecx, reset_provenance_and_padding, data_bytes: reset_padding.then_some(RangeSet(Vec::new())), + may_dangle: false, }; v.visit_value(val)?; v.reset_padding(val)?; diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 617b619a7df90..66e804d972b76 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -917,6 +917,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { RetagInfo { cause: self.retag_cause, in_field: self.in_field }, )?; self.ecx.write_immediate(*val, place)?; + interp_ok(()) } } @@ -964,6 +965,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // even if field retagging is not enabled. *shrug*) self.walk_value(place)?; } + ty::Adt(adt, _) if adt.is_maybe_dangling() => { + // Skip traversing for everything inside of `MaybeDangling` + } _ => { // Not a reference/pointer/box. Recurse. let in_field = mem::replace(&mut self.in_field, true); // remember and restore old value diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 1a7c0af2988a3..a205502327307 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -523,6 +523,9 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // even if field retagging is not enabled. *shrug*) self.walk_value(place)?; } + ty::Adt(adt, _) if adt.is_maybe_dangling() => { + // Skip traversing for everything inside of `MaybeDangling` + } _ => { // Not a reference/pointer/box. Recurse. self.walk_value(place)?; diff --git a/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.rs b/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.rs new file mode 100644 index 0000000000000..92917c88b7312 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.rs @@ -0,0 +1,15 @@ +// Test that an unaligned `MaybeDangling<&u8>` is still detected as UB. +// +//@compile-flags: -Zmiri-disable-stacked-borrows +#![feature(maybe_dangling)] + +use std::mem::{MaybeDangling, transmute}; + +fn main() { + let a = [1u16, 0u16]; + unsafe { + let unaligned = MaybeDangling::new(a.as_ptr().byte_add(1)); + transmute::, MaybeDangling<&u16>>(unaligned) + //~^ ERROR: Undefined Behavior: constructing invalid value: encountered an unaligned reference + }; +} diff --git a/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.stderr b/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.stderr new file mode 100644 index 0000000000000..9a50a031cf96e --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/maybe_dangling_unalighed.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: constructing invalid value: encountered an unaligned reference (required ALIGN byte alignment but found ALIGN) + --> tests/fail/unaligned_pointers/maybe_dangling_unalighed.rs:LL:CC + | +LL | transmute::, MaybeDangling<&u16>>(unaligned) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/validity/maybe_dangling_null.rs b/src/tools/miri/tests/fail/validity/maybe_dangling_null.rs new file mode 100644 index 0000000000000..21dba864a3f12 --- /dev/null +++ b/src/tools/miri/tests/fail/validity/maybe_dangling_null.rs @@ -0,0 +1,13 @@ +// Test that a null `MaybeDangling<&u8>` is still detected as UB. +// +//@compile-flags: -Zmiri-disable-stacked-borrows +#![feature(maybe_dangling)] + +use std::mem::{MaybeDangling, transmute}; +use std::ptr::null; + +fn main() { + let null = MaybeDangling::new(null()); + unsafe { transmute::, MaybeDangling<&u8>>(null) }; + //~^ ERROR: Undefined Behavior: constructing invalid value: encountered a null reference +} diff --git a/src/tools/miri/tests/fail/validity/maybe_dangling_null.stderr b/src/tools/miri/tests/fail/validity/maybe_dangling_null.stderr new file mode 100644 index 0000000000000..650234777ccf4 --- /dev/null +++ b/src/tools/miri/tests/fail/validity/maybe_dangling_null.stderr @@ -0,0 +1,13 @@ +error: Undefined Behavior: constructing invalid value: encountered a null reference + --> tests/fail/validity/maybe_dangling_null.rs:LL:CC + | +LL | unsafe { transmute::, MaybeDangling<&u8>>(null) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass/both_borrows/maybe_dangling.rs b/src/tools/miri/tests/pass/both_borrows/maybe_dangling.rs new file mode 100644 index 0000000000000..fe2b526d94b19 --- /dev/null +++ b/src/tools/miri/tests/pass/both_borrows/maybe_dangling.rs @@ -0,0 +1,59 @@ +// Check that `MaybeDangling` actually prevents UB when it wraps dangling +// boxes and references +// +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(maybe_dangling)] + +use std::mem::{self, MaybeDangling}; +use std::ptr::drop_in_place; + +fn main() { + boxy(); + reference(); + write_through_shared_ref(); +} + +fn boxy() { + let mut x = MaybeDangling::new(Box::new(1)); + + // make the box dangle + unsafe { drop_in_place(x.as_mut()) }; + + // move the dangling box (without `MaybeDangling` this causes UB) + let x: MaybeDangling> = x; + + mem::forget(x); +} + +fn reference() { + let x = { + let local = 0; + + // erase the lifetime to make a dangling reference + unsafe { + mem::transmute::, MaybeDangling<&u32>>(MaybeDangling::new(&local)) + } + }; + + // move the dangling reference (without `MaybeDangling` this causes UB) + let _x: MaybeDangling<&u32> = x; +} + +fn write_through_shared_ref() { + // Under the current models, we do not forbid writing through + // `MaybeDangling<&i32>`. That's not yet finally decided, but meanwhile + // ensure we document this and notice when it changes. + + unsafe { + let mutref = &mut 0; + write_through_shr(mem::transmute(mutref)); + } + + fn write_through_shr(x: MaybeDangling<&i32>) { + unsafe { + let y: *mut i32 = mem::transmute(x); + y.write(1); + } + } +} diff --git a/src/tools/miri/tests/pass/stacked_borrows/stack-printing.stdout b/src/tools/miri/tests/pass/stacked_borrows/stack-printing.stdout index de7da309271da..296339e738455 100644 --- a/src/tools/miri/tests/pass/stacked_borrows/stack-printing.stdout +++ b/src/tools/miri/tests/pass/stacked_borrows/stack-printing.stdout @@ -1,6 +1,6 @@ 0..1: [ SharedReadWrite ] 0..1: [ SharedReadWrite ] 0..1: [ SharedReadWrite ] -0..1: [ SharedReadWrite Unique Unique Unique Unique Unique Unique Unique Unique Unique Unique Unique ] -0..1: [ SharedReadWrite Disabled Disabled Disabled Disabled Disabled Disabled Disabled Disabled Disabled Disabled Disabled SharedReadOnly ] +0..1: [ SharedReadWrite Unique Unique Unique Unique Unique Unique Unique ] +0..1: [ SharedReadWrite Disabled Disabled Disabled Disabled Disabled Disabled Disabled SharedReadOnly ] 0..1: [ unknown-bottom(..) ]