Rollup merge of #69839 - RalfJung:miri-error-cleanup, r=oli-obk

Miri error reform

Some time ago we started moving Miri errors into a few distinct categories, but we never classified all the old errors. That's what this PR does.

~~This is on top of #69762; [relative diff](RalfJung/rust@validity-errors...RalfJung:miri-error-cleanup).~~

r? @oli-obk

Fixes rust-lang/const-eval#4

Author: Centril

src/librustc/mir/interpret/allocation.rs

@@ -41,6 +41,7 @@ pub struct Allocation<Tag = (), Extra = ()> {
     /// The size of the allocation. Currently, must always equal `bytes.len()`.
     pub size: Size,
     /// The alignment of the allocation to detect unaligned reads.
+    /// (`Align` guarantees that this is a power of two.)
     pub align: Align,
     /// `true` if the allocation is mutable.
     /// Also used by codegen to determine if a static should be put into mutable memory,
@@ -314,7 +315,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra<Tag>> Allocation<Tag, Extra> {
                 &self.get_bytes(cx, ptr, size_with_null)?[..size]
             }
             // This includes the case where `offset` is out-of-bounds to begin with.
-            None => throw_unsup!(UnterminatedCString(ptr.erase_tag())),
+            None => throw_ub!(UnterminatedCString(ptr.erase_tag())),
         })
     }
 
@@ -573,7 +574,7 @@ impl<'tcx, Tag, Extra> Allocation<Tag, Extra> {
     fn check_defined(&self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
         self.undef_mask
             .is_range_defined(ptr.offset, ptr.offset + size)
-            .or_else(|idx| throw_unsup!(ReadUndefBytes(idx)))
+            .or_else(|idx| throw_ub!(InvalidUndefBytes(Some(Pointer::new(ptr.alloc_id, idx)))))
     }
 
     pub fn mark_definedness(&mut self, ptr: Pointer<Tag>, size: Size, new_state: bool) {

src/librustc/mir/interpret/error.rs

@@ -161,7 +161,13 @@ pub struct AllocId(pub u64);
 
 impl fmt::Debug for AllocId {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(fmt, "alloc{}", self.0)
+        fmt::Display::fmt(self, fmt)
+    }
+}
+
+impl fmt::Display for AllocId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "alloc{}", self.0)
     }
 }
 
@@ -351,12 +357,6 @@ impl<'s> AllocDecodingSession<'s> {
     }
 }
 
-impl fmt::Display for AllocId {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.0)
-    }
-}
-
 /// An allocation in the global (tcx-managed) memory can be either a function pointer,
 /// a static, or a "real" allocation with some data in it.
 #[derive(Debug, Clone, Eq, PartialEq, Hash, RustcDecodable, RustcEncodable, HashStable)]

src/librustc/mir/interpret/mod.rs

@@ -213,20 +213,4 @@ impl<'tcx, Tag> Pointer<Tag> {
     pub fn erase_tag(self) -> Pointer {
         Pointer { alloc_id: self.alloc_id, offset: self.offset, tag: () }
     }
-
-    /// Test if the pointer is "inbounds" of an allocation of the given size.
-    /// A pointer is "inbounds" even if its offset is equal to the size; this is
-    /// a "one-past-the-end" pointer.
-    #[inline(always)]
-    pub fn check_inbounds_alloc(
-        self,
-        allocation_size: Size,
-        msg: CheckInAllocMsg,
-    ) -> InterpResult<'tcx, ()> {
-        if self.offset > allocation_size {
-            throw_unsup!(PointerOutOfBounds { ptr: self.erase_tag(), msg, allocation_size })
-        } else {
-            Ok(())
-        }
-    }
 }

src/librustc/mir/interpret/pointer.rs

@@ -455,18 +455,19 @@ impl<'tcx, Tag> Scalar<Tag> {
     }
 
     pub fn to_bool(self) -> InterpResult<'tcx, bool> {
-        match self {
-            Scalar::Raw { data: 0, size: 1 } => Ok(false),
-            Scalar::Raw { data: 1, size: 1 } => Ok(true),
-            _ => throw_unsup!(InvalidBool),
+        let val = self.to_u8()?;
+        match val {
+            0 => Ok(false),
+            1 => Ok(true),
+            _ => throw_ub!(InvalidBool(val)),
         }
     }
 
     pub fn to_char(self) -> InterpResult<'tcx, char> {
         let val = self.to_u32()?;
         match ::std::char::from_u32(val) {
             Some(c) => Ok(c),
-            None => throw_unsup!(InvalidChar(val as u128)),
+            None => throw_ub!(InvalidChar(val)),
         }
     }
 
@@ -609,7 +610,7 @@ impl<'tcx, Tag> ScalarMaybeUndef<Tag> {
     pub fn not_undef(self) -> InterpResult<'static, Scalar<Tag>> {
         match self {
             ScalarMaybeUndef::Scalar(scalar) => Ok(scalar),
-            ScalarMaybeUndef::Undef => throw_unsup!(ReadUndefBytes(Size::ZERO)),
+            ScalarMaybeUndef::Undef => throw_ub!(InvalidUndefBytes(None)),
         }
     }
 

