check that entire ref is in-bounds before recursing; add macro for validation msgs on error

Author: RalfJung

src/librustc_mir/interpret/memory.rs

@@ -284,7 +284,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
     /// If you want to check bounds before doing a memory access, be sure to
     /// check the pointer one past the end of your access, then everything will
     /// work out exactly.
-    pub fn check_bounds(&self, ptr: Pointer, access: bool) -> EvalResult<'tcx> {
+    pub fn check_bounds_ptr(&self, ptr: Pointer, access: bool) -> EvalResult<'tcx> {
         let alloc = self.get(ptr.alloc_id)?;
         let allocation_size = alloc.bytes.len() as u64;
         if ptr.offset.bytes() > allocation_size {
@@ -296,6 +296,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         }
         Ok(())
     }
+
+    /// Check if the memory range beginning at `ptr` and of size `Size` is "in-bounds".
+    #[inline(always)]
+    pub fn check_bounds(&self, ptr: Pointer, size: Size, access: bool) -> EvalResult<'tcx> {
+        // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow)
+        self.check_bounds_ptr(ptr.offset(size, &*self)?, access)
+    }
 }
 
 /// Allocation accessors
@@ -524,8 +531,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
     ) -> EvalResult<'tcx, &[u8]> {
         assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`");
         self.check_align(ptr.into(), align)?;
-        // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow)
-        self.check_bounds(ptr.offset(size, &*self)?, true)?;
+        self.check_bounds(ptr, size, true)?;
 
         if check_defined_and_ptr {
             self.check_defined(ptr, size)?;
@@ -569,8 +575,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
     ) -> EvalResult<'tcx, &mut [u8]> {
         assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`");
         self.check_align(ptr.into(), align)?;
-        // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow)
-        self.check_bounds(ptr.offset(size, &self)?, true)?;
+        self.check_bounds(ptr, size, true)?;
 
         self.mark_definedness(ptr, size, true)?;
         self.clear_relocations(ptr, size)?;

src/librustc_mir/interpret/validity.rs

@@ -22,7 +22,7 @@ use super::{
     OpTy, MPlaceTy, Machine, EvalContext, ScalarMaybeUndef
 };
 
