From cdeef61425ec177d2eb0f84d02a9d25000c954dd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Aug 2018 20:42:52 +0200 Subject: [PATCH 1/7] move some Scalar helpers from miri here, and use them where appropriate --- src/librustc/mir/interpret/mod.rs | 7 +- src/librustc/mir/interpret/value.rs | 97 +++++++++++++++++++- src/librustc_mir/interpret/cast.rs | 102 ++++++++------------- src/librustc_mir/interpret/eval_context.rs | 3 +- src/librustc_mir/interpret/intrinsics.rs | 17 +--- src/librustc_mir/interpret/memory.rs | 34 +++---- src/librustc_mir/interpret/operand.rs | 5 +- src/librustc_mir/interpret/operator.rs | 42 +++------ src/librustc_mir/interpret/place.rs | 30 +++--- src/librustc_mir/interpret/step.rs | 23 ++--- src/librustc_mir/interpret/terminator.rs | 9 +- src/librustc_mir/interpret/traits.rs | 17 ++-- src/librustc_mir/interpret/validity.rs | 5 +- 13 files changed, 210 insertions(+), 181 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 147f9ccad7c38..da5216bd1befe 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/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) } diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index d793bb1cc63ca..11a4f8b884e7f 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/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,12 +70,14 @@ 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, @@ -82,10 +85,12 @@ impl<'tcx> Scalar { } } + #[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, 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, 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,6 +213,7 @@ impl<'tcx> Scalar { } } + #[inline] pub fn is_bits(self) -> bool { match self { Scalar::Bits { .. } => true, @@ -172,6 +221,7 @@ impl<'tcx> Scalar { } } + #[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 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) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index d0d1c5d6610d0..9bbaa0e7ef699 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -14,8 +14,7 @@ use syntax::ast::{FloatTy, IntTy, UintTy}; use rustc_apfloat::ieee::{Single, Double}; use rustc::mir::interpret::{ - Scalar, EvalResult, Pointer, PointerArithmetic, EvalErrorKind, - truncate, sign_extend + Scalar, EvalResult, Pointer, PointerArithmetic, EvalErrorKind, truncate }; use rustc::mir::CastKind; use rustc_apfloat::Float; @@ -70,10 +69,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { .discriminant_for_variant(*self.tcx, index) .val; return self.write_scalar( - Scalar::Bits { - bits: discr_val, - size: dst_layout.size.bytes() as u8, - }, + Scalar::from_uint(discr_val, dst_layout.size), dest); } } @@ -198,41 +194,39 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { match dest_layout.ty.sty { Int(_) | Uint(_) => { let v = self.truncate(v, dest_layout); - Ok(Scalar::Bits { - bits: v, - size: dest_layout.size.bytes() as u8, - }) + Ok(Scalar::from_uint(v, dest_layout.size)) } - Float(FloatTy::F32) if signed => Ok(Scalar::Bits { - bits: Single::from_i128(v as i128).value.to_bits(), - size: 4, - }), - Float(FloatTy::F64) if signed => Ok(Scalar::Bits { - bits: Double::from_i128(v as i128).value.to_bits(), - size: 8, - }), - Float(FloatTy::F32) => Ok(Scalar::Bits { - bits: Single::from_u128(v).value.to_bits(), - size: 4, - }), - Float(FloatTy::F64) => Ok(Scalar::Bits { - bits: Double::from_u128(v).value.to_bits(), - size: 8, - }), + Float(FloatTy::F32) if signed => Ok(Scalar::from_uint( + Single::from_i128(v as i128).value.to_bits(), + Size::from_bits(32) + )), + Float(FloatTy::F64) if signed => Ok(Scalar::from_uint( + Double::from_i128(v as i128).value.to_bits(), + Size::from_bits(64) + )), + Float(FloatTy::F32) => Ok(Scalar::from_uint( + Single::from_u128(v).value.to_bits(), + Size::from_bits(32) + )), + Float(FloatTy::F64) => Ok(Scalar::from_uint( + Double::from_u128(v).value.to_bits(), + Size::from_bits(64) + )), Char => { - assert_eq!(v as u8 as u128, v); - Ok(Scalar::Bits { bits: v, size: 4 }) + // `u8` to `char` cast + debug_assert_eq!(v as u8 as u128, v); + Ok(Scalar::from_uint(v, Size::from_bytes(4))) }, // No alignment check needed for raw pointers. // But we have to truncate to target ptr size. RawPtr(_) => { - Ok(Scalar::Bits { - bits: self.memory.truncate_to_ptr(v).0 as u128, - size: self.memory.pointer_size().bytes() as u8, - }) + Ok(Scalar::from_uint( + self.truncate_to_ptr(v).0, + self.pointer_size(), + )) }, // Casts to bool are not permitted by rustc, no need to handle them here. @@ -251,56 +245,40 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { match dest_ty.sty { // float -> uint Uint(t) => { - let width = t.bit_width().unwrap_or(self.memory.pointer_size().bits() as usize); + let width = t.bit_width().unwrap_or(self.pointer_size().bits() as usize); let v = match fty { FloatTy::F32 => Single::from_bits(bits).to_u128(width).value, FloatTy::F64 => Double::from_bits(bits).to_u128(width).value, }; // This should already fit the bit width - Ok(Scalar::Bits { - bits: v, - size: (width / 8) as u8, - }) + Ok(Scalar::from_uint(v, Size::from_bits(width as u64))) }, // float -> int Int(t) => { - let width = t.bit_width().unwrap_or(self.memory.pointer_size().bits() as usize); + let width = t.bit_width().unwrap_or(self.pointer_size().bits() as usize); let v = match fty { FloatTy::F32 => Single::from_bits(bits).to_i128(width).value, FloatTy::F64 => Double::from_bits(bits).to_i128(width).value, }; - // We got an i128, but we may need something smaller. We have to truncate ourselves. - let truncated = truncate(v as u128, Size::from_bits(width as u64)); - assert_eq!(sign_extend(truncated, Size::from_bits(width as u64)) as i128, v, - "truncating and extending changed the value?!?"); - Ok(Scalar::Bits { - bits: truncated, - size: (width / 8) as u8, - }) + Ok(Scalar::from_int(v, Size::from_bits(width as u64))) }, // f64 -> f32 Float(FloatTy::F32) if fty == FloatTy::F64 => { - Ok(Scalar::Bits { - bits: Single::to_bits(Double::from_bits(bits).convert(&mut false).value), - size: 4, - }) + Ok(Scalar::from_uint( + Single::to_bits(Double::from_bits(bits).convert(&mut false).value), + Size::from_bits(32), + )) }, // f32 -> f64 Float(FloatTy::F64) if fty == FloatTy::F32 => { - Ok(Scalar::Bits { - bits: Double::to_bits(Single::from_bits(bits).convert(&mut false).value), - size: 8, - }) + Ok(Scalar::from_uint( + Double::to_bits(Single::from_bits(bits).convert(&mut false).value), + Size::from_bits(64), + )) }, // identity cast - Float(FloatTy:: F64) => Ok(Scalar::Bits { - bits, - size: 8, - }), - Float(FloatTy:: F32) => Ok(Scalar::Bits { - bits, - size: 4, - }), + Float(FloatTy:: F64) => Ok(Scalar::from_uint(bits, Size::from_bits(64))), + Float(FloatTy:: F32) => Ok(Scalar::from_uint(bits, Size::from_bits(32))), _ => err!(Unimplemented(format!("float to {:?} cast", dest_ty))), } } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 455c3fc281a6b..6e144ba7ed2ee 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -270,7 +270,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for &'a EvalContext<' } impl<'c, 'b, 'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout - for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M> { + for &'c &'b mut EvalContext<'a, 'mir, 'tcx, M> +{ #[inline] fn data_layout(&self) -> &layout::TargetDataLayout { &self.tcx.data_layout diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 35e3253ca7f8e..db27df3723c0e 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -41,7 +41,7 @@ fn numeric_intrinsic<'tcx>( "bswap" => (bits << extra).swap_bytes(), _ => bug!("not a numeric intrinsic: {}", name), }; - Ok(Scalar::Bits { bits: bits_out, size: size.bytes() as u8 }) + Ok(Scalar::from_uint(bits_out, size)) } impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { @@ -59,30 +59,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { "min_align_of" => { let elem_ty = substs.type_at(0); let elem_align = self.layout_of(elem_ty)?.align.abi(); - let align_val = Scalar::Bits { - bits: elem_align as u128, - size: dest.layout.size.bytes() as u8, - }; + let align_val = Scalar::from_uint(elem_align, dest.layout.size); self.write_scalar(align_val, dest)?; } "size_of" => { let ty = substs.type_at(0); let size = self.layout_of(ty)?.size.bytes() as u128; - let size_val = Scalar::Bits { - bits: size, - size: dest.layout.size.bytes() as u8, - }; + let size_val = Scalar::from_uint(size, dest.layout.size); self.write_scalar(size_val, dest)?; } "type_id" => { let ty = substs.type_at(0); let type_id = self.tcx.type_id_hash(ty) as u128; - let id_val = Scalar::Bits { - bits: type_id, - size: dest.layout.size.bytes() as u8, - }; + let id_val = Scalar::from_uint(type_id, dest.layout.size); self.write_scalar(id_val, dest)?; } "ctpop" | "cttz" | "cttz_nonzero" | "ctlz" | "ctlz_nonzero" | "bswap" => { diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 240f977a5a0e8..4063447fbb151 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -23,7 +23,8 @@ use std::ptr; use rustc::ty::{self, Instance, query::TyCtxtAt}; use rustc::ty::layout::{self, Align, TargetDataLayout, Size, HasDataLayout}; use rustc::mir::interpret::{Pointer, AllocId, Allocation, ScalarMaybeUndef, GlobalId, - EvalResult, Scalar, EvalErrorKind, AllocType, truncate}; + EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, + truncate}; pub use rustc::mir::interpret::{write_target_uint, read_target_uint}; use rustc_data_structures::fx::{FxHashSet, FxHashMap, FxHasher}; @@ -60,6 +61,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for &'a Memory<'a, 'm &self.tcx.data_layout } } +impl<'a, 'b, 'c, 'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout + for &'b &'c mut Memory<'a, 'mir, 'tcx, M> +{ + #[inline] + fn data_layout(&self) -> &TargetDataLayout { + &self.tcx.data_layout + } +} impl<'a, 'mir, 'tcx, M> Eq for Memory<'a, 'mir, 'tcx, M> where M: Machine<'mir, 'tcx>, @@ -277,14 +286,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(()) } - pub fn pointer_size(&self) -> Size { - self.tcx.data_layout.pointer_size - } - - pub fn endianness(&self) -> layout::Endian { - self.tcx.data_layout.endian - } - /// Check that the pointer is aligned AND non-NULL. This supports scalars /// for the benefit of other parts of miri that need to check alignment even for ZST. pub fn check_align(&self, ptr: Scalar, required_align: Align) -> EvalResult<'tcx> { @@ -773,7 +774,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ) -> EvalResult<'tcx, ScalarMaybeUndef> { // Make sure we don't read part of a pointer as a pointer self.check_relocation_edges(ptr, size)?; - let endianness = self.endianness(); // get_bytes_unchecked tests alignment let bytes = self.get_bytes_unchecked(ptr, size, ptr_align.min(self.int_align(size)))?; // Undef check happens *after* we established that the alignment is correct. @@ -784,7 +784,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { return Ok(ScalarMaybeUndef::Undef); } // Now we do the actual reading - let bits = read_target_uint(endianness, bytes).unwrap(); + let bits = read_target_uint(self.tcx.data_layout.endian, bytes).unwrap(); // See if we got a pointer if size != self.pointer_size() { if self.relocations(ptr, size)?.len() != 0 { @@ -801,10 +801,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } // We don't. Just return the bits. - Ok(ScalarMaybeUndef::Scalar(Scalar::Bits { - bits, - size: size.bytes() as u8, - })) + Ok(ScalarMaybeUndef::Scalar(Scalar::from_uint(bits, size))) } pub fn read_ptr_sized(&self, ptr: Pointer, ptr_align: Align) @@ -820,8 +817,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { val: ScalarMaybeUndef, type_size: Size, ) -> EvalResult<'tcx> { - let endianness = self.endianness(); - let val = match val { ScalarMaybeUndef::Scalar(scalar) => scalar, ScalarMaybeUndef::Undef => return self.mark_definedness(ptr, type_size, false), @@ -835,7 +830,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Scalar::Bits { bits, size } => { assert_eq!(size as u64, type_size.bytes()); - assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, + debug_assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, "Unexpected value of size {} when writing to memory", size); bits }, @@ -843,8 +838,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { { // get_bytes_mut checks alignment + let endian = self.tcx.data_layout.endian; let dst = self.get_bytes_mut(ptr, type_size, ptr_align)?; - write_target_uint(endianness, dst, bytes).unwrap(); + write_target_uint(endian, dst, bytes).unwrap(); } // See if we have to also write a relocation diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9681b705d7eba..b2a668b961f16 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -42,10 +42,7 @@ impl<'tcx> Value { len: u64, cx: impl HasDataLayout ) -> Self { - Value::ScalarPair(val.into(), Scalar::Bits { - bits: len as u128, - size: cx.data_layout().pointer_size.bytes() as u8, - }.into()) + Value::ScalarPair(val.into(), Scalar::from_uint(len, cx.data_layout().pointer_size).into()) } pub fn new_dyn_trait(val: Scalar, vtable: Pointer) -> Self { diff --git a/src/librustc_mir/interpret/operator.rs b/src/librustc_mir/interpret/operator.rs index 13ed527e3496b..4f4f00e320a96 100644 --- a/src/librustc_mir/interpret/operator.rs +++ b/src/librustc_mir/interpret/operator.rs @@ -9,7 +9,7 @@ // except according to those terms. use rustc::mir; -use rustc::ty::{self, layout::TyLayout}; +use rustc::ty::{self, layout::{Size, TyLayout}}; use syntax::ast::FloatTy; use rustc_apfloat::ieee::{Double, Single}; use rustc_apfloat::Float; @@ -105,10 +105,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { ($ty:path, $size:expr) => {{ let l = <$ty>::from_bits(l); let r = <$ty>::from_bits(r); - let bitify = |res: ::rustc_apfloat::StatusAnd<$ty>| Scalar::Bits { - bits: res.value.to_bits(), - size: $size, - }; + let bitify = |res: ::rustc_apfloat::StatusAnd<$ty>| + Scalar::from_uint(res.value.to_bits(), Size::from_bytes($size)); let val = match bin_op { Eq => Scalar::from_bool(l == r), Ne => Scalar::from_bool(l != r), @@ -169,10 +167,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { } }; let truncated = self.truncate(result, left_layout); - return Ok((Scalar::Bits { - bits: truncated, - size: size.bytes() as u8, - }, oflo)); + return Ok((Scalar::from_uint(truncated, size), oflo)); } // For the remaining ops, the types must be the same on both sides @@ -220,7 +215,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Rem | Div => { // int_min / -1 if r == -1 && l == (1 << (size.bits() - 1)) { - return Ok((Scalar::Bits { bits: l, size: size.bytes() as u8 }, true)); + return Ok((Scalar::from_uint(l, size), true)); } }, _ => {}, @@ -232,16 +227,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let max = 1 << (size.bits() - 1); oflo = result >= max || result < -max; } + // this may be out-of-bounds for the result type, so we have to truncate ourselves let result = result as u128; let truncated = self.truncate(result, left_layout); - return Ok((Scalar::Bits { - bits: truncated, - size: size.bytes() as u8, - }, oflo)); + return Ok((Scalar::from_uint(truncated, size), oflo)); } } - let size = left_layout.size.bytes() as u8; + let size = left_layout.size; // only ints left let val = match bin_op { @@ -253,11 +246,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Gt => Scalar::from_bool(l > r), Ge => Scalar::from_bool(l >= r), - BitOr => Scalar::Bits { bits: l | r, size }, - BitAnd => Scalar::Bits { bits: l & r, size }, - BitXor => Scalar::Bits { bits: l ^ r, size }, + BitOr => Scalar::from_uint(l | r, size), + BitAnd => Scalar::from_uint(l & r, size), + BitXor => Scalar::from_uint(l ^ r, size), Add | Sub | Mul | Rem | Div => { + debug_assert!(!left_layout.abi.is_signed()); let op: fn(u128, u128) -> (u128, bool) = match bin_op { Add => u128::overflowing_add, Sub => u128::overflowing_sub, @@ -270,10 +264,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { }; let (result, oflo) = op(l, r); let truncated = self.truncate(result, left_layout); - return Ok((Scalar::Bits { - bits: truncated, - size, - }, oflo || truncated != result)); + return Ok((Scalar::from_uint(truncated, size), oflo || truncated != result)); } _ => { @@ -373,7 +364,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { (Neg, FloatTy::F64) => Double::to_bits(-Double::from_bits(val)), _ => bug!("Invalid float op {:?}", un_op) }; - Ok(Scalar::Bits { bits: res, size: layout.size.bytes() as u8 }) + Ok(Scalar::from_uint(res, layout.size)) } _ => { assert!(layout.ty.is_integral()); @@ -386,10 +377,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { } }; // res needs tuncating - Ok(Scalar::Bits { - bits: self.truncate(res, layout), - size: layout.size.bytes() as u8, - }) + Ok(Scalar::from_uint(self.truncate(res, layout), layout.size)) } } } diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 0a6fef3008433..5bf6b2b46b7a6 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -20,7 +20,7 @@ use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout}; use rustc_data_structures::indexed_vec::Idx; use rustc::mir::interpret::{ - GlobalId, Scalar, EvalResult, Pointer, ScalarMaybeUndef + GlobalId, Scalar, EvalResult, Pointer, ScalarMaybeUndef, PointerArithmetic }; use super::{EvalContext, Machine, Value, ValTy, Operand, OpTy, MemoryKind}; @@ -344,10 +344,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { ty::Array(inner, _) => (None, self.tcx.mk_array(inner, inner_len)), ty::Slice(..) => { - let len = Scalar::Bits { - bits: inner_len.into(), - size: self.memory.pointer_size().bytes() as u8 - }; + let len = Scalar::from_uint(inner_len, self.pointer_size()); (Some(len), base.layout.ty) } _ => @@ -716,10 +713,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let discr_val = (discr_val << shift) >> shift; let discr_dest = self.place_field(dest, 0)?; - self.write_scalar(Scalar::Bits { - bits: discr_val, - size: size.bytes() as u8, - }, discr_dest)?; + self.write_scalar(Scalar::from_uint(discr_val, size), discr_dest)?; } layout::Variants::NicheFilling { dataful_variant, @@ -733,10 +727,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { self.place_field(dest, 0)?; let niche_value = ((variant_index - niche_variants.start()) as u128) .wrapping_add(niche_start); - self.write_scalar(Scalar::Bits { - bits: niche_value, - size: niche_dest.layout.size.bytes() as u8, - }, niche_dest)?; + self.write_scalar( + Scalar::from_uint(niche_value, niche_dest.layout.size), + niche_dest + )?; } } } @@ -766,11 +760,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let layout = self.layout_of(ty)?; // More sanity checks - let (size, align) = self.read_size_and_align_from_vtable(vtable)?; - assert_eq!(size, layout.size); - assert_eq!(align.abi(), layout.align.abi()); // only ABI alignment is preserved - // FIXME: More checks for the vtable? We could make sure it is exactly - // the one one would expect for this type. + if cfg!(debug_assertions) { + let (size, align) = self.read_size_and_align_from_vtable(vtable)?; + assert_eq!(size, layout.size); + assert_eq!(align.abi(), layout.align.abi()); // only ABI alignment is preserved + } let mplace = MPlaceTy { mplace: MemPlace { extra: None, ..*mplace }, diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 933f06d3d10a0..114ef093ec2fd 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -14,7 +14,7 @@ use rustc::mir; use rustc::ty::layout::LayoutOf; -use rustc::mir::interpret::{EvalResult, Scalar}; +use rustc::mir::interpret::{EvalResult, Scalar, PointerArithmetic}; use super::{EvalContext, Machine}; @@ -269,12 +269,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let src = self.eval_place(place)?; let mplace = self.force_allocation(src)?; let len = mplace.len(&self)?; - let size = self.memory.pointer_size().bytes() as u8; + let size = self.pointer_size(); self.write_scalar( - Scalar::Bits { - bits: len as u128, - size, - }, + Scalar::from_uint(len, size), dest, )?; } @@ -294,12 +291,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let layout = self.layout_of(ty)?; assert!(!layout.is_unsized(), "SizeOf nullary MIR operator called for unsized type"); - let size = self.memory.pointer_size().bytes() as u8; + let size = self.pointer_size(); self.write_scalar( - Scalar::Bits { - bits: layout.size.bytes() as u128, - size, - }, + Scalar::from_uint(layout.size.bytes(), size), dest, )?; } @@ -313,11 +307,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Discriminant(ref place) => { let place = self.eval_place(place)?; let discr_val = self.read_discriminant(self.place_to_op(place)?)?.0; - let size = dest.layout.size.bytes() as u8; - self.write_scalar(Scalar::Bits { - bits: discr_val, - size, - }, dest)?; + let size = dest.layout.size; + self.write_scalar(Scalar::from_uint(discr_val, size), dest)?; } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 11826e0ce0c25..8fc7c11c6ab8f 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -16,7 +16,7 @@ use rustc::ty::layout::LayoutOf; use syntax::source_map::Span; use rustc_target::spec::abi::Abi; -use rustc::mir::interpret::{EvalResult, Scalar}; +use rustc::mir::interpret::{EvalResult, Scalar, PointerArithmetic}; use super::{ EvalContext, Machine, Value, OpTy, Place, PlaceTy, ValTy, Operand, StackPopCleanup }; @@ -60,10 +60,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { for (index, &const_int) in values.iter().enumerate() { // Compare using binary_op - let const_int = Scalar::Bits { - bits: const_int, - size: discr.layout.size.bytes() as u8 - }; + let const_int = Scalar::from_uint(const_int, discr.layout.size); let (res, _) = self.binary_op(mir::BinOp::Eq, discr, ValTy { value: Value::Scalar(const_int.into()), layout: discr.layout } @@ -411,7 +408,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { } // cannot use the shim here, because that will only result in infinite recursion ty::InstanceDef::Virtual(_, idx) => { - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); let ptr_align = self.tcx.data_layout.pointer_align; let ptr = self.ref_to_mplace(self.read_value(args[0])?)?; let vtable = ptr.vtable()?; diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 74567b4297491..0e09f65f0a8ea 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -10,7 +10,7 @@ use rustc::ty::{self, Ty}; use rustc::ty::layout::{Size, Align, LayoutOf}; -use rustc::mir::interpret::{Scalar, Pointer, EvalResult}; +use rustc::mir::interpret::{Scalar, Pointer, EvalResult, PointerArithmetic}; use syntax::ast::Mutability; @@ -35,7 +35,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let size = layout.size.bytes(); let align = layout.align.abi(); - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); let ptr_align = self.tcx.data_layout.pointer_align; let methods = self.tcx.vtable_methods(trait_ref); let vtable = self.memory.allocate( @@ -49,15 +49,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { self.memory.write_ptr_sized(vtable, ptr_align, Scalar::Ptr(drop).into())?; let size_ptr = vtable.offset(ptr_size, &self)?; - self.memory.write_ptr_sized(size_ptr, ptr_align, Scalar::Bits { - bits: size as u128, - size: ptr_size.bytes() as u8, - }.into())?; + self.memory.write_ptr_sized(size_ptr, ptr_align, Scalar::from_uint(size, ptr_size).into())?; let align_ptr = vtable.offset(ptr_size * 2, &self)?; - self.memory.write_ptr_sized(align_ptr, ptr_align, Scalar::Bits { - bits: align as u128, - size: ptr_size.bytes() as u8, - }.into())?; + self.memory.write_ptr_sized(align_ptr, ptr_align, + Scalar::from_uint(align, ptr_size).into())?; for (i, method) in methods.iter().enumerate() { if let Some((def_id, substs)) = *method { @@ -97,7 +92,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { &self, vtable: Pointer, ) -> EvalResult<'tcx, (Size, Align)> { - let pointer_size = self.memory.pointer_size(); + let pointer_size = self.pointer_size(); let pointer_align = self.tcx.data_layout.pointer_align; let size = self.memory.read_ptr_sized(vtable.offset(pointer_size, self)?,pointer_align)? .to_bits(pointer_size)? as u64; diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index d50fd6e13c106..ef2ae8f5337c6 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -15,7 +15,7 @@ use rustc::ty::layout::{self, Size, Primitive}; use rustc::ty::{self, Ty}; use rustc_data_structures::fx::FxHashSet; use rustc::mir::interpret::{ - Scalar, AllocType, EvalResult, ScalarMaybeUndef, EvalErrorKind + Scalar, AllocType, EvalResult, ScalarMaybeUndef, EvalErrorKind, PointerArithmetic }; use super::{ @@ -118,7 +118,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { bits }, Scalar::Ptr(_) => { - let ptr_size = self.memory.pointer_size(); + let ptr_size = self.pointer_size(); let ptr_max = u128::max_value() >> (128 - ptr_size.bits()); return if lo > hi { if lo - hi == 1 { @@ -376,6 +376,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { "non-pointer vtable in fat pointer", path ), } + // FIXME: More checks for the vtable. } ty::Slice(..) | ty::Str => { match ptr.extra.unwrap().to_usize(self) { From b8a40aad1b309bdeac2bf2a7d1509aa568f84093 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 27 Aug 2018 13:34:12 +0200 Subject: [PATCH 2/7] memory: make getting the alloc for a static an associate function for easier calling --- src/librustc_mir/interpret/memory.rs | 84 ++++++++++++++-------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 4063447fbb151..b08e8c230bd34 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -122,45 +122,6 @@ impl<'a, 'mir, 'tcx, M> Hash for Memory<'a, 'mir, 'tcx, M> } } -/// Helper function to obtain the global (tcx) allocation for a static -fn const_eval_static<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>>( - tcx: TyCtxtAt<'a, 'tcx, 'tcx>, - id: AllocId -) -> EvalResult<'tcx, &'tcx Allocation> { - let alloc = tcx.alloc_map.lock().get(id); - let def_id = match alloc { - Some(AllocType::Memory(mem)) => { - return Ok(mem) - } - Some(AllocType::Function(..)) => { - return err!(DerefFunctionPointer) - } - Some(AllocType::Static(did)) => { - did - } - None => - return err!(DanglingPointerDeref), - }; - // We got a "lazy" static that has not been computed yet, do some work - trace!("static_alloc: Need to compute {:?}", def_id); - if tcx.is_foreign_item(def_id) { - return M::find_foreign_static(tcx, def_id); - } - let instance = Instance::mono(tcx.tcx, def_id); - let gid = GlobalId { - instance, - promoted: None, - }; - tcx.const_eval(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| { - // no need to report anything, the const_eval call takes care of that for statics - assert!(tcx.is_static(def_id).is_some()); - EvalErrorKind::ReferencedConstant(err).into() - }).map(|val| { - // FIXME We got our static (will be a ByRef), now we make a *copy*?!? - tcx.const_to_allocation(val) - }) -} - impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn new(tcx: TyCtxtAt<'a, 'tcx, 'tcx>, data: M::MemoryData) -> Self { Memory { @@ -343,13 +304,52 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// Allocation accessors impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { + /// Helper function to obtain the global (tcx) allocation for a static + fn get_static_alloc( + tcx: TyCtxtAt<'a, 'tcx, 'tcx>, + id: AllocId, + ) -> EvalResult<'tcx, &'tcx Allocation> { + let alloc = tcx.alloc_map.lock().get(id); + let def_id = match alloc { + Some(AllocType::Memory(mem)) => { + return Ok(mem) + } + Some(AllocType::Function(..)) => { + return err!(DerefFunctionPointer) + } + Some(AllocType::Static(did)) => { + did + } + None => + return err!(DanglingPointerDeref), + }; + // We got a "lazy" static that has not been computed yet, do some work + trace!("static_alloc: Need to compute {:?}", def_id); + if tcx.is_foreign_item(def_id) { + return M::find_foreign_static(tcx, def_id); + } + let instance = Instance::mono(tcx.tcx, def_id); + let gid = GlobalId { + instance, + promoted: None, + }; + tcx.const_eval(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| { + // no need to report anything, the const_eval call takes care of that for statics + assert!(tcx.is_static(def_id).is_some()); + EvalErrorKind::ReferencedConstant(err).into() + }).map(|val| { + // FIXME We got our static (will be a ByRef), now we make a *copy*?!? + tcx.const_to_allocation(val) + }) + } + pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation> { match self.alloc_map.get(&id) { // Normal alloc? Some(alloc) => Ok(&alloc.1), // Static. No need to make any copies, just provide read access to the global static // memory in tcx. - None => const_eval_static::(self.tcx, id), + None => Self::get_static_alloc(self.tcx, id), } } @@ -381,7 +381,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { if ptr.offset.bytes() != 0 { return err!(InvalidFunctionPointer); } - debug!("reading fn ptr: {}", ptr.alloc_id); + trace!("reading fn ptr: {}", ptr.alloc_id); match self.tcx.alloc_map.lock().get(ptr.alloc_id) { Some(AllocType::Function(instance)) => Ok(instance), _ => Err(EvalErrorKind::ExecuteMemory.into()), @@ -610,7 +610,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { id: AllocId, kind: MemoryKind, ) -> EvalResult<'tcx> { - let alloc = const_eval_static::(self.tcx, id)?; + let alloc = Self::get_static_alloc(self.tcx, id)?; if alloc.mutability == Mutability::Immutable { return err!(ModifiedConstantMemory); } From ec056d51889349818138d0c92cebcd2eeb4eb730 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 27 Aug 2018 13:34:35 +0200 Subject: [PATCH 3/7] re-do argument passing one more time to finally be sane --- src/librustc/ich/impls_ty.rs | 7 +- src/librustc/mir/interpret/error.rs | 24 +- src/librustc/ty/structural_impls.rs | 4 +- src/librustc_mir/interpret/terminator.rs | 343 +++++++++++------------ src/librustc_mir/transform/const_prop.rs | 4 +- 5 files changed, 189 insertions(+), 193 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index c598f99a2b54d..6250e12f43043 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/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) }, diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index ab38f8fa72135..84d55c84ce154 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/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) => diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index e3ef74f44db7f..60b85e8a8eb9a 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/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, diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 8fc7c11c6ab8f..08ee30e5948a6 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -10,13 +10,12 @@ use std::borrow::Cow; -use rustc::mir; -use rustc::ty::{self, Ty}; -use rustc::ty::layout::LayoutOf; +use rustc::{mir, ty}; +use rustc::ty::layout::{self, TyLayout, LayoutOf}; use syntax::source_map::Span; use rustc_target::spec::abi::Abi; -use rustc::mir::interpret::{EvalResult, Scalar, PointerArithmetic}; +use rustc::mir::interpret::{EvalResult, PointerArithmetic, EvalErrorKind, Scalar}; use super::{ EvalContext, Machine, Value, OpTy, Place, PlaceTy, ValTy, Operand, StackPopCleanup }; @@ -59,7 +58,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let mut target_block = targets[targets.len() - 1]; for (index, &const_int) in values.iter().enumerate() { - // Compare using binary_op + // Compare using binary_op, to also support pointer values let const_int = Scalar::from_uint(const_int, discr.layout.size); let (res, _) = self.binary_op(mir::BinOp::Eq, discr, @@ -86,37 +85,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { }; let func = self.eval_operand(func, None)?; - let (fn_def, sig) = match func.layout.ty.sty { + let (fn_def, abi) = match func.layout.ty.sty { ty::FnPtr(sig) => { + let caller_abi = sig.abi(); let fn_ptr = self.read_scalar(func)?.to_ptr()?; let instance = self.memory.get_fn(fn_ptr)?; - let instance_ty = instance.ty(*self.tcx); - match instance_ty.sty { - ty::FnDef(..) => { - let sig = self.tcx.normalize_erasing_late_bound_regions( - self.param_env, - &sig, - ); - let real_sig = instance_ty.fn_sig(*self.tcx); - let real_sig = self.tcx.normalize_erasing_late_bound_regions( - self.param_env, - &real_sig, - ); - if !self.check_sig_compat(sig, real_sig)? { - return err!(FunctionPointerTyMismatch(real_sig, sig)); - } - (instance, sig) - } - _ => bug!("unexpected fn ptr to ty: {:?}", instance_ty), - } + (instance, caller_abi) } ty::FnDef(def_id, substs) => { let sig = func.layout.ty.fn_sig(*self.tcx); - let sig = self.tcx.normalize_erasing_late_bound_regions( - self.param_env, - &sig, - ); - (self.resolve(def_id, substs)?, sig) + (self.resolve(def_id, substs)?, sig.abi()) }, _ => { let msg = format!("can't handle callee of type {:?}", func.layout.ty); @@ -126,11 +104,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let args = self.eval_operands(args)?; self.eval_fn_call( fn_def, + terminator.source_info.span, + abi, &args[..], dest, ret, - terminator.source_info.span, - Some(sig), )?; } @@ -165,6 +143,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { if expected == cond_val { self.goto_block(Some(target))?; } else { + // Compute error message use rustc::mir::interpret::EvalErrorKind::*; return match *msg { BoundsCheck { ref len, ref index } => { @@ -187,11 +166,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { } } - Yield { .. } => unimplemented!("{:#?}", terminator.kind), - GeneratorDrop => unimplemented!(), - DropAndReplace { .. } => unimplemented!(), - Resume => unimplemented!(), - Abort => unimplemented!(), + Yield { .. } | + GeneratorDrop | + DropAndReplace { .. } | + Resume | + Abort => unimplemented!("{:#?}", terminator.kind), FalseEdges { .. } => bug!("should have been eliminated by\ `simplify_branches` mir pass"), FalseUnwind { .. } => bug!("should have been eliminated by\ @@ -202,91 +181,67 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Ok(()) } - /// Decides whether it is okay to call the method with signature `real_sig` - /// using signature `sig`. - /// FIXME: This should take into account the platform-dependent ABI description. - fn check_sig_compat( - &mut self, - sig: ty::FnSig<'tcx>, - real_sig: ty::FnSig<'tcx>, - ) -> EvalResult<'tcx, bool> { - fn check_ty_compat<'tcx>(ty: Ty<'tcx>, real_ty: Ty<'tcx>) -> bool { - if ty == real_ty { - return true; - } // This is actually a fast pointer comparison - return match (&ty.sty, &real_ty.sty) { - // Permit changing the pointer type of raw pointers and references as well as - // mutability of raw pointers. - // FIXME: Should not be allowed when fat pointers are involved. - (&ty::RawPtr(_), &ty::RawPtr(_)) => true, - (&ty::Ref(_, _, _), &ty::Ref(_, _, _)) => { - ty.is_mutable_pointer() == real_ty.is_mutable_pointer() - } - // rule out everything else - _ => false, - }; + fn check_argument_compat( + caller: TyLayout<'tcx>, + callee: TyLayout<'tcx>, + ) -> bool { + if caller.ty == callee.ty { + // No question + return true; } - - if sig.abi == real_sig.abi && sig.variadic == real_sig.variadic && - sig.inputs_and_output.len() == real_sig.inputs_and_output.len() && - sig.inputs_and_output - .iter() - .zip(real_sig.inputs_and_output) - .all(|(ty, real_ty)| check_ty_compat(ty, real_ty)) - { - // Definitely good. - return Ok(true); + // Compare layout + match (&caller.abi, &callee.abi) { + (layout::Abi::Scalar(ref caller), layout::Abi::Scalar(ref callee)) => + // Different valid ranges are okay (once we enforce validity, + // that will take care to make it UB to leave the range, just + // like for transmute). + caller.value == callee.value, + // Be conservative + _ => false } + } - if sig.variadic || real_sig.variadic { - // We're not touching this - return Ok(false); + /// Pass a single argument, checking the types for compatibility. + fn pass_argument( + &mut self, + skip_zst: bool, + caller_arg: &mut impl Iterator>, + callee_arg: PlaceTy<'tcx>, + ) -> EvalResult<'tcx> { + if skip_zst && callee_arg.layout.is_zst() { + // Nothing to do. + trace!("Skipping callee ZST"); + return Ok(()); } - - // We need to allow what comes up when a non-capturing closure is cast to a fn(). - match (sig.abi, real_sig.abi) { - (Abi::Rust, Abi::RustCall) // check the ABIs. This makes the test here non-symmetric. - if check_ty_compat(sig.output(), real_sig.output()) - && real_sig.inputs_and_output.len() == 3 => { - // First argument of real_sig must be a ZST - let fst_ty = real_sig.inputs_and_output[0]; - if self.layout_of(fst_ty)?.is_zst() { - // Second argument must be a tuple matching the argument list of sig - let snd_ty = real_sig.inputs_and_output[1]; - match snd_ty.sty { - ty::Tuple(tys) if sig.inputs().len() == tys.len() => - if sig.inputs() - .iter() - .zip(tys) - .all(|(ty, real_ty)| check_ty_compat(ty, real_ty)) { - return Ok(true) - }, - _ => {} - } - } - } - _ => {} - }; - - // Nope, this doesn't work. - return Ok(false); + let caller_arg = caller_arg.next() + .ok_or_else(|| EvalErrorKind::FunctionArgCountMismatch)?; + if skip_zst { + debug_assert!(!caller_arg.layout.is_zst(), "ZSTs must have been already filtered out"); + } + // Now, check + if !Self::check_argument_compat(caller_arg.layout, callee_arg.layout) { + return err!(FunctionArgMismatch(caller_arg.layout.ty, callee_arg.layout.ty)); + } + self.copy_op(caller_arg, callee_arg) } /// Call this function -- pushing the stack frame and initializing the arguments. - /// `sig` is optional in case of FnPtr/FnDef -- but mandatory for closures! fn eval_fn_call( &mut self, instance: ty::Instance<'tcx>, + span: Span, + caller_abi: Abi, args: &[OpTy<'tcx>], dest: Option>, ret: Option, - span: Span, - sig: Option>, ) -> EvalResult<'tcx> { trace!("eval_fn_call: {:#?}", instance); match instance.def { ty::InstanceDef::Intrinsic(..) => { + if caller_abi != Abi::RustIntrinsic { + return err!(FunctionAbiMismatch(caller_abi, Abi::RustIntrinsic)); + } // The intrinsic itself cannot diverge, so if we got here without a return // place... (can happen e.g. for transmute returning `!`) let dest = match dest { @@ -305,6 +260,26 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { ty::InstanceDef::DropGlue(..) | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { + // ABI check + { + let callee_abi = { + let instance_ty = instance.ty(*self.tcx); + match instance_ty.sty { + ty::FnDef(..) => + instance_ty.fn_sig(*self.tcx).abi(), + ty::Closure(..) => Abi::RustCall, + ty::Generator(..) => Abi::Rust, + _ => bug!("unexpected callee ty: {:?}", instance_ty), + } + }; + // Rust and RustCall are compatible + let normalize_abi = |abi| if abi == Abi::RustCall { Abi::Rust } else { abi }; + if normalize_abi(caller_abi) != normalize_abi(callee_abi) { + return err!(FunctionAbiMismatch(caller_abi, callee_abi)); + } + } + + // We need MIR for this fn let mir = match M::find_fn(self, instance, args, dest, ret)? { Some(mir) => mir, None => return Ok(()), @@ -322,89 +297,91 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { StackPopCleanup::Goto(ret), )?; - // If we didn't get a signture, ask `fn_sig` - let sig = sig.unwrap_or_else(|| { - let fn_sig = instance.ty(*self.tcx).fn_sig(*self.tcx); - self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig) - }); - assert_eq!(sig.inputs().len(), args.len()); - // We can't test the types, as it is fine if the types are ABI-compatible but - // not equal. - - // Figure out how to pass which arguments. - // FIXME: Somehow this is horribly full of special cases here, and codegen has - // none of that. What is going on? - trace!( - "ABI: {:?}, args: {:#?}", - sig.abi, - args.iter() - .map(|arg| (arg.layout.ty, format!("{:?}", **arg))) - .collect::>() - ); - trace!( - "spread_arg: {:?}, locals: {:#?}", - mir.spread_arg, - mir.args_iter() - .map(|local| - (local, self.layout_of_local(self.cur_frame(), local).unwrap().ty) - ) - .collect::>() - ); - - // We have two iterators: Where the arguments come from, - // and where they go to. - - // For where they come from: If the ABI is RustCall, we untuple the - // last incoming argument. These do not have the same type, - // so to keep the code paths uniform we accept an allocation - // (for RustCall ABI only). - let args_effective : Cow<[OpTy<'tcx>]> = - if sig.abi == Abi::RustCall && !args.is_empty() { - // Untuple - let (&untuple_arg, args) = args.split_last().unwrap(); - trace!("eval_fn_call: Will pass last argument by untupling"); - Cow::from(args.iter().map(|&a| Ok(a)) - .chain((0..untuple_arg.layout.fields.count()).into_iter() - .map(|i| self.operand_field(untuple_arg, i as u64)) + // We want to pop this frame again in case there was an error, to put + // the blame in the right location. Until the 2018 edition is used in + // the compiler, we have to do this with an immediately invoked function. + let res = (||{ + trace!( + "caller ABI: {:?}, args: {:#?}", + caller_abi, + args.iter() + .map(|arg| (arg.layout.ty, format!("{:?}", **arg))) + .collect::>() + ); + trace!( + "spread_arg: {:?}, locals: {:#?}", + mir.spread_arg, + mir.args_iter() + .map(|local| + (local, self.layout_of_local(self.cur_frame(), local).unwrap().ty) ) - .collect::>>>()?) - } else { - // Plain arg passing - Cow::from(args) + .collect::>() + ); + + // Figure out how to pass which arguments. + // We have two iterators: Where the arguments come from, + // and where they go to. + let skip_zst = match caller_abi { + Abi::Rust | Abi::RustCall => true, + _ => false }; - // Now we have to spread them out across the callee's locals, - // taking into account the `spread_arg`. - let mut args_iter = args_effective.iter(); - let mut local_iter = mir.args_iter(); - // HACK: ClosureOnceShim calls something that expects a ZST as - // first argument, but the callers do not actually pass that ZST. - // Codegen doesn't care because ZST arguments do not even exist there. - match instance.def { - ty::InstanceDef::ClosureOnceShim { .. } if sig.abi == Abi::Rust => { - let local = local_iter.next().unwrap(); + // For where they come from: If the ABI is RustCall, we untuple the + // last incoming argument. These two iterators do not have the same type, + // so to keep the code paths uniform we accept an allocation + // (for RustCall ABI only). + let caller_args : Cow<[OpTy<'tcx>]> = + if caller_abi == Abi::RustCall && !args.is_empty() { + // Untuple + let (&untuple_arg, args) = args.split_last().unwrap(); + trace!("eval_fn_call: Will pass last argument by untupling"); + Cow::from(args.iter().map(|&a| Ok(a)) + .chain((0..untuple_arg.layout.fields.count()).into_iter() + .map(|i| self.operand_field(untuple_arg, i as u64)) + ) + .collect::>>>()?) + } else { + // Plain arg passing + Cow::from(args) + }; + // Skip ZSTs + let mut caller_iter = caller_args.iter() + .filter(|op| !skip_zst || !op.layout.is_zst()) + .map(|op| *op); + + // Now we have to spread them out across the callee's locals, + // taking into account the `spread_arg`. If we could write + // this is a single iterator (that handles `spread_arg`), then + // `pass_argument` would be the loop body. It takes care to + // not advance `caller_iter` for ZSTs. + let mut locals_iter = mir.args_iter(); + while let Some(local) = locals_iter.next() { let dest = self.eval_place(&mir::Place::Local(local))?; - assert!(dest.layout.is_zst()); - } - _ => {} - } - // Now back to norml argument passing. - while let Some(local) = local_iter.next() { - let dest = self.eval_place(&mir::Place::Local(local))?; - if Some(local) == mir.spread_arg { - // Must be a tuple - for i in 0..dest.layout.fields.count() { - let dest = self.place_field(dest, i as u64)?; - self.copy_op(*args_iter.next().unwrap(), dest)?; + if Some(local) == mir.spread_arg { + // Must be a tuple + for i in 0..dest.layout.fields.count() { + let dest = self.place_field(dest, i as u64)?; + self.pass_argument(skip_zst, &mut caller_iter, dest)?; + } + } else { + // Normal argument + self.pass_argument(skip_zst, &mut caller_iter, dest)?; } - } else { - // Normal argument - self.copy_op(*args_iter.next().unwrap(), dest)?; } + // Now we should have no more caller args + if caller_iter.next().is_some() { + trace!("Caller has too many args over"); + return err!(FunctionArgCountMismatch); + } + Ok(()) + })(); + match res { + Err(err) => { + self.stack.pop(); + Err(err) + } + Ok(v) => Ok(v) } - // Now we should be done - assert!(args_iter.next().is_none()); - Ok(()) } // cannot use the shim here, because that will only result in infinite recursion ty::InstanceDef::Virtual(_, idx) => { @@ -428,7 +405,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { args[0].op = Operand::Immediate(Value::Scalar(ptr.ptr.into())); // strip vtable trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function - self.eval_fn_call(instance, &args, dest, ret, span, sig) + self.eval_fn_call(instance, span, caller_abi, &args, dest, ret) } } } @@ -464,11 +441,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { self.eval_fn_call( instance, + span, + Abi::Rust, &[arg], Some(dest), Some(target), - span, - None, ) } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 1715930dbb61c..8d4e67b96e5b7 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -153,7 +153,9 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { | MachineError(_) // at runtime these transformations might make sense // FIXME: figure out the rules and start linting - | FunctionPointerTyMismatch(..) + | FunctionAbiMismatch(..) + | FunctionArgMismatch(..) + | FunctionArgCountMismatch // fine at runtime, might be a register address or sth | ReadBytesAsPointer // fine at runtime From 1d498d5a4311524fe89a2a0cc8056839a049b28c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Aug 2018 01:14:29 +0200 Subject: [PATCH 4/7] make ptr_op finally reponsible for all ops involving pointers; make ValTy constructor private Also remove public OpTy constructors, but a pub(crate) constructor remains --- src/librustc_mir/const_eval.rs | 18 +++++------ src/librustc_mir/interpret/cast.rs | 4 +-- src/librustc_mir/interpret/machine.rs | 10 +++--- src/librustc_mir/interpret/operand.rs | 35 +++------------------ src/librustc_mir/interpret/operator.rs | 39 ++++++++++++++++-------- src/librustc_mir/interpret/terminator.rs | 6 ++-- src/librustc_mir/transform/const_prop.rs | 28 ++++++++++------- 7 files changed, 63 insertions(+), 77 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 295fca839c812..70addf2c907e6 100644 --- a/src/librustc_mir/const_eval.rs +++ b/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>( diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 9bbaa0e7ef699..83264acf76a31 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -48,13 +48,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Misc => { let src = self.read_value(src)?; if self.type_is_fat_ptr(src_layout.ty) { - match (src.value, self.type_is_fat_ptr(dest.layout.ty)) { + match (*src, self.type_is_fat_ptr(dest.layout.ty)) { // pointers to extern types (Value::Scalar(_),_) | // slices and trait objects to other slices/trait objects (Value::ScalarPair(..), true) => { // No change to value - self.write_value(src.value, dest)?; + self.write_value(*src, dest)?; } // slices and trait objects to thin pointers (dropping the metadata) (Value::ScalarPair(data, _), false) => { diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index a8fae2b4871ed..57af63d63d9c4 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -69,20 +69,18 @@ pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash { def_id: DefId, ) -> EvalResult<'tcx, &'tcx Allocation>; - /// Called for all binary operations except on float types. - /// - /// Returns `None` if the operation should be handled by the integer - /// op code in order to share more code between machines + /// Called for all binary operations on integer(-like) types when one operand is a pointer + /// value, and for the `Offset` operation that is inherently about pointers. /// /// Returns a (value, overflowed) pair if the operation succeeded - fn try_ptr_op<'a>( + fn ptr_op<'a>( ecx: &EvalContext<'a, 'mir, 'tcx, Self>, bin_op: mir::BinOp, left: Scalar, left_layout: TyLayout<'tcx>, right: Scalar, right_layout: TyLayout<'tcx>, - ) -> EvalResult<'tcx, Option<(Scalar, bool)>>; + ) -> EvalResult<'tcx, (Scalar, bool)>; /// Heap allocations via the `box` keyword /// diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index b2a668b961f16..6f9e0cf3e37b6 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -15,7 +15,7 @@ use std::hash::{Hash, Hasher}; use std::convert::TryInto; use rustc::{mir, ty}; -use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, IntegerExt}; +use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt}; use rustc_data_structures::indexed_vec::Idx; use rustc::mir::interpret::{ @@ -85,7 +85,7 @@ impl<'tcx> Value { // as input for binary and cast operations. #[derive(Copy, Clone, Debug)] pub struct ValTy<'tcx> { - pub value: Value, + value: Value, pub layout: TyLayout<'tcx>, } @@ -107,16 +107,6 @@ pub enum Operand { } impl Operand { - #[inline] - pub fn from_ptr(ptr: Pointer, align: Align) -> Self { - Operand::Indirect(MemPlace::from_ptr(ptr, align)) - } - - #[inline] - pub fn from_scalar_value(val: Scalar) -> Self { - Operand::Immediate(Value::Scalar(val.into())) - } - #[inline] pub fn to_mem_place(self) -> MemPlace { match self { @@ -138,7 +128,7 @@ impl Operand { #[derive(Copy, Clone, Debug)] pub struct OpTy<'tcx> { - crate op: Operand, // ideally we'd make this private, but we are not there yet + crate op: Operand, // ideally we'd make this private, but const_prop needs this pub layout: TyLayout<'tcx>, } @@ -184,23 +174,6 @@ impl<'tcx> PartialEq for OpTy<'tcx> { } impl<'tcx> Eq for OpTy<'tcx> {} -impl<'tcx> OpTy<'tcx> { - #[inline] - pub fn from_ptr(ptr: Pointer, align: Align, layout: TyLayout<'tcx>) -> Self { - OpTy { op: Operand::from_ptr(ptr, align), layout } - } - - #[inline] - pub fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { - OpTy { op: Operand::from_ptr(ptr, layout.align), layout } - } - - #[inline] - pub fn from_scalar_value(val: Scalar, layout: TyLayout<'tcx>) -> Self { - OpTy { op: Operand::Immediate(Value::Scalar(val.into())), layout } - } -} - // Use the existing layout if given (but sanity check in debug mode), // or compute the layout. #[inline(always)] @@ -507,7 +480,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { ConstValue::ByRef(id, alloc, offset) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. - Ok(Operand::from_ptr(Pointer::new(id, offset), alloc.align)) + Ok(Operand::Indirect(MemPlace::from_ptr(Pointer::new(id, offset), alloc.align))) }, ConstValue::ScalarPair(a, b) => Ok(Operand::Immediate(Value::ScalarPair(a.into(), b))), diff --git a/src/librustc_mir/interpret/operator.rs b/src/librustc_mir/interpret/operator.rs index 4f4f00e320a96..d07d37d43b13e 100644 --- a/src/librustc_mir/interpret/operator.rs +++ b/src/librustc_mir/interpret/operator.rs @@ -28,7 +28,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { right: ValTy<'tcx>, dest: PlaceTy<'tcx>, ) -> EvalResult<'tcx> { - let (val, overflowed) = self.binary_op(op, left, right)?; + let (val, overflowed) = self.binary_op_val(op, left, right)?; let val = Value::ScalarPair(val.into(), Scalar::from_bool(overflowed).into()); self.write_value(val, dest) } @@ -42,7 +42,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { right: ValTy<'tcx>, dest: PlaceTy<'tcx>, ) -> EvalResult<'tcx> { - let (val, _overflowed) = self.binary_op(op, left, right)?; + let (val, _overflowed) = self.binary_op_val(op, left, right)?; self.write_scalar(val, dest) } } @@ -282,16 +282,31 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { Ok((val, false)) } + /// Convenience wrapper that's useful when keeping the layout together with the + /// value. + #[inline] + pub fn binary_op_val( + &self, + bin_op: mir::BinOp, + left: ValTy<'tcx>, + right: ValTy<'tcx>, + ) -> EvalResult<'tcx, (Scalar, bool)> { + self.binary_op( + bin_op, + left.to_scalar()?, left.layout, + right.to_scalar()?, right.layout, + ) + } + /// Returns the result of the specified operation and whether it overflowed. pub fn binary_op( &self, bin_op: mir::BinOp, - ValTy { value: left, layout: left_layout }: ValTy<'tcx>, - ValTy { value: right, layout: right_layout }: ValTy<'tcx>, + left: Scalar, + left_layout: TyLayout<'tcx>, + right: Scalar, + right_layout: TyLayout<'tcx>, ) -> EvalResult<'tcx, (Scalar, bool)> { - let left = left.to_scalar()?; - let right = right.to_scalar()?; - trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})", bin_op, left, left_layout.ty, right, right_layout.ty); @@ -322,15 +337,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { right_layout.ty.is_fn()); // Handle operations that support pointer values - if let Some(handled) = - M::try_ptr_op(self, bin_op, left, left_layout, right, right_layout)? - { - return Ok(handled); + if left.is_ptr() || right.is_ptr() || bin_op == mir::BinOp::Offset { + return M::ptr_op(self, bin_op, left, left_layout, right, right_layout); } // Everything else only works with "proper" bits - let left = left.to_bits(left_layout.size)?; - let right = right.to_bits(right_layout.size)?; + let left = left.to_bits(left_layout.size).expect("we checked is_ptr"); + let right = right.to_bits(right_layout.size).expect("we checked is_ptr"); self.binary_int_op(bin_op, left, left_layout, right, right_layout) } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 08ee30e5948a6..de86810627409 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi; use rustc::mir::interpret::{EvalResult, PointerArithmetic, EvalErrorKind, Scalar}; use super::{ - EvalContext, Machine, Value, OpTy, Place, PlaceTy, ValTy, Operand, StackPopCleanup + EvalContext, Machine, Value, OpTy, Place, PlaceTy, Operand, StackPopCleanup }; impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { @@ -61,8 +61,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { // Compare using binary_op, to also support pointer values let const_int = Scalar::from_uint(const_int, discr.layout.size); let (res, _) = self.binary_op(mir::BinOp::Eq, - discr, - ValTy { value: Value::Scalar(const_int.into()), layout: discr.layout } + discr.to_scalar()?, discr.layout, + const_int, discr.layout, )?; if res.to_bool()? { target_block = targets[index]; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8d4e67b96e5b7..e2b1a255eaca2 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -22,7 +22,7 @@ use rustc::mir::interpret::{ }; use rustc::ty::{TyCtxt, self, Instance}; use interpret::{EvalContext, CompileTimeEvaluator, eval_promoted, mk_borrowck_eval_cx}; -use interpret::{Value, OpTy, MemoryKind}; +use interpret::{self, Value, OpTy, MemoryKind}; use transform::{MirPass, MirSource}; use syntax::source_map::{Span, DUMMY_SP}; use rustc::ty::subst::Substs; @@ -358,13 +358,15 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { Rvalue::Len(_) => None, Rvalue::NullaryOp(NullOp::SizeOf, ty) => { type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some(( - OpTy::from_scalar_value( - Scalar::Bits { - bits: n as u128, - size: self.tcx.data_layout.pointer_size.bytes() as u8, - }, - self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?, - ), + OpTy { + op: interpret::Operand::Immediate(Value::Scalar( + Scalar::Bits { + bits: n as u128, + size: self.tcx.data_layout.pointer_size.bytes() as u8, + }.into() + )), + layout: self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?, + }, span, ))) } @@ -399,7 +401,11 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { // Now run the actual operation. this.ecx.unary_op(op, prim, arg.layout) })?; - Some((OpTy::from_scalar_value(val, place_layout), span)) + let res = OpTy { + op: interpret::Operand::Immediate(Value::Scalar(val.into())), + layout: place_layout, + }; + Some((res, span)) } Rvalue::CheckedBinaryOp(op, ref left, ref right) | Rvalue::BinaryOp(op, ref left, ref right) => { @@ -454,7 +460,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { })?; trace!("const evaluating {:?} for {:?} and {:?}", op, left, right); let (val, overflow) = self.use_ecx(source_info, |this| { - this.ecx.binary_op(op, l, r) + this.ecx.binary_op_val(op, l, r) })?; let val = if let Rvalue::CheckedBinaryOp(..) = *rvalue { Value::ScalarPair( @@ -470,7 +476,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { Value::Scalar(val.into()) }; let res = OpTy { - op: ::interpret::Operand::Immediate(val), + op: interpret::Operand::Immediate(val), layout: place_layout, }; Some((res, span)) From 365b90c6373336365e5464e22862ede0831117ae Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 Aug 2018 17:49:24 +0200 Subject: [PATCH 5/7] refactor memory access methods a bit --- src/librustc_mir/interpret/memory.rs | 87 +++++++++++++++------------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index b08e8c230bd34..7d2310244ce5f 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -287,7 +287,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'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. + /// 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(&self, ptr: Pointer, access: bool) -> EvalResult<'tcx> { let alloc = self.get(ptr.alloc_id)?; let allocation_size = alloc.bytes.len() as u64; @@ -498,21 +502,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// Byte accessors impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { - /// This checks alignment! - fn get_bytes_unchecked( + /// The last argument controls whether we error out when there are undefined + /// or pointer bytes. You should never call this, call `get_bytes` or + /// `get_bytes_unchecked` instead, + fn get_bytes_internal( &self, ptr: Pointer, size: Size, align: Align, + check_defined_and_ptr: bool, ) -> EvalResult<'tcx, &[u8]> { - // Zero-sized accesses can use dangling pointers, - // but they still have to be aligned and non-NULL + assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); self.check_align(ptr.into(), align)?; - if size.bytes() == 0 { - return Ok(&[]); - } // 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.offset(size, &*self)?, true)?; + + if check_defined_and_ptr { + self.check_defined(ptr, size)?; + if self.relocations(ptr, size)?.len() != 0 { + return err!(ReadPointerAsBytes); + } + } + let alloc = self.get(ptr.alloc_id)?; assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); assert_eq!(size.bytes() as usize as u64, size.bytes()); @@ -520,48 +531,42 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(&alloc.bytes[offset..offset + size.bytes() as usize]) } - /// This checks alignment! - fn get_bytes_unchecked_mut( + #[inline] + fn get_bytes(&self, ptr: Pointer, size: Size, align: Align) -> EvalResult<'tcx, &[u8]> { + self.get_bytes_internal(ptr, size, align, true) + } + + /// It is the caller's responsibility to handle undefined and pointer bytes. + #[inline] + fn get_bytes_with_undef_and_ptr( + &self, + ptr: Pointer, + size: Size, + align: Align + ) -> EvalResult<'tcx, &[u8]> { + self.get_bytes_internal(ptr, size, align, false) + } + + fn get_bytes_mut( &mut self, ptr: Pointer, size: Size, align: Align, ) -> EvalResult<'tcx, &mut [u8]> { - // Zero-sized accesses can use dangling pointers, - // but they still have to be aligned and non-NULL + assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); self.check_align(ptr.into(), align)?; - if size.bytes() == 0 { - return Ok(&mut []); - } // 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.offset(size, &self)?, true)?; + + self.mark_definedness(ptr, size, true)?; + self.clear_relocations(ptr, size)?; + let alloc = self.get_mut(ptr.alloc_id)?; assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); assert_eq!(size.bytes() as usize as u64, size.bytes()); let offset = ptr.offset.bytes() as usize; Ok(&mut alloc.bytes[offset..offset + size.bytes() as usize]) } - - fn get_bytes(&self, ptr: Pointer, size: Size, align: Align) -> EvalResult<'tcx, &[u8]> { - assert_ne!(size.bytes(), 0); - if self.relocations(ptr, size)?.len() != 0 { - return err!(ReadPointerAsBytes); - } - self.check_defined(ptr, size)?; - self.get_bytes_unchecked(ptr, size, align) - } - - fn get_bytes_mut( - &mut self, - ptr: Pointer, - size: Size, - align: Align, - ) -> EvalResult<'tcx, &mut [u8]> { - assert_ne!(size.bytes(), 0); - self.clear_relocations(ptr, size)?; - self.mark_definedness(ptr, size, true)?; - self.get_bytes_unchecked_mut(ptr, size, align) - } } /// Reading and writing @@ -672,7 +677,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { }; // This also checks alignment. - let src_bytes = self.get_bytes_unchecked(src, size, src_align)?.as_ptr(); + let src_bytes = self.get_bytes_with_undef_and_ptr(src, size, src_align)?.as_ptr(); let dest_bytes = self.get_bytes_mut(dest, size * length, dest_align)?.as_mut_ptr(); // SAFE: The above indexing would have panicked if there weren't at least `size` bytes @@ -775,7 +780,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // Make sure we don't read part of a pointer as a pointer self.check_relocation_edges(ptr, size)?; // get_bytes_unchecked tests alignment - let bytes = self.get_bytes_unchecked(ptr, size, ptr_align.min(self.int_align(size)))?; + let bytes = self.get_bytes_with_undef_and_ptr( + ptr, size, ptr_align.min(self.int_align(size)) + )?; // Undef check happens *after* we established that the alignment is correct. // We must not return Ok() for unaligned pointers! if !self.is_defined(ptr, size)? { From b06a8db26e660505601b764e5d702fc17d7d73ee Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 Aug 2018 10:07:27 +0200 Subject: [PATCH 6/7] audit the relocations code, and clean it up a little --- src/librustc/mir/interpret/mod.rs | 4 +- src/librustc_mir/interpret/memory.rs | 64 ++++++++++++++++++---------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index da5216bd1befe..d40dbae09d2cb 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -496,7 +496,9 @@ pub struct Allocation { /// Note that the bytes of a pointer represent the offset of the pointer pub bytes: Vec, /// 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, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 7d2310244ce5f..18e96bf2040ba 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -504,7 +504,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// The last argument controls whether we error out when there are undefined /// or pointer bytes. You should never call this, call `get_bytes` or - /// `get_bytes_unchecked` instead, + /// `get_bytes_with_undef_and_ptr` instead, fn get_bytes_internal( &self, ptr: Pointer, @@ -519,9 +519,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { if check_defined_and_ptr { self.check_defined(ptr, size)?; - if self.relocations(ptr, size)?.len() != 0 { - return err!(ReadPointerAsBytes); - } + self.check_relocations(ptr, size)?; + } else { + // We still don't want relocations on the *edges* + self.check_relocation_edges(ptr, size)?; } let alloc = self.get(ptr.alloc_id)?; @@ -537,6 +538,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } /// It is the caller's responsibility to handle undefined and pointer bytes. + /// However, this still checks that there are no relocations on the egdes. #[inline] fn get_bytes_with_undef_and_ptr( &self, @@ -547,6 +549,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { self.get_bytes_internal(ptr, size, align, false) } + /// Just calling this already marks everything as defined and removes relocations, + /// so be sure to actually put data there! fn get_bytes_mut( &mut self, ptr: Pointer, @@ -654,11 +658,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } let src = src.to_ptr()?; let dest = dest.to_ptr()?; - self.check_relocation_edges(src, size)?; // first copy the relocations to a temporary buffer, because // `get_bytes_mut` will clear the relocations, which is correct, // since we don't want to keep any relocations at the target. + // (`get_bytes_with_undef_and_ptr` below checks that there are no + // relocations overlapping the edges; those would not be handled correctly). let relocations = { let relocations = self.relocations(src, size)?; let mut new_relocations = Vec::with_capacity(relocations.len() * (length as usize)); @@ -676,7 +681,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { new_relocations }; - // This also checks alignment. + // This also checks alignment, and relocation edges on the src. let src_bytes = self.get_bytes_with_undef_and_ptr(src, size, src_align)?.as_ptr(); let dest_bytes = self.get_bytes_mut(dest, size * length, dest_align)?.as_mut_ptr(); @@ -710,8 +715,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } + // copy definedness to the destination self.copy_undef_mask(src, dest, size, length)?; - // copy back the relocations + // copy the relocations to the destination self.get_mut(dest.alloc_id)?.relocations.insert_presorted(relocations); Ok(()) @@ -724,9 +730,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { match alloc.bytes[offset..].iter().position(|&c| c == 0) { Some(size) => { let p1 = Size::from_bytes((size + 1) as u64); - if self.relocations(ptr, p1)?.len() != 0 { - return err!(ReadPointerAsBytes); - } + self.check_relocations(ptr, p1)?; self.check_defined(ptr, p1)?; Ok(&alloc.bytes[offset..offset + size]) } @@ -777,9 +781,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ptr_align: Align, size: Size ) -> EvalResult<'tcx, ScalarMaybeUndef> { - // Make sure we don't read part of a pointer as a pointer - self.check_relocation_edges(ptr, size)?; - // get_bytes_unchecked tests alignment + // get_bytes_unchecked tests alignment and relocation edges let bytes = self.get_bytes_with_undef_and_ptr( ptr, size, ptr_align.min(self.int_align(size)) )?; @@ -794,9 +796,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { let bits = read_target_uint(self.tcx.data_layout.endian, bytes).unwrap(); // See if we got a pointer if size != self.pointer_size() { - if self.relocations(ptr, size)?.len() != 0 { - return err!(ReadPointerAsBytes); - } + // *Now* better make sure that the inside also is free of relocations. + self.check_relocations(ptr, size)?; } else { let alloc = self.get(ptr.alloc_id)?; match alloc.relocations.get(&ptr.offset) { @@ -887,16 +888,35 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// Relocations impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { + /// Return all relocations overlapping with the given ptr-offset pair. fn relocations( &self, ptr: Pointer, size: Size, ) -> EvalResult<'tcx, &[(Size, AllocId)]> { + // We have to go back `pointer_size - 1` bytes, as that one would still overlap with + // the beginning of this range. let start = ptr.offset.bytes().saturating_sub(self.pointer_size().bytes() - 1); - let end = ptr.offset + size; + let end = ptr.offset + size; // this does overflow checking Ok(self.get(ptr.alloc_id)?.relocations.range(Size::from_bytes(start)..end)) } + /// Check that there ar eno relocations overlapping with the given range. + #[inline(always)] + fn check_relocations(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + if self.relocations(ptr, size)?.len() != 0 { + err!(ReadPointerAsBytes) + } else { + Ok(()) + } + } + + /// Remove all relocations inside the given range. + /// If there are relocations overlapping with the edges, they + /// are removed as well *and* the bytes they cover are marked as + /// uninitialized. This is a somewhat odd "spooky action at a distance", + /// but it allows strictly more code to run than if we would just error + /// immediately in that case. fn clear_relocations(&mut self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { // Find the start and end of the given range and its outermost relocations. let (first, last) = { @@ -929,12 +949,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(()) } + /// Error if there are relocations overlapping with the egdes of the + /// given memory range. + #[inline] fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { - let overlapping_start = self.relocations(ptr, Size::ZERO)?.len(); - let overlapping_end = self.relocations(ptr.offset(size, self)?, Size::ZERO)?.len(); - if overlapping_start + overlapping_end != 0 { - return err!(ReadPointerAsBytes); - } + self.check_relocations(ptr, Size::ZERO)?; + self.check_relocations(ptr.offset(size, self)?, Size::ZERO)?; Ok(()) } } From 97d693a19a001a38f18cfd49a89ddfb8b7d3fa94 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 30 Aug 2018 11:39:40 +0200 Subject: [PATCH 7/7] assert sanity in memory --- src/librustc_mir/interpret/memory.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 18e96bf2040ba..91fc6453446a8 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -258,13 +258,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } Scalar::Bits { bits, size } => { assert_eq!(size as u64, self.pointer_size().bytes()); - // FIXME: what on earth does this line do? docs or fix needed! - let v = ((bits as u128) % (1 << self.pointer_size().bytes())) as u64; - if v == 0 { + assert!(bits < (1u128 << self.pointer_size().bits())); + if bits == 0 { return err!(InvalidNullPointerUsage); } - // the base address if the "integer allocation" is 0 and hence always aligned - (v, required_align) + // the "base address" is 0 and hence always aligned + (bits as u64, required_align) } }; // Check alignment