Auto merge of #53779 - RalfJung:miri-refactor, r=oli-obk

Miri refactor: Final round

Tying up some loose ends that I noticed in the previous PRs -- and finally getting argument passing into a shape where @eddyb says it is "okay", which is a big improvement over the previous verdict that I cannot quote in public. ;)

Also move a bunch of useful helpers to construct `Scalar` from miri to here.

Cc @eddyb
r? @oli-obk

Author: bors

src/librustc/ich/impls_ty.rs

@@ -512,6 +512,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
         mem::discriminant(&self).hash_stable(hcx, hasher);
 
         match *self {
+            FunctionArgCountMismatch |
             DanglingPointerDeref |
             DoubleFree |
             InvalidMemoryAccess |
@@ -558,7 +559,11 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
             },
             ReferencedConstant(ref err) => err.hash_stable(hcx, hasher),
             MachineError(ref err) => err.hash_stable(hcx, hasher),
-            FunctionPointerTyMismatch(a, b) => {
+            FunctionAbiMismatch(a, b) => {
+                a.hash_stable(hcx, hasher);
+                b.hash_stable(hcx, hasher)
+            },
+            FunctionArgMismatch(a, b) => {
                 a.hash_stable(hcx, hasher);
                 b.hash_stable(hcx, hasher)
             },

src/librustc/mir/interpret/error.rs

@@ -11,9 +11,10 @@
 use std::{fmt, env};
 
 use mir;
-use ty::{FnSig, Ty, layout};
+use ty::{Ty, layout};
 use ty::layout::{Size, Align};
 use rustc_data_structures::sync::Lrc;
+use rustc_target::spec::abi::Abi;
 
 use super::{
     Pointer, Lock, AccessKind
@@ -182,7 +183,10 @@ pub enum EvalErrorKind<'tcx, O> {
     /// This variant is used by machines to signal their own errors that do not
     /// match an existing variant
     MachineError(String),
-    FunctionPointerTyMismatch(FnSig<'tcx>, FnSig<'tcx>),
+
+    FunctionAbiMismatch(Abi, Abi),
+    FunctionArgMismatch(Ty<'tcx>, Ty<'tcx>),
+    FunctionArgCountMismatch,
     NoMirFor(String),
     UnterminatedCString(Pointer),
     DanglingPointerDeref,
@@ -290,8 +294,8 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
         use self::EvalErrorKind::*;
         match *self {
             MachineError(ref inner) => inner,
-            FunctionPointerTyMismatch(..) =>
-                "tried to call a function through a function pointer of a different type",
+            FunctionAbiMismatch(..) | FunctionArgMismatch(..) | FunctionArgCountMismatch =>
+                "tried to call a function through a function pointer of incompatible type",
             InvalidMemoryAccess =>
                 "tried to access memory through an invalid pointer",
             DanglingPointerDeref =>
@@ -459,9 +463,15 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> {
                 write!(f, "type validation failed: {}", err)
             }
             NoMirFor(ref func) => write!(f, "no mir for `{}`", func),
-            FunctionPointerTyMismatch(sig, got) =>
-                write!(f, "tried to call a function with sig {} through a \
-                       function pointer of type {}", sig, got),
+            FunctionAbiMismatch(caller_abi, callee_abi) =>
+                write!(f, "tried to call a function with ABI {:?} using caller ABI {:?}",
+                    callee_abi, caller_abi),
+            FunctionArgMismatch(caller_ty, callee_ty) =>
+                write!(f, "tried to call a function with argument of type {:?} \
+                           passing data of type {:?}",
+                    callee_ty, caller_ty),
+            FunctionArgCountMismatch =>
+                write!(f, "tried to call a function with incorrect number of arguments"),
             BoundsCheck { ref len, ref index } =>
                 write!(f, "index out of bounds: the len is {:?} but the index is {:?}", len, index),
             ReallocatedWrongMemoryKind(ref old, ref new) =>

src/librustc/mir/interpret/mod.rs

@@ -85,9 +85,14 @@ pub struct GlobalId<'tcx> {
 pub trait PointerArithmetic: layout::HasDataLayout {
     // These are not supposed to be overridden.
 
+    #[inline(always)]
+    fn pointer_size(self) -> Size {
+        self.data_layout().pointer_size
+    }
+
     //// Trunace the given value to the pointer size; also return whether there was an overflow
     fn truncate_to_ptr(self, val: u128) -> (u64, bool) {
-        let max_ptr_plus_1 = 1u128 << self.data_layout().pointer_size.bits();
+        let max_ptr_plus_1 = 1u128 << self.pointer_size().bits();
         ((val % max_ptr_plus_1) as u64, val >= max_ptr_plus_1)
     }
 
@@ -491,7 +496,9 @@ pub struct Allocation {
     /// Note that the bytes of a pointer represent the offset of the pointer
     pub bytes: Vec<u8>,
     /// Maps from byte addresses to allocations.
-    /// Only the first byte of a pointer is inserted into the map.
+    /// Only the first byte of a pointer is inserted into the map; i.e.,
+    /// every entry in this map applies to `pointer_size` consecutive bytes starting
+    /// at the given offset.
     pub relocations: Relocations,
     /// Denotes undefined memory. Reading from undefined memory is forbidden in miri
     pub undef_mask: UndefMask,

src/librustc/mir/interpret/value.rs

@@ -14,7 +14,7 @@ use ty::layout::{HasDataLayout, Size};
 use ty::subst::Substs;
 use hir::def_id::DefId;
 
-use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend};
+use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend, truncate};
 
 /// Represents a constant value in Rust. Scalar and ScalarPair are optimizations which
 /// matches the LocalValue optimizations for easy conversions between Value and ConstValue.
@@ -58,6 +58,7 @@ impl<'tcx> ConstValue<'tcx> {
         self.try_to_scalar()?.to_ptr().ok()
     }
 
+    #[inline]
     pub fn new_slice(
         val: Scalar,
         len: u64,
@@ -69,23 +70,27 @@ impl<'tcx> ConstValue<'tcx> {
         }.into())
     }
 
+    #[inline]
     pub fn new_dyn_trait(val: Scalar, vtable: Pointer) -> Self {
         ConstValue::ScalarPair(val, Scalar::Ptr(vtable).into())
     }
 }
 
 impl<'tcx> Scalar {
+    #[inline]
     pub fn ptr_null(cx: impl HasDataLayout) -> Self {
         Scalar::Bits {
             bits: 0,
             size: cx.data_layout().pointer_size.bytes() as u8,
         }
     }
 
+    #[inline]
     pub fn zst() -> Self {
         Scalar::Bits { bits: 0, size: 0 }
     }
 
+    #[inline]
     pub fn ptr_signed_offset(self, i: i64, cx: impl HasDataLayout) -> EvalResult<'tcx, Self> {
         let layout = cx.data_layout();
         match self {
@@ -100,6 +105,7 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
     pub fn ptr_offset(self, i: Size, cx: impl HasDataLayout) -> EvalResult<'tcx, Self> {
         let layout = cx.data_layout();
         match self {
@@ -114,6 +120,7 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
     pub fn ptr_wrapping_signed_offset(self, i: i64, cx: impl HasDataLayout) -> Self {
         let layout = cx.data_layout();
         match self {
@@ -128,6 +135,7 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
     pub fn is_null_ptr(self, cx: impl HasDataLayout) -> bool {
         match self {
             Scalar::Bits { bits, size } =>  {
@@ -138,14 +146,53 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
+    pub fn is_null(self) -> bool {
+        match self {
+            Scalar::Bits { bits, .. } => bits == 0,
+            Scalar::Ptr(_) => false
+        }
+    }
+
+    #[inline]
     pub fn from_bool(b: bool) -> Self {
         Scalar::Bits { bits: b as u128, size: 1 }
     }
 
+    #[inline]
     pub fn from_char(c: char) -> Self {
         Scalar::Bits { bits: c as u128, size: 4 }
     }
 
+    #[inline]
+    pub fn from_uint(i: impl Into<u128>, size: Size) -> Self {
+        let i = i.into();
+        debug_assert_eq!(truncate(i, size), i,
+                    "Unsigned value {} does not fit in {} bits", i, size.bits());
+        Scalar::Bits { bits: i, size: size.bytes() as u8 }
+    }
+
+    #[inline]
+    pub fn from_int(i: impl Into<i128>, size: Size) -> Self {
+        let i = i.into();
+        // `into` performed sign extension, we have to truncate
+        let truncated = truncate(i as u128, size);
+        debug_assert_eq!(sign_extend(truncated, size) as i128, i,
+                    "Signed value {} does not fit in {} bits", i, size.bits());
+        Scalar::Bits { bits: truncated, size: size.bytes() as u8 }
+    }
+
+    #[inline]
+    pub fn from_f32(f: f32) -> Self {
+        Scalar::Bits { bits: f.to_bits() as u128, size: 4 }
+    }
+
+    #[inline]
+    pub fn from_f64(f: f64) -> Self {
+        Scalar::Bits { bits: f.to_bits() as u128, size: 8 }
+    }
+
+    #[inline]
     pub fn to_bits(self, target_size: Size) -> EvalResult<'tcx, u128> {
         match self {
             Scalar::Bits { bits, size } => {
@@ -157,6 +204,7 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
     pub fn to_ptr(self) -> EvalResult<'tcx, Pointer> {
         match self {
             Scalar::Bits { bits: 0, .. } => err!(InvalidNullPointerUsage),
@@ -165,13 +213,15 @@ impl<'tcx> Scalar {
         }
     }
 
+    #[inline]
     pub fn is_bits(self) -> bool {
         match self {
             Scalar::Bits { .. } => true,
             _ => false,
         }
     }
 
+    #[inline]
     pub fn is_ptr(self) -> bool {
         match self {
             Scalar::Ptr(_) => true,
@@ -209,6 +259,13 @@ impl<'tcx> Scalar {
         Ok(b as u32)
     }
 
+    pub fn to_u64(self) -> EvalResult<'static, u64> {
+        let sz = Size::from_bits(64);
+        let b = self.to_bits(sz)?;
+        assert_eq!(b as u64 as u128, b);
+        Ok(b as u64)
+    }
+
     pub fn to_usize(self, cx: impl HasDataLayout) -> EvalResult<'static, u64> {
         let b = self.to_bits(cx.data_layout().pointer_size)?;
         assert_eq!(b as u64 as u128, b);
@@ -231,12 +288,30 @@ impl<'tcx> Scalar {
         Ok(b as i32)
     }
 
+    pub fn to_i64(self) -> EvalResult<'static, i64> {
+        let sz = Size::from_bits(64);
+        let b = self.to_bits(sz)?;
+        let b = sign_extend(b, sz) as i128;
+        assert_eq!(b as i64 as i128, b);
+        Ok(b as i64)
+    }
+
     pub fn to_isize(self, cx: impl HasDataLayout) -> EvalResult<'static, i64> {
         let b = self.to_bits(cx.data_layout().pointer_size)?;
         let b = sign_extend(b, cx.data_layout().pointer_size) as i128;
         assert_eq!(b as i64 as i128, b);
         Ok(b as i64)
     }
+
+    #[inline]
+    pub fn to_f32(self) -> EvalResult<'static, f32> {
+        Ok(f32::from_bits(self.to_u32()?))
+    }
+
+    #[inline]
+    pub fn to_f64(self) -> EvalResult<'static, f64> {
+        Ok(f64::from_bits(self.to_u64()?))
+    }
 }
 
 impl From<Pointer> for Scalar {
@@ -308,6 +383,16 @@ impl<'tcx> ScalarMaybeUndef {
         self.not_undef()?.to_char()
     }
 
+    #[inline(always)]
+    pub fn to_f32(self) -> EvalResult<'tcx, f32> {
+        self.not_undef()?.to_f32()
+    }
+
+    #[inline(always)]
+    pub fn to_f64(self) -> EvalResult<'tcx, f64> {
+        self.not_undef()?.to_f64()
+    }
+
     #[inline(always)]
     pub fn to_u8(self) -> EvalResult<'tcx, u8> {
         self.not_undef()?.to_u8()
@@ -318,6 +403,11 @@ impl<'tcx> ScalarMaybeUndef {
         self.not_undef()?.to_u32()
     }
 
