From f036fe0d32a077a8d69f20c876389779e88e3ea5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 25 Aug 2017 16:20:13 +0200 Subject: [PATCH] refactor memory read API: provide only size-based, no type-based methods --- src/librustc_mir/interpret/cast.rs | 1 + src/librustc_mir/interpret/eval_context.rs | 143 ++++++++++++++----- src/librustc_mir/interpret/memory.rs | 61 ++++---- src/librustc_mir/interpret/terminator/mod.rs | 109 +------------- src/librustc_mir/interpret/traits.rs | 8 +- src/librustc_mir/interpret/value.rs | 22 +-- tests/run-pass-fullmir/integer-ops.rs | 4 + 7 files changed, 164 insertions(+), 184 deletions(-) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index c6016509d238..2f45347d113c 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -10,6 +10,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { src_ty: Ty<'tcx>, dest_ty: Ty<'tcx>, ) -> EvalResult<'tcx, PrimVal> { + trace!("Casting {:?}: {:?} to {:?}", val, src_ty, dest_ty); let src_kind = self.ty_to_primval_kind(src_ty)?; match val { diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 9fbc32c4f005..1841e1554056 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -998,16 +998,17 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { let ptr = self.force_allocation(lval)?.to_ptr()?; let discr_val = self.read_discriminant_value(ptr, ty)?; if let ty::TyAdt(adt_def, _) = ty.sty { + trace!("Read discriminant {}, valid discriminants {:?}", discr_val, adt_def.discriminants(self.tcx).collect::>()); if adt_def.discriminants(self.tcx).all(|v| { discr_val != v.to_u128_unchecked() }) { return err!(InvalidDiscriminant); } + self.write_primval(dest, PrimVal::Bytes(discr_val), dest_ty)?; } else { bug!("rustc only generates Rvalue::Discriminant for enums"); } - self.write_primval(dest, PrimVal::Bytes(discr_val), dest_ty)?; } } @@ -1295,6 +1296,96 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { } } + pub fn read_discriminant_value( + &self, + adt_ptr: MemoryPointer, + adt_ty: Ty<'tcx>, + ) -> EvalResult<'tcx, u128> { + use rustc::ty::layout::Layout::*; + let adt_layout = self.type_layout(adt_ty)?; + //trace!("read_discriminant_value {:#?}", adt_layout); + + let discr_val = match *adt_layout { + General { discr, .. } => { + let discr_size = discr.size().bytes(); + self.memory.read_primval(adt_ptr, discr_size, false)?.to_bytes()? + } + + CEnum { + discr, + signed, + .. + } => { + let discr_size = discr.size().bytes(); + self.memory.read_primval(adt_ptr, discr_size, signed)?.to_bytes()? + } + + RawNullablePointer { nndiscr, value } => { + let discr_size = value.size(&self.tcx.data_layout).bytes(); + trace!("rawnullablepointer with size {}", discr_size); + self.read_nonnull_discriminant_value( + adt_ptr, + nndiscr as u128, + discr_size, + )? + } + + StructWrappedNullablePointer { + nndiscr, + ref discrfield_source, + .. + } => { + let (offset, TyAndPacked { ty, packed }) = self.nonnull_offset_and_ty( + adt_ty, + nndiscr, + discrfield_source, + )?; + let nonnull = adt_ptr.offset(offset.bytes(), &*self)?; + trace!("struct wrapped nullable pointer type: {}", ty); + // only the pointer part of a fat pointer is used for this space optimization + let discr_size = self.type_size(ty)?.expect( + "bad StructWrappedNullablePointer discrfield", + ); + self.read_maybe_aligned(!packed, |ectx| { + ectx.read_nonnull_discriminant_value(nonnull, nndiscr as u128, discr_size) + })? + } + + // The discriminant_value intrinsic returns 0 for non-sum types. + Array { .. } | + FatPointer { .. } | + Scalar { .. } | + Univariant { .. } | + Vector { .. } | + UntaggedUnion { .. } => 0, + }; + + Ok(discr_val) + } + + fn read_nonnull_discriminant_value( + &self, + ptr: MemoryPointer, + nndiscr: u128, + discr_size: u64, + ) -> EvalResult<'tcx, u128> { + trace!( + "read_nonnull_discriminant_value: {:?}, {}, {}", + ptr, + nndiscr, + discr_size + ); + // We are only interested in 0 vs. non-0, the sign does not matter for this + let null = match self.memory.read_primval(ptr, discr_size, false)? { + PrimVal::Bytes(0) => true, + PrimVal::Bytes(_) | + PrimVal::Ptr(..) => false, + PrimVal::Undef => return err!(ReadUndefBytes), + }; + assert!(nndiscr == 0 || nndiscr == 1); + Ok(if !null { nndiscr } else { 1 - nndiscr }) + } + pub fn read_global_as_value(&self, gid: GlobalId) -> Value { Value::ByRef(*self.globals.get(&gid).expect("global not cached")) } @@ -1676,18 +1767,19 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { ptr: MemoryPointer, pointee_ty: Ty<'tcx>, ) -> EvalResult<'tcx, Value> { - let p = self.memory.read_ptr(ptr)?; + let ptr_size = self.memory.pointer_size(); + let p : Pointer = self.memory.read_ptr_sized_unsigned(ptr)?.into(); if self.type_is_sized(pointee_ty) { Ok(p.to_value()) } else { trace!("reading fat pointer extra of type {}", pointee_ty); - let extra = ptr.offset(self.memory.pointer_size(), self)?; + let extra = ptr.offset(ptr_size, self)?; match self.tcx.struct_tail(pointee_ty).sty { ty::TyDynamic(..) => Ok(p.to_value_with_vtable( - self.memory.read_ptr(extra)?.to_ptr()?, + self.memory.read_ptr_sized_unsigned(extra)?.to_ptr()?, )), ty::TySlice(..) | ty::TyStr => Ok( - p.to_value_with_len(self.memory.read_usize(extra)?), + p.to_value_with_len(self.memory.read_ptr_sized_unsigned(extra)?.to_bytes()? as u64), ), _ => bug!("unsized primval ptr read from {:?}", pointee_ty), } @@ -1697,10 +1789,11 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { fn try_read_value(&self, ptr: Pointer, ty: Ty<'tcx>) -> EvalResult<'tcx, Option> { use syntax::ast::FloatTy; + let ptr = ptr.to_ptr()?; let val = match ty.sty { - ty::TyBool => PrimVal::from_bool(self.memory.read_bool(ptr.to_ptr()?)?), + ty::TyBool => PrimVal::from_bool(self.memory.read_bool(ptr)?), ty::TyChar => { - let c = self.memory.read_uint(ptr.to_ptr()?, 4)? as u32; + let c = self.memory.read_primval(ptr, 4, false)?.to_bytes()? as u32; match ::std::char::from_u32(c) { Some(ch) => PrimVal::from_char(ch), None => return err!(InvalidChar(c as u128)), @@ -1717,15 +1810,7 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { I128 => 16, Is => self.memory.pointer_size(), }; - // if we transmute a ptr to an isize, reading it back into a primval shouldn't panic - // Due to read_ptr ignoring the sign, we need to jump around some hoops - match self.memory.read_int(ptr.to_ptr()?, size) { - Err(EvalError { kind: EvalErrorKind::ReadPointerAsBytes, .. }) if size == self.memory.pointer_size() => - // Reading as an int failed because we are seeing ptr bytes *and* we are actually reading at ptr size. - // Let's try again, reading a ptr this time. - self.memory.read_ptr(ptr.to_ptr()?)?.into_inner_primval(), - other => PrimVal::from_i128(other?), - } + self.memory.read_primval(ptr, size, true)? } ty::TyUint(uint_ty) => { @@ -1738,36 +1823,24 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { U128 => 16, Us => self.memory.pointer_size(), }; - // if we transmute a ptr to an usize, reading it back into a primval shouldn't panic - // for consistency's sake, we use the same code as above - match self.memory.read_uint(ptr.to_ptr()?, size) { - Err(EvalError { kind: EvalErrorKind::ReadPointerAsBytes, .. }) - if size == self.memory.pointer_size() => { - self.memory.read_ptr(ptr.to_ptr()?)?.into_inner_primval() - } - other => PrimVal::from_u128(other?), - } + self.memory.read_primval(ptr, size, false)? } - ty::TyFloat(FloatTy::F32) => PrimVal::from_f32(self.memory.read_f32(ptr.to_ptr()?)?), - ty::TyFloat(FloatTy::F64) => PrimVal::from_f64(self.memory.read_f64(ptr.to_ptr()?)?), + ty::TyFloat(FloatTy::F32) => PrimVal::from_f32(self.memory.read_f32(ptr)?), + ty::TyFloat(FloatTy::F64) => PrimVal::from_f64(self.memory.read_f64(ptr)?), - ty::TyFnPtr(_) => self.memory.read_ptr(ptr.to_ptr()?)?.into_inner_primval(), + ty::TyFnPtr(_) => self.memory.read_ptr_sized_unsigned(ptr)?, ty::TyRef(_, ref tam) | - ty::TyRawPtr(ref tam) => return self.read_ptr(ptr.to_ptr()?, tam.ty).map(Some), + ty::TyRawPtr(ref tam) => return self.read_ptr(ptr, tam.ty).map(Some), ty::TyAdt(def, _) => { if def.is_box() { - return self.read_ptr(ptr.to_ptr()?, ty.boxed_ty()).map(Some); + return self.read_ptr(ptr, ty.boxed_ty()).map(Some); } use rustc::ty::layout::Layout::*; if let CEnum { discr, signed, .. } = *self.type_layout(ty)? { let size = discr.size().bytes(); - if signed { - PrimVal::from_i128(self.memory.read_int(ptr.to_ptr()?, size)?) - } else { - PrimVal::from_u128(self.memory.read_uint(ptr.to_ptr()?, size)?) - } + self.memory.read_primval(ptr, size, signed)? } else { return Ok(None); } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 8c7a36f866da..d24f469de405 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -1171,24 +1171,39 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { Ok(()) } - pub fn read_ptr(&self, ptr: MemoryPointer) -> EvalResult<'tcx, Pointer> { - let size = self.pointer_size(); + pub fn read_primval(&self, ptr: MemoryPointer, size: u64, signed: bool) -> EvalResult<'tcx, PrimVal> { self.check_relocation_edges(ptr, size)?; // Make sure we don't read part of a pointer as a pointer let endianess = self.endianess(); - let bytes = self.get_bytes_unchecked(ptr, size, size)?; + let bytes = self.get_bytes_unchecked(ptr, size, 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.check_defined(ptr, size).is_err() { return Ok(PrimVal::Undef.into()); } - let offset = read_target_uint(endianess, bytes).unwrap(); - assert_eq!(offset as u64 as u128, offset); - let offset = offset as u64; - let alloc = self.get(ptr.alloc_id)?; - match alloc.relocations.get(&ptr.offset) { - Some(&alloc_id) => Ok(PrimVal::Ptr(MemoryPointer::new(alloc_id, offset)).into()), - None => Ok(PrimVal::Bytes(offset as u128).into()), + // Now we do the actual reading + let bytes = if signed { + read_target_int(endianess, bytes).unwrap() as u128 + } else { + read_target_uint(endianess, bytes).unwrap() + }; + // See if we got a pointer + if size != self.pointer_size() { + if self.relocations(ptr, size)?.count() != 0 { + return err!(ReadPointerAsBytes); + } + } else { + let alloc = self.get(ptr.alloc_id)?; + match alloc.relocations.get(&ptr.offset) { + Some(&alloc_id) => return Ok(PrimVal::Ptr(MemoryPointer::new(alloc_id, bytes as u64))), + None => {}, + } } + // We don't. Just return the bytes. + Ok(PrimVal::Bytes(bytes)) + } + + pub fn read_ptr_sized_unsigned(&self, ptr: MemoryPointer) -> EvalResult<'tcx, PrimVal> { + self.read_primval(ptr, self.pointer_size(), false) } pub fn write_ptr(&mut self, dest: MemoryPointer, ptr: MemoryPointer) -> EvalResult<'tcx> { @@ -1242,6 +1257,8 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } fn int_align(&self, size: u64) -> EvalResult<'tcx, u64> { + // We assume pointer-sized integers have the same alignment as pointers. + // We also assume singed and unsigned integers of the same size have the same alignment. match size { 1 => Ok(self.layout.i8_align.abi()), 2 => Ok(self.layout.i16_align.abi()), @@ -1252,13 +1269,6 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { } } - pub fn read_int(&self, ptr: MemoryPointer, size: u64) -> EvalResult<'tcx, i128> { - let align = self.int_align(size)?; - self.get_bytes(ptr, size, align).map(|b| { - read_target_int(self.endianess(), b).unwrap() - }) - } - pub fn write_int(&mut self, ptr: MemoryPointer, n: i128, size: u64) -> EvalResult<'tcx> { let align = self.int_align(size)?; let endianess = self.endianess(); @@ -1267,13 +1277,6 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { Ok(()) } - pub fn read_uint(&self, ptr: MemoryPointer, size: u64) -> EvalResult<'tcx, u128> { - let align = self.int_align(size)?; - self.get_bytes(ptr, size, align).map(|b| { - read_target_uint(self.endianess(), b).unwrap() - }) - } - pub fn write_uint(&mut self, ptr: MemoryPointer, n: u128, size: u64) -> EvalResult<'tcx> { let align = self.int_align(size)?; let endianess = self.endianess(); @@ -1282,19 +1285,11 @@ impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { Ok(()) } - pub fn read_isize(&self, ptr: MemoryPointer) -> EvalResult<'tcx, i64> { - self.read_int(ptr, self.pointer_size()).map(|i| i as i64) - } - pub fn write_isize(&mut self, ptr: MemoryPointer, n: i64) -> EvalResult<'tcx> { let size = self.pointer_size(); self.write_int(ptr, n as i128, size) } - pub fn read_usize(&self, ptr: MemoryPointer) -> EvalResult<'tcx, u64> { - self.read_uint(ptr, self.pointer_size()).map(|i| i as u64) - } - pub fn write_usize(&mut self, ptr: MemoryPointer, n: u64) -> EvalResult<'tcx> { let size = self.pointer_size(); self.write_uint(ptr, n as u128, size) @@ -1494,6 +1489,7 @@ fn read_target_uint(endianess: layout::Endian, mut source: &[u8]) -> Result source.read_uint128::(source.len()), } } + fn read_target_int(endianess: layout::Endian, mut source: &[u8]) -> Result { match endianess { layout::Endian::Little => source.read_int128::(source.len()), @@ -1501,6 +1497,7 @@ fn read_target_int(endianess: layout::Endian, mut source: &[u8]) -> Result> EvalContext<'a, 'tcx, M> { ty::InstanceDef::Virtual(_, idx) => { let ptr_size = self.memory.pointer_size(); let (ptr, vtable) = args[0].into_ptr_vtable_pair(&self.memory)?; - let fn_ptr = self.memory.read_ptr( - vtable.offset(ptr_size * (idx as u64 + 3), &self)?, - )?; - let instance = self.memory.get_fn(fn_ptr.to_ptr()?)?; + let fn_ptr = self.memory.read_ptr_sized_unsigned( + vtable.offset(ptr_size * (idx as u64 + 3), &self)? + )?.to_ptr()?; + let instance = self.memory.get_fn(fn_ptr)?; let mut args = args.to_vec(); let ty = self.get_field_ty(args[0].ty, 0)?.ty; // TODO: packed flag is ignored args[0].ty = ty; @@ -408,98 +407,4 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { } } } - - pub fn read_discriminant_value( - &self, - adt_ptr: MemoryPointer, - adt_ty: Ty<'tcx>, - ) -> EvalResult<'tcx, u128> { - use rustc::ty::layout::Layout::*; - let adt_layout = self.type_layout(adt_ty)?; - //trace!("read_discriminant_value {:#?}", adt_layout); - - let discr_val = match *adt_layout { - General { discr, .. } | - CEnum { - discr, - signed: false, - .. - } => { - let discr_size = discr.size().bytes(); - self.memory.read_uint(adt_ptr, discr_size)? - } - - CEnum { - discr, - signed: true, - .. - } => { - let discr_size = discr.size().bytes(); - self.memory.read_int(adt_ptr, discr_size)? as u128 - } - - RawNullablePointer { nndiscr, value } => { - let discr_size = value.size(&self.tcx.data_layout).bytes(); - trace!("rawnullablepointer with size {}", discr_size); - self.read_nonnull_discriminant_value( - adt_ptr, - nndiscr as u128, - discr_size, - )? - } - - StructWrappedNullablePointer { - nndiscr, - ref discrfield_source, - .. - } => { - let (offset, TyAndPacked { ty, packed }) = self.nonnull_offset_and_ty( - adt_ty, - nndiscr, - discrfield_source, - )?; - let nonnull = adt_ptr.offset(offset.bytes(), &*self)?; - trace!("struct wrapped nullable pointer type: {}", ty); - // only the pointer part of a fat pointer is used for this space optimization - let discr_size = self.type_size(ty)?.expect( - "bad StructWrappedNullablePointer discrfield", - ); - self.read_maybe_aligned(!packed, |ectx| { - ectx.read_nonnull_discriminant_value(nonnull, nndiscr as u128, discr_size) - })? - } - - // The discriminant_value intrinsic returns 0 for non-sum types. - Array { .. } | - FatPointer { .. } | - Scalar { .. } | - Univariant { .. } | - Vector { .. } | - UntaggedUnion { .. } => 0, - }; - - Ok(discr_val) - } - - fn read_nonnull_discriminant_value( - &self, - ptr: MemoryPointer, - nndiscr: u128, - discr_size: u64, - ) -> EvalResult<'tcx, u128> { - trace!( - "read_nonnull_discriminant_value: {:?}, {}, {}", - ptr, - nndiscr, - discr_size - ); - let not_null = match self.memory.read_uint(ptr, discr_size) { - Ok(0) => false, - Ok(_) | - Err(EvalError { kind: EvalErrorKind::ReadPointerAsBytes, .. }) => true, - Err(e) => return Err(e), - }; - assert!(nndiscr == 0 || nndiscr == 1); - Ok(if not_null { nndiscr } else { 1 - nndiscr }) - } } diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 07d7de854b99..284e9811c9fd 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -105,10 +105,10 @@ impl<'a, 'tcx, M: Machine<'tcx>> EvalContext<'a, 'tcx, M> { vtable: MemoryPointer, ) -> EvalResult<'tcx, (u64, u64)> { let pointer_size = self.memory.pointer_size(); - let size = self.memory.read_usize(vtable.offset(pointer_size, self)?)?; - let align = self.memory.read_usize( - vtable.offset(pointer_size * 2, self)?, - )?; + let size = self.memory.read_ptr_sized_unsigned(vtable.offset(pointer_size, self)?)?.to_bytes()? as u64; + let align = self.memory.read_ptr_sized_unsigned( + vtable.offset(pointer_size * 2, self)? + )?.to_bytes()? as u64; Ok((size, align)) } diff --git a/src/librustc_mir/interpret/value.rs b/src/librustc_mir/interpret/value.rs index 8abb0b86bf8f..e052ec1e391c 100644 --- a/src/librustc_mir/interpret/value.rs +++ b/src/librustc_mir/interpret/value.rs @@ -176,13 +176,13 @@ impl<'a, 'tcx: 'a> Value { mem: &Memory<'a, 'tcx, M>, ) -> EvalResult<'tcx, Pointer> { use self::Value::*; - match *self { + Ok(match *self { ByRef(PtrAndAlign { ptr, aligned }) => { - mem.read_maybe_aligned(aligned, |mem| mem.read_ptr(ptr.to_ptr()?)) + mem.read_maybe_aligned(aligned, |mem| mem.read_ptr_sized_unsigned(ptr.to_ptr()?))? } ByVal(ptr) | - ByValPair(ptr, _) => Ok(ptr.into()), - } + ByValPair(ptr, _) => ptr, + }.into()) } pub(super) fn into_ptr_vtable_pair>( @@ -196,11 +196,11 @@ impl<'a, 'tcx: 'a> Value { aligned, }) => { mem.read_maybe_aligned(aligned, |mem| { - let ptr = mem.read_ptr(ref_ptr.to_ptr()?)?; - let vtable = mem.read_ptr( + let ptr = mem.read_ptr_sized_unsigned(ref_ptr.to_ptr()?)?.into(); + let vtable = mem.read_ptr_sized_unsigned( ref_ptr.offset(mem.pointer_size(), mem.layout)?.to_ptr()?, - )?; - Ok((ptr, vtable.to_ptr()?)) + )?.to_ptr()?; + Ok((ptr, vtable)) }) } @@ -222,10 +222,10 @@ impl<'a, 'tcx: 'a> Value { aligned, }) => { mem.read_maybe_aligned(aligned, |mem| { - let ptr = mem.read_ptr(ref_ptr.to_ptr()?)?; - let len = mem.read_usize( + let ptr = mem.read_ptr_sized_unsigned(ref_ptr.to_ptr()?)?.into(); + let len = mem.read_ptr_sized_unsigned( ref_ptr.offset(mem.pointer_size(), mem.layout)?.to_ptr()?, - )?; + )?.to_bytes()? as u64; Ok((ptr, len)) }) } diff --git a/tests/run-pass-fullmir/integer-ops.rs b/tests/run-pass-fullmir/integer-ops.rs index e761cdd6237c..0964b1b32b5c 100644 --- a/tests/run-pass-fullmir/integer-ops.rs +++ b/tests/run-pass-fullmir/integer-ops.rs @@ -14,6 +14,10 @@ use std::i32; pub fn main() { + // This tests that do (not) do sign extension properly when loading integers + assert_eq!(u32::max_value() as i64, 4294967295); + assert_eq!(i32::min_value() as i64, -2147483648); + assert_eq!(i8::min_value(), -128); assert_eq!(i8::max_value(), 127);