Rollup merge of rust-lang#55894 - RalfJung:validation-enums, r=oli-obk

miri enum discriminant handling: Fix treatment of pointers, better error when it is undef

r? @oli-obk

Author: pietroalbini

src/librustc/ich/impls_ty.rs

@@ -451,7 +451,7 @@ impl_stable_hash_for!(
         FunctionRetMismatch(a, b),
         NoMirFor(s),
         UnterminatedCString(ptr),
-        PointerOutOfBounds { ptr, access, allocation_size },
+        PointerOutOfBounds { ptr, check, allocation_size },
         InvalidBoolOp(bop),
         Unimplemented(s),
         BoundsCheck { len, index },
@@ -471,6 +471,11 @@ impl_stable_hash_for!(
     }
 );
 
+impl_stable_hash_for!(enum mir::interpret::InboundsCheck {
+    Live,
+    MaybeDead
+});
+
 impl_stable_hash_for!(enum mir::interpret::Lock {
     NoLock,
     WriteLock(dl),

src/librustc/mir/interpret/allocation.rs

@@ -19,6 +19,14 @@ use mir;
 use std::ops::{Deref, DerefMut};
 use rustc_data_structures::sorted_map::SortedMap;
 
+/// Used by `check_bounds` to indicate whether the pointer needs to be just inbounds
+/// or also inbounds of a *live* allocation.
+#[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable)]
+pub enum InboundsCheck {
+    Live,
+    MaybeDead,
+}
+
 #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
 pub struct Allocation<Tag=(),Extra=()> {
     /// The actual bytes of the allocation.

src/librustc/mir/interpret/error.rs

@@ -15,7 +15,7 @@ use ty::{Ty, layout};
 use ty::layout::{Size, Align, LayoutError};
 use rustc_target::spec::abi::Abi;
 
-use super::{Pointer, Scalar};
+use super::{Pointer, InboundsCheck, ScalarMaybeUndef};
 
 use backtrace::Backtrace;
 
@@ -240,10 +240,10 @@ pub enum EvalErrorKind<'tcx, O> {
     InvalidMemoryAccess,
     InvalidFunctionPointer,
     InvalidBool,
-    InvalidDiscriminant(Scalar),
+    InvalidDiscriminant(ScalarMaybeUndef),
     PointerOutOfBounds {
         ptr: Pointer,
-        access: bool,
+        check: InboundsCheck,
         allocation_size: Size,
     },
     InvalidNullPointerUsage,
@@ -457,9 +457,13 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::EvalErrorKind::*;
         match *self {
-            PointerOutOfBounds { ptr, access, allocation_size } => {
-                write!(f, "{} at offset {}, outside bounds of allocation {} which has size {}",
-                       if access { "memory access" } else { "pointer computed" },
+            PointerOutOfBounds { ptr, check, allocation_size } => {
+                write!(f, "Pointer must be in-bounds{} at offset {}, but is outside bounds of \
+                           allocation {} which has size {}",
+                       match check {
+                           InboundsCheck::Live => " and live",
+                           InboundsCheck::MaybeDead => "",
+                       },
                        ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes())
             },
             ValidationFailure(ref err) => {

src/librustc/mir/interpret/mod.rs

@@ -28,7 +28,7 @@ pub use self::error::{
 pub use self::value::{Scalar, ConstValue, ScalarMaybeUndef};
 
 pub use self::allocation::{
-    Allocation, AllocationExtra,
+    InboundsCheck, Allocation, AllocationExtra,
     Relocations, UndefMask,
 };
 

src/librustc_mir/interpret/memory.rs

@@ -28,7 +28,7 @@ use rustc_data_structures::fx::{FxHashSet, FxHashMap};
 use syntax::ast::Mutability;
 
 use super::{
-    Pointer, AllocId, Allocation, ConstValue, GlobalId, AllocationExtra,
+    Pointer, AllocId, Allocation, ConstValue, GlobalId, AllocationExtra, InboundsCheck,
     EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic,
     Machine, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled,
 };
@@ -249,17 +249,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         // Check non-NULL/Undef, extract offset
         let (offset, alloc_align) = match ptr {
             Scalar::Ptr(ptr) => {
-                let (size, align) = self.get_size_and_align(ptr.alloc_id);
                 // check this is not NULL -- which we can ensure only if this is in-bounds
                 // of some (potentially dead) allocation.
-                if ptr.offset > size {
-                    return err!(PointerOutOfBounds {
-                        ptr: ptr.erase_tag(),
-                        access: true,
-                        allocation_size: size,
-                    });
-                };
-                // keep data for alignment check
+                self.check_bounds_ptr(ptr, InboundsCheck::MaybeDead)?;
+                // data required for alignment check
+                let (_, align) = self.get_size_and_align(ptr.alloc_id);
                 (ptr.offset.bytes(), align)
             }
             Scalar::Bits { bits, size } => {
@@ -293,18 +287,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
 
     /// Check if the pointer is "in-bounds". Notice that a pointer pointing at the end
     /// of an allocation (i.e., at the first *inaccessible* location) *is* considered
-    /// in-bounds!  This follows C's/LLVM's rules.  The `access` boolean is just used
-    /// for the error message.
-    /// 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_ptr(&self, ptr: Pointer<M::PointerTag>, access: bool) -> EvalResult<'tcx> {
-        let alloc = self.get(ptr.alloc_id)?;
-        let allocation_size = alloc.bytes.len() as u64;
+    /// in-bounds!  This follows C's/LLVM's rules.  `check` indicates whether we
+    /// additionally require the pointer to be pointing to a *live* (still allocated)
+    /// allocation.
+    /// If you want to check bounds before doing a memory access, better use `check_bounds`.
+    pub fn check_bounds_ptr(
+        &self,
+        ptr: Pointer<M::PointerTag>,
+        check: InboundsCheck,
+    ) -> EvalResult<'tcx> {
+        let allocation_size = match check {
+            InboundsCheck::Live => {
+                let alloc = self.get(ptr.alloc_id)?;
+                alloc.bytes.len() as u64
+            }
+            InboundsCheck::MaybeDead => {
+                self.get_size_and_align(ptr.alloc_id).0.bytes()
+            }
+        };
         if ptr.offset.bytes() > allocation_size {
             return err!(PointerOutOfBounds {
                 ptr: ptr.erase_tag(),
-                access,
+                check,
                 allocation_size: Size::from_bytes(allocation_size),
             });
         }
@@ -317,10 +321,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
         &self,
         ptr: Pointer<M::PointerTag>,
         size: Size,
-        access: bool
+        check: InboundsCheck,
     ) -> 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)
+        self.check_bounds_ptr(ptr.offset(size, &*self)?, check)
     }
 }
 
@@ -626,7 +630,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)?;
-        self.check_bounds(ptr, size, true)?;
+        self.check_bounds(ptr, size, InboundsCheck::Live)?;
 
         if check_defined_and_ptr {
             self.check_defined(ptr, size)?;
@@ -677,7 +681,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)?;
-        self.check_bounds(ptr, size, true)?;
+        self.check_bounds(ptr, size, InboundsCheck::Live)?;
 
         self.mark_definedness(ptr, size, true)?;
         self.clear_relocations(ptr, size)?;

src/librustc_mir/interpret/operand.rs

@@ -19,7 +19,7 @@ use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerEx
 use rustc::mir::interpret::{
     GlobalId, AllocId,
     ConstValue, Pointer, Scalar,
-    EvalResult, EvalErrorKind
+    EvalResult, EvalErrorKind, InboundsCheck,
 };
 use super::{EvalContext, Machine, MemPlace, MPlaceTy, MemoryKind};
 pub use rustc::mir::interpret::ScalarMaybeUndef;
@@ -601,7 +601,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         // read raw discriminant value
         let discr_op = self.operand_field(rval, 0)?;
         let discr_val = self.read_immediate(discr_op)?;
-        let raw_discr = discr_val.to_scalar()?;
+        let raw_discr = discr_val.to_scalar_or_undef();
         trace!("discr value: {:?}", raw_discr);
         // post-process
         Ok(match rval.layout.variants {
@@ -647,28 +647,33 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                 let variants_start = niche_variants.start().as_u32() as u128;
                 let variants_end = niche_variants.end().as_u32() as u128;
                 match raw_discr {
-                    Scalar::Ptr(_) => {
-                        // The niche must be just 0 (which a pointer value never is)
-                        assert!(niche_start == 0);
-                        assert!(variants_start == variants_end);
+                    ScalarMaybeUndef::Scalar(Scalar::Ptr(ptr)) => {
+                        // The niche must be just 0 (which an inbounds pointer value never is)
+                        let ptr_valid = niche_start == 0 && variants_start == variants_end &&
+                            self.memory.check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok();
+                        if !ptr_valid {
+                            return err!(InvalidDiscriminant(raw_discr.erase_tag()));
+                        }
                         (dataful_variant.as_u32() as u128, dataful_variant)
                     },
-                    Scalar::Bits { bits: raw_discr, size } => {
+                    ScalarMaybeUndef::Scalar(Scalar::Bits { bits: raw_discr, size }) => {
                         assert_eq!(size as u64, discr_val.layout.size.bytes());
-                        let discr = raw_discr.wrapping_sub(niche_start)
+                        let adjusted_discr = raw_discr.wrapping_sub(niche_start)
                             .wrapping_add(variants_start);
-                        if variants_start <= discr && discr <= variants_end {
-                            let index = discr as usize;
-                            assert_eq!(index as u128, discr);
+                        if variants_start <= adjusted_discr && adjusted_discr <= variants_end {
+                            let index = adjusted_discr as usize;
+                            assert_eq!(index as u128, adjusted_discr);
                             assert!(index < rval.layout.ty
                                 .ty_adt_def()
                                 .expect("tagged layout for non adt")
                                 .variants.len());
-                            (discr, VariantIdx::from_usize(index))
+                            (adjusted_discr, VariantIdx::from_usize(index))
                         } else {
                             (dataful_variant.as_u32() as u128, dataful_variant)
                         }
                     },
+                    ScalarMaybeUndef::Undef =>
+                        return err!(InvalidDiscriminant(ScalarMaybeUndef::Undef)),
                 }
             }
         })

src/librustc_mir/interpret/validity.rs

@@ -17,7 +17,7 @@ use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf, VariantIdx};
 use rustc::ty;
 use rustc_data_structures::fx::FxHashSet;
 use rustc::mir::interpret::{
-    Scalar, AllocType, EvalResult, EvalErrorKind,
+    Scalar, AllocType, EvalResult, EvalErrorKind, InboundsCheck,
 };
 
 use super::{
@@ -274,7 +274,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>
                     ),
                 EvalErrorKind::ReadPointerAsBytes =>
                     validation_failure!(
-                        "a pointer", self.path, "plain bytes"
+                        "a pointer", self.path, "plain (non-pointer) bytes"
                     ),
                 _ => Err(err),
             }
@@ -304,7 +304,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>
                 if self.const_mode {
                     // Integers/floats in CTFE: Must be scalar bits, pointers are dangerous
                     try_validation!(value.to_bits(size),
-                        value, self.path, "initialized plain bits");
+                        value, self.path, "initialized plain (non-pointer) bytes");
                 } else {
                     // At run-time, for now, we accept *anything* for these types, including
                     // undef. We should fix that, but let's start low.
@@ -394,7 +394,8 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>
                         }
                         // Maintain the invariant that the place we are checking is
                         // already verified to be in-bounds.
-                        try_validation!(self.ecx.memory.check_bounds(ptr, size, false),
+                        try_validation!(
+                            self.ecx.memory.check_bounds(ptr, size, InboundsCheck::Live),
                             "dangling (not entirely in bounds) reference", self.path);
                     }
                     // Check if we have encountered this pointer+layout combination

src/test/ui/consts/const-eval/const-pointer-values-in-various-types.stderr

@@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:24:5
    |
 LL |     const I32_REF_USIZE_UNION: usize = unsafe { Nonsense { int_32_ref: &3 }.u };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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
 
@@ -36,7 +36,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:36:5
    |
 LL |     const I32_REF_U64_UNION: u64 = unsafe { Nonsense { int_32_ref: &3 }.uint_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:51:5
    |
 LL |     const I32_REF_I64_UNION: i64 = unsafe { Nonsense { int_32_ref: &3 }.int_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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
 
@@ -96,7 +96,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:60:5
    |
 LL |     const I32_REF_F64_UNION: f64 = unsafe { Nonsense { int_32_ref: &3 }.float_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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
 
@@ -144,7 +144,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:78:5
    |
 LL |     const STR_U64_UNION: u64 = unsafe { Nonsense { stringy: "3" }.uint_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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
 
@@ -184,7 +184,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:93:5
    |
 LL |     const STR_I64_UNION: i64 = unsafe { Nonsense { stringy: "3" }.int_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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
 
@@ -208,7 +208,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/const-pointer-values-in-various-types.rs:102:5
    |
 LL |     const STR_F64_UNION: f64 = unsafe { Nonsense { stringy: "3" }.float_64 };
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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/issue-52442.stderr

@@ -8,7 +8,7 @@ error[E0080]: it is undefined behavior to use this value
   --> $DIR/issue-52442.rs:12:11
    |
 LL |     [();  { &loop { break } as *const _ as usize } ]; //~ ERROR unimplemented expression type
-   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
+   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain (non-pointer) bytes
    |
    = 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