src/librustc/mir/interpret/value.rs

@@ -240,7 +240,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         Ok(Some(match ecx.load_mir(instance.def, None) {
             Ok(body) => *body,
             Err(err) => {
-                if let err_unsup!(NoMirFor(ref path)) = err.kind {
+                if let err_unsup!(NoMirFor(did)) = err.kind {
+                    let path = ecx.tcx.def_path_str(did);
                     return Err(ConstEvalErrKind::NeedsRfc(format!(
                         "calling extern function `{}`",
                         path

src/librustc_mir/const_eval/machine.rs

@@ -138,7 +138,7 @@ pub enum LocalValue<Tag = (), Id = AllocId> {
 impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> {
     pub fn access(&self) -> InterpResult<'tcx, Operand<Tag>> {
         match self.value {
-            LocalValue::Dead => throw_unsup!(DeadLocal),
+            LocalValue::Dead => throw_ub!(DeadLocal),
             LocalValue::Uninitialized => {
                 bug!("The type checker should prevent reading from a never-written local")
             }
@@ -152,7 +152,7 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> {
         &mut self,
     ) -> InterpResult<'tcx, Result<&mut LocalValue<Tag>, MemPlace<Tag>>> {
         match self.value {
-            LocalValue::Dead => throw_unsup!(DeadLocal),
+            LocalValue::Dead => throw_ub!(DeadLocal),
             LocalValue::Live(Operand::Indirect(mplace)) => Ok(Err(mplace)),
             ref mut local @ LocalValue::Live(Operand::Immediate(_))
             | ref mut local @ LocalValue::Uninitialized => Ok(Ok(local)),
@@ -326,7 +326,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 if self.tcx.is_mir_available(did) {
                     Ok(self.tcx.optimized_mir(did).unwrap_read_only())
                 } else {
-                    throw_unsup!(NoMirFor(self.tcx.def_path_str(def_id)))
+                    throw_unsup!(NoMirFor(def_id))
                 }
             }
             _ => Ok(self.tcx.instance_mir(instance)),

src/librustc_mir/interpret/eval_context.rs

@@ -327,7 +327,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
         if let Err(error) = interned {
             // This can happen when e.g. the tag of an enum is not a valid discriminant. We do have
             // to read enum discriminants in order to find references in enum variant fields.
-            if let err_unsup!(ValidationFailure(_)) = error.kind {
+            if let err_ub!(ValidationFailure(_)) = error.kind {
                 let err = crate::const_eval::error_to_const_error(&ecx, error);
                 match err.struct_error(
                     ecx.tcx,
@@ -390,7 +390,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
             }
         } else if ecx.memory.dead_alloc_map.contains_key(&alloc_id) {
             // dangling pointer
-            throw_unsup!(ValidationFailure("encountered dangling pointer in final constant".into()))
+            throw_ub_format!("encountered dangling pointer in final constant")
         } else if ecx.tcx.alloc_map.lock().get(alloc_id).is_none() {
             // We have hit an `AllocId` that is neither in local or global memory and isn't marked
             // as dangling by local memory.

src/librustc_mir/interpret/intern.rs

@@ -135,7 +135,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let bits = self.force_bits(val, layout_of.size)?;
                 let kind = match layout_of.abi {
                     ty::layout::Abi::Scalar(ref scalar) => scalar.value,
-                    _ => throw_unsup!(TypeNotPrimitive(ty)),
+                    _ => bug!("{} called on invalid type {:?}", intrinsic_name, ty),
                 };
                 let (nonzero, intrinsic_name) = match intrinsic_name {
                     sym::cttz_nonzero => (true, sym::cttz),
@@ -246,9 +246,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     let layout = self.layout_of(substs.type_at(0))?;
                     let r_val = self.force_bits(r.to_scalar()?, layout.size)?;
                     if let sym::unchecked_shl | sym::unchecked_shr = intrinsic_name {
-                        throw_ub_format!("Overflowing shift by {} in `{}`", r_val, intrinsic_name);
+                        throw_ub_format!("overflowing shift by {} in `{}`", r_val, intrinsic_name);
                     } else {
-                        throw_ub_format!("Overflow executing `{}`", intrinsic_name);
+                        throw_ub_format!("overflow executing `{}`", intrinsic_name);
                     }
                 }
                 self.write_scalar(val, dest)?;

src/librustc_mir/interpret/intrinsics.rs

@@ -286,8 +286,10 @@ pub trait Machine<'mir, 'tcx>: Sized {
         int: u64,
     ) -> InterpResult<'tcx, Pointer<Self::PointerTag>> {
         Err((if int == 0 {
-            err_unsup!(InvalidNullPointerUsage)
+            // This is UB, seriously.
+            err_ub!(InvalidIntPointerUsage(0))
         } else {
+            // This is just something we cannot support during const-eval.
             err_unsup!(ReadBytesAsPointer)
         })
         .into())