-macro_rules! validation_failure{
+macro_rules! validation_failure {
     ($what:expr, $where:expr, $details:expr) => {{
         let where_ = path_format($where);
         let where_ = if where_.is_empty() {
@@ -49,6 +49,15 @@ macro_rules! validation_failure{
     }};
 }
 
+macro_rules! try_validation {
+    ($e:expr, $what:expr, $where:expr) => {{
+        match $e {
+            Ok(x) => x,
+            Err(_) => return validation_failure!($what, $where),
+        }
+    }}
+}
+
 /// We want to show a nice path to the invalid field for diagnotsics,
 /// but avoid string operations in the happy case where no error happens.
 /// So we track a `Vec<PathElem>` where `PathElem` contains all the data we
@@ -230,8 +239,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         path: &mut Vec<PathElem>,
         ref_tracking: Option<&mut RefTracking<'tcx>>,
     ) -> EvalResult<'tcx> {
-        // Skip recursion for some external statics
-        if let Scalar::Ptr(ptr) = place.ptr {
+        // Before we do anything else, make sure this is entirely in-bounds.
+        if !place.layout.is_zst() {
+            let ptr = try_validation!(place.ptr.to_ptr(),
+                "integer pointer in non-ZST reference", path);
+            let size = self.size_and_align_of(place.extra, place.layout)?.0;
+            try_validation!(self.memory.check_bounds(ptr, size, false),
+                "dangling reference (not entirely in bounds)", path);
+            // Skip recursion for some external statics
             let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id);
             if let Some(AllocType::Static(did)) = alloc_kind {
                 // statics from other crates are already checked.
@@ -257,7 +272,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         Ok(())
     }
 
-    /// This function checks the data at `op`.
+    /// This function checks the data at `op`.  `op` is assumed to cover valid memory if it
+    /// is an indirect operand.
     /// 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!
@@ -305,13 +321,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 let dest = match dest.layout.ty.sty {
                     ty::Dynamic(..) => {
                         let dest = dest.to_mem_place(); // immediate trait objects are not a thing
-                        match self.unpack_dyn_trait(dest) {
-                            Ok(res) => res.1.into(),
-                            Err(_) =>
-                                return validation_failure!(
-                                    "invalid vtable in fat pointer", path
-                                ),
-                        }
+                        try_validation!(self.unpack_dyn_trait(dest),
+                            "invalid vtable in fat pointer", path).1.into()
                     }
                     _ => dest
                 };
@@ -337,20 +348,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     // expectation.
                     layout::Abi::Scalar(ref scalar_layout) => {
                         let size = scalar_layout.value.size(self);
-                        let value = match self.read_value(dest) {
-                            Ok(val) => val,
-                            Err(err) => match err.kind {
-                                EvalErrorKind::PointerOutOfBounds { .. } |
-                                EvalErrorKind::ReadUndefBytes(_) =>
-                                    return validation_failure!(
-                                        "uninitialized or out-of-bounds memory", path
-                                    ),
-                                _ =>
-                                    return validation_failure!(
-                                        "unrepresentable data", path
-                                    ),
-                            }
-                        };
+                        let value = try_validation!(self.read_value(dest),
+                            "uninitialized or unrepresentable data", path);
                         let scalar = value.to_scalar_or_undef();
                         self.validate_scalar(scalar, size, scalar_layout, &path, dest.layout.ty)?;
                         // Recursively check *safe* references
@@ -367,35 +366,24 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 if dest.layout.ty.builtin_deref(true).is_some() =>
             {
                 // This is a fat pointer.
-                let ptr = match self.read_value(dest.into())
-                    .and_then(|val| self.ref_to_mplace(val))
-                {
-                    Ok(ptr) => ptr,
-                    Err(_) =>
-                        return validation_failure!(
-                            "undefined location or metadata in fat pointer", path
-                        ),
-                };
+                let ptr = try_validation!(self.read_value(dest.into()),
+                    "undefined location in fat pointer", path);
+                let ptr = try_validation!(self.ref_to_mplace(ptr),
+                    "undefined metadata in fat pointer", path);
                 // check metadata early, for better diagnostics
                 match self.tcx.struct_tail(ptr.layout.ty).sty {
                     ty::Dynamic(..) => {
-                        match ptr.extra.unwrap().to_ptr() {
-                            Ok(_) => {},
-                            Err(_) =>
-                                return validation_failure!(
-                                    "non-pointer vtable in fat pointer", path
-                                ),
-                        }
+                        let vtable = try_validation!(ptr.extra.unwrap().to_ptr(),
+                            "non-pointer vtable in fat pointer", path);
+                        try_validation!(self.read_drop_type_from_vtable(vtable),
+                            "invalid drop fn in vtable", path);
+                        try_validation!(self.read_size_and_align_from_vtable(vtable),
+                            "invalid size or align in vtable", path);
                         // FIXME: More checks for the vtable.
                     }
                     ty::Slice(..) | ty::Str => {
-                        match ptr.extra.unwrap().to_usize(self) {
-                            Ok(_) => {},
-                            Err(_) =>
-                                return validation_failure!(
-                                    "non-integer slice length in fat pointer", path
-                                ),
-                        }
+                        try_validation!(ptr.extra.unwrap().to_usize(self),
+                            "non-integer slice length in fat pointer", path);
                     }
                     _ =>
                         bug!("Unexpected unsized type tail: {:?}",
@@ -418,23 +406,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 match dest.layout.ty.sty {
                     // Special handling for strings to verify UTF-8
                     ty::Str => {
-                        match self.read_str(dest) {
-                            Ok(_) => {},
-                            Err(err) => match err.kind {
-                                EvalErrorKind::PointerOutOfBounds { .. } |
-                                EvalErrorKind::ReadUndefBytes(_) =>
-                                    // The error here looks slightly different than it does
-                                    // for slices, because we do not report the index into the
-                                    // str at which we are OOB.
-                                    return validation_failure!(
-                                        "uninitialized or out-of-bounds memory", path
-                                    ),
-                                _ =>
-                                    return validation_failure!(
-                                        "non-UTF-8 data in str", path
-                                    ),
-                            }
-                        }
+                        try_validation!(self.read_str(dest),
+                            "uninitialized or non-UTF-8 data in str", path);
                     }
                     // Special handling for arrays/slices of builtin integer types
                     ty::Array(tys, ..) | ty::Slice(tys) if {
@@ -470,18 +443,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                                             "undefined bytes", path
                                         )
                                     },
-                                    EvalErrorKind::PointerOutOfBounds { allocation_size, .. } => {
-                                        // If the array access is out-of-bounds, the first
-                                        // undefined access is the after the end of the array.
-                                        let i = (allocation_size.bytes() * ty_size) as usize;
-                                        path.push(PathElem::ArrayElem(i));
-                                    },
-                                    _ => (),
+                                    // Other errors shouldn't be possible
+                                    _ => return Err(err),
                                 }
-
-                                return validation_failure!(
-                                    "uninitialized or out-of-bounds memory", path
-                                )
                             }
                         }
                     },

src/test/ui/consts/const-eval/ub-uninhabit.rs

@@ -9,14 +9,18 @@
 // except according to those terms.
 
 union Foo {
-    a: u8,
+    a: usize,
     b: Bar,
+    c: &'static Bar,
 }
 
 #[derive(Copy, Clone)]
 enum Bar {}
 
-const BAD_BAD_BAD: Bar = unsafe { Foo { a: 1 }.b};
+const BAD_BAD_BAD: Bar = unsafe { Foo { a: 1 }.b };
+//~^ ERROR this constant likely exhibits undefined behavior
+
+const BAD_BAD_REF: &Bar = unsafe { Foo { a: 1 }.c };
 //~^ ERROR this constant likely exhibits undefined behavior
 
 fn main() {

src/test/ui/consts/const-eval/ub-uninhabit.stderr

@@ -1,11 +1,19 @@
 error[E0080]: this constant likely exhibits undefined behavior
-  --> $DIR/ub-uninhabit.rs:19:1
+  --> $DIR/ub-uninhabit.rs:20:1
    |
-LL | const BAD_BAD_BAD: Bar = unsafe { Foo { a: 1 }.b};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type
+LL | const BAD_BAD_BAD: Bar = unsafe { Foo { a: 1 }.b };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type
    |
    = 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 previous error
+error[E0080]: this constant likely exhibits undefined behavior
+  --> $DIR/ub-uninhabit.rs:23:1
+   |
+LL | const BAD_BAD_REF: &Bar = unsafe { Foo { a: 1 }.c };
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at .<deref>
+   |
+   = 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 2 previous errors
 
 For more information about this error, try `rustc --explain E0080`.

src/test/ui/consts/const-eval/ub-usize-in-ref.rs

@@ -8,15 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// compile-pass
-
 union Foo {
     a: &'static u8,
     b: usize,
 }
 
-// This might point to an invalid address, but that's the user's problem
 const USIZE_AS_STATIC_REF: &'static u8 = unsafe { Foo { b: 1337 }.a};
+//~^ ERROR this constant likely exhibits undefined behavior
 
 fn main() {
 }

src/test/ui/consts/const-eval/ub-usize-in-ref.stderr

@@ -0,0 +1,11 @@
+error[E0080]: this constant likely exhibits undefined behavior
+  --> $DIR/ub-usize-in-ref.rs:16:1
+   |
+LL | const USIZE_AS_STATIC_REF: &'static u8 = unsafe { Foo { b: 1337 }.a};
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 previous error
+
+For more information about this error, try `rustc --explain E0080`.

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

@@ -2,7 +2,7 @@ error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/union-ub-fat-ptr.rs:87:1
    |
 LL | const B: &str = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.str};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or out-of-bounds memory at .<deref>
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (not entirely in bounds)
    |
    = 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
 
@@ -26,7 +26,7 @@ error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/union-ub-fat-ptr.rs:99:1
    |
 LL | const B2: &[u8] = unsafe { SliceTransmute { repr: SliceRepr { ptr: &42, len: 999 } }.slice};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or out-of-bounds memory at .<deref>[1]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered dangling reference (not entirely in bounds)
    |
    = 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
 
@@ -42,15 +42,15 @@ error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/union-ub-fat-ptr.rs:106:1
    |
 LL | const D: &Trait = unsafe { DynTransmute { repr: DynRepr { ptr: &92, vtable: &3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid vtable in fat pointer at .<deref>
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable
    |
    = 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:109:1
    |
 LL | const E: &Trait = unsafe { DynTransmute { repr2: DynRepr2 { ptr: &92, vtable: &3 } }.rust};
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid vtable in fat pointer at .<deref>
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered invalid drop fn in vtable
    |
    = 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
 
@@ -98,15 +98,15 @@ error[E0080]: this constant likely exhibits undefined behavior
   --> $DIR/union-ub-fat-ptr.rs:132:1
    |
 LL | const J1: &str = unsafe { SliceTransmute { slice: &[0xFF] }.str };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-UTF-8 data in str at .<deref>
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at .<deref>
    |
    = 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:135:1
    |
 LL | const J2: &MyStr = unsafe { SliceTransmute { slice: &[0xFF] }.my_str };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered non-UTF-8 data in str at .<deref>.0
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at .<deref>.0
    |
    = 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