From d8c5bc7ec6ea2501bdbd2853551aa83175d6e8d0 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 30 Jul 2019 23:10:51 +0200 Subject: [PATCH] Replace usage of alloc.bytes in interpret There is now a dedicate `len` method which avoids the need to access the bytes. Access the length as `Size` can also be done by a direct member. The constructors guarantee that these representations are convertable. Access which relies on the bytes, such as snapshot, can use direct raw access by reference as it does not care about undef and relocations or properly checks them seperately. --- src/librustc/mir/interpret/allocation.rs | 2 ++ src/librustc_mir/interpret/intrinsics.rs | 2 +- .../interpret/intrinsics/type_name.rs | 2 +- src/librustc_mir/interpret/memory.rs | 25 +++++++++++-------- src/librustc_mir/interpret/snapshot.rs | 20 +++++++++++++-- 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index e0ada7e2f11a..bfbfffeb3b8c 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -13,6 +13,8 @@ use rustc_data_structures::sorted_map::SortedMap; use rustc_target::abi::HasDataLayout; use std::borrow::Cow; +// NOTE: When adding new fields, make sure to adjust the Snapshot impl in +// `src/librustc_mir/interpret/snapshot.rs`. #[derive( Clone, Debug, diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 89c5be137a4e..a48a4abf796b 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -81,7 +81,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let alloc = alloc_type_name(self.tcx.tcx, substs.type_at(0)); let name_id = self.tcx.alloc_map.lock().create_memory_alloc(alloc); let id_ptr = self.memory.tag_static_base_pointer(name_id.into()); - let alloc_len = alloc.bytes.len() as u64; + let alloc_len = alloc.size.bytes(); let name_val = Immediate::new_slice(Scalar::Ptr(id_ptr), alloc_len, self); self.write_immediate(name_val, dest)?; } diff --git a/src/librustc_mir/interpret/intrinsics/type_name.rs b/src/librustc_mir/interpret/intrinsics/type_name.rs index f207cfc6b39c..032d16a49db4 100644 --- a/src/librustc_mir/interpret/intrinsics/type_name.rs +++ b/src/librustc_mir/interpret/intrinsics/type_name.rs @@ -221,7 +221,7 @@ pub fn type_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> &'tcx ty::Const<'tcx> val: ConstValue::Slice { data: alloc, start: 0, - end: alloc.bytes.len(), + end: alloc.len(), }, ty: tcx.mk_static_str(), }) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index f572651f02b5..15180265ad29 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -210,7 +210,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let new_ptr = self.allocate(new_size, new_align, kind); let old_size = match old_size_and_align { Some((size, _align)) => size, - None => Size::from_bytes(self.get(ptr.alloc_id)?.bytes.len() as u64), + None => self.get(ptr.alloc_id)?.size, }; self.copy( ptr, @@ -271,20 +271,20 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { )) } if let Some((size, align)) = old_size_and_align { - if size.bytes() != alloc.bytes.len() as u64 || align != alloc.align { - let bytes = Size::from_bytes(alloc.bytes.len() as u64); + if size != alloc.size || align != alloc.align { + let bytes = alloc.size; throw_unsup!(IncorrectAllocationInformation(size, bytes, align, alloc.align)) } } // Let the machine take some extra action - let size = Size::from_bytes(alloc.bytes.len() as u64); + let size = alloc.size; AllocationExtra::memory_deallocated(&mut alloc, ptr, size)?; // Don't forget to remember size and align of this now-dead allocation let old = self.dead_alloc_map.insert( ptr.alloc_id, - (Size::from_bytes(alloc.bytes.len() as u64), alloc.align) + (alloc.size, alloc.align) ); if old.is_some() { bug!("Nothing can be deallocated twice"); @@ -555,7 +555,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // a) cause cycles in case `id` refers to a static // b) duplicate a static's allocation in miri if let Some((_, alloc)) = self.alloc_map.get(id) { - return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); + return Ok((alloc.size, alloc.align)); } // # Function pointers @@ -583,7 +583,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Some(GlobalAlloc::Memory(alloc)) => // Need to duplicate the logic here, because the global allocations have // different associated types than the interpreter-local ones. - Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)), + Ok((alloc.size, alloc.align)), Some(GlobalAlloc::Function(_)) => bug!("We already checked function pointers above"), // The rest must be dead. @@ -645,7 +645,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let prefix_len = msg.len(); let mut relocations = vec![]; - for i in 0..(alloc.bytes.len() as u64) { + for i in 0..alloc.size.bytes() { let i = Size::from_bytes(i); if let Some(&(_, target_id)) = alloc.relocations.get(&i) { if allocs_seen.insert(target_id) { @@ -655,7 +655,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)).is_ok() { // this `as usize` is fine, since `i` came from a `usize` - write!(msg, "{:02x} ", alloc.bytes[i.bytes() as usize]).unwrap(); + let i = i.bytes() as usize; + + // Checked definedness (and thus range) and relocations. This access also doesn't + // influence interpreter execution but is only for debugging. + let bytes = alloc.inspect_with_undef_and_ptr_outside_interpreter(i..i+1); + write!(msg, "{:02x} ", bytes[0]).unwrap(); } else { msg.push_str("__ "); } @@ -664,7 +669,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { trace!( "{}({} bytes, alignment {}){}", msg, - alloc.bytes.len(), + alloc.size.bytes(), alloc.align.bytes(), extra ); diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 47289064f4d0..0f0c422d8d3f 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -15,7 +15,7 @@ use rustc::mir::interpret::{ }; use rustc::ty::{self, TyCtxt}; -use rustc::ty::layout::Align; +use rustc::ty::layout::{Align, Size}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::indexed_vec::IndexVec; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; @@ -276,6 +276,7 @@ struct AllocationSnapshot<'a> { relocations: Relocations<(), AllocIdSnapshot<'a>>, undef_mask: &'a UndefMask, align: &'a Align, + size: &'a Size, mutability: &'a Mutability, } @@ -285,12 +286,27 @@ impl<'a, Ctx> Snapshot<'a, Ctx> for &'a Allocation type Item = AllocationSnapshot<'a>; fn snapshot(&self, ctx: &'a Ctx) -> Self::Item { - let Allocation { bytes, relocations, undef_mask, align, mutability, extra: () } = self; + let Allocation { + relocations, + size, + align, + mutability, + extra: (), + .. + } = self; + + let all_bytes = 0..self.len(); + // This 'inspect' is okay since following access respects undef and relocations. This does + // influence interpreter exeuction, but only to detect the error of cycles in evalution + // dependencies. + let bytes = self.inspect_with_undef_and_ptr_outside_interpreter(all_bytes); + let undef_mask = self.undef_mask(); AllocationSnapshot { bytes, undef_mask, align, + size, mutability, relocations: relocations.snapshot(ctx), }