Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
switch validation of scalars to be type-driven
This does not actually regress anything. It would regress NonNull, but we didn't handle that correctly previously either.
- Loading branch information
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ use rustc::ty::layout::{self, Size}; | |
| use rustc::ty::{self, Ty}; | ||
| use rustc_data_structures::fx::FxHashSet; | ||
| use rustc::mir::interpret::{ | ||
| Scalar, AllocType, EvalResult, EvalErrorKind | ||
| }; | ||
|
|
||
| use super::{ | ||
|
|
@@ -50,6 +50,13 @@ macro_rules! validation_failure { | |
| } | ||
|
|
||
| macro_rules! try_validation { | ||
| ($e:expr, $what:expr, $where:expr, $details:expr) => {{ | ||
| match $e { | ||
| Ok(x) => x, | ||
| Err(_) => return validation_failure!($what, $where, $details), | ||
| } | ||
| }}; | ||
|
|
||
| ($e:expr, $what:expr, $where:expr) => {{ | ||
| match $e { | ||
| Ok(x) => x, | ||
|
|
@@ -121,114 +128,65 @@ fn path_format(path: &Vec<PathElem>) -> String { | |
| out | ||
| } | ||
|
|
||
| fn scalar_format(value: ScalarMaybeUndef) -> String { | ||
| match value { | ||
| ScalarMaybeUndef::Undef => | ||
| "uninitialized bytes".to_owned(), | ||
| ScalarMaybeUndef::Scalar(Scalar::Ptr(_)) => | ||
| "a pointer".to_owned(), | ||
| ScalarMaybeUndef::Scalar(Scalar::Bits { bits, .. }) => | ||
| bits.to_string(), | ||
| } | ||
| } | ||
|
|
||
| impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { | ||
| fn validate_scalar( | ||
| &self, | ||
| value: ScalarMaybeUndef, | ||
| size: Size, | ||
| path: &Vec<PathElem>, | ||
| ty: Ty, | ||
| ) -> EvalResult<'tcx> { | ||
| trace!("validate scalar: {:#?}, {:#?}, {}", value, size, ty); | ||
|
|
||
| // Go over all the primitive types | ||
| match ty.sty { | ||
| ty::Bool => { | ||
| try_validation!(value.to_bool(), | ||
| scalar_format(value), path, "a boolean"); | ||
| }, | ||
| ty::Char => { | ||
| try_validation!(value.to_char(), | ||
| scalar_format(value), path, "a valid unicode codepoint"); | ||
| }, | ||
| ty::Float(_) | ty::Int(_) | ty::Uint(_) => { | ||
| // Must be scalar bits | ||
| try_validation!(value.to_bits(size), | ||
| scalar_format(value), path, "initialized plain bits"); | ||
| } | ||
| ty::RawPtr(_) => { | ||
| // Anything but undef goes | ||
| try_validation!(value.not_undef(), | ||
| scalar_format(value), path, "a raw pointer"); | ||
| }, | ||
| ty::Ref(..) => { | ||
| // This is checked by the recursive reference handling, nothing to do here. | ||
| debug_assert!(ty.builtin_deref(true).is_some() && !ty.is_unsafe_ptr()); | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong. 
RalfJung
Author
Owner
|
||
| } | ||
| ty::FnPtr(_sig) => { | ||
| let ptr = try_validation!(value.to_ptr(), | ||
| scalar_format(value), path, "a pointer"); | ||
| let _fn = try_validation!(self.memory.get_fn(ptr), | ||
| scalar_format(value), path, "a function pointer"); | ||
| // TODO: Check if the signature matches | ||
| } | ||
| ty::FnDef(..) => { | ||
| // This is a zero-sized type with all relevant data sitting in the type. | ||
| // There is nothing to validate. | ||
| } | ||
| _ => bug!("Unexpected primitive type {}", ty) | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Validate a reference, potentially recursively. `place` is assumed to already be | ||
|
|
@@ -240,10 +198,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| ref_tracking: Option<&mut RefTracking<'tcx>>, | ||
| ) -> EvalResult<'tcx> { | ||
| // Before we do anything else, make sure this is entirely in-bounds. | ||
| let (size, align) = self.size_and_align_of(place.extra, place.layout)?; | ||
| try_validation!(self.memory.check_align(place.ptr, align), | ||
| "unaligned reference", path); | ||
| if !place.layout.is_zst() { | ||
| let ptr = try_validation!(place.ptr.to_ptr(), | ||
| "integer pointer in non-ZST reference", path); | ||
| try_validation!(self.memory.check_bounds(ptr, size, false), | ||
| "dangling reference (not entirely in bounds)", path); | ||
| // Skip recursion for some external statics | ||
|
|
@@ -277,6 +237,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| /// It will error if the bits at the destination do not match the ones described by the layout. | ||
| /// The `path` may be pushed to, but the part that is present when the function | ||
| /// starts must not be changed! | ||
| /// | ||
| /// `ref_tracking` can be None to avoid recursive checking below references. | ||
| /// This also toggles between "run-time" (no recursion) and "compile-time" (with recursion) | ||
| /// validation (e.g., pointer values are fine in integers at runtime). | ||
| pub fn validate_operand( | ||
| &self, | ||
| dest: OpTy<'tcx>, | ||
|
|
@@ -346,12 +310,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| return validation_failure!("a value of an uninhabited type", path), | ||
| // check that the scalar is a valid pointer or that its bit range matches the | ||
| // expectation. | ||
| layout::Abi::Scalar(_) => { | ||
| let value = try_validation!(self.read_value(dest), | ||
| "uninitialized or unrepresentable data", path); | ||
| let scalar = value.to_scalar_or_undef(); | ||
| self.validate_scalar(scalar, dest.layout.size, &path, dest.layout.ty)?; | ||
| // Recursively check *safe* references | ||
| if dest.layout.ty.builtin_deref(true).is_some() && | ||
| !dest.layout.ty.is_unsafe_ptr() | ||
|
|
@@ -365,7 +328,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| layout::FieldPlacement::Arbitrary { .. } | ||
| if dest.layout.ty.builtin_deref(true).is_some() => | ||
| { | ||
| // This is a fat pointer. We also check fat raw pointers, their metadata must | ||
| // be valid! | ||
| let ptr = try_validation!(self.read_value(dest.into()), | ||
| "undefined location in fat pointer", path); | ||
| let ptr = try_validation!(self.ref_to_mplace(ptr), | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| error[E0080]: this constant likely exhibits undefined behavior | ||
| --> $DIR/ub-ref.rs:15:1 | ||
| | | ||
| LL | const UNALIGNED: &u16 = unsafe { mem::transmute(&[0u8; 4]) }; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered unaligned reference | ||
| | | ||
| = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior | ||
|
|
||
| error[E0080]: this constant likely exhibits undefined behavior | ||
| --> $DIR/ub-ref.rs:18:1 | ||
| | | ||
| LL | const REF_AS_USIZE: usize = unsafe { mem::transmute(&0) }; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits | ||
| | | ||
| = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior | ||
|
|
||
| error[E0080]: this constant likely exhibits undefined behavior | ||
| --> $DIR/ub-ref.rs:21:1 | ||
| | | ||
| LL | const USIZE_AS_REF: &'static u8 = unsafe { mem::transmute(1337usize) }; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered integer pointer in non-ZST reference | ||
| | | ||
| = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior | ||
|
|
||
| error: aborting due to 3 previous errors | ||
|
|
||
| For more information about this error, try `rustc --explain E0080`. |
Oops, something went wrong.
I don't get this debug assert. are you sanity checking the methods on
Ty?