+    #[inline(always)]
+    pub fn to_u64(self) -> EvalResult<'tcx, u64> {
+        self.not_undef()?.to_u64()
+    }
+
     #[inline(always)]
     pub fn to_usize(self, cx: impl HasDataLayout) -> EvalResult<'tcx, u64> {
         self.not_undef()?.to_usize(cx)
@@ -333,6 +423,11 @@ impl<'tcx> ScalarMaybeUndef {
         self.not_undef()?.to_i32()
     }
 
+    #[inline(always)]
+    pub fn to_i64(self) -> EvalResult<'tcx, i64> {
+        self.not_undef()?.to_i64()
+    }
+
     #[inline(always)]
     pub fn to_isize(self, cx: impl HasDataLayout) -> EvalResult<'tcx, i64> {
         self.not_undef()?.to_isize(cx)

src/librustc/ty/structural_impls.rs

@@ -487,10 +487,12 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
         use ::mir::interpret::EvalErrorKind::*;
         Some(match *self {
             MachineError(ref err) => MachineError(err.clone()),
-            FunctionPointerTyMismatch(a, b) => FunctionPointerTyMismatch(
+            FunctionAbiMismatch(a, b) => FunctionAbiMismatch(a, b),
+            FunctionArgMismatch(a, b) => FunctionArgMismatch(
                 tcx.lift(&a)?,
                 tcx.lift(&b)?,
             ),
+            FunctionArgCountMismatch => FunctionArgCountMismatch,
             NoMirFor(ref s) => NoMirFor(s.clone()),
             UnterminatedCString(ptr) => UnterminatedCString(ptr),
             DanglingPointerDeref => DanglingPointerDeref,

src/librustc_mir/const_eval.rs

@@ -288,21 +288,17 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeEvaluator {
         )
     }
 
-    fn try_ptr_op<'a>(
+    fn ptr_op<'a>(
         _ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
         _bin_op: mir::BinOp,
-        left: Scalar,
+        _left: Scalar,
         _left_layout: TyLayout<'tcx>,
-        right: Scalar,
+        _right: Scalar,
         _right_layout: TyLayout<'tcx>,
-    ) -> EvalResult<'tcx, Option<(Scalar, bool)>> {
-        if left.is_bits() && right.is_bits() {
-            Ok(None)
-        } else {
-            Err(
-                ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
-            )
-        }
+    ) -> EvalResult<'tcx, (Scalar, bool)> {
+        Err(
+            ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
+        )
     }
 
     fn find_foreign_static<'a>(