From 9b9eecf964235f781806ce3e026c1522eb35e013 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Sat, 13 Jul 2019 04:00:06 +0200 Subject: [PATCH 01/12] Store allocation size, make bytes, undef_mask private Direct access to the bytes was previously a problem (#62931) where components would read their contents without properly checking relocations and/or definedness. Making bytes private instead of purely renaming them also helps in allowing amendments to their allocation scheme (such as eliding allocation for undef of constant regions). --- src/librustc/ich/impls_ty.rs | 8 +++++- src/librustc/mir/interpret/allocation.rs | 35 ++++++++++++++++++------ src/librustc/ty/print/pretty.rs | 10 +++++-- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 0ddc9211f9811..e33991bb1b6f8 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -176,15 +176,21 @@ impl<'a> HashStable> for mir::interpret::Allocation { hasher: &mut StableHasher, ) { let mir::interpret::Allocation { - bytes, relocations, undef_mask, align, mutability, + relocations, align, mutability, size, extra: _, + .. /* private bytes and undef_mask */ } = self; + + let bytes = self.inspect_with_undef_and_ptr_outside_interpreter(0..self.len()); + let undef_mask = self.undef_mask(); + bytes.hash_stable(hcx, hasher); relocations.len().hash_stable(hcx, hasher); for reloc in relocations.iter() { reloc.hash_stable(hcx, hasher); } undef_mask.hash_stable(hcx, hasher); + size.hash_stable(hcx, hasher); align.hash_stable(hcx, hasher); mutability.hash_stable(hcx, hasher); } diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index ce04cca96e0f9..61b237c0bb395 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -16,15 +16,17 @@ use std::borrow::Cow; #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] pub struct Allocation { /// The actual bytes of the allocation. - /// Note that the bytes of a pointer represent the offset of the pointer - pub bytes: Vec, + /// Note that the bytes of a pointer represent the offset of the pointer. + bytes: Vec, /// Maps from byte addresses to extra data for each pointer. /// 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, + /// Denotes which part of this allocation is initialized. + undef_mask: UndefMask, + /// The size of the allocation. Currently, must always equal `bytes.len()`. + pub size: Size, /// The alignment of the allocation to detect unaligned reads. pub align: Align, /// Whether the allocation is mutable. @@ -85,11 +87,12 @@ impl Allocation { /// Creates a read-only allocation initialized by the given bytes pub fn from_bytes<'a>(slice: impl Into>, align: Align) -> Self { let bytes = slice.into().into_owned(); - let undef_mask = UndefMask::new(Size::from_bytes(bytes.len() as u64), true); + let size = Size::from_bytes(bytes.len() as u64); Self { bytes, relocations: Relocations::new(), - undef_mask, + undef_mask: UndefMask::new(size, true), + size, align, mutability: Mutability::Immutable, extra: (), @@ -106,6 +109,7 @@ impl Allocation { bytes: vec![0; size.bytes() as usize], relocations: Relocations::new(), undef_mask: UndefMask::new(size, false), + size, align, mutability: Mutability::Mutable, extra: (), @@ -113,6 +117,21 @@ impl Allocation { } } +/// Raw accessors. Provide access to otherwise private bytes. +impl Allocation { + pub fn len(&self) -> usize { + self.size.bytes() as usize + } + + /// Look at a slice which may describe undefined bytes or describe a relocation. This differs + /// from `get_bytes_with_undef_and_ptr` in that it does no relocation checks (even on the + /// edges) at all. It further ignores `AllocationExtra` callbacks. + /// This must not be used for reads affecting the interpreter execution. + pub fn inspect_with_undef_and_ptr_outside_interpreter(&self, range: Range) -> &[u8] { + &self.bytes[range] + } +} + impl<'tcx> rustc_serialize::UseSpecializedDecodable for &'tcx Allocation {} /// Byte accessors @@ -132,9 +151,9 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { ); let end = end.bytes() as usize; assert!( - end <= self.bytes.len(), + end <= self.len(), "Out-of-bounds access at offset {}, size {} in allocation of size {}", - offset.bytes(), size.bytes(), self.bytes.len() + offset.bytes(), size.bytes(), self.len() ); (offset.bytes() as usize)..end } diff --git a/src/librustc/ty/print/pretty.rs b/src/librustc/ty/print/pretty.rs index bf6741dde43b5..a72ecdb5745bb 100644 --- a/src/librustc/ty/print/pretty.rs +++ b/src/librustc/ty/print/pretty.rs @@ -944,10 +944,16 @@ pub trait PrettyPrinter<'tcx>: .get_bytes(&self.tcx(), ptr, Size::from_bytes(n)).unwrap()) }, (ConstValue::Slice { data, start, end }, ty::Slice(t)) if *t == u8 => { - Some(&data.bytes[start..end]) + // The `inspect` here is okay since we checked the bounds, and there are no + // relocations (we have an active slice reference here). We don't use this + // result to affect interpreter execution. + Some(data.inspect_with_undef_and_ptr_outside_interpreter(start..end)) }, (ConstValue::Slice { data, start, end }, ty::Str) => { - let slice = &data.bytes[start..end]; + // The `inspect` here is okay since we checked the bounds, and there are no + // relocations (we have an active `str` reference here). We don't use this + // result to affect interpreter execution. + let slice = data.inspect_with_undef_and_ptr_outside_interpreter(start..end); let s = ::std::str::from_utf8(slice) .expect("non utf8 str from miri"); p!(write("{:?}", s)); From 2228b3f08631b5e9b32e6b6c82e758fe4bcbbd60 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Sat, 17 Aug 2019 19:49:51 +0200 Subject: [PATCH 02/12] Derive HashStable for Allocation Requires a manual implementation for Relocations since dereferencing to SortedMap is not always implemented but that one is far more trivial. Added fields could otherwise be silently forgotten since private fields make destructing outside the module possible only with `..` pattern which would then also be applicable to newly added public fields. --- src/librustc/ich/impls_ty.rs | 26 ++++++++---------------- src/librustc/mir/interpret/allocation.rs | 13 +++++++++++- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index e33991bb1b6f8..be7669fcad875 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -168,31 +168,21 @@ impl<'a> HashStable> for mir::interpret::AllocId { } } -// Allocations treat their relocations specially -impl<'a> HashStable> for mir::interpret::Allocation { +// `Relocations` with default type parameters is a sorted map. +impl<'a, Tag> HashStable> +for mir::interpret::Relocations +where + Tag: HashStable>, +{ fn hash_stable( &self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher, ) { - let mir::interpret::Allocation { - relocations, align, mutability, size, - extra: _, - .. /* private bytes and undef_mask */ - } = self; - - let bytes = self.inspect_with_undef_and_ptr_outside_interpreter(0..self.len()); - let undef_mask = self.undef_mask(); - - bytes.hash_stable(hcx, hasher); - relocations.len().hash_stable(hcx, hasher); - for reloc in relocations.iter() { + self.len().hash_stable(hcx, hasher); + for reloc in self.iter() { reloc.hash_stable(hcx, hasher); } - undef_mask.hash_stable(hcx, hasher); - size.hash_stable(hcx, hasher); - align.hash_stable(hcx, hasher); - mutability.hash_stable(hcx, hasher); } } diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 61b237c0bb395..363fcaae5b266 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -13,7 +13,18 @@ use rustc_data_structures::sorted_map::SortedMap; use rustc_target::abi::HasDataLayout; use std::borrow::Cow; -#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] +#[derive( + Clone, + Debug, + Eq, + PartialEq, + PartialOrd, + Ord, + Hash, + RustcEncodable, + RustcDecodable, + HashStable, +)] pub struct Allocation { /// The actual bytes of the allocation. /// Note that the bytes of a pointer represent the offset of the pointer. From 98cff6928925256faffbe3e9ce9f14d0dfdbb6c0 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 30 Jul 2019 23:38:02 +0200 Subject: [PATCH 03/12] Move copy of undef_mask into allocation This also means that the compressed representation chosen may be optimized together with any changes to the undef_mask. --- src/librustc/mir/interpret/allocation.rs | 85 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 58 +--------------- 2 files changed, 88 insertions(+), 55 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 363fcaae5b266..e0ada7e2f11a3 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -566,6 +566,91 @@ impl<'tcx, Tag, Extra> Allocation { } } +/// Run-length encoding of the undef mask. +/// Used to copy parts of a mask multiple times to another allocation. +pub struct AllocationDefinedness { + ranges: smallvec::SmallVec::<[u64; 1]>, + first: bool, +} + +/// Transferring the definedness mask to other allocations. +impl Allocation { + /// Creates a run-length encoding of the undef_mask. + pub fn compress_defined_range( + &self, + src: Pointer, + size: Size, + ) -> AllocationDefinedness { + // Since we are copying `size` bytes from `src` to `dest + i * size` (`for i in 0..repeat`), + // a naive undef mask copying algorithm would repeatedly have to read the undef mask from + // the source and write it to the destination. Even if we optimized the memory accesses, + // we'd be doing all of this `repeat` times. + // Therefor we precompute a compressed version of the undef mask of the source value and + // then write it back `repeat` times without computing any more information from the source. + + // a precomputed cache for ranges of defined/undefined bits + // 0000010010001110 will become + // [5, 1, 2, 1, 3, 3, 1] + // where each element toggles the state + + let mut ranges = smallvec::SmallVec::<[u64; 1]>::new(); + let first = self.undef_mask.get(src.offset); + let mut cur_len = 1; + let mut cur = first; + + for i in 1..size.bytes() { + // FIXME: optimize to bitshift the current undef block's bits and read the top bit + if self.undef_mask.get(src.offset + Size::from_bytes(i)) == cur { + cur_len += 1; + } else { + ranges.push(cur_len); + cur_len = 1; + cur = !cur; + } + } + + ranges.push(cur_len); + + AllocationDefinedness { ranges, first, } + } + + /// Apply multiple instances of the run-length encoding to the undef_mask. + pub fn mark_compressed_range( + &mut self, + defined: &AllocationDefinedness, + dest: Pointer, + size: Size, + repeat: u64, + ) { + // an optimization where we can just overwrite an entire range of definedness bits if + // they are going to be uniformly `1` or `0`. + if defined.ranges.len() <= 1 { + self.undef_mask.set_range_inbounds( + dest.offset, + dest.offset + size * repeat, + defined.first, + ); + return; + } + + for mut j in 0..repeat { + j *= size.bytes(); + j += dest.offset.bytes(); + let mut cur = defined.first; + for range in &defined.ranges { + let old_j = j; + j += range; + self.undef_mask.set_range_inbounds( + Size::from_bytes(old_j), + Size::from_bytes(j), + cur, + ); + cur = !cur; + } + } + } +} + /// Relocations #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)] pub struct Relocations(SortedMap); diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index aef09df4537be..f572651f02b56 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -894,65 +894,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // The bits have to be saved locally before writing to dest in case src and dest overlap. assert_eq!(size.bytes() as usize as u64, size.bytes()); - let undef_mask = &self.get(src.alloc_id)?.undef_mask; - - // Since we are copying `size` bytes from `src` to `dest + i * size` (`for i in 0..repeat`), - // a naive undef mask copying algorithm would repeatedly have to read the undef mask from - // the source and write it to the destination. Even if we optimized the memory accesses, - // we'd be doing all of this `repeat` times. - // Therefor we precompute a compressed version of the undef mask of the source value and - // then write it back `repeat` times without computing any more information from the source. - - // a precomputed cache for ranges of defined/undefined bits - // 0000010010001110 will become - // [5, 1, 2, 1, 3, 3, 1] - // where each element toggles the state - let mut ranges = smallvec::SmallVec::<[u64; 1]>::new(); - let first = undef_mask.get(src.offset); - let mut cur_len = 1; - let mut cur = first; - for i in 1..size.bytes() { - // FIXME: optimize to bitshift the current undef block's bits and read the top bit - if undef_mask.get(src.offset + Size::from_bytes(i)) == cur { - cur_len += 1; - } else { - ranges.push(cur_len); - cur_len = 1; - cur = !cur; - } - } + let src_alloc = self.get(src.alloc_id)?; + let compressed = src_alloc.compress_defined_range(src, size); // now fill in all the data let dest_allocation = self.get_mut(dest.alloc_id)?; - // an optimization where we can just overwrite an entire range of definedness bits if - // they are going to be uniformly `1` or `0`. - if ranges.is_empty() { - dest_allocation.undef_mask.set_range_inbounds( - dest.offset, - dest.offset + size * repeat, - first, - ); - return Ok(()) - } + dest_allocation.mark_compressed_range(&compressed, dest, size, repeat); - // remember to fill in the trailing bits - ranges.push(cur_len); - - for mut j in 0..repeat { - j *= size.bytes(); - j += dest.offset.bytes(); - let mut cur = first; - for range in &ranges { - let old_j = j; - j += range; - dest_allocation.undef_mask.set_range_inbounds( - Size::from_bytes(old_j), - Size::from_bytes(j), - cur, - ); - cur = !cur; - } - } Ok(()) } From d8c5bc7ec6ea2501bdbd2853551aa83175d6e8d0 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Tue, 30 Jul 2019 23:10:51 +0200 Subject: [PATCH 04/12] 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 e0ada7e2f11a3..bfbfffeb3b8c6 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 89c5be137a4e5..a48a4abf796b2 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 f207cfc6b39cd..032d16a49db4b 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 f572651f02b56..15180265ad29f 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 47289064f4d0d..0f0c422d8d3f4 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), } From 7b941e368fac6612eacf3423eac0149ef5ca3bb3 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 14 Aug 2019 02:26:18 +0200 Subject: [PATCH 05/12] Expose encapsulated undef mask as immutable --- src/librustc/mir/interpret/allocation.rs | 5 +++++ src/librustc_mir/interpret/memory.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index bfbfffeb3b8c6..d3f87f16313d0 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -143,6 +143,11 @@ impl Allocation { pub fn inspect_with_undef_and_ptr_outside_interpreter(&self, range: Range) -> &[u8] { &self.bytes[range] } + + /// View the undef mask. + pub fn undef_mask(&self) -> &UndefMask { + &self.undef_mask + } } impl<'tcx> rustc_serialize::UseSpecializedDecodable for &'tcx Allocation {} diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 15180265ad29f..f7576a41e0f00 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -653,7 +653,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } relocations.push((i, target_id)); } - if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)).is_ok() { + 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` let i = i.bytes() as usize; From df58fcffbcc2dea6eb31934424ce74b3636c5b68 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 14 Aug 2019 04:40:03 +0200 Subject: [PATCH 06/12] Fix codegen with explicit allocation byte access --- src/librustc_codegen_llvm/consts.rs | 55 +++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 0077df3cf5eea..26eb870cfc0d4 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -35,11 +35,21 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll assert_eq!(offset as usize as u64, offset); let offset = offset as usize; if offset > next_offset { - llvals.push(cx.const_bytes(&alloc.bytes[next_offset..offset])); + // This `inspect` is okay since we have check that it is not within a relocation, it is + // within the bounds of the allocation, and it doesn't affect interpreter execution (we + // inspect the result after interpreter execution). Any undef byte is replaced with + // some arbitrary byte value. + // + // FIXME: relay undef bytes to codegen as undef const bytes + let bytes = alloc.inspect_with_undef_and_ptr_outside_interpreter(next_offset..offset); + llvals.push(cx.const_bytes(bytes)); } let ptr_offset = read_target_uint( dl.endian, - &alloc.bytes[offset..(offset + pointer_size)], + // This `inspect` is okay since it is within the bounds of the allocation, it doesn't + // affect interpreter execution (we inspect the result after interpreter execution), + // and we properly interpret the relocation as a relocation pointer offset. + alloc.inspect_with_undef_and_ptr_outside_interpreter(offset..(offset + pointer_size)), ).expect("const_alloc_to_llvm: could not read relocation pointer") as u64; llvals.push(cx.scalar_to_backend( Pointer::new(alloc_id, Size::from_bytes(ptr_offset)).into(), @@ -51,8 +61,16 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll )); next_offset = offset + pointer_size; } - if alloc.bytes.len() >= next_offset { - llvals.push(cx.const_bytes(&alloc.bytes[next_offset ..])); + if alloc.len() >= next_offset { + let range = next_offset..alloc.len(); + // This `inspect` is okay since we have check that it is after all relocations, it is + // within the bounds of the allocation, and it doesn't affect interpreter execution (we + // inspect the result after interpreter execution). Any undef byte is replaced with some + // arbitrary byte value. + // + // FIXME: relay undef bytes to codegen as undef const bytes + let bytes = alloc.inspect_with_undef_and_ptr_outside_interpreter(range); + llvals.push(cx.const_bytes(bytes)); } cx.const_struct(&llvals, true) @@ -437,7 +455,23 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> { // // We could remove this hack whenever we decide to drop macOS 10.10 support. if self.tcx.sess.target.target.options.is_like_osx { - let sect_name = if alloc.bytes.iter().all(|b| *b == 0) { + assert_eq!(alloc.relocations.len(), 0); + + let is_zeroed = { + // Treats undefined bytes as if they were defined with the byte value that + // happens to be currently assigned in mir. This is valid since reading + // undef bytes may yield arbitrary values. + // + // FIXME: ignore undef bytes even with representation `!= 0`. + // + // The `inspect` method is okay here because we checked relocations, and + // because we are doing this access to inspect the final interpreter state + // (not as part of the interpreter execution). + alloc.inspect_with_undef_and_ptr_outside_interpreter(0..alloc.len()) + .iter() + .all(|b| *b == 0) + }; + let sect_name = if is_zeroed { CStr::from_bytes_with_nul_unchecked(b"__DATA,__thread_bss\0") } else { CStr::from_bytes_with_nul_unchecked(b"__DATA,__thread_data\0") @@ -456,10 +490,17 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> { section.as_str().as_ptr() as *const _, section.as_str().len() as c_uint, ); + assert!(alloc.relocations.is_empty()); + + // The `inspect` method is okay here because we checked relocations, and + // because we are doing this access to inspect the final interpreter state (not + // as part of the interpreter execution). + let bytes = alloc.inspect_with_undef_and_ptr_outside_interpreter( + 0..alloc.len()); let alloc = llvm::LLVMMDStringInContext( self.llcx, - alloc.bytes.as_ptr() as *const _, - alloc.bytes.len() as c_uint, + bytes.as_ptr() as *const _, + bytes.len() as c_uint, ); let data = [section, alloc]; let meta = llvm::LLVMMDNodeInContext(self.llcx, data.as_ptr(), 2); From 85d6b7b9d3d946b35826298e3d04381f96427433 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 28 Aug 2019 03:58:42 +0200 Subject: [PATCH 07/12] Address naming and comments from reviews --- src/librustc/mir/interpret/allocation.rs | 8 ++++---- src/librustc_codegen_llvm/consts.rs | 6 +++--- src/librustc_mir/interpret/memory.rs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index d3f87f16313d0..dcfa2e5cb4691 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -136,7 +136,7 @@ impl Allocation { self.size.bytes() as usize } - /// Look at a slice which may describe undefined bytes or describe a relocation. This differs + /// Looks at a slice which may describe undefined bytes or describe a relocation. This differs /// from `get_bytes_with_undef_and_ptr` in that it does no relocation checks (even on the /// edges) at all. It further ignores `AllocationExtra` callbacks. /// This must not be used for reads affecting the interpreter execution. @@ -144,7 +144,7 @@ impl Allocation { &self.bytes[range] } - /// View the undef mask. + /// Returns the undef mask. pub fn undef_mask(&self) -> &UndefMask { &self.undef_mask } @@ -583,7 +583,7 @@ pub struct AllocationDefinedness { /// Transferring the definedness mask to other allocations. impl Allocation { /// Creates a run-length encoding of the undef_mask. - pub fn compress_defined_range( + pub fn compress_undef_range( &self, src: Pointer, size: Size, @@ -622,7 +622,7 @@ impl Allocation { } /// Apply multiple instances of the run-length encoding to the undef_mask. - pub fn mark_compressed_range( + pub fn mark_compressed_undef_range( &mut self, defined: &AllocationDefinedness, dest: Pointer, diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 26eb870cfc0d4..8725e69b11270 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -35,9 +35,9 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll assert_eq!(offset as usize as u64, offset); let offset = offset as usize; if offset > next_offset { - // This `inspect` is okay since we have check that it is not within a relocation, it is - // within the bounds of the allocation, and it doesn't affect interpreter execution (we - // inspect the result after interpreter execution). Any undef byte is replaced with + // This `inspect` is okay since we have checked that it is not within a relocation, it + // is within the bounds of the allocation, and it doesn't affect interpreter execution + // (we inspect the result after interpreter execution). Any undef byte is replaced with // some arbitrary byte value. // // FIXME: relay undef bytes to codegen as undef const bytes diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index f7576a41e0f00..7f7729ae5e0f4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -900,11 +900,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { assert_eq!(size.bytes() as usize as u64, size.bytes()); let src_alloc = self.get(src.alloc_id)?; - let compressed = src_alloc.compress_defined_range(src, size); + let compressed = src_alloc.compress_undef_range(src, size); // now fill in all the data let dest_allocation = self.get_mut(dest.alloc_id)?; - dest_allocation.mark_compressed_range(&compressed, dest, size, repeat); + dest_allocation.mark_compressed_undef_range(&compressed, dest, size, repeat); Ok(()) } From 6fe31fefd80cd1c4300b03b1e55c63de12134eed Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Thu, 29 Aug 2019 18:02:51 +0200 Subject: [PATCH 08/12] Make allocation relocation field private --- src/librustc/mir/interpret/allocation.rs | 13 +++++++++---- src/librustc_mir/interpret/intern.rs | 4 ++-- src/librustc_mir/interpret/memory.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 4 ++-- src/librustc_typeck/check/mod.rs | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index dcfa2e5cb4691..75319a6783167 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -35,7 +35,7 @@ pub struct Allocation { /// 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, + relocations: Relocations, /// Denotes which part of this allocation is initialized. undef_mask: UndefMask, /// The size of the allocation. Currently, must always equal `bytes.len()`. @@ -148,6 +148,11 @@ impl Allocation { pub fn undef_mask(&self) -> &UndefMask { &self.undef_mask } + + /// Returns the relocation list. + pub fn relocations(&self) -> &Relocations { + &self.relocations + } } impl<'tcx> rustc_serialize::UseSpecializedDecodable for &'tcx Allocation {} @@ -459,7 +464,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Relocations impl<'tcx, Tag: Copy, Extra> Allocation { /// Returns all relocations overlapping with the given ptr-offset pair. - pub fn relocations( + pub fn get_relocations( &self, cx: &impl HasDataLayout, ptr: Pointer, @@ -480,7 +485,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { ptr: Pointer, size: Size, ) -> InterpResult<'tcx> { - if self.relocations(cx, ptr, size).is_empty() { + if self.get_relocations(cx, ptr, size).is_empty() { Ok(()) } else { throw_unsup!(ReadPointerAsBytes) @@ -502,7 +507,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // Find the start and end of the given range and its outermost relocations. let (first, last) = { // Find all relocations overlapping the given range. - let relocations = self.relocations(cx, ptr, size); + let relocations = self.get_relocations(cx, ptr, size); if relocations.is_empty() { return Ok(()); } diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 32ba70a81c997..4cbbc0ffe17cc 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -94,7 +94,7 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { alloc.mutability = mutability; // link the alloc id to the actual allocation let alloc = tcx.intern_const_alloc(alloc); - self.leftover_relocations.extend(alloc.relocations.iter().map(|&(_, ((), reloc))| reloc)); + self.leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); tcx.alloc_map.lock().set_alloc_id_memory(ptr.alloc_id, alloc); Ok(None) } @@ -316,7 +316,7 @@ pub fn intern_const_alloc_recursive( // So we hand-roll the interning logic here again let alloc = tcx.intern_const_alloc(alloc); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); - for &(_, ((), reloc)) in alloc.relocations.iter() { + for &(_, ((), reloc)) in alloc.relocations().iter() { if leftover_relocations.insert(reloc) { todo.push(reloc); } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 7f7729ae5e0f4..26b3f0be8c2b8 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -647,7 +647,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { for i in 0..alloc.size.bytes() { let i = Size::from_bytes(i); - if let Some(&(_, target_id)) = alloc.relocations.get(&i) { + if let Some(&(_, target_id)) = alloc.relocations().get(&i) { if allocs_seen.insert(target_id) { allocs_to_print.push_back(target_id); } @@ -809,7 +809,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // (`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.get(src.alloc_id)?.relocations(self, src, size); + let relocations = self.get(src.alloc_id)?.get_relocations(self, src, size); if relocations.is_empty() { // nothing to copy, ignore even the `length` loop Vec::new() diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 12d763bb7910a..a9403502f64d3 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1202,7 +1202,7 @@ fn collect_miri<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut Vec { trace!("collecting {:?} with {:#?}", alloc_id, alloc); - for &((), inner) in alloc.relocations.values() { + for &((), inner) in alloc.relocations().values() { collect_miri(tcx, inner, output); } }, @@ -1268,7 +1268,7 @@ fn collect_const<'tcx>( collect_miri(tcx, ptr.alloc_id, output), ConstValue::Slice { data: alloc, start: _, end: _ } | ConstValue::ByRef { alloc, .. } => { - for &((), id) in alloc.relocations.values() { + for &((), id) in alloc.relocations().values() { collect_miri(tcx, id, output); } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 14fc0d6347e4b..e68104c6df808 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1570,7 +1570,7 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt<'_>, id: DefId, span: Span) } else { bug!("Matching on non-ByRef static") }; - if alloc.relocations.len() != 0 { + if alloc.relocations().len() != 0 { let msg = "statics with a custom `#[link_section]` must be a \ simple list of bytes on the wasm target with no \ extra levels of indirection such as references"; From bee2d3748ef472e416b4a690af89543ed0edd302 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Fri, 30 Aug 2019 04:17:18 +0200 Subject: [PATCH 09/12] Move relocation range copies into allocation --- src/librustc/mir/interpret/allocation.rs | 50 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 30 ++------------ 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 75319a6783167..8f47bf9d0fd5a 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -693,6 +693,56 @@ impl DerefMut for Relocations { } } +/// A partial, owned list of relocations to transfer into another allocation. +pub struct AllocationRelocations { + relative_relocations: Vec<(Size, (Tag, AllocId))>, +} + +impl Allocation { + pub fn prepare_relocation_copy( + &self, + cx: &impl HasDataLayout, + src: Pointer, + size: Size, + dest: Pointer, + length: u64, + ) -> AllocationRelocations { + let relocations = self.get_relocations(cx, src, size); + if relocations.is_empty() { + return AllocationRelocations { relative_relocations: Vec::new() }; + } + + let mut new_relocations = Vec::with_capacity(relocations.len() * (length as usize)); + + for i in 0..length { + new_relocations.extend( + relocations + .iter() + .map(|&(offset, reloc)| { + // compute offset for current repetition + let dest_offset = dest.offset + (i * size); + ( + // shift offsets from source allocation to destination allocation + offset + dest_offset - src.offset, + reloc, + ) + }) + ); + } + + AllocationRelocations { + relative_relocations: new_relocations, + } + } + + pub fn mark_relocation_range( + &mut self, + relocations: AllocationRelocations, + ) { + self.relocations.insert_presorted(relocations.relative_relocations); + } +} + //////////////////////////////////////////////////////////////////////////////// // Undefined byte tracking //////////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 26b3f0be8c2b8..6e0d77235fe2b 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -808,32 +808,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // 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.get(src.alloc_id)?.get_relocations(self, src, size); - if relocations.is_empty() { - // nothing to copy, ignore even the `length` loop - Vec::new() - } else { - let mut new_relocations = Vec::with_capacity(relocations.len() * (length as usize)); - for i in 0..length { - new_relocations.extend( - relocations - .iter() - .map(|&(offset, reloc)| { - // compute offset for current repetition - let dest_offset = dest.offset + (i * size); - ( - // shift offsets from source allocation to destination allocation - offset + dest_offset - src.offset, - reloc, - ) - }) - ); - } - - new_relocations - } - }; + let relocations = self.get(src.alloc_id)? + .prepare_relocation_copy(self, src, size, dest, length); let tcx = self.tcx.tcx; @@ -880,7 +856,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // copy definedness to the destination self.copy_undef_mask(src, dest, size, length)?; // copy the relocations to the destination - self.get_mut(dest.alloc_id)?.relocations.insert_presorted(relocations); + self.get_mut(dest.alloc_id)?.mark_relocation_range(relocations); Ok(()) } From 7388cb4cf81523fb2709518ec8b6a16c329b72e5 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Fri, 30 Aug 2019 04:19:29 +0200 Subject: [PATCH 10/12] Fixup remaining direct relocation field references --- src/librustc_codegen_llvm/consts.rs | 8 ++++---- src/librustc_mir/interpret/snapshot.rs | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index 8725e69b11270..958666cb8858a 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -25,12 +25,12 @@ use rustc::hir::{self, CodegenFnAttrs, CodegenFnAttrFlags}; use std::ffi::{CStr, CString}; pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value { - let mut llvals = Vec::with_capacity(alloc.relocations.len() + 1); + let mut llvals = Vec::with_capacity(alloc.relocations().len() + 1); let dl = cx.data_layout(); let pointer_size = dl.pointer_size.bytes() as usize; let mut next_offset = 0; - for &(offset, ((), alloc_id)) in alloc.relocations.iter() { + for &(offset, ((), alloc_id)) in alloc.relocations().iter() { let offset = offset.bytes(); assert_eq!(offset as usize as u64, offset); let offset = offset as usize; @@ -455,7 +455,7 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> { // // We could remove this hack whenever we decide to drop macOS 10.10 support. if self.tcx.sess.target.target.options.is_like_osx { - assert_eq!(alloc.relocations.len(), 0); + assert_eq!(alloc.relocations().len(), 0); let is_zeroed = { // Treats undefined bytes as if they were defined with the byte value that @@ -490,7 +490,7 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> { section.as_str().as_ptr() as *const _, section.as_str().len() as c_uint, ); - assert!(alloc.relocations.is_empty()); + assert!(alloc.relocations().is_empty()); // The `inspect` method is okay here because we checked relocations, and // because we are doing this access to inspect the final interpreter state (not diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 0f0c422d8d3f4..2cac8bb0c517e 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -287,7 +287,6 @@ impl<'a, Ctx> Snapshot<'a, Ctx> for &'a Allocation fn snapshot(&self, ctx: &'a Ctx) -> Self::Item { let Allocation { - relocations, size, align, mutability, @@ -300,7 +299,9 @@ impl<'a, Ctx> Snapshot<'a, Ctx> for &'a Allocation // 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(); + let relocations = self.relocations(); AllocationSnapshot { bytes, From 823c3b984541b13c2083b7bd8025d8d679a2c13b Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Sat, 31 Aug 2019 17:01:56 +0200 Subject: [PATCH 11/12] Improve documentation around allocation accessors --- src/librustc/mir/interpret/allocation.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 8f47bf9d0fd5a..8e824aeb7916a 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -581,8 +581,10 @@ impl<'tcx, Tag, Extra> Allocation { /// Run-length encoding of the undef mask. /// Used to copy parts of a mask multiple times to another allocation. pub struct AllocationDefinedness { + /// The lengths of ranges that are run-length encoded. ranges: smallvec::SmallVec::<[u64; 1]>, - first: bool, + /// The definedness of the first range. + initial: bool, } /// Transferring the definedness mask to other allocations. @@ -606,9 +608,9 @@ impl Allocation { // where each element toggles the state let mut ranges = smallvec::SmallVec::<[u64; 1]>::new(); - let first = self.undef_mask.get(src.offset); + let initial = self.undef_mask.get(src.offset); let mut cur_len = 1; - let mut cur = first; + let mut cur = initial; for i in 1..size.bytes() { // FIXME: optimize to bitshift the current undef block's bits and read the top bit @@ -623,7 +625,7 @@ impl Allocation { ranges.push(cur_len); - AllocationDefinedness { ranges, first, } + AllocationDefinedness { ranges, initial, } } /// Apply multiple instances of the run-length encoding to the undef_mask. @@ -640,7 +642,7 @@ impl Allocation { self.undef_mask.set_range_inbounds( dest.offset, dest.offset + size * repeat, - defined.first, + defined.initial, ); return; } @@ -648,7 +650,7 @@ impl Allocation { for mut j in 0..repeat { j *= size.bytes(); j += dest.offset.bytes(); - let mut cur = defined.first; + let mut cur = defined.initial; for range in &defined.ranges { let old_j = j; j += range; @@ -725,9 +727,9 @@ impl Allocation { // shift offsets from source allocation to destination allocation offset + dest_offset - src.offset, reloc, - ) + ) }) - ); + ); } AllocationRelocations { @@ -735,6 +737,9 @@ impl Allocation { } } + /// Apply a relocation copy. + /// The affected range, as defined in the parameters to `prepare_relocation_copy` is expected + /// to be clear of relocations. pub fn mark_relocation_range( &mut self, relocations: AllocationRelocations, From f3c435eb780dec5c1d390a3b43cdfb4f1528d70d Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Sat, 31 Aug 2019 21:21:29 +0200 Subject: [PATCH 12/12] Reorder AllocationDefinedness members This improves the clarity of the documentation a bit since they can reference each other when reading the member docs in sequence. --- src/librustc/mir/interpret/allocation.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 8e824aeb7916a..db24c14083768 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -581,10 +581,11 @@ impl<'tcx, Tag, Extra> Allocation { /// Run-length encoding of the undef mask. /// Used to copy parts of a mask multiple times to another allocation. pub struct AllocationDefinedness { - /// The lengths of ranges that are run-length encoded. - ranges: smallvec::SmallVec::<[u64; 1]>, /// The definedness of the first range. initial: bool, + /// The lengths of ranges that are run-length encoded. + /// The definedness of the ranges alternate starting with `initial`. + ranges: smallvec::SmallVec::<[u64; 1]>, } /// Transferring the definedness mask to other allocations.