Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 15.5k
miri/const eval: support MaybeDangling#150446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<RangeSet>, | ||
| /// 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. | ||
RalfJung marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // 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::<Option<AllocId>>::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. | ||
RalfJung marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // 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::<Option<AllocId>>::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)?; | ||
WaffleLapkin marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() => { | ||
WaffleLapkin marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // 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 | ||
RalfJung marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<*const u16>, MaybeDangling<&u16>>(unaligned) | ||
| //~^ ERROR: Undefined Behavior: constructing invalid value: encountered an unaligned reference | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<*const u16>, 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 | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't even a borrow checking thing. Please move the test to |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<*const u8>, MaybeDangling<&u8>>(null) }; | ||
| //~^ ERROR: Undefined Behavior: constructing invalid value: encountered a null reference | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<*const u8>, 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 | ||
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<Box<u32>> = 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<&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); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| 0..1: [ SharedReadWrite<TAG> ] | ||
| 0..1: [ SharedReadWrite<TAG> ] | ||
| 0..1: [ SharedReadWrite<TAG> ] | ||
| 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> ] | ||
| 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> ] | ||
| 0..1: [ SharedReadWrite<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> Unique<TAG> ] | ||
| 0..1: [ SharedReadWrite<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> Disabled<TAG> SharedReadOnly<TAG> ] | ||
| 0..1: [ unknown-bottom(..<TAG>) ] |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.