From 69a320f40d802060c27d022e98e91bba0a4cc537 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Oct 2018 20:05:12 +0200 Subject: [PATCH] also validate everything that has a Scalar layout, to catch NonNull --- src/librustc_mir/interpret/validity.rs | 94 +++++++++++++++++-- .../consts/const-eval/transmute-const.stderr | 2 +- src/test/ui/consts/const-eval/ub-enum.stderr | 2 +- src/test/ui/consts/const-eval/ub-nonnull.rs | 25 +++++ .../ui/consts/const-eval/ub-nonnull.stderr | 27 ++++++ .../ui/consts/const-eval/union-ice.stderr | 2 +- src/test/ui/consts/const-eval/union-ub.stderr | 2 +- src/test/ui/union-ub-fat-ptr.stderr | 8 +- 8 files changed, 146 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/consts/const-eval/ub-nonnull.rs create mode 100644 src/test/ui/consts/const-eval/ub-nonnull.stderr diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 23f09cf108e..a174645b7ef 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -140,14 +140,15 @@ fn scalar_format(value: ScalarMaybeUndef) -> String { } impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { - fn validate_scalar( + /// Make sure that `value` is valid for `ty` + fn validate_scalar_type( &self, value: ScalarMaybeUndef, size: Size, path: &Vec, ty: Ty, ) -> EvalResult<'tcx> { - trace!("validate scalar: {:#?}, {:#?}, {}", value, size, ty); + trace!("validate scalar by type: {:#?}, {:#?}, {}", value, size, ty); // Go over all the primitive types match ty.sty { @@ -189,6 +190,62 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok(()) } + /// Make sure that `value` matches the + fn validate_scalar_layout( + &self, + value: ScalarMaybeUndef, + size: Size, + path: &Vec, + layout: &layout::Scalar, + ) -> EvalResult<'tcx> { + trace!("validate scalar by layout: {:#?}, {:#?}, {:#?}", value, size, layout); + let (lo, hi) = layout.valid_range.clone().into_inner(); + if lo == u128::min_value() && hi == u128::max_value() { + // Nothing to check + return Ok(()); + } + // At least one value is excluded. Get the bits. + let value = try_validation!(value.not_undef(), + scalar_format(value), path, format!("something in the range {:?}", layout.valid_range)); + let bits = match value { + Scalar::Ptr(_) => { + // Comparing a ptr with a range is not meaningfully possible. + // In principle, *if* the pointer is inbonds, we could exclude NULL, but + // that does not seem worth it. + return Ok(()); + } + Scalar::Bits { bits, size: value_size } => { + assert_eq!(value_size as u64, size.bytes()); + bits + } + }; + // Now compare. This is slightly subtle because this is a special "wrap-around" range. + use std::ops::RangeInclusive; + let in_range = |bound: RangeInclusive| bound.contains(&bits); + if lo > hi { + // wrapping around + if in_range(0..=hi) || in_range(lo..=u128::max_value()) { + Ok(()) + } else { + validation_failure!( + bits, + path, + format!("something in the range {:?} or {:?}", 0..=hi, lo..=u128::max_value()) + ) + } + } else { + if in_range(layout.valid_range.clone()) { + Ok(()) + } else { + validation_failure!( + bits, + path, + format!("something in the range {:?}", layout.valid_range) + ) + } + } + } + /// Validate a reference, potentially recursively. `place` is assumed to already be /// dereferenced, i.e. it describes the target. fn validate_ref( @@ -297,6 +354,22 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Remember the length, in case we need to truncate let path_len = path.len(); + // If this is a scalar, validate the scalar layout. + // Things can be aggregates and have scalar layout at the same time, and that + // is very relevant for `NonNull` and similar structs: We need to validate them + // at their scalar layout *before* descending into their fields. + match dest.layout.abi { + layout::Abi::Uninhabited => + return validation_failure!("a value of an uninhabited type", path), + layout::Abi::Scalar(ref layout) => { + let value = try_validation!(self.read_scalar(dest), + "uninitialized or unrepresentable data", path); + self.validate_scalar_layout(value, dest.layout.size, &path, layout)?; + } + // FIXME: Should we do something for ScalarPair? Vector? + _ => {} + } + // Validate all fields match dest.layout.fields { // Primitives appear as Union with 0 fields -- except for fat pointers. @@ -305,21 +378,26 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // fields to get a proper `path`. layout::FieldPlacement::Union(0) => { match dest.layout.abi { - // nothing to do, whatever the pointer points to, it is never going to be read - layout::Abi::Uninhabited => - 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)?; + self.validate_scalar_type( + value.to_scalar_or_undef(), + 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() { - self.validate_ref(self.ref_to_mplace(value)?, path, ref_tracking)?; + self.validate_ref( + self.ref_to_mplace(value)?, + path, + ref_tracking + )?; } }, _ => bug!("bad abi for FieldPlacement::Union(0): {:#?}", dest.layout.abi), diff --git a/src/test/ui/consts/const-eval/transmute-const.stderr b/src/test/ui/consts/const-eval/transmute-const.stderr index 2e82c0963c3..c9beca7aa30 100644 --- a/src/test/ui/consts/const-eval/transmute-const.stderr +++ b/src/test/ui/consts/const-eval/transmute-const.stderr @@ -2,7 +2,7 @@ error[E0080]: this static likely exhibits undefined behavior --> $DIR/transmute-const.rs:15:1 | LL | static FOO: bool = unsafe { mem::transmute(3u8) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3, but expected something in the range 0..=1 | = 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 diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 20105a46c1d..243343c94b0 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -18,7 +18,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/ub-enum.rs:45:1 | LL | const BAD_ENUM_CHAR : Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at .Some.0.1, but expected a valid unicode codepoint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at .Some.0.1, but expected something in the range 0..=1114111 | = 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 diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs new file mode 100644 index 00000000000..2b07eee3ccb --- /dev/null +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -0,0 +1,25 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(const_transmute)] + +use std::mem; +use std::ptr::NonNull; +use std::num::{NonZeroU8, NonZeroUsize}; + +const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; +//~^ ERROR this constant likely exhibits undefined behavior + +const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; +//~^ ERROR this constant likely exhibits undefined behavior +const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; +//~^ ERROR this constant likely exhibits undefined behavior + +fn main() {} diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr new file mode 100644 index 00000000000..5ebec560da6 --- /dev/null +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -0,0 +1,27 @@ +error[E0080]: this constant likely exhibits undefined behavior + --> $DIR/ub-nonnull.rs:17:1 + | +LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something in the range 1..=18446744073709551615 + | + = 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-nonnull.rs:20:1 + | +LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something in the range 1..=255 + | + = 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-nonnull.rs:22:1 + | +LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something in the range 1..=18446744073709551615 + | + = 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`. diff --git a/src/test/ui/consts/const-eval/union-ice.stderr b/src/test/ui/consts/const-eval/union-ice.stderr index 4484dd6a147..2848bc62aee 100644 --- a/src/test/ui/consts/const-eval/union-ice.stderr +++ b/src/test/ui/consts/const-eval/union-ice.stderr @@ -13,7 +13,7 @@ LL | / const FIELD_PATH: Struct = Struct { //~ ERROR this constant likely exhibi LL | | a: 42, LL | | b: unsafe { UNION.field3 }, LL | | }; - | |__^ type validation failed: encountered uninitialized bytes at .b, but expected initialized plain bits + | |__^ type validation failed: encountered uninitialized bytes at .b, but expected something in the range 0..=18446744073709551615 | = 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 diff --git a/src/test/ui/consts/const-eval/union-ub.stderr b/src/test/ui/consts/const-eval/union-ub.stderr index 5da7e1a4dbf..2a04dae337b 100644 --- a/src/test/ui/consts/const-eval/union-ub.stderr +++ b/src/test/ui/consts/const-eval/union-ub.stderr @@ -2,7 +2,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ub.rs:36:1 | LL | const BAD_BOOL: bool = unsafe { DummyUnion { u8: 42 }.bool}; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 0..=1 | = 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 diff --git a/src/test/ui/union-ub-fat-ptr.stderr b/src/test/ui/union-ub-fat-ptr.stderr index 0471ef075f8..08aaa40e2f3 100644 --- a/src/test/ui/union-ub-fat-ptr.stderr +++ b/src/test/ui/union-ub-fat-ptr.stderr @@ -66,7 +66,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ub-fat-ptr.rs:116:1 | LL | const G: &Trait = &unsafe { BoolTransmute { val: 3 }.bl }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ., but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ., but expected something in the range 0..=1 | = 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 @@ -74,7 +74,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ub-fat-ptr.rs:119:1 | LL | const H: &[bool] = &[unsafe { BoolTransmute { val: 3 }.bl }]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .[0], but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .[0], but expected something in the range 0..=1 | = 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 @@ -82,7 +82,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ub-fat-ptr.rs:125:1 | LL | const I2: &MySliceBool = &MySlice(unsafe { BoolTransmute { val: 3 }.bl }, [false]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..0, but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..0, but expected something in the range 0..=1 | = 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 @@ -90,7 +90,7 @@ error[E0080]: this constant likely exhibits undefined behavior --> $DIR/union-ub-fat-ptr.rs:128:1 | LL | const I3: &MySliceBool = &MySlice(true, [unsafe { BoolTransmute { val: 3 }.bl }]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..1[0], but expected a boolean + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at ..1[0], but expected something in the range 0..=1 | = 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