From aa760a522588a15b7167f51296df8e1f87bbae50 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 16 Aug 2018 11:38:16 +0200 Subject: [PATCH] finally remove all traces of signs from memory --- src/librustc/mir/interpret/mod.rs | 32 ++++++---- src/librustc/mir/interpret/value.rs | 2 +- src/librustc_mir/hair/pattern/mod.rs | 8 +-- src/librustc_mir/interpret/cast.rs | 71 ++++++++++++++-------- src/librustc_mir/interpret/eval_context.rs | 24 +++----- src/librustc_mir/interpret/memory.rs | 18 +++--- src/librustc_mir/interpret/mod.rs | 3 +- src/librustc_mir/interpret/place.rs | 15 ++--- src/librustc_mir/interpret/traits.rs | 8 +-- 9 files changed, 93 insertions(+), 88 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 50b8c271233e..ca664c6e18b4 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -567,18 +567,6 @@ pub fn write_target_uint( } } -pub fn write_target_int( - endianness: layout::Endian, - mut target: &mut [u8], - data: i128, -) -> Result<(), io::Error> { - let len = target.len(); - match endianness { - layout::Endian::Little => target.write_int128::(data, len), - layout::Endian::Big => target.write_int128::(data, len), - } -} - pub fn read_target_uint(endianness: layout::Endian, mut source: &[u8]) -> Result { match endianness { layout::Endian::Little => source.read_uint128::(source.len()), @@ -586,6 +574,26 @@ pub fn read_target_uint(endianness: layout::Endian, mut source: &[u8]) -> Result } } +//////////////////////////////////////////////////////////////////////////////// +// Methods to faciliate working with signed integers stored in a u128 +//////////////////////////////////////////////////////////////////////////////// + +pub fn sign_extend(value: u128, size: Size) -> u128 { + let size = size.bits(); + // sign extend + let shift = 128 - size; + // shift the unsigned value to the left + // and back to the right as signed (essentially fills with FF on the left) + (((value << shift) as i128) >> shift) as u128 +} + +pub fn truncate(value: u128, size: Size) -> u128 { + let size = size.bits(); + let shift = 128 - size; + // truncate (shift left to drop out leftover values, shift right to fill with zeroes) + (value << shift) >> shift +} + //////////////////////////////////////////////////////////////////////////////// // Undefined byte tracking //////////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 3f8130ec04ca..e661da8792a9 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -191,7 +191,7 @@ pub enum Scalar { /// The raw bytes of a simple value. Bits { /// The first `size` bytes are the value. - /// Do not try to read less or more bytes that that + /// Do not try to read less or more bytes that that. The remaining bytes must be 0. size: u8, bits: u128, }, diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 0bb0faf08484..16d6a08981ab 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -16,10 +16,10 @@ mod check_match; pub use self::check_match::check_crate; pub(crate) use self::check_match::check_match; -use interpret::{const_field, const_variant_index, self}; +use interpret::{const_field, const_variant_index}; use rustc::mir::{fmt_const_val, Field, BorrowKind, Mutability}; -use rustc::mir::interpret::{Scalar, GlobalId, ConstValue}; +use rustc::mir::interpret::{Scalar, GlobalId, ConstValue, sign_extend}; use rustc::ty::{self, TyCtxt, AdtDef, Ty, Region}; use rustc::ty::subst::{Substs, Kind}; use rustc::hir::{self, PatKind, RangeEnd}; @@ -1086,8 +1086,8 @@ pub fn compare_const_vals<'a, 'tcx>( ty::TyInt(_) => { let layout = tcx.layout_of(ty).ok()?; assert!(layout.abi.is_signed()); - let a = interpret::sign_extend(a, layout.size); - let b = interpret::sign_extend(b, layout.size); + let a = sign_extend(a, layout.size); + let b = sign_extend(b, layout.size); Some((a as i128).cmp(&(b as i128))) }, _ => Some(a.cmp(&b)), diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 036b84ee1fb7..b4d36afa0f80 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -1,9 +1,12 @@ use rustc::ty::{self, Ty, TypeAndMut}; -use rustc::ty::layout::{self, TyLayout}; +use rustc::ty::layout::{self, TyLayout, Size}; use syntax::ast::{FloatTy, IntTy, UintTy}; use rustc_apfloat::ieee::{Single, Double}; -use rustc::mir::interpret::{Scalar, EvalResult, Pointer, PointerArithmetic, EvalErrorKind}; +use rustc::mir::interpret::{ + Scalar, EvalResult, Pointer, PointerArithmetic, EvalErrorKind, + truncate, sign_extend +}; use rustc::mir::CastKind; use rustc_apfloat::Float; @@ -144,11 +147,26 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { match val { Scalar::Ptr(ptr) => self.cast_from_ptr(ptr, dest_layout.ty), Scalar::Bits { bits, size } => { - assert_eq!(size as u64, src_layout.size.bytes()); - match src_layout.ty.sty { - TyFloat(fty) => self.cast_from_float(bits, fty, dest_layout.ty), - _ => self.cast_from_int(bits, src_layout, dest_layout), + debug_assert_eq!(size as u64, src_layout.size.bytes()); + debug_assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, + "Unexpected value of size {} before casting", size); + + let res = match src_layout.ty.sty { + TyFloat(fty) => self.cast_from_float(bits, fty, dest_layout.ty)?, + _ => self.cast_from_int(bits, src_layout, dest_layout)?, + }; + + // Sanity check + match res { + Scalar::Ptr(_) => bug!("Fabricated a ptr value from an int...?"), + Scalar::Bits { bits, size } => { + debug_assert_eq!(size as u64, dest_layout.size.bytes()); + debug_assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, + "Unexpected value of size {} after casting", size); + } } + // Done + Ok(res) } } } @@ -218,30 +236,31 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { // float -> uint TyUint(t) => { let width = t.bit_width().unwrap_or(self.memory.pointer_size().bits() as usize); - match fty { - FloatTy::F32 => Ok(Scalar::Bits { - bits: Single::from_bits(bits).to_u128(width).value, - size: (width / 8) as u8, - }), - FloatTy::F64 => Ok(Scalar::Bits { - bits: Double::from_bits(bits).to_u128(width).value, - size: (width / 8) as u8, - }), - } + 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, + }) }, // float -> int TyInt(t) => { let width = t.bit_width().unwrap_or(self.memory.pointer_size().bits() as usize); - match fty { - FloatTy::F32 => Ok(Scalar::Bits { - bits: Single::from_bits(bits).to_i128(width).value as u128, - size: (width / 8) as u8, - }), - FloatTy::F64 => Ok(Scalar::Bits { - bits: Double::from_bits(bits).to_i128(width).value as u128, - size: (width / 8) as u8, - }), - } + 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, + }) }, // f64 -> f32 TyFloat(FloatTy::F32) if fty == FloatTy::F64 => { diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 2b97860187ab..539428b9e517 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -18,6 +18,7 @@ use rustc::mir::interpret::{ GlobalId, Scalar, FrameInfo, AllocType, EvalResult, EvalErrorKind, ScalarMaybeUndef, + truncate, sign_extend, }; use syntax::source_map::{self, Span}; @@ -906,10 +907,12 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M } } + #[inline(always)] pub fn frame(&self) -> &Frame<'mir, 'tcx> { self.stack.last().expect("no call frames exist") } + #[inline(always)] pub fn frame_mut(&mut self) -> &mut Frame<'mir, 'tcx> { self.stack.last_mut().expect("no call frames exist") } @@ -1028,13 +1031,15 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M (frames, self.tcx.span) } + #[inline(always)] pub fn sign_extend(&self, value: u128, ty: TyLayout<'_>) -> u128 { assert!(ty.abi.is_signed()); - super::sign_extend(value, ty.size) + sign_extend(value, ty.size) } + #[inline(always)] pub fn truncate(&self, value: u128, ty: TyLayout<'_>) -> u128 { - super::truncate(value, ty.size) + truncate(value, ty.size) } fn dump_field_name(&self, s: &mut String, ty: Ty<'tcx>, i: usize, variant: usize) -> ::std::fmt::Result { @@ -1105,18 +1110,3 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M } } -pub fn sign_extend(value: u128, size: Size) -> u128 { - let size = size.bits(); - // sign extend - let shift = 128 - size; - // shift the unsigned value to the left - // and back to the right as signed (essentially fills with FF on the left) - (((value << shift) as i128) >> shift) as u128 -} - -pub fn truncate(value: u128, size: Size) -> u128 { - let size = size.bits(); - let shift = 128 - size; - // truncate (shift left to drop out leftover values, shift right to fill with zeroes) - (value << shift) >> shift -} diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 9cd82d84d7d0..72bd32efe7bf 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -8,8 +8,8 @@ use rustc::ty::ParamEnv; use rustc::ty::query::TyCtxtAt; use rustc::ty::layout::{self, Align, TargetDataLayout, Size}; use rustc::mir::interpret::{Pointer, AllocId, Allocation, AccessKind, ScalarMaybeUndef, - EvalResult, Scalar, EvalErrorKind, GlobalId, AllocType}; -pub use rustc::mir::interpret::{write_target_uint, write_target_int, read_target_uint}; + EvalResult, Scalar, EvalErrorKind, GlobalId, AllocType, truncate}; +pub use rustc::mir::interpret::{write_target_uint, read_target_uint}; use rustc_data_structures::fx::{FxHashSet, FxHashMap, FxHasher}; use syntax::ast::Mutability; @@ -791,7 +791,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { val: ScalarMaybeUndef, type_size: Size, type_align: Align, - signed: bool, ) -> EvalResult<'tcx> { let endianness = self.endianness(); self.check_align(ptr, ptr_align)?; @@ -815,6 +814,8 @@ 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, + "Unexpected value of size {} when writing to memory", size); bits }, }; @@ -823,12 +824,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { { let dst = self.get_bytes_mut(ptr, type_size, ptr_align.min(type_align))?; - // TODO: Why do we still need `signed` here? We do NOT have it for loading! - if signed { - write_target_int(endianness, dst, bytes as i128).unwrap(); - } else { - write_target_uint(endianness, dst, bytes).unwrap(); - } + write_target_uint(endianness, dst, bytes).unwrap(); } // See if we have to also write a relocation @@ -845,9 +841,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(()) } - pub fn write_ptr_sized_unsigned(&mut self, ptr: Pointer, ptr_align: Align, val: ScalarMaybeUndef) -> EvalResult<'tcx> { + pub fn write_ptr_sized(&mut self, ptr: Pointer, ptr_align: Align, val: ScalarMaybeUndef) -> EvalResult<'tcx> { let ptr_size = self.pointer_size(); - self.write_scalar(ptr.into(), ptr_align, val, ptr_size, ptr_align, false) + self.write_scalar(ptr.into(), ptr_align, val, ptr_size, ptr_align) } fn int_align(&self, size: Size) -> Align { diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 452708179009..167fb5d184cf 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -13,8 +13,7 @@ mod traits; mod const_eval; pub use self::eval_context::{ - EvalContext, Frame, StackPopCleanup, - sign_extend, truncate, LocalValue, + EvalContext, Frame, StackPopCleanup, LocalValue, }; pub use self::place::{Place, PlaceExtra, PlaceTy, MemPlace, MPlaceTy}; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index e92143c56f80..8b8c050acb7b 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -551,15 +551,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { // correct if we never look at this data with the wrong type. match value { Value::Scalar(scalar) => { - let signed = match dest.layout.abi { - layout::Abi::Scalar(ref scal) => match scal.value { - layout::Primitive::Int(_, signed) => signed, - _ => false, - }, - _ => false, - }; self.memory.write_scalar( - dest.ptr, dest.align, scalar, dest.layout.size, dest.layout.align, signed + dest.ptr, dest.align, scalar, dest.layout.size, dest.layout.align ) } Value::ScalarPair(a_val, b_val) => { @@ -572,9 +565,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let a_ptr = dest.ptr; let b_offset = a_size.abi_align(b_align); let b_ptr = a_ptr.ptr_offset(b_offset, &self)?.into(); - // TODO: What about signedess? - self.memory.write_scalar(a_ptr, dest.align, a_val, a_size, a_align, false)?; - self.memory.write_scalar(b_ptr, dest.align, b_val, b_size, b_align, false) + + self.memory.write_scalar(a_ptr, dest.align, a_val, a_size, a_align)?; + self.memory.write_scalar(b_ptr, dest.align, b_val, b_size, b_align) } } } diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 1cc8644629ee..18718cc3dcd6 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -36,15 +36,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let drop = ::monomorphize::resolve_drop_in_place(*self.tcx, ty); let drop = self.memory.create_fn_alloc(drop); - self.memory.write_ptr_sized_unsigned(vtable, ptr_align, Scalar::Ptr(drop).into())?; + 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_unsigned(size_ptr, ptr_align, Scalar::Bits { + self.memory.write_ptr_sized(size_ptr, ptr_align, Scalar::Bits { bits: size as u128, size: ptr_size.bytes() as u8, }.into())?; let align_ptr = vtable.offset(ptr_size * 2, &self)?; - self.memory.write_ptr_sized_unsigned(align_ptr, ptr_align, Scalar::Bits { + self.memory.write_ptr_sized(align_ptr, ptr_align, Scalar::Bits { bits: align as u128, size: ptr_size.bytes() as u8, }.into())?; @@ -54,7 +54,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let instance = self.resolve(def_id, substs)?; let fn_ptr = self.memory.create_fn_alloc(instance); let method_ptr = vtable.offset(ptr_size * (3 + i as u64), &self)?; - self.memory.write_ptr_sized_unsigned(method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?; + self.memory.write_ptr_sized(method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?; } }