Skip to content

Miri refactor: Final round #53779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
mem::discriminant(&self).hash_stable(hcx, hasher);

match *self {
FunctionArgCountMismatch |
DanglingPointerDeref |
DoubleFree |
InvalidMemoryAccess |
Expand Down Expand Up @@ -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)
},
Expand Down
24 changes: 17 additions & 7 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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) =>
Expand Down
11 changes: 9 additions & 2 deletions src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
Expand Down
97 changes: 96 additions & 1 deletion src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -58,6 +58,7 @@ impl<'tcx> ConstValue<'tcx> {
self.try_to_scalar()?.to_ptr().ok()
}

#[inline]
pub fn new_slice(
val: Scalar,
len: u64,
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -128,6 +135,7 @@ impl<'tcx> Scalar {
}
}

#[inline]
pub fn is_null_ptr(self, cx: impl HasDataLayout) -> bool {
match self {
Scalar::Bits { bits, size } => {
Expand All @@ -138,14 +146,53 @@ impl<'tcx> Scalar {
}
}

#[inline]
pub fn is_null(self) -> bool {
match self {
Scalar::Bits { bits, .. } => bits == 0,
Scalar::Ptr(_) => false
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh... when did this happen? I feel it wasn't present in my original review, but I may have missed it.

I don't like this function because for pointers, it cannot reliably tell you whether this is NULL or not. There might have been overflow. That's why I deliberately removed this function recently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh lol, never mind, I am looking at the wrong PR. We have too many PRs called "miri refactor".^^


#[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 } => {
Expand All @@ -157,6 +204,7 @@ impl<'tcx> Scalar {
}
}

#[inline]
pub fn to_ptr(self) -> EvalResult<'tcx, Pointer> {
match self {
Scalar::Bits { bits: 0, .. } => err!(InvalidNullPointerUsage),
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 7 additions & 11 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down
Loading