also validate everything that has a Scalar layout, to catch NonNull

Author: RalfJung

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<PathElem>,
         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<PathElem>,
+        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<u128>| 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),

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
 

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
 

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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<u8> = 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() {}

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<u8> = 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`.

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
 

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
 

src/test/ui/union-ub-fat-ptr.stderr

@@ -66,31 +66,31 @@ 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 .<deref>, but expected a boolean
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>, 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
 
 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 .<deref>[0], but expected a boolean
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>[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
 
 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 .<deref>.0, but expected a boolean
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>.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
 
 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 .<deref>.1[0], but expected a boolean
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 3 at .<deref>.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