From 98cd2ad4ea84408ecd9d064f4bd90fe406fb7ddd Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:30:52 +0100 Subject: [PATCH 01/34] Move some methods from `Memory` to `Allocation` --- src/librustc/mir/interpret/allocation.rs | 84 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 84 ------------------------ 2 files changed, 84 insertions(+), 84 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 3250ea266a587..1a88a959b4eb0 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -49,6 +49,90 @@ pub struct Allocation { pub extra: Extra, } +/// Byte accessors +impl<'tcx, Tag, Extra> Allocation { + /// 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_with_undef_and_ptr` instead, + /// + /// This function also guarantees that the resulting pointer will remain stable + /// even when new allocations are pushed to the `HashMap`. `copy_repeatedly` relies + /// on that. + fn get_bytes_internal( + &self, + ptr: Pointer, + size: Size, + align: Align, + check_defined_and_ptr: bool, + ) -> EvalResult<'tcx, &[u8]> { + assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); + self.check_align(ptr.into(), align)?; + self.check_bounds(ptr, size, InboundsCheck::Live)?; + + if check_defined_and_ptr { + self.check_defined(ptr, size)?; + 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)?; + AllocationExtra::memory_read(alloc, ptr, size)?; + + 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(&alloc.bytes[offset..offset + size.bytes() as usize]) + } + + #[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. + /// However, this still checks that there are no relocations on the *edges*. + #[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) + } + + /// 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, + size: Size, + align: Align, + ) -> EvalResult<'tcx, &mut [u8]> { + assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); + self.check_align(ptr.into(), align)?; + self.check_bounds(ptr, size, InboundsCheck::Live)?; + + self.mark_definedness(ptr, size, true)?; + self.clear_relocations(ptr, size)?; + + let alloc = self.get_mut(ptr.alloc_id)?; + AllocationExtra::memory_written(alloc, ptr, size)?; + + 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]) + } +} + pub trait AllocationExtra: ::std::fmt::Debug + Default + Clone { /// Hook for performing extra checks on a memory read access. /// diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 898600d8322d2..759892451bd94 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -609,90 +609,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } -/// Byte accessors -impl<'a, 'mir, 'tcx, M: Machine<'a, '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_with_undef_and_ptr` instead, - /// - /// This function also guarantees that the resulting pointer will remain stable - /// even when new allocations are pushed to the `HashMap`. `copy_repeatedly` relies - /// on that. - fn get_bytes_internal( - &self, - ptr: Pointer, - size: Size, - align: Align, - check_defined_and_ptr: bool, - ) -> EvalResult<'tcx, &[u8]> { - assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_align(ptr.into(), align)?; - self.check_bounds(ptr, size, InboundsCheck::Live)?; - - if check_defined_and_ptr { - self.check_defined(ptr, size)?; - 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)?; - AllocationExtra::memory_read(alloc, ptr, size)?; - - 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(&alloc.bytes[offset..offset + size.bytes() as usize]) - } - - #[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. - /// However, this still checks that there are no relocations on the *edges*. - #[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) - } - - /// 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, - size: Size, - align: Align, - ) -> EvalResult<'tcx, &mut [u8]> { - assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_align(ptr.into(), align)?; - self.check_bounds(ptr, size, InboundsCheck::Live)?; - - self.mark_definedness(ptr, size, true)?; - self.clear_relocations(ptr, size)?; - - let alloc = self.get_mut(ptr.alloc_id)?; - AllocationExtra::memory_written(alloc, ptr, size)?; - - 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]) - } -} - /// Interning (for CTFE) impl<'a, 'mir, 'tcx, M> Memory<'a, 'mir, 'tcx, M> where From d40a7713d3f044dc6f590acfc55f74f44f8295e8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:32:30 +0100 Subject: [PATCH 02/34] Preliminary code adjustment to let the compiler complain about missing methods --- src/librustc/mir/interpret/allocation.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 1a88a959b4eb0..669302852d451 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -60,7 +60,7 @@ impl<'tcx, Tag, Extra> Allocation { /// on that. fn get_bytes_internal( &self, - ptr: Pointer, + ptr: Pointer, size: Size, align: Align, check_defined_and_ptr: bool, @@ -89,7 +89,7 @@ impl<'tcx, Tag, Extra> Allocation { #[inline] fn get_bytes( &self, - ptr: Pointer, + ptr: Pointer, size: Size, align: Align ) -> EvalResult<'tcx, &[u8]> { @@ -101,7 +101,7 @@ impl<'tcx, Tag, Extra> Allocation { #[inline] fn get_bytes_with_undef_and_ptr( &self, - ptr: Pointer, + ptr: Pointer, size: Size, align: Align ) -> EvalResult<'tcx, &[u8]> { @@ -112,7 +112,7 @@ impl<'tcx, Tag, Extra> Allocation { /// so be sure to actually put data there! fn get_bytes_mut( &mut self, - ptr: Pointer, + ptr: Pointer, size: Size, align: Align, ) -> EvalResult<'tcx, &mut [u8]> { From eb30ce8acb6617fbe6182a3b5b9078dea4027a90 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:34:04 +0100 Subject: [PATCH 03/34] Move relocation methods from `Memory` to `Allocation` --- src/librustc/mir/interpret/allocation.rs | 73 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 73 ------------------------ 2 files changed, 73 insertions(+), 73 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 669302852d451..2334ca2f91a9a 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -133,6 +133,79 @@ impl<'tcx, Tag, Extra> Allocation { } } +/// Relocations +impl<'tcx, Tag, Extra> Allocation { + /// Return all relocations overlapping with the given ptr-offset pair. + fn relocations( + &self, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx, &[(Size, (M::PointerTag, 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; // 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) = { + // Find all relocations overlapping the given range. + let relocations = self.relocations(ptr, size)?; + if relocations.is_empty() { + return Ok(()); + } + + (relocations.first().unwrap().0, + relocations.last().unwrap().0 + self.pointer_size()) + }; + let start = ptr.offset; + let end = start + size; + + let alloc = self.get_mut(ptr.alloc_id)?; + + // Mark parts of the outermost relocations as undefined if they partially fall outside the + // given range. + if first < start { + alloc.undef_mask.set_range(first, start, false); + } + if last > end { + alloc.undef_mask.set_range(end, last, false); + } + + // Forget all the relocations. + alloc.relocations.remove_range(first..last); + + Ok(()) + } + + /// Error if there are relocations overlapping with the edges of the + /// given memory range. + #[inline] + fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + self.check_relocations(ptr, Size::ZERO)?; + self.check_relocations(ptr.offset(size, self)?, Size::ZERO)?; + Ok(()) + } +} + pub trait AllocationExtra: ::std::fmt::Debug + Default + Clone { /// Hook for performing extra checks on a memory read access. /// diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 759892451bd94..cb52e595e02be 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -955,79 +955,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } -/// Relocations -impl<'a, 'mir, 'tcx, M: Machine<'a, '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, (M::PointerTag, 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; // 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) = { - // Find all relocations overlapping the given range. - let relocations = self.relocations(ptr, size)?; - if relocations.is_empty() { - return Ok(()); - } - - (relocations.first().unwrap().0, - relocations.last().unwrap().0 + self.pointer_size()) - }; - let start = ptr.offset; - let end = start + size; - - let alloc = self.get_mut(ptr.alloc_id)?; - - // Mark parts of the outermost relocations as undefined if they partially fall outside the - // given range. - if first < start { - alloc.undef_mask.set_range(first, start, false); - } - if last > end { - alloc.undef_mask.set_range(end, last, false); - } - - // Forget all the relocations. - alloc.relocations.remove_range(first..last); - - Ok(()) - } - - /// Error if there are relocations overlapping with the edges of the - /// given memory range. - #[inline] - fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { - self.check_relocations(ptr, Size::ZERO)?; - self.check_relocations(ptr.offset(size, self)?, Size::ZERO)?; - Ok(()) - } -} - /// Undefined bytes impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // FIXME: Add a fast version for the common, nonoverlapping case From d98c46ce57d562ebcfb01ec814ff8d90d47ff7ea Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:35:32 +0100 Subject: [PATCH 04/34] Move undef mask methods from `Memory` to `Allocation` --- src/librustc/mir/interpret/allocation.rs | 33 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 29 --------------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 2334ca2f91a9a..ad4bf415b8de0 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -206,6 +206,39 @@ impl<'tcx, Tag, Extra> Allocation { } } + +/// Undefined bytes +impl<'tcx, Tag, Extra> Allocation { + /// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes` + /// error which will report the first byte which is undefined. + #[inline] + fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + let alloc = self.get(ptr.alloc_id)?; + alloc.undef_mask.is_range_defined( + ptr.offset, + ptr.offset + size, + ).or_else(|idx| err!(ReadUndefBytes(idx))) + } + + pub fn mark_definedness( + &mut self, + ptr: Pointer, + size: Size, + new_state: bool, + ) -> EvalResult<'tcx> { + if size.bytes() == 0 { + return Ok(()); + } + let alloc = self.get_mut(ptr.alloc_id)?; + alloc.undef_mask.set_range( + ptr.offset, + ptr.offset + size, + new_state, + ); + Ok(()) + } +} + pub trait AllocationExtra: ::std::fmt::Debug + Default + Clone { /// Hook for performing extra checks on a memory read access. /// diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index cb52e595e02be..d8ae107a22b56 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -984,33 +984,4 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(()) } - - /// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes` - /// error which will report the first byte which is undefined. - #[inline] - fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { - let alloc = self.get(ptr.alloc_id)?; - alloc.undef_mask.is_range_defined( - ptr.offset, - ptr.offset + size, - ).or_else(|idx| err!(ReadUndefBytes(idx))) - } - - pub fn mark_definedness( - &mut self, - ptr: Pointer, - size: Size, - new_state: bool, - ) -> EvalResult<'tcx> { - if size.bytes() == 0 { - return Ok(()); - } - let alloc = self.get_mut(ptr.alloc_id)?; - alloc.undef_mask.set_range( - ptr.offset, - ptr.offset + size, - new_state, - ); - Ok(()) - } } From 7c9d786e5007004917d73749fad32bc3bff94cce Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:37:54 +0100 Subject: [PATCH 05/34] Move alignment and bounds check from `Memory` to `Allocation` --- src/librustc/mir/interpret/allocation.rs | 45 ++++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 42 ---------------------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index ad4bf415b8de0..b2737ae203fba 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -49,6 +49,51 @@ pub struct Allocation { pub extra: Extra, } +/// Alignment and bounds checks +impl<'tcx, Tag, Extra> Allocation { + /// 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. `check` indicates whether we + /// additionally require the pointer to be pointing to a *live* (still allocated) + /// allocation. + /// If you want to check bounds before doing a memory access, better use `check_bounds`. + pub fn check_bounds_ptr( + &self, + ptr: Pointer, + check: InboundsCheck, + ) -> EvalResult<'tcx> { + let allocation_size = match check { + InboundsCheck::Live => { + let alloc = self.get(ptr.alloc_id)?; + alloc.bytes.len() as u64 + } + InboundsCheck::MaybeDead => { + self.get_size_and_align(ptr.alloc_id).0.bytes() + } + }; + if ptr.offset.bytes() > allocation_size { + return err!(PointerOutOfBounds { + ptr: ptr.erase_tag(), + check, + allocation_size: Size::from_bytes(allocation_size), + }); + } + Ok(()) + } + + /// Check if the memory range beginning at `ptr` and of size `Size` is "in-bounds". + #[inline(always)] + pub fn check_bounds( + &self, + ptr: Pointer, + size: Size, + check: InboundsCheck, + ) -> EvalResult<'tcx> { + // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow) + self.check_bounds_ptr(ptr.offset(size, &*self)?, check) + } +} + /// Byte accessors impl<'tcx, Tag, Extra> Allocation { /// The last argument controls whether we error out when there are undefined diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index d8ae107a22b56..9f8e8fc921af8 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -284,48 +284,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, '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. `check` indicates whether we - /// additionally require the pointer to be pointing to a *live* (still allocated) - /// allocation. - /// If you want to check bounds before doing a memory access, better use `check_bounds`. - pub fn check_bounds_ptr( - &self, - ptr: Pointer, - check: InboundsCheck, - ) -> EvalResult<'tcx> { - let allocation_size = match check { - InboundsCheck::Live => { - let alloc = self.get(ptr.alloc_id)?; - alloc.bytes.len() as u64 - } - InboundsCheck::MaybeDead => { - self.get_size_and_align(ptr.alloc_id).0.bytes() - } - }; - if ptr.offset.bytes() > allocation_size { - return err!(PointerOutOfBounds { - ptr: ptr.erase_tag(), - check, - allocation_size: Size::from_bytes(allocation_size), - }); - } - Ok(()) - } - - /// Check if the memory range beginning at `ptr` and of size `Size` is "in-bounds". - #[inline(always)] - pub fn check_bounds( - &self, - ptr: Pointer, - size: Size, - check: InboundsCheck, - ) -> EvalResult<'tcx> { - // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow) - self.check_bounds_ptr(ptr.offset(size, &*self)?, check) - } } /// Allocation accessors From c392fbbbf16981dc6368de7ea6a8797892221283 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:38:35 +0100 Subject: [PATCH 06/34] Adjust generics to `Allocation` parameters --- src/librustc/mir/interpret/allocation.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index b2737ae203fba..05f9f482d5327 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -59,7 +59,7 @@ impl<'tcx, Tag, Extra> Allocation { /// If you want to check bounds before doing a memory access, better use `check_bounds`. pub fn check_bounds_ptr( &self, - ptr: Pointer, + ptr: Pointer, check: InboundsCheck, ) -> EvalResult<'tcx> { let allocation_size = match check { @@ -85,7 +85,7 @@ impl<'tcx, Tag, Extra> Allocation { #[inline(always)] pub fn check_bounds( &self, - ptr: Pointer, + ptr: Pointer, size: Size, check: InboundsCheck, ) -> EvalResult<'tcx> { @@ -183,9 +183,9 @@ impl<'tcx, Tag, Extra> Allocation { /// Return all relocations overlapping with the given ptr-offset pair. fn relocations( &self, - ptr: Pointer, + ptr: Pointer, size: Size, - ) -> EvalResult<'tcx, &[(Size, (M::PointerTag, AllocId))]> { + ) -> EvalResult<'tcx, &[(Size, (Tag, 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); @@ -195,7 +195,7 @@ impl<'tcx, Tag, Extra> Allocation { /// Check that there ar eno relocations overlapping with the given range. #[inline(always)] - fn check_relocations(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + fn check_relocations(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { if self.relocations(ptr, size)?.len() != 0 { err!(ReadPointerAsBytes) } else { @@ -209,7 +209,7 @@ impl<'tcx, Tag, Extra> Allocation { /// 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> { + 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) = { // Find all relocations overlapping the given range. @@ -244,7 +244,7 @@ impl<'tcx, Tag, Extra> Allocation { /// Error if there are relocations overlapping with the edges of the /// given memory range. #[inline] - fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { self.check_relocations(ptr, Size::ZERO)?; self.check_relocations(ptr.offset(size, self)?, Size::ZERO)?; Ok(()) @@ -257,7 +257,7 @@ impl<'tcx, Tag, Extra> Allocation { /// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes` /// error which will report the first byte which is undefined. #[inline] - fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { + fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { let alloc = self.get(ptr.alloc_id)?; alloc.undef_mask.is_range_defined( ptr.offset, @@ -267,7 +267,7 @@ impl<'tcx, Tag, Extra> Allocation { pub fn mark_definedness( &mut self, - ptr: Pointer, + ptr: Pointer, size: Size, new_state: bool, ) -> EvalResult<'tcx> { From 04210f3e169b31613f70e5de9550a4f46039e7bc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:39:04 +0100 Subject: [PATCH 07/34] Access `self` instead of `alloc` --- src/librustc/mir/interpret/allocation.rs | 44 ++++++++---------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 05f9f482d5327..aba0da32d4544 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -60,21 +60,12 @@ impl<'tcx, Tag, Extra> Allocation { pub fn check_bounds_ptr( &self, ptr: Pointer, - check: InboundsCheck, ) -> EvalResult<'tcx> { - let allocation_size = match check { - InboundsCheck::Live => { - let alloc = self.get(ptr.alloc_id)?; - alloc.bytes.len() as u64 - } - InboundsCheck::MaybeDead => { - self.get_size_and_align(ptr.alloc_id).0.bytes() - } - }; + let allocation_size = self.bytes.len() as u64; if ptr.offset.bytes() > allocation_size { return err!(PointerOutOfBounds { ptr: ptr.erase_tag(), - check, + check: InboundsCheck::Live, allocation_size: Size::from_bytes(allocation_size), }); } @@ -87,15 +78,14 @@ impl<'tcx, Tag, Extra> Allocation { &self, ptr: Pointer, size: Size, - check: InboundsCheck, ) -> EvalResult<'tcx> { // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow) - self.check_bounds_ptr(ptr.offset(size, &*self)?, check) + self.check_bounds_ptr(ptr.offset(size, &*self)?) } } /// Byte accessors -impl<'tcx, Tag, Extra> Allocation { +impl<'tcx, Tag, Extra: AllocationExtra> Allocation { /// 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_with_undef_and_ptr` instead, @@ -122,13 +112,12 @@ impl<'tcx, Tag, Extra> Allocation { self.check_relocation_edges(ptr, size)?; } - let alloc = self.get(ptr.alloc_id)?; - AllocationExtra::memory_read(alloc, ptr, size)?; + AllocationExtra::memory_read(self, ptr, size)?; 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(&alloc.bytes[offset..offset + size.bytes() as usize]) + Ok(&self.bytes[offset..offset + size.bytes() as usize]) } #[inline] @@ -168,13 +157,12 @@ impl<'tcx, Tag, Extra> Allocation { self.mark_definedness(ptr, size, true)?; self.clear_relocations(ptr, size)?; - let alloc = self.get_mut(ptr.alloc_id)?; - AllocationExtra::memory_written(alloc, ptr, size)?; + AllocationExtra::memory_written(self, ptr, size)?; 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]) + Ok(&mut self.bytes[offset..offset + size.bytes() as usize]) } } @@ -190,7 +178,7 @@ impl<'tcx, Tag, Extra> Allocation { // the beginning of this range. let start = ptr.offset.bytes().saturating_sub(self.pointer_size().bytes() - 1); let end = ptr.offset + size; // this does overflow checking - Ok(self.get(ptr.alloc_id)?.relocations.range(Size::from_bytes(start)..end)) + Ok(self.relocations.range(Size::from_bytes(start)..end)) } /// Check that there ar eno relocations overlapping with the given range. @@ -224,19 +212,17 @@ impl<'tcx, Tag, Extra> Allocation { let start = ptr.offset; let end = start + size; - let alloc = self.get_mut(ptr.alloc_id)?; - // Mark parts of the outermost relocations as undefined if they partially fall outside the // given range. if first < start { - alloc.undef_mask.set_range(first, start, false); + self.undef_mask.set_range(first, start, false); } if last > end { - alloc.undef_mask.set_range(end, last, false); + self.undef_mask.set_range(end, last, false); } // Forget all the relocations. - alloc.relocations.remove_range(first..last); + self.relocations.remove_range(first..last); Ok(()) } @@ -258,8 +244,7 @@ impl<'tcx, Tag, Extra> Allocation { /// error which will report the first byte which is undefined. #[inline] fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { - let alloc = self.get(ptr.alloc_id)?; - alloc.undef_mask.is_range_defined( + self.undef_mask.is_range_defined( ptr.offset, ptr.offset + size, ).or_else(|idx| err!(ReadUndefBytes(idx))) @@ -274,8 +259,7 @@ impl<'tcx, Tag, Extra> Allocation { if size.bytes() == 0 { return Ok(()); } - let alloc = self.get_mut(ptr.alloc_id)?; - alloc.undef_mask.set_range( + self.undef_mask.set_range( ptr.offset, ptr.offset + size, new_state, From ad11856431ea3c0808951eed9fcbefbd82590ef2 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 08:56:41 +0100 Subject: [PATCH 08/34] Fiddle a `HasDataLayout` through the allocation methods --- src/librustc/mir/interpret/allocation.rs | 60 ++++++++++++++++-------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index aba0da32d4544..1856232573c2a 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -18,6 +18,7 @@ use std::iter; use mir; use std::ops::{Deref, DerefMut}; use rustc_data_structures::sorted_map::SortedMap; +use rustc_target::abi::HasDataLayout; /// Used by `check_bounds` to indicate whether the pointer needs to be just inbounds /// or also inbounds of a *live* allocation. @@ -76,16 +77,17 @@ impl<'tcx, Tag, Extra> Allocation { #[inline(always)] pub fn check_bounds( &self, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, ) -> EvalResult<'tcx> { // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow) - self.check_bounds_ptr(ptr.offset(size, &*self)?) + self.check_bounds_ptr(ptr.offset(size, cx)?) } } /// Byte accessors -impl<'tcx, Tag, Extra: AllocationExtra> Allocation { +impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// 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_with_undef_and_ptr` instead, @@ -95,6 +97,7 @@ impl<'tcx, Tag, Extra: AllocationExtra> Allocation { /// on that. fn get_bytes_internal( &self, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, align: Align, @@ -102,14 +105,14 @@ impl<'tcx, Tag, Extra: AllocationExtra> Allocation { ) -> EvalResult<'tcx, &[u8]> { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); self.check_align(ptr.into(), align)?; - self.check_bounds(ptr, size, InboundsCheck::Live)?; + self.check_bounds(cx, ptr, size)?; if check_defined_and_ptr { self.check_defined(ptr, size)?; - self.check_relocations(ptr, size)?; + self.check_relocations(cx, ptr, size)?; } else { // We still don't want relocations on the *edges* - self.check_relocation_edges(ptr, size)?; + self.check_relocation_edges(cx, ptr, size)?; } AllocationExtra::memory_read(self, ptr, size)?; @@ -123,11 +126,12 @@ impl<'tcx, Tag, Extra: AllocationExtra> Allocation { #[inline] fn get_bytes( &self, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, align: Align ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(ptr, size, align, true) + self.get_bytes_internal(cx, ptr, size, align, true) } /// It is the caller's responsibility to handle undefined and pointer bytes. @@ -135,27 +139,29 @@ impl<'tcx, Tag, Extra: AllocationExtra> Allocation { #[inline] fn get_bytes_with_undef_and_ptr( &self, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, align: Align ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(ptr, size, align, false) + self.get_bytes_internal(cx, 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, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, align: Align, ) -> EvalResult<'tcx, &mut [u8]> { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); self.check_align(ptr.into(), align)?; - self.check_bounds(ptr, size, InboundsCheck::Live)?; + self.check_bounds(cx, ptr, size)?; self.mark_definedness(ptr, size, true)?; - self.clear_relocations(ptr, size)?; + self.clear_relocations(cx, ptr, size)?; AllocationExtra::memory_written(self, ptr, size)?; @@ -167,24 +173,30 @@ impl<'tcx, Tag, Extra: AllocationExtra> Allocation { } /// Relocations -impl<'tcx, Tag, Extra> Allocation { +impl<'tcx, Tag: Copy, Extra> Allocation { /// Return all relocations overlapping with the given ptr-offset pair. fn relocations( &self, + cx: &impl HasDataLayout, ptr: Pointer, size: Size, ) -> EvalResult<'tcx, &[(Size, (Tag, 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 start = ptr.offset.bytes().saturating_sub(cx.data_layout().pointer_size.bytes() - 1); let end = ptr.offset + size; // this does overflow checking Ok(self.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 { + fn check_relocations( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { + if self.relocations(cx, ptr, size)?.len() != 0 { err!(ReadPointerAsBytes) } else { Ok(()) @@ -197,17 +209,22 @@ impl<'tcx, Tag, Extra> Allocation { /// 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> { + fn clear_relocations( + &mut self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { // 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(ptr, size)?; + let relocations = self.relocations(cx, ptr, size)?; if relocations.is_empty() { return Ok(()); } (relocations.first().unwrap().0, - relocations.last().unwrap().0 + self.pointer_size()) + relocations.last().unwrap().0 + cx.data_layout().pointer_size) }; let start = ptr.offset; let end = start + size; @@ -230,9 +247,14 @@ impl<'tcx, Tag, Extra> Allocation { /// Error if there are relocations overlapping with the edges of the /// given memory range. #[inline] - fn check_relocation_edges(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> { - self.check_relocations(ptr, Size::ZERO)?; - self.check_relocations(ptr.offset(size, self)?, Size::ZERO)?; + fn check_relocation_edges( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx> { + self.check_relocations(cx, ptr, Size::ZERO)?; + self.check_relocations(cx, ptr.offset(size, cx)?, Size::ZERO)?; Ok(()) } } From 9ecde5712ec5b393c7a0bb0074c957446ed9886b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 09:00:41 +0100 Subject: [PATCH 09/34] Move some byte and scalar accessors from `Memory` to `Allocation` --- src/librustc/mir/interpret/allocation.rs | 200 +++++++++++++++++++++++ src/librustc_mir/interpret/memory.rs | 197 ---------------------- 2 files changed, 200 insertions(+), 197 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 1856232573c2a..cba51981021a9 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -86,6 +86,206 @@ impl<'tcx, Tag, Extra> Allocation { } } +/// Reading and writing +impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { + pub fn read_c_str(&self, ptr: Pointer) -> EvalResult<'tcx, &[u8]> { + let alloc = self.get(ptr.alloc_id)?; + assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); + let offset = ptr.offset.bytes() as usize; + match alloc.bytes[offset..].iter().position(|&c| c == 0) { + Some(size) => { + let p1 = Size::from_bytes((size + 1) as u64); + self.check_relocations(ptr, p1)?; + self.check_defined(ptr, p1)?; + Ok(&alloc.bytes[offset..offset + size]) + } + None => err!(UnterminatedCString(ptr.erase_tag())), + } + } + + pub fn check_bytes( + &self, + ptr: Scalar, + size: Size, + allow_ptr_and_undef: bool, + ) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + let align = Align::from_bytes(1).unwrap(); + if size.bytes() == 0 { + self.check_align(ptr, align)?; + return Ok(()); + } + let ptr = ptr.to_ptr()?; + // Check bounds, align and relocations on the edges + self.get_bytes_with_undef_and_ptr(ptr, size, align)?; + // Check undef and ptr + if !allow_ptr_and_undef { + self.check_defined(ptr, size)?; + self.check_relocations(ptr, size)?; + } + Ok(()) + } + + pub fn read_bytes(&self, ptr: Scalar, size: Size) -> EvalResult<'tcx, &[u8]> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + let align = Align::from_bytes(1).unwrap(); + if size.bytes() == 0 { + self.check_align(ptr, align)?; + return Ok(&[]); + } + self.get_bytes(ptr.to_ptr()?, size, align) + } + + pub fn write_bytes(&mut self, ptr: Scalar, src: &[u8]) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + let align = Align::from_bytes(1).unwrap(); + if src.is_empty() { + self.check_align(ptr, align)?; + return Ok(()); + } + let bytes = self.get_bytes_mut(ptr.to_ptr()?, Size::from_bytes(src.len() as u64), align)?; + bytes.clone_from_slice(src); + Ok(()) + } + + pub fn write_repeat( + &mut self, + ptr: Scalar, + val: u8, + count: Size + ) -> EvalResult<'tcx> { + // Empty accesses don't need to be valid pointers, but they should still be non-NULL + let align = Align::from_bytes(1).unwrap(); + if count.bytes() == 0 { + self.check_align(ptr, align)?; + return Ok(()); + } + let bytes = self.get_bytes_mut(ptr.to_ptr()?, count, align)?; + for b in bytes { + *b = val; + } + Ok(()) + } + + /// Read a *non-ZST* scalar + pub fn read_scalar( + &self, + ptr: Pointer, + ptr_align: Align, + size: Size + ) -> EvalResult<'tcx, ScalarMaybeUndef> { + // 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)) + )?; + // 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() { + // this inflates undefined bytes to the entire scalar, even if only a few + // bytes are undefined + return Ok(ScalarMaybeUndef::Undef); + } + // Now we do the actual reading + let bits = read_target_uint(self.tcx.data_layout.endian, bytes).unwrap(); + // See if we got a pointer + if size != self.pointer_size() { + // *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) { + Some(&(tag, alloc_id)) => { + let ptr = Pointer::new_with_tag(alloc_id, Size::from_bytes(bits as u64), tag); + return Ok(ScalarMaybeUndef::Scalar(ptr.into())) + } + None => {}, + } + } + // We don't. Just return the bits. + Ok(ScalarMaybeUndef::Scalar(Scalar::from_uint(bits, size))) + } + + pub fn read_ptr_sized( + &self, + ptr: Pointer, + ptr_align: Align + ) -> EvalResult<'tcx, ScalarMaybeUndef> { + self.read_scalar(ptr, ptr_align, self.pointer_size()) + } + + /// Write a *non-ZST* scalar + pub fn write_scalar( + &mut self, + ptr: Pointer, + ptr_align: Align, + val: ScalarMaybeUndef, + type_size: Size, + ) -> EvalResult<'tcx> { + let val = match val { + ScalarMaybeUndef::Scalar(scalar) => scalar, + ScalarMaybeUndef::Undef => return self.mark_definedness(ptr, type_size, false), + }; + + let bytes = match val { + Scalar::Ptr(val) => { + assert_eq!(type_size, self.pointer_size()); + val.offset.bytes() as u128 + } + + Scalar::Bits { bits, size } => { + assert_eq!(size as u64, type_size.bytes()); + debug_assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, + "Unexpected value of size {} when writing to memory", size); + bits + }, + }; + + { + // 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(endian, dst, bytes).unwrap(); + } + + // See if we have to also write a relocation + match val { + Scalar::Ptr(val) => { + self.get_mut(ptr.alloc_id)?.relocations.insert( + ptr.offset, + (val.tag, val.alloc_id), + ); + } + _ => {} + } + + Ok(()) + } + + 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) + } + + fn int_align(&self, size: Size) -> Align { + // We assume pointer-sized integers have the same alignment as pointers. + // We also assume signed and unsigned integers of the same size have the same alignment. + let ity = match size.bytes() { + 1 => layout::I8, + 2 => layout::I16, + 4 => layout::I32, + 8 => layout::I64, + 16 => layout::I128, + _ => bug!("bad integer size: {}", size.bytes()), + }; + ity.align(self).abi + } +} + /// Byte accessors impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// The last argument controls whether we error out when there are undefined diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 9f8e8fc921af8..69789637927d9 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -714,203 +714,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(()) } - - pub fn read_c_str(&self, ptr: Pointer) -> EvalResult<'tcx, &[u8]> { - let alloc = self.get(ptr.alloc_id)?; - assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); - let offset = ptr.offset.bytes() as usize; - match alloc.bytes[offset..].iter().position(|&c| c == 0) { - Some(size) => { - let p1 = Size::from_bytes((size + 1) as u64); - self.check_relocations(ptr, p1)?; - self.check_defined(ptr, p1)?; - Ok(&alloc.bytes[offset..offset + size]) - } - None => err!(UnterminatedCString(ptr.erase_tag())), - } - } - - pub fn check_bytes( - &self, - ptr: Scalar, - size: Size, - allow_ptr_and_undef: bool, - ) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - let align = Align::from_bytes(1).unwrap(); - if size.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(()); - } - let ptr = ptr.to_ptr()?; - // Check bounds, align and relocations on the edges - self.get_bytes_with_undef_and_ptr(ptr, size, align)?; - // Check undef and ptr - if !allow_ptr_and_undef { - self.check_defined(ptr, size)?; - self.check_relocations(ptr, size)?; - } - Ok(()) - } - - pub fn read_bytes(&self, ptr: Scalar, size: Size) -> EvalResult<'tcx, &[u8]> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - let align = Align::from_bytes(1).unwrap(); - if size.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(&[]); - } - self.get_bytes(ptr.to_ptr()?, size, align) - } - - pub fn write_bytes(&mut self, ptr: Scalar, src: &[u8]) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - let align = Align::from_bytes(1).unwrap(); - if src.is_empty() { - self.check_align(ptr, align)?; - return Ok(()); - } - let bytes = self.get_bytes_mut(ptr.to_ptr()?, Size::from_bytes(src.len() as u64), align)?; - bytes.clone_from_slice(src); - Ok(()) - } - - pub fn write_repeat( - &mut self, - ptr: Scalar, - val: u8, - count: Size - ) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL - let align = Align::from_bytes(1).unwrap(); - if count.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(()); - } - let bytes = self.get_bytes_mut(ptr.to_ptr()?, count, align)?; - for b in bytes { - *b = val; - } - Ok(()) - } - - /// Read a *non-ZST* scalar - pub fn read_scalar( - &self, - ptr: Pointer, - ptr_align: Align, - size: Size - ) -> EvalResult<'tcx, ScalarMaybeUndef> { - // 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)) - )?; - // 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() { - // this inflates undefined bytes to the entire scalar, even if only a few - // bytes are undefined - return Ok(ScalarMaybeUndef::Undef); - } - // Now we do the actual reading - let bits = read_target_uint(self.tcx.data_layout.endian, bytes).unwrap(); - // See if we got a pointer - if size != self.pointer_size() { - // *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) { - Some(&(tag, alloc_id)) => { - let ptr = Pointer::new_with_tag(alloc_id, Size::from_bytes(bits as u64), tag); - return Ok(ScalarMaybeUndef::Scalar(ptr.into())) - } - None => {}, - } - } - // We don't. Just return the bits. - Ok(ScalarMaybeUndef::Scalar(Scalar::from_uint(bits, size))) - } - - pub fn read_ptr_sized( - &self, - ptr: Pointer, - ptr_align: Align - ) -> EvalResult<'tcx, ScalarMaybeUndef> { - self.read_scalar(ptr, ptr_align, self.pointer_size()) - } - - /// Write a *non-ZST* scalar - pub fn write_scalar( - &mut self, - ptr: Pointer, - ptr_align: Align, - val: ScalarMaybeUndef, - type_size: Size, - ) -> EvalResult<'tcx> { - let val = match val { - ScalarMaybeUndef::Scalar(scalar) => scalar, - ScalarMaybeUndef::Undef => return self.mark_definedness(ptr, type_size, false), - }; - - let bytes = match val { - Scalar::Ptr(val) => { - assert_eq!(type_size, self.pointer_size()); - val.offset.bytes() as u128 - } - - Scalar::Bits { bits, size } => { - assert_eq!(size as u64, type_size.bytes()); - debug_assert_eq!(truncate(bits, Size::from_bytes(size.into())), bits, - "Unexpected value of size {} when writing to memory", size); - bits - }, - }; - - { - // 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(endian, dst, bytes).unwrap(); - } - - // See if we have to also write a relocation - match val { - Scalar::Ptr(val) => { - self.get_mut(ptr.alloc_id)?.relocations.insert( - ptr.offset, - (val.tag, val.alloc_id), - ); - } - _ => {} - } - - Ok(()) - } - - 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) - } - - fn int_align(&self, size: Size) -> Align { - // We assume pointer-sized integers have the same alignment as pointers. - // We also assume signed and unsigned integers of the same size have the same alignment. - let ity = match size.bytes() { - 1 => layout::I8, - 2 => layout::I16, - 4 => layout::I32, - 8 => layout::I64, - 16 => layout::I128, - _ => bug!("bad integer size: {}", size.bytes()), - }; - ity.align(self).abi - } } /// Undefined bytes From 07e7804110c77d572012f913a341523bdbaac4dd Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 12 Nov 2018 13:26:53 +0100 Subject: [PATCH 10/34] Adjust rustc_mir::interpret to changes in `Allocation`/`Memory` methods --- src/librustc/mir/interpret/allocation.rs | 112 ++++++++++++++--------- src/librustc_mir/interpret/memory.rs | 16 +++- src/librustc_mir/interpret/operand.rs | 17 +++- src/librustc_mir/interpret/place.rs | 15 ++- src/librustc_mir/interpret/terminator.rs | 3 +- src/librustc_mir/interpret/traits.rs | 31 +++++-- src/librustc_mir/interpret/validity.rs | 28 ++++-- 7 files changed, 146 insertions(+), 76 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index cba51981021a9..6ef7a5a266d37 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -10,9 +10,12 @@ //! The virtual memory representation of the MIR interpreter -use super::{Pointer, EvalResult, AllocId}; +use super::{ + Pointer, EvalResult, AllocId, ScalarMaybeUndef, write_target_uint, read_target_uint, Scalar, + truncate, +}; -use ty::layout::{Size, Align}; +use ty::layout::{self, Size, Align}; use syntax::ast::Mutability; use std::iter; use mir; @@ -88,16 +91,19 @@ impl<'tcx, Tag, Extra> Allocation { /// Reading and writing impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { - pub fn read_c_str(&self, ptr: Pointer) -> EvalResult<'tcx, &[u8]> { - let alloc = self.get(ptr.alloc_id)?; + pub fn read_c_str( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + ) -> EvalResult<'tcx, &[u8]> { assert_eq!(ptr.offset.bytes() as usize as u64, ptr.offset.bytes()); let offset = ptr.offset.bytes() as usize; - match alloc.bytes[offset..].iter().position(|&c| c == 0) { + match self.bytes[offset..].iter().position(|&c| c == 0) { Some(size) => { let p1 = Size::from_bytes((size + 1) as u64); - self.check_relocations(ptr, p1)?; + self.check_relocations(cx, ptr, p1)?; self.check_defined(ptr, p1)?; - Ok(&alloc.bytes[offset..offset + size]) + Ok(&self.bytes[offset..offset + size]) } None => err!(UnterminatedCString(ptr.erase_tag())), } @@ -105,7 +111,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { pub fn check_bytes( &self, - ptr: Scalar, + cx: &impl HasDataLayout, + ptr: Pointer, size: Size, allow_ptr_and_undef: bool, ) -> EvalResult<'tcx> { @@ -115,42 +122,54 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { self.check_align(ptr, align)?; return Ok(()); } - let ptr = ptr.to_ptr()?; // Check bounds, align and relocations on the edges - self.get_bytes_with_undef_and_ptr(ptr, size, align)?; + self.get_bytes_with_undef_and_ptr(cx, ptr, size, align)?; // Check undef and ptr if !allow_ptr_and_undef { self.check_defined(ptr, size)?; - self.check_relocations(ptr, size)?; + self.check_relocations(cx, ptr, size)?; } Ok(()) } - pub fn read_bytes(&self, ptr: Scalar, size: Size) -> EvalResult<'tcx, &[u8]> { + pub fn read_bytes( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + ) -> EvalResult<'tcx, &[u8]> { // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); if size.bytes() == 0 { self.check_align(ptr, align)?; return Ok(&[]); } - self.get_bytes(ptr.to_ptr()?, size, align) + self.get_bytes(cx, ptr, size, align) } - pub fn write_bytes(&mut self, ptr: Scalar, src: &[u8]) -> EvalResult<'tcx> { + pub fn write_bytes( + &mut self, + cx: &impl HasDataLayout, + ptr: Pointer, + src: &[u8], + ) -> EvalResult<'tcx> { // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); if src.is_empty() { self.check_align(ptr, align)?; return Ok(()); } - let bytes = self.get_bytes_mut(ptr.to_ptr()?, Size::from_bytes(src.len() as u64), align)?; + let bytes = self.get_bytes_mut( + cx, ptr, Size::from_bytes(src.len() as u64), align, + )?; bytes.clone_from_slice(src); Ok(()) } pub fn write_repeat( &mut self, - ptr: Scalar, + cx: &impl HasDataLayout, + ptr: Pointer, val: u8, count: Size ) -> EvalResult<'tcx> { @@ -160,7 +179,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { self.check_align(ptr, align)?; return Ok(()); } - let bytes = self.get_bytes_mut(ptr.to_ptr()?, count, align)?; + let bytes = self.get_bytes_mut(cx, ptr, count, align)?; for b in bytes { *b = val; } @@ -170,13 +189,14 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Read a *non-ZST* scalar pub fn read_scalar( &self, - ptr: Pointer, + cx: &impl HasDataLayout, + ptr: Pointer, ptr_align: Align, size: Size - ) -> EvalResult<'tcx, ScalarMaybeUndef> { + ) -> EvalResult<'tcx, ScalarMaybeUndef> { // 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)) + cx, ptr, size, ptr_align.min(self.int_align(cx, size)) )?; // Undef check happens *after* we established that the alignment is correct. // We must not return Ok() for unaligned pointers! @@ -186,14 +206,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { return Ok(ScalarMaybeUndef::Undef); } // Now we do the actual reading - let bits = read_target_uint(self.tcx.data_layout.endian, bytes).unwrap(); + let bits = read_target_uint(cx.data_layout().endian, bytes).unwrap(); // See if we got a pointer - if size != self.pointer_size() { + if size != cx.data_layout().pointer_size { // *Now* better make sure that the inside also is free of relocations. - self.check_relocations(ptr, size)?; + self.check_relocations(cx, ptr, size)?; } else { - let alloc = self.get(ptr.alloc_id)?; - match alloc.relocations.get(&ptr.offset) { + match self.relocations.get(&ptr.offset) { Some(&(tag, alloc_id)) => { let ptr = Pointer::new_with_tag(alloc_id, Size::from_bytes(bits as u64), tag); return Ok(ScalarMaybeUndef::Scalar(ptr.into())) @@ -207,18 +226,20 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { pub fn read_ptr_sized( &self, - ptr: Pointer, + cx: &impl HasDataLayout, + ptr: Pointer, ptr_align: Align - ) -> EvalResult<'tcx, ScalarMaybeUndef> { - self.read_scalar(ptr, ptr_align, self.pointer_size()) + ) -> EvalResult<'tcx, ScalarMaybeUndef> { + self.read_scalar(cx, ptr, ptr_align, cx.data_layout().pointer_size) } /// Write a *non-ZST* scalar pub fn write_scalar( &mut self, - ptr: Pointer, + cx: &impl HasDataLayout, + ptr: Pointer, ptr_align: Align, - val: ScalarMaybeUndef, + val: ScalarMaybeUndef, type_size: Size, ) -> EvalResult<'tcx> { let val = match val { @@ -228,7 +249,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { let bytes = match val { Scalar::Ptr(val) => { - assert_eq!(type_size, self.pointer_size()); + assert_eq!(type_size, cx.data_layout().pointer_size); val.offset.bytes() as u128 } @@ -242,15 +263,15 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { { // get_bytes_mut checks alignment - let endian = self.tcx.data_layout.endian; - let dst = self.get_bytes_mut(ptr, type_size, ptr_align)?; + let endian = cx.data_layout().endian; + let dst = self.get_bytes_mut(cx, ptr, type_size, ptr_align)?; write_target_uint(endian, dst, bytes).unwrap(); } // See if we have to also write a relocation match val { Scalar::Ptr(val) => { - self.get_mut(ptr.alloc_id)?.relocations.insert( + self.relocations.insert( ptr.offset, (val.tag, val.alloc_id), ); @@ -263,15 +284,20 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { pub fn write_ptr_sized( &mut self, - ptr: Pointer, + cx: &impl HasDataLayout, + ptr: Pointer, ptr_align: Align, - val: ScalarMaybeUndef + val: ScalarMaybeUndef ) -> EvalResult<'tcx> { - let ptr_size = self.pointer_size(); - self.write_scalar(ptr.into(), ptr_align, val, ptr_size) + let ptr_size = cx.data_layout().pointer_size; + self.write_scalar(cx, ptr.into(), ptr_align, val, ptr_size) } - fn int_align(&self, size: Size) -> Align { + fn int_align( + &self, + cx: &impl HasDataLayout, + size: Size, + ) -> Align { // We assume pointer-sized integers have the same alignment as pointers. // We also assume signed and unsigned integers of the same size have the same alignment. let ity = match size.bytes() { @@ -282,7 +308,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { 16 => layout::I128, _ => bug!("bad integer size: {}", size.bytes()), }; - ity.align(self).abi + ity.align(cx).abi } } @@ -337,7 +363,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// It is the caller's responsibility to handle undefined and pointer bytes. /// However, this still checks that there are no relocations on the *edges*. #[inline] - fn get_bytes_with_undef_and_ptr( + pub fn get_bytes_with_undef_and_ptr( &self, cx: &impl HasDataLayout, ptr: Pointer, @@ -349,7 +375,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Just calling this already marks everything as defined and removes relocations, /// so be sure to actually put data there! - fn get_bytes_mut( + pub fn get_bytes_mut( &mut self, cx: &impl HasDataLayout, ptr: Pointer, @@ -375,7 +401,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Relocations impl<'tcx, Tag: Copy, Extra> Allocation { /// Return all relocations overlapping with the given ptr-offset pair. - fn relocations( + pub fn relocations( &self, cx: &impl HasDataLayout, ptr: Pointer, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 69789637927d9..3119d9ed0ffa3 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -21,7 +21,7 @@ use std::ptr; use std::borrow::Cow; use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; -use rustc::ty::layout::{self, Align, TargetDataLayout, Size, HasDataLayout}; +use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout}; pub use rustc::mir::interpret::{truncate, write_target_uint, read_target_uint}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; @@ -30,7 +30,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InboundsCheck, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, - Machine, AllocMap, MayLeak, ScalarMaybeUndef, ErrorHandled, + Machine, AllocMap, MayLeak, ErrorHandled, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -655,7 +655,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, '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.relocations(src, size)?; + let relocations = self.get(src.alloc_id)?.relocations(self, src, size)?; let mut new_relocations = Vec::with_capacity(relocations.len() * (length as usize)); for i in 0..length { new_relocations.extend( @@ -671,9 +671,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { new_relocations }; + let tcx = self.tcx.tcx; + // 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(); + let src_bytes = self.get(src.alloc_id)? + .get_bytes_with_undef_and_ptr(&tcx, src, size, src_align)? + .as_ptr(); + let dest_bytes = self.get_mut(dest.alloc_id)? + .get_bytes_mut(&tcx, dest, size * length, dest_align)? + .as_mut_ptr(); // SAFE: The above indexing would have panicked if there weren't at least `size` bytes // behind `src` and `dest`. Also, we use the overlapping-safe `ptr::copy` if `src` and diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 8238d580022a8..3f5f0ebed72d0 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -278,7 +278,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let ptr = ptr.to_ptr()?; match mplace.layout.abi { layout::Abi::Scalar(..) => { - let scalar = self.memory.read_scalar(ptr, ptr_align, mplace.layout.size)?; + let scalar = self.memory + .get(ptr.alloc_id)? + .read_scalar(self, ptr, ptr_align, mplace.layout.size)?; Ok(Some(Immediate::Scalar(scalar))) } layout::Abi::ScalarPair(ref a, ref b) => { @@ -288,8 +290,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let b_offset = a_size.align_to(b.align(self).abi); assert!(b_offset.bytes() > 0); // we later use the offset to test which field to use let b_ptr = ptr.offset(b_offset, self)?.into(); - let a_val = self.memory.read_scalar(a_ptr, ptr_align, a_size)?; - let b_val = self.memory.read_scalar(b_ptr, ptr_align, b_size)?; + let a_val = self.memory + .get(ptr.alloc_id)? + .read_scalar(self, a_ptr, ptr_align, a_size)?; + let b_val = self.memory + .get(ptr.alloc_id)? + .read_scalar(self, b_ptr, ptr_align, b_size)?; Ok(Some(Immediate::ScalarPair(a_val, b_val))) } _ => Ok(None), @@ -345,7 +351,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, &str> { let len = mplace.len(self)?; - let bytes = self.memory.read_bytes(mplace.ptr, Size::from_bytes(len as u64))?; + let ptr = mplace.ptr.to_ptr()?; + let bytes = self.memory + .get(ptr.alloc_id)? + .read_bytes(self, ptr, Size::from_bytes(len as u64))?; let str = ::std::str::from_utf8(bytes) .map_err(|err| EvalErrorKind::ValidationFailure(err.to_string()))?; Ok(str) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 7ef3dd5f7201e..e2b6c00ba382c 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -718,6 +718,7 @@ where } let ptr = ptr.to_ptr()?; + let tcx = &*self.tcx; // FIXME: We should check that there are dest.layout.size many bytes available in // memory. The code below is not sufficient, with enough padding it might not // cover all the bytes! @@ -729,8 +730,8 @@ where dest.layout) } - self.memory.write_scalar( - ptr, ptr_align.min(dest.layout.align.abi), scalar, dest.layout.size + self.memory.get_mut(ptr.alloc_id)?.write_scalar( + tcx, ptr, ptr_align.min(dest.layout.align.abi), scalar, dest.layout.size ) } Immediate::ScalarPair(a_val, b_val) => { @@ -742,14 +743,18 @@ where let (a_size, b_size) = (a.size(self), b.size(self)); let (a_align, b_align) = (a.align(self).abi, b.align(self).abi); let b_offset = a_size.align_to(b_align); - let b_ptr = ptr.offset(b_offset, self)?.into(); + let b_ptr = ptr.offset(b_offset, self)?; // It is tempting to verify `b_offset` against `layout.fields.offset(1)`, // but that does not work: We could be a newtype around a pair, then the // fields do not match the `ScalarPair` components. - self.memory.write_scalar(ptr, ptr_align.min(a_align), a_val, a_size)?; - self.memory.write_scalar(b_ptr, ptr_align.min(b_align), b_val, b_size) + self.memory + .get_mut(ptr.alloc_id)? + .write_scalar(tcx, ptr, ptr_align.min(a_align), a_val, a_size)?; + self.memory + .get_mut(b_ptr.alloc_id)? + .write_scalar(tcx, b_ptr, ptr_align.min(b_align), b_val, b_size) } } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index fd17a4a71295b..9e59611125d92 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -404,7 +404,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let ptr_align = self.tcx.data_layout.pointer_align.abi; let ptr = self.deref_operand(args[0])?; let vtable = ptr.vtable()?; - let fn_ptr = self.memory.read_ptr_sized( + let fn_ptr = self.memory.get(vtable.alloc_id)?.read_ptr_sized( + self, vtable.offset(ptr_size * (idx as u64 + 3), self)?, ptr_align )?.to_ptr()?; diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index f11fd45b753fc..8e39574c01e98 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -55,23 +55,31 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ptr_align, MemoryKind::Vtable, )?.with_default_tag(); + let tcx = &*self.tcx; - let drop = ::monomorphize::resolve_drop_in_place(*self.tcx, ty); + let drop = ::monomorphize::resolve_drop_in_place(*tcx, ty); let drop = self.memory.create_fn_alloc(drop).with_default_tag(); - self.memory.write_ptr_sized(vtable, ptr_align, Scalar::Ptr(drop).into())?; + self.memory + .get_mut(vtable.alloc_id)? + .write_ptr_sized(tcx, 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::from_uint(size, ptr_size).into())?; + self.memory + .get_mut(size_ptr.alloc_id)? + .write_ptr_sized(tcx, 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::from_uint(align, ptr_size).into())?; + self.memory + .get_mut(align_ptr.alloc_id)? + .write_ptr_sized(tcx, 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 { let instance = self.resolve(def_id, substs)?; let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag(); let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?; - self.memory.write_ptr_sized(method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?; + self.memory + .get_mut(method_ptr.alloc_id)? + .write_ptr_sized(tcx, method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?; } } @@ -88,7 +96,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ) -> EvalResult<'tcx, (ty::Instance<'tcx>, ty::Ty<'tcx>)> { // we don't care about the pointee type, we just want a pointer let pointer_align = self.tcx.data_layout.pointer_align.abi; - let drop_fn = self.memory.read_ptr_sized(vtable, pointer_align)?.to_ptr()?; + let drop_fn = self.memory + .get(vtable.alloc_id)? + .read_ptr_sized(self, vtable, pointer_align)? + .to_ptr()?; let drop_instance = self.memory.get_fn(drop_fn)?; trace!("Found drop fn: {:?}", drop_instance); let fn_sig = drop_instance.ty(*self.tcx).fn_sig(*self.tcx); @@ -104,9 +115,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ) -> EvalResult<'tcx, (Size, Align)> { let pointer_size = self.pointer_size(); let pointer_align = self.tcx.data_layout.pointer_align.abi; - let size = self.memory.read_ptr_sized(vtable.offset(pointer_size, self)?,pointer_align)? + let alloc = self.memory.get(vtable.alloc_id)?; + let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?, pointer_align)? .to_bits(pointer_size)? as u64; - let align = self.memory.read_ptr_sized( + let align = alloc.read_ptr_sized( + self, vtable.offset(pointer_size * 2, 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 6d1cacfa1479c..b3a82cd70232a 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -21,7 +21,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, MPlaceTy, Machine, EvalContext, ValueVisitor + OpTy, MPlaceTy, Machine, EvalContext, ValueVisitor, Operand, }; macro_rules! validation_failure { @@ -396,7 +396,9 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // Maintain the invariant that the place we are checking is // already verified to be in-bounds. try_validation!( - self.ecx.memory.check_bounds(ptr, size, InboundsCheck::Live), + self.ecx.memory + .get(ptr.alloc_id)? + .check_bounds(self.ecx, ptr, size), "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination @@ -520,12 +522,14 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> _ => false, } } => { - let mplace = if op.layout.is_zst() { + let mplace = match *op { // it's a ZST, the memory content cannot matter - MPlaceTy::dangling(op.layout, self.ecx) - } else { - // non-ZST array/slice/str cannot be immediate - op.to_mem_place() + Operand::Immediate(_) if op.layout.is_zst() => + // invent an aligned mplace + MPlaceTy::dangling(op.layout, self.ecx), + // FIXME: what about single element arrays? They can be Scalar layout I think + Operand::Immediate(_) => bug!("non-ZST array/slice cannot be immediate"), + Operand::Indirect(_) => op.to_mem_place(), }; // This is the length of the array/slice. let len = mplace.len(self.ecx)?; @@ -534,6 +538,11 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // This is the size in bytes of the whole array. let size = ty_size * len; + if op.layout.is_zst() { + return self.ecx.memory.check_align(mplace.ptr, op.layout.align); + } + let ptr = mplace.ptr.to_ptr()?; + // NOTE: Keep this in sync with the handling of integer and float // types above, in `visit_primitive`. // In run-time mode, we accept pointers in here. This is actually more @@ -543,8 +552,9 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // to reject those pointers, we just do not have the machinery to // talk about parts of a pointer. // We also accept undef, for consistency with the type-based checks. - match self.ecx.memory.check_bytes( - mplace.ptr, + match self.ecx.memory.get(ptr.alloc_id)?.check_bytes( + self.ecx, + ptr, size, /*allow_ptr_and_undef*/!self.const_mode, ) { From 3a0e8254b06c8a9c8e4b775a18524797f1e3d44c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 09:44:59 +0100 Subject: [PATCH 11/34] Remove unnecessary `Result` (function always returned `Ok`) --- src/librustc/mir/interpret/allocation.rs | 12 ++++++------ src/librustc_mir/interpret/memory.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 6ef7a5a266d37..42bcd3a902760 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -406,12 +406,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - ) -> EvalResult<'tcx, &[(Size, (Tag, AllocId))]> { + ) -> &[(Size, (Tag, 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(cx.data_layout().pointer_size.bytes() - 1); let end = ptr.offset + size; // this does overflow checking - Ok(self.relocations.range(Size::from_bytes(start)..end)) + self.relocations.range(Size::from_bytes(start)..end) } /// Check that there ar eno relocations overlapping with the given range. @@ -422,10 +422,10 @@ impl<'tcx, Tag: Copy, Extra> Allocation { ptr: Pointer, size: Size, ) -> EvalResult<'tcx> { - if self.relocations(cx, ptr, size)?.len() != 0 { - err!(ReadPointerAsBytes) - } else { + if self.relocations(cx, ptr, size).is_empty() { Ok(()) + } else { + err!(ReadPointerAsBytes) } } @@ -444,7 +444,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.relocations(cx, ptr, size); if relocations.is_empty() { return Ok(()); } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 3119d9ed0ffa3..896a7a25b5d95 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -655,7 +655,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, '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)?.relocations(self, src, size); let mut new_relocations = Vec::with_capacity(relocations.len() * (length as usize)); for i in 0..length { new_relocations.extend( From a835555474c87def84099df412816d5edfa2b9cb Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 10:19:12 +0100 Subject: [PATCH 12/34] Make zst accesses in allocations take the regular path. Speeding up zst accesses should be done on a higher level. --- src/librustc/mir/interpret/allocation.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 42bcd3a902760..ba2755b29f0b8 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -116,12 +116,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { size: Size, allow_ptr_and_undef: bool, ) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); - if size.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(()); - } // Check bounds, align and relocations on the edges self.get_bytes_with_undef_and_ptr(cx, ptr, size, align)?; // Check undef and ptr @@ -138,12 +133,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { ptr: Pointer, size: Size, ) -> EvalResult<'tcx, &[u8]> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); - if size.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(&[]); - } self.get_bytes(cx, ptr, size, align) } @@ -153,12 +143,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { ptr: Pointer, src: &[u8], ) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); - if src.is_empty() { - self.check_align(ptr, align)?; - return Ok(()); - } let bytes = self.get_bytes_mut( cx, ptr, Size::from_bytes(src.len() as u64), align, )?; @@ -173,12 +158,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { val: u8, count: Size ) -> EvalResult<'tcx> { - // Empty accesses don't need to be valid pointers, but they should still be non-NULL let align = Align::from_bytes(1).unwrap(); - if count.bytes() == 0 { - self.check_align(ptr, align)?; - return Ok(()); - } let bytes = self.get_bytes_mut(cx, ptr, count, align)?; for b in bytes { *b = val; @@ -329,7 +309,6 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { align: Align, check_defined_and_ptr: bool, ) -> EvalResult<'tcx, &[u8]> { - assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); self.check_align(ptr.into(), align)?; self.check_bounds(cx, ptr, size)?; From df1ed0c2a68d0a38d98434b734ed042f7b976e5e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 14:32:39 +0100 Subject: [PATCH 13/34] Make a method that doesn't need `Self` a free function instead --- src/librustc/mir/interpret/allocation.rs | 35 ++++++++++++------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index ba2755b29f0b8..bafa32df8486d 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -176,7 +176,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { ) -> EvalResult<'tcx, ScalarMaybeUndef> { // get_bytes_unchecked tests alignment and relocation edges let bytes = self.get_bytes_with_undef_and_ptr( - cx, ptr, size, ptr_align.min(self.int_align(cx, size)) + cx, ptr, size, ptr_align.min(int_align(cx, size)) )?; // Undef check happens *after* we established that the alignment is correct. // We must not return Ok() for unaligned pointers! @@ -272,24 +272,23 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { let ptr_size = cx.data_layout().pointer_size; self.write_scalar(cx, ptr.into(), ptr_align, val, ptr_size) } +} - fn int_align( - &self, - cx: &impl HasDataLayout, - size: Size, - ) -> Align { - // We assume pointer-sized integers have the same alignment as pointers. - // We also assume signed and unsigned integers of the same size have the same alignment. - let ity = match size.bytes() { - 1 => layout::I8, - 2 => layout::I16, - 4 => layout::I32, - 8 => layout::I64, - 16 => layout::I128, - _ => bug!("bad integer size: {}", size.bytes()), - }; - ity.align(cx).abi - } +fn int_align( + cx: &impl HasDataLayout, + size: Size, +) -> Align { + // We assume pointer-sized integers have the same alignment as pointers. + // We also assume signed and unsigned integers of the same size have the same alignment. + let ity = match size.bytes() { + 1 => layout::I8, + 2 => layout::I16, + 4 => layout::I32, + 8 => layout::I64, + 16 => layout::I128, + _ => bug!("bad integer size: {}", size.bytes()), + }; + ity.align(cx).abi } /// Byte accessors From 20dee47a66a802c0e7f0b8884632592ad9311357 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 14:54:00 +0100 Subject: [PATCH 14/34] Add regression test for integral pointers in zst str slice fat pointers --- src/test/ui/consts/int_ptr_for_zst_slices.rs | 3 +++ .../ui/consts/int_ptr_for_zst_slices.stderr | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/test/ui/consts/int_ptr_for_zst_slices.rs create mode 100644 src/test/ui/consts/int_ptr_for_zst_slices.stderr diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.rs b/src/test/ui/consts/int_ptr_for_zst_slices.rs new file mode 100644 index 0000000000000..ece8d3d1505c5 --- /dev/null +++ b/src/test/ui/consts/int_ptr_for_zst_slices.rs @@ -0,0 +1,3 @@ +const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; + +fn main() {} \ No newline at end of file diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.stderr b/src/test/ui/consts/int_ptr_for_zst_slices.stderr new file mode 100644 index 0000000000000..c7b78d98cc880 --- /dev/null +++ b/src/test/ui/consts/int_ptr_for_zst_slices.stderr @@ -0,0 +1,23 @@ +error[E0308]: mismatched types + --> $DIR/int_ptr_for_zst_slices.rs:1:28 + | +LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected str, found slice + | + = note: expected type `&'static str` + found type `&[str]` + +error[E0277]: the size for values of type `str` cannot be known at compilation time + --> $DIR/int_ptr_for_zst_slices.rs:1:75 + | +LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; + | ^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `std::marker::Sized` is not implemented for `str` + = note: to learn more, visit + = note: slice and array elements must have `Sized` type + +error: aborting due to 2 previous errors + +Some errors occurred: E0277, E0308. +For more information about an error, try `rustc --explain E0277`. From cc2f46e55a3fe52affe0dd4aee2ececd75c78031 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 14:55:18 +0100 Subject: [PATCH 15/34] Reorder methods in `allocation.rs` --- src/librustc/mir/interpret/allocation.rs | 170 +++++++++++------------ 1 file changed, 85 insertions(+), 85 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index bafa32df8486d..2e08baa6f5171 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -89,6 +89,91 @@ impl<'tcx, Tag, Extra> Allocation { } } +/// Byte accessors +impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { + /// 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_with_undef_and_ptr` instead, + /// + /// This function also guarantees that the resulting pointer will remain stable + /// even when new allocations are pushed to the `HashMap`. `copy_repeatedly` relies + /// on that. + fn get_bytes_internal( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + align: Align, + check_defined_and_ptr: bool, + ) -> EvalResult<'tcx, &[u8]> { + self.check_align(ptr.into(), align)?; + self.check_bounds(cx, ptr, size)?; + + if check_defined_and_ptr { + self.check_defined(ptr, size)?; + self.check_relocations(cx, ptr, size)?; + } else { + // We still don't want relocations on the *edges* + self.check_relocation_edges(cx, ptr, size)?; + } + + AllocationExtra::memory_read(self, ptr, size)?; + + 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(&self.bytes[offset..offset + size.bytes() as usize]) + } + + #[inline] + fn get_bytes( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + align: Align + ) -> EvalResult<'tcx, &[u8]> { + self.get_bytes_internal(cx, ptr, size, align, true) + } + + /// It is the caller's responsibility to handle undefined and pointer bytes. + /// However, this still checks that there are no relocations on the *edges*. + #[inline] + pub fn get_bytes_with_undef_and_ptr( + &self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + align: Align + ) -> EvalResult<'tcx, &[u8]> { + self.get_bytes_internal(cx, ptr, size, align, false) + } + + /// Just calling this already marks everything as defined and removes relocations, + /// so be sure to actually put data there! + pub fn get_bytes_mut( + &mut self, + cx: &impl HasDataLayout, + ptr: Pointer, + size: Size, + align: Align, + ) -> EvalResult<'tcx, &mut [u8]> { + assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); + self.check_align(ptr.into(), align)?; + self.check_bounds(cx, ptr, size)?; + + self.mark_definedness(ptr, size, true)?; + self.clear_relocations(cx, ptr, size)?; + + AllocationExtra::memory_written(self, ptr, size)?; + + 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 self.bytes[offset..offset + size.bytes() as usize]) + } +} + /// Reading and writing impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { pub fn read_c_str( @@ -291,91 +376,6 @@ fn int_align( ity.align(cx).abi } -/// Byte accessors -impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { - /// 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_with_undef_and_ptr` instead, - /// - /// This function also guarantees that the resulting pointer will remain stable - /// even when new allocations are pushed to the `HashMap`. `copy_repeatedly` relies - /// on that. - fn get_bytes_internal( - &self, - cx: &impl HasDataLayout, - ptr: Pointer, - size: Size, - align: Align, - check_defined_and_ptr: bool, - ) -> EvalResult<'tcx, &[u8]> { - self.check_align(ptr.into(), align)?; - self.check_bounds(cx, ptr, size)?; - - if check_defined_and_ptr { - self.check_defined(ptr, size)?; - self.check_relocations(cx, ptr, size)?; - } else { - // We still don't want relocations on the *edges* - self.check_relocation_edges(cx, ptr, size)?; - } - - AllocationExtra::memory_read(self, ptr, size)?; - - 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(&self.bytes[offset..offset + size.bytes() as usize]) - } - - #[inline] - fn get_bytes( - &self, - cx: &impl HasDataLayout, - ptr: Pointer, - size: Size, - align: Align - ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(cx, ptr, size, align, true) - } - - /// It is the caller's responsibility to handle undefined and pointer bytes. - /// However, this still checks that there are no relocations on the *edges*. - #[inline] - pub fn get_bytes_with_undef_and_ptr( - &self, - cx: &impl HasDataLayout, - ptr: Pointer, - size: Size, - align: Align - ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(cx, ptr, size, align, false) - } - - /// Just calling this already marks everything as defined and removes relocations, - /// so be sure to actually put data there! - pub fn get_bytes_mut( - &mut self, - cx: &impl HasDataLayout, - ptr: Pointer, - size: Size, - align: Align, - ) -> EvalResult<'tcx, &mut [u8]> { - assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_align(ptr.into(), align)?; - self.check_bounds(cx, ptr, size)?; - - self.mark_definedness(ptr, size, true)?; - self.clear_relocations(cx, ptr, size)?; - - AllocationExtra::memory_written(self, ptr, size)?; - - 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 self.bytes[offset..offset + size.bytes() as usize]) - } -} - /// Relocations impl<'tcx, Tag: Copy, Extra> Allocation { /// Return all relocations overlapping with the given ptr-offset pair. From ebf03363f2d2385bc2248549cd93fca42779699a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 15:36:05 +0100 Subject: [PATCH 16/34] Properly test for int pointers in fat pointers to str slices of zero chars --- src/test/ui/consts/int_ptr_for_zst_slices.rs | 7 +++-- .../ui/consts/int_ptr_for_zst_slices.stderr | 26 +++++-------------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.rs b/src/test/ui/consts/int_ptr_for_zst_slices.rs index ece8d3d1505c5..809cc15be0c3f 100644 --- a/src/test/ui/consts/int_ptr_for_zst_slices.rs +++ b/src/test/ui/consts/int_ptr_for_zst_slices.rs @@ -1,3 +1,6 @@ -const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; +#![feature(const_raw_ptr_deref)] -fn main() {} \ No newline at end of file +const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const str) }; +//~^ ERROR it is undefined behaviour to use this value + +fn main() {} diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.stderr b/src/test/ui/consts/int_ptr_for_zst_slices.stderr index c7b78d98cc880..437e6952e740a 100644 --- a/src/test/ui/consts/int_ptr_for_zst_slices.stderr +++ b/src/test/ui/consts/int_ptr_for_zst_slices.stderr @@ -1,23 +1,11 @@ -error[E0308]: mismatched types - --> $DIR/int_ptr_for_zst_slices.rs:1:28 +error[E0080]: it is undefined behavior to use this value + --> $DIR/int_ptr_for_zst_slices.rs:3:1 | -LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected str, found slice +LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const str) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at . | - = note: expected type `&'static str` - found type `&[str]` + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error[E0277]: the size for values of type `str` cannot be known at compilation time - --> $DIR/int_ptr_for_zst_slices.rs:1:75 - | -LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const [str]) }; - | ^^^^^^^^^^^^ doesn't have a size known at compile-time - | - = help: the trait `std::marker::Sized` is not implemented for `str` - = note: to learn more, visit - = note: slice and array elements must have `Sized` type - -error: aborting due to 2 previous errors +error: aborting due to previous error -Some errors occurred: E0277, E0308. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0080`. From ef332959dc42f25da567ae9a345ad395a19234a1 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 16:13:51 +0100 Subject: [PATCH 17/34] Reintroduce zst-slice reading `read_bytes` method on `Memory` --- src/librustc_mir/interpret/memory.rs | 16 ++++++++++++++++ src/librustc_mir/interpret/operand.rs | 5 +---- src/test/ui/consts/int_ptr_for_zst_slices.rs | 3 ++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 896a7a25b5d95..b468943cbf0b1 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -567,6 +567,22 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } +/// Byte Accessors +impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { + pub fn read_bytes( + &self, + ptr: Scalar, + size: Size, + ) -> EvalResult<'tcx, &[u8]> { + if size.bytes() == 0 { + Ok(&[]) + } else { + let ptr = ptr.to_ptr()?; + self.get(ptr.alloc_id)?.read_bytes(self, ptr, size) + } + } +} + /// Interning (for CTFE) impl<'a, 'mir, 'tcx, M> Memory<'a, 'mir, 'tcx, M> where diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3f5f0ebed72d0..0fb5b59e4420b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -351,10 +351,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, &str> { let len = mplace.len(self)?; - let ptr = mplace.ptr.to_ptr()?; - let bytes = self.memory - .get(ptr.alloc_id)? - .read_bytes(self, ptr, Size::from_bytes(len as u64))?; + let bytes = self.memory.read_bytes(mplace.ptr, Size::from_bytes(len as u64))?; let str = ::std::str::from_utf8(bytes) .map_err(|err| EvalErrorKind::ValidationFailure(err.to_string()))?; Ok(str) diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.rs b/src/test/ui/consts/int_ptr_for_zst_slices.rs index 809cc15be0c3f..afa2c6a5b9e1c 100644 --- a/src/test/ui/consts/int_ptr_for_zst_slices.rs +++ b/src/test/ui/consts/int_ptr_for_zst_slices.rs @@ -1,6 +1,7 @@ +// compile-pass + #![feature(const_raw_ptr_deref)] const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const str) }; -//~^ ERROR it is undefined behaviour to use this value fn main() {} From 87bd5d13d8a175a0be82fe2b1db5588036c4bdb6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 17:23:26 +0100 Subject: [PATCH 18/34] Remove stderr file, because the test passes now --- src/test/ui/consts/int_ptr_for_zst_slices.stderr | 11 ----------- 1 file changed, 11 deletions(-) delete mode 100644 src/test/ui/consts/int_ptr_for_zst_slices.stderr diff --git a/src/test/ui/consts/int_ptr_for_zst_slices.stderr b/src/test/ui/consts/int_ptr_for_zst_slices.stderr deleted file mode 100644 index 437e6952e740a..0000000000000 --- a/src/test/ui/consts/int_ptr_for_zst_slices.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0080]: it is undefined behavior to use this value - --> $DIR/int_ptr_for_zst_slices.rs:3:1 - | -LL | const FOO: &str = unsafe { &*(1_usize as *const [u8; 0] as *const [u8] as *const str) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized or non-UTF-8 data in str at . - | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0080`. From 65b702c6b108dfa5e2c3fa891d4f96755293ddb3 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 14 Nov 2018 10:56:09 +0100 Subject: [PATCH 19/34] Update miri submodule --- Cargo.lock | 8 ++++---- src/tools/miri | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7d5a1a507bab..dc9296b81e223 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -306,7 +306,7 @@ dependencies = [ "clippy-mini-macro-test 0.2.0", "clippy_dev 0.0.1", "clippy_lints 0.0.212", - "compiletest_rs 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)", + "compiletest_rs 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)", "derive-new 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "regex 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -423,7 +423,7 @@ dependencies = [ [[package]] name = "compiletest_rs" -version = "0.3.16" +version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "diff 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1315,7 +1315,7 @@ dependencies = [ "byteorder 1.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "cargo_metadata 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "colored 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)", - "compiletest_rs 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)", + "compiletest_rs 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.5.12 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "vergen 3.0.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3274,7 +3274,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum colored 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b0aa3473e85a3161b59845d6096b289bb577874cafeaf75ea1b1beaa6572c7fc" "checksum commoncrypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d056a8586ba25a1e4d61cb090900e495952c7886786fc55f909ab2f819b69007" "checksum commoncrypto-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1fed34f46747aa73dfaa578069fd8279d2818ade2b55f38f22a9401c7f4083e2" -"checksum compiletest_rs 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)" = "75e809f56d6aa9575b67924b0af686c4f4c1380314f47947e235e9ff7fa94bed" +"checksum compiletest_rs 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)" = "89747fe073b7838343bd2c2445e7a7c2e0d415598f8925f0fa9205b9cdfc48cb" "checksum core-foundation 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cc3532ec724375c7cb7ff0a097b714fde180bb1f6ed2ab27cfcd99ffca873cd2" "checksum core-foundation-sys 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a3fb15cdbdd9cf8b82d97d0296bb5cd3631bba58d6e31650a002a8e7fb5721f9" "checksum crossbeam 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "24ce9782d4d5c53674646a6a4c1863a21a8fc0cb649b3c94dfc16e45071dea19" diff --git a/src/tools/miri b/src/tools/miri index bbb1d80703f27..06758c48bd7a7 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit bbb1d80703f272a5592ceeb3832a489776512251 +Subproject commit 06758c48bd7a77bb5aa43fc50cf344540ba5afef From b820cc79a93a8867b1fe51ab628663671843a4ed Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 14 Nov 2018 11:45:10 +0100 Subject: [PATCH 20/34] Clean up array/slice of primitive validation --- src/librustc_mir/interpret/validity.rs | 23 ++++++++----------- src/test/ui/consts/validate_never_arrays.rs | 5 ++++ .../ui/consts/validate_never_arrays.stderr | 11 +++++++++ 3 files changed, 26 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/consts/validate_never_arrays.rs create mode 100644 src/test/ui/consts/validate_never_arrays.stderr diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index b3a82cd70232a..4b9ded4c17ee9 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -21,7 +21,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, MPlaceTy, Machine, EvalContext, ValueVisitor, Operand, + OpTy, Machine, EvalContext, ValueVisitor, }; macro_rules! validation_failure { @@ -522,25 +522,22 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> _ => false, } } => { - let mplace = match *op { - // it's a ZST, the memory content cannot matter - Operand::Immediate(_) if op.layout.is_zst() => - // invent an aligned mplace - MPlaceTy::dangling(op.layout, self.ecx), - // FIXME: what about single element arrays? They can be Scalar layout I think - Operand::Immediate(_) => bug!("non-ZST array/slice cannot be immediate"), - Operand::Indirect(_) => op.to_mem_place(), - }; + if op.layout.is_zst() { + return Ok(()); + } + // non-ZST array cannot be immediate, slices are never immediate + let mplace = op.to_mem_place(); // This is the length of the array/slice. let len = mplace.len(self.ecx)?; + // zero length slices have nothing to be checked + if len == 0 { + return Ok(()); + } // This is the element type size. let ty_size = self.ecx.layout_of(tys)?.size; // This is the size in bytes of the whole array. let size = ty_size * len; - if op.layout.is_zst() { - return self.ecx.memory.check_align(mplace.ptr, op.layout.align); - } let ptr = mplace.ptr.to_ptr()?; // NOTE: Keep this in sync with the handling of integer and float diff --git a/src/test/ui/consts/validate_never_arrays.rs b/src/test/ui/consts/validate_never_arrays.rs new file mode 100644 index 0000000000000..9610b7b22f161 --- /dev/null +++ b/src/test/ui/consts/validate_never_arrays.rs @@ -0,0 +1,5 @@ +#![feature(const_raw_ptr_deref, never_type)] + +const FOO: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) }; //~ ERROR undefined behavior + +fn main() {} diff --git a/src/test/ui/consts/validate_never_arrays.stderr b/src/test/ui/consts/validate_never_arrays.stderr new file mode 100644 index 0000000000000..0b639240bb348 --- /dev/null +++ b/src/test/ui/consts/validate_never_arrays.stderr @@ -0,0 +1,11 @@ +error[E0080]: it is undefined behavior to use this value + --> $DIR/validate_never_arrays.rs:3:1 + | +LL | const FOO: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) }; //~ ERROR undefined behavior + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at .[0] + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0080`. From d3139b9c4157957aa40f212f5d423db4359001a9 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 14 Nov 2018 12:19:56 +0100 Subject: [PATCH 21/34] Rebase fallout --- src/librustc_mir/interpret/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index b468943cbf0b1..c7c6dd1284646 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -30,7 +30,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InboundsCheck, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, + Machine, AllocMap, MayLeak, ErrorHandled, AllocationExtra, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] From 1c08ced99589114164f057bc4cd020a6bad48af9 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 14 Nov 2018 13:28:38 +0100 Subject: [PATCH 22/34] Explain early abort legality --- src/librustc_mir/interpret/validity.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 4b9ded4c17ee9..8ce5a0365cf65 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -522,6 +522,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> _ => false, } } => { + // bailing out for zsts is ok, since the array element type can only be int/float if op.layout.is_zst() { return Ok(()); } From 8b04b098695951f561ef4eedf33c6fbc63728caa Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 15 Nov 2018 14:43:58 +0100 Subject: [PATCH 23/34] Move alignment checks out of `Allocation` --- src/librustc/mir/interpret/allocation.rs | 69 +++++------------------- src/librustc_mir/interpret/memory.rs | 12 ++--- src/librustc_mir/interpret/operand.rs | 11 ++-- src/librustc_mir/interpret/place.rs | 16 +++--- src/librustc_mir/interpret/terminator.rs | 3 +- src/librustc_mir/interpret/traits.rs | 17 +++--- 6 files changed, 43 insertions(+), 85 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 2e08baa6f5171..b2c4454880575 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -15,7 +15,7 @@ use super::{ truncate, }; -use ty::layout::{self, Size, Align}; +use ty::layout::{Size, Align}; use syntax::ast::Mutability; use std::iter; use mir; @@ -103,10 +103,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - align: Align, check_defined_and_ptr: bool, ) -> EvalResult<'tcx, &[u8]> { - self.check_align(ptr.into(), align)?; self.check_bounds(cx, ptr, size)?; if check_defined_and_ptr { @@ -126,14 +124,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { } #[inline] - fn get_bytes( + pub fn get_bytes( &self, cx: &impl HasDataLayout, ptr: Pointer, size: Size, - align: Align ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(cx, ptr, size, align, true) + self.get_bytes_internal(cx, ptr, size, true) } /// It is the caller's responsibility to handle undefined and pointer bytes. @@ -144,9 +141,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - align: Align ) -> EvalResult<'tcx, &[u8]> { - self.get_bytes_internal(cx, ptr, size, align, false) + self.get_bytes_internal(cx, ptr, size, false) } /// Just calling this already marks everything as defined and removes relocations, @@ -156,10 +152,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - align: Align, ) -> EvalResult<'tcx, &mut [u8]> { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_align(ptr.into(), align)?; self.check_bounds(cx, ptr, size)?; self.mark_definedness(ptr, size, true)?; @@ -201,9 +195,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { size: Size, allow_ptr_and_undef: bool, ) -> EvalResult<'tcx> { - let align = Align::from_bytes(1).unwrap(); // Check bounds, align and relocations on the edges - self.get_bytes_with_undef_and_ptr(cx, ptr, size, align)?; + self.get_bytes_with_undef_and_ptr(cx, ptr, size)?; // Check undef and ptr if !allow_ptr_and_undef { self.check_defined(ptr, size)?; @@ -212,26 +205,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(()) } - pub fn read_bytes( - &self, - cx: &impl HasDataLayout, - ptr: Pointer, - size: Size, - ) -> EvalResult<'tcx, &[u8]> { - let align = Align::from_bytes(1).unwrap(); - self.get_bytes(cx, ptr, size, align) - } - pub fn write_bytes( &mut self, cx: &impl HasDataLayout, ptr: Pointer, src: &[u8], ) -> EvalResult<'tcx> { - let align = Align::from_bytes(1).unwrap(); - let bytes = self.get_bytes_mut( - cx, ptr, Size::from_bytes(src.len() as u64), align, - )?; + let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?; bytes.clone_from_slice(src); Ok(()) } @@ -243,8 +223,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { val: u8, count: Size ) -> EvalResult<'tcx> { - let align = Align::from_bytes(1).unwrap(); - let bytes = self.get_bytes_mut(cx, ptr, count, align)?; + let bytes = self.get_bytes_mut(cx, ptr, count)?; for b in bytes { *b = val; } @@ -256,13 +235,10 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { &self, cx: &impl HasDataLayout, ptr: Pointer, - ptr_align: Align, size: Size ) -> EvalResult<'tcx, ScalarMaybeUndef> { - // get_bytes_unchecked tests alignment and relocation edges - let bytes = self.get_bytes_with_undef_and_ptr( - cx, ptr, size, ptr_align.min(int_align(cx, size)) - )?; + // get_bytes_unchecked tests relocation edges + let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, 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() { @@ -293,9 +269,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { &self, cx: &impl HasDataLayout, ptr: Pointer, - ptr_align: Align ) -> EvalResult<'tcx, ScalarMaybeUndef> { - self.read_scalar(cx, ptr, ptr_align, cx.data_layout().pointer_size) + self.read_scalar(cx, ptr, cx.data_layout().pointer_size) } /// Write a *non-ZST* scalar @@ -303,7 +278,6 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { &mut self, cx: &impl HasDataLayout, ptr: Pointer, - ptr_align: Align, val: ScalarMaybeUndef, type_size: Size, ) -> EvalResult<'tcx> { @@ -327,9 +301,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { }; { - // get_bytes_mut checks alignment let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, ptr, type_size, ptr_align)?; + let dst = self.get_bytes_mut(cx, ptr, type_size)?; write_target_uint(endian, dst, bytes).unwrap(); } @@ -351,31 +324,13 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { &mut self, cx: &impl HasDataLayout, ptr: Pointer, - ptr_align: Align, val: ScalarMaybeUndef ) -> EvalResult<'tcx> { let ptr_size = cx.data_layout().pointer_size; - self.write_scalar(cx, ptr.into(), ptr_align, val, ptr_size) + self.write_scalar(cx, ptr.into(), val, ptr_size) } } -fn int_align( - cx: &impl HasDataLayout, - size: Size, -) -> Align { - // We assume pointer-sized integers have the same alignment as pointers. - // We also assume signed and unsigned integers of the same size have the same alignment. - let ity = match size.bytes() { - 1 => layout::I8, - 2 => layout::I16, - 4 => layout::I32, - 8 => layout::I64, - 16 => layout::I128, - _ => bug!("bad integer size: {}", size.bytes()), - }; - ity.align(cx).abi -} - /// Relocations impl<'tcx, Tag: Copy, Extra> Allocation { /// Return all relocations overlapping with the given ptr-offset pair. diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c7c6dd1284646..2e47d7e69d9a5 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -578,7 +578,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok(&[]) } else { let ptr = ptr.to_ptr()?; - self.get(ptr.alloc_id)?.read_bytes(self, ptr, size) + self.get(ptr.alloc_id)?.get_bytes(self, ptr, size) } } } @@ -656,10 +656,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { length: u64, nonoverlapping: bool, ) -> EvalResult<'tcx> { + self.check_align(src, src_align)?; + self.check_align(dest, dest_align)?; if size.bytes() == 0 { // Nothing to do for ZST, other than checking alignment and non-NULLness. - self.check_align(src, src_align)?; - self.check_align(dest, dest_align)?; return Ok(()); } let src = src.to_ptr()?; @@ -689,12 +689,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { let tcx = self.tcx.tcx; - // This also checks alignment, and relocation edges on the src. + // This checks relocation edges on the src. let src_bytes = self.get(src.alloc_id)? - .get_bytes_with_undef_and_ptr(&tcx, src, size, src_align)? + .get_bytes_with_undef_and_ptr(&tcx, src, size)? .as_ptr(); let dest_bytes = self.get_mut(dest.alloc_id)? - .get_bytes_mut(&tcx, dest, size * length, dest_align)? + .get_bytes_mut(&tcx, dest, size * length)? .as_mut_ptr(); // SAFE: The above indexing would have panicked if there weren't at least `size` bytes diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 0fb5b59e4420b..ba995afddc813 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -275,12 +275,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> return Ok(Some(Immediate::Scalar(Scalar::zst().into()))); } + // check for integer pointers before alignment to report better errors let ptr = ptr.to_ptr()?; + self.memory.check_align(ptr.into(), ptr_align)?; match mplace.layout.abi { layout::Abi::Scalar(..) => { let scalar = self.memory .get(ptr.alloc_id)? - .read_scalar(self, ptr, ptr_align, mplace.layout.size)?; + .read_scalar(self, ptr, mplace.layout.size)?; Ok(Some(Immediate::Scalar(scalar))) } layout::Abi::ScalarPair(ref a, ref b) => { @@ -289,13 +291,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let a_ptr = ptr; let b_offset = a_size.align_to(b.align(self).abi); assert!(b_offset.bytes() > 0); // we later use the offset to test which field to use - let b_ptr = ptr.offset(b_offset, self)?.into(); + let b_ptr = ptr.offset(b_offset, self)?; let a_val = self.memory .get(ptr.alloc_id)? - .read_scalar(self, a_ptr, ptr_align, a_size)?; + .read_scalar(self, a_ptr, a_size)?; + self.memory.check_align(b_ptr.into(), b.align(self))?; let b_val = self.memory .get(ptr.alloc_id)? - .read_scalar(self, b_ptr, ptr_align, b_size)?; + .read_scalar(self, b_ptr, b_size)?; Ok(Some(Immediate::ScalarPair(a_val, b_val))) } _ => Ok(None), diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index e2b6c00ba382c..6317cfb94d27f 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -713,11 +713,12 @@ where // Nothing to do for ZSTs, other than checking alignment if dest.layout.is_zst() { - self.memory.check_align(ptr, ptr_align)?; - return Ok(()); + return self.memory.check_align(ptr, ptr_align); } + // check for integer pointers before alignment to report better errors let ptr = ptr.to_ptr()?; + self.memory.check_align(ptr.into(), ptr_align)?; let tcx = &*self.tcx; // FIXME: We should check that there are dest.layout.size many bytes available in // memory. The code below is not sufficient, with enough padding it might not @@ -729,9 +730,8 @@ where _ => bug!("write_immediate_to_mplace: invalid Scalar layout: {:#?}", dest.layout) } - self.memory.get_mut(ptr.alloc_id)?.write_scalar( - tcx, ptr, ptr_align.min(dest.layout.align.abi), scalar, dest.layout.size + tcx, ptr, scalar, dest.layout.size ) } Immediate::ScalarPair(a_val, b_val) => { @@ -741,20 +741,22 @@ where dest.layout) }; let (a_size, b_size) = (a.size(self), b.size(self)); - let (a_align, b_align) = (a.align(self).abi, b.align(self).abi); + let b_align = b.align(self).abi; let b_offset = a_size.align_to(b_align); let b_ptr = ptr.offset(b_offset, self)?; + self.memory.check_align(b_ptr.into(), ptr_align.min(b_align))?; + // It is tempting to verify `b_offset` against `layout.fields.offset(1)`, // but that does not work: We could be a newtype around a pair, then the // fields do not match the `ScalarPair` components. self.memory .get_mut(ptr.alloc_id)? - .write_scalar(tcx, ptr, ptr_align.min(a_align), a_val, a_size)?; + .write_scalar(tcx, ptr, a_val, a_size)?; self.memory .get_mut(b_ptr.alloc_id)? - .write_scalar(tcx, b_ptr, ptr_align.min(b_align), b_val, b_size) + .write_scalar(tcx, b_ptr, b_val, b_size) } } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 9e59611125d92..617d204fe1042 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -401,13 +401,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, '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.pointer_size(); - let ptr_align = self.tcx.data_layout.pointer_align.abi; let ptr = self.deref_operand(args[0])?; let vtable = ptr.vtable()?; + self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?; let fn_ptr = self.memory.get(vtable.alloc_id)?.read_ptr_sized( self, vtable.offset(ptr_size * (idx as u64 + 3), self)?, - ptr_align )?.to_ptr()?; let instance = self.memory.get_fn(fn_ptr)?; diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 8e39574c01e98..37979c8ee664c 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -61,16 +61,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let drop = self.memory.create_fn_alloc(drop).with_default_tag(); self.memory .get_mut(vtable.alloc_id)? - .write_ptr_sized(tcx, vtable, ptr_align, Scalar::Ptr(drop).into())?; + .write_ptr_sized(tcx, vtable, Scalar::Ptr(drop).into())?; let size_ptr = vtable.offset(ptr_size, self)?; self.memory .get_mut(size_ptr.alloc_id)? - .write_ptr_sized(tcx, size_ptr, ptr_align, Scalar::from_uint(size, ptr_size).into())?; + .write_ptr_sized(tcx, size_ptr, Scalar::from_uint(size, ptr_size).into())?; let align_ptr = vtable.offset(ptr_size * 2, self)?; self.memory .get_mut(align_ptr.alloc_id)? - .write_ptr_sized(tcx, align_ptr, ptr_align, Scalar::from_uint(align, ptr_size).into())?; + .write_ptr_sized(tcx, align_ptr, Scalar::from_uint(align, ptr_size).into())?; for (i, method) in methods.iter().enumerate() { if let Some((def_id, substs)) = *method { @@ -79,7 +79,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?; self.memory .get_mut(method_ptr.alloc_id)? - .write_ptr_sized(tcx, method_ptr, ptr_align, Scalar::Ptr(fn_ptr).into())?; + .write_ptr_sized(tcx, method_ptr, Scalar::Ptr(fn_ptr).into())?; } } @@ -95,10 +95,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> vtable: Pointer, ) -> EvalResult<'tcx, (ty::Instance<'tcx>, ty::Ty<'tcx>)> { // we don't care about the pointee type, we just want a pointer - let pointer_align = self.tcx.data_layout.pointer_align.abi; + self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?; let drop_fn = self.memory .get(vtable.alloc_id)? - .read_ptr_sized(self, vtable, pointer_align)? + .read_ptr_sized(self, vtable)? .to_ptr()?; let drop_instance = self.memory.get_fn(drop_fn)?; trace!("Found drop fn: {:?}", drop_instance); @@ -114,14 +114,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> vtable: Pointer, ) -> EvalResult<'tcx, (Size, Align)> { let pointer_size = self.pointer_size(); - let pointer_align = self.tcx.data_layout.pointer_align.abi; + self.memory.check_align(vtable.into(), self.tcx.data_layout.pointer_align.abi)?; let alloc = self.memory.get(vtable.alloc_id)?; - let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?, pointer_align)? + let size = alloc.read_ptr_sized(self, vtable.offset(pointer_size, self)?)? .to_bits(pointer_size)? as u64; let align = alloc.read_ptr_sized( self, vtable.offset(pointer_size * 2, self)?, - pointer_align )?.to_bits(pointer_size)? as u64; Ok((Size::from_bytes(size), Align::from_bytes(align).unwrap())) } From 9d57adf2ba57420903b4c9b4dbb492c3fcf691b8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 15 Nov 2018 14:48:34 +0100 Subject: [PATCH 24/34] Explain {read,write}_scalar failure to cope with zsts --- src/librustc/mir/interpret/allocation.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index b2c4454880575..2ecd5eb5a6269 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -231,6 +231,11 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { } /// Read a *non-ZST* scalar + /// + /// zsts can't be read out of two reasons: + /// * byteorder cannot work with zero element buffers + /// * in oder to obtain a `Pointer` we need to check for ZSTness anyway due to integer pointers + /// being valid for ZSTs pub fn read_scalar( &self, cx: &impl HasDataLayout, @@ -274,6 +279,11 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { } /// Write a *non-ZST* scalar + /// + /// zsts can't be read out of two reasons: + /// * byteorder cannot work with zero element buffers + /// * in oder to obtain a `Pointer` we need to check for ZSTness anyway due to integer pointers + /// being valid for ZSTs pub fn write_scalar( &mut self, cx: &impl HasDataLayout, From 927c5aab47c624fe9f4cf02289a2acccd425432c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 15 Nov 2018 17:47:11 +0100 Subject: [PATCH 25/34] Use correct alignment for fat pointer extra part --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index ba995afddc813..61c3c6f24ffb8 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -295,7 +295,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let a_val = self.memory .get(ptr.alloc_id)? .read_scalar(self, a_ptr, a_size)?; - self.memory.check_align(b_ptr.into(), b.align(self))?; + self.memory.check_align(b_ptr.into(), b.align(self).min(ptr_align))?; let b_val = self.memory .get(ptr.alloc_id)? .read_scalar(self, b_ptr, b_size)?; From 9b8e82ad242ba438b0db983248c59281b4355891 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 15 Nov 2018 18:05:15 +0100 Subject: [PATCH 26/34] Use correct alignment checks for scalars and zsts, too --- src/librustc_mir/interpret/operand.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 61c3c6f24ffb8..4ec01b6ca1032 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -271,13 +271,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> if mplace.layout.is_zst() { // Not all ZSTs have a layout we would handle below, so just short-circuit them // all here. - self.memory.check_align(ptr, ptr_align)?; + self.memory.check_align(ptr, ptr_align.min(mplace.layout.align))?; return Ok(Some(Immediate::Scalar(Scalar::zst().into()))); } // check for integer pointers before alignment to report better errors let ptr = ptr.to_ptr()?; - self.memory.check_align(ptr.into(), ptr_align)?; + self.memory.check_align(ptr.into(), ptr_align.min(mplace.layout.align))?; match mplace.layout.abi { layout::Abi::Scalar(..) => { let scalar = self.memory From 10102d1f0aa0e7e612ea2a51d5329af928b07f03 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 16 Nov 2018 11:16:34 +0100 Subject: [PATCH 27/34] comment nit Co-Authored-By: oli-obk --- src/librustc_mir/interpret/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 2e47d7e69d9a5..1cb9543b36631 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -659,7 +659,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { self.check_align(src, src_align)?; self.check_align(dest, dest_align)?; if size.bytes() == 0 { - // Nothing to do for ZST, other than checking alignment and non-NULLness. + // Nothing to do for ZST, other than checking alignment and non-NULLness which already happened. return Ok(()); } let src = src.to_ptr()?; From a5ef2d1b54c833783a64a98d5d585bb44218497a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 16 Nov 2018 11:46:58 +0100 Subject: [PATCH 28/34] Array and slice projections need to update the place alignment --- src/librustc_mir/interpret/operand.rs | 7 ++++--- src/librustc_mir/interpret/place.rs | 12 +++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4ec01b6ca1032..9588a931c4a35 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -271,13 +271,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> if mplace.layout.is_zst() { // Not all ZSTs have a layout we would handle below, so just short-circuit them // all here. - self.memory.check_align(ptr, ptr_align.min(mplace.layout.align))?; + self.memory.check_align(ptr, ptr_align)?; return Ok(Some(Immediate::Scalar(Scalar::zst().into()))); } // check for integer pointers before alignment to report better errors let ptr = ptr.to_ptr()?; - self.memory.check_align(ptr.into(), ptr_align.min(mplace.layout.align))?; + self.memory.check_align(ptr.into(), ptr_align)?; match mplace.layout.abi { layout::Abi::Scalar(..) => { let scalar = self.memory @@ -295,7 +295,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let a_val = self.memory .get(ptr.alloc_id)? .read_scalar(self, a_ptr, a_size)?; - self.memory.check_align(b_ptr.into(), b.align(self).min(ptr_align))?; + let b_align = ptr_align.restrict_for_offset(b_offset); + self.memory.check_align(b_ptr.into(), b_align)?; let b_val = self.memory .get(ptr.alloc_id)? .read_scalar(self, b_ptr, b_size)?; diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 6317cfb94d27f..d93aca7f4e1d9 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -393,8 +393,9 @@ where let dl = &self.tcx.data_layout; Ok((0..len).map(move |i| { let ptr = base.ptr.ptr_offset(i * stride, dl)?; + let align = base.align.restrict_for_offset(i * stride); Ok(MPlaceTy { - mplace: MemPlace { ptr, align: base.align, meta: None }, + mplace: MemPlace { ptr, align, meta: None }, layout }) })) @@ -417,6 +418,7 @@ where _ => bug!("Unexpected layout of index access: {:#?}", base.layout), }; let ptr = base.ptr.ptr_offset(from_offset, self)?; + let align = base.align.restrict_for_offset(from_offset); // Compute meta and new layout let inner_len = len - to - from; @@ -435,7 +437,7 @@ where let layout = self.layout_of(ty)?; Ok(MPlaceTy { - mplace: MemPlace { ptr, align: base.align, meta }, + mplace: MemPlace { ptr, align, meta }, layout }) } @@ -741,11 +743,11 @@ where dest.layout) }; let (a_size, b_size) = (a.size(self), b.size(self)); - let b_align = b.align(self).abi; - let b_offset = a_size.align_to(b_align); + let b_offset = a_size.align_to(b.align(self).abi); + let b_align = ptr_align.restrict_for_offset(b_offset); let b_ptr = ptr.offset(b_offset, self)?; - self.memory.check_align(b_ptr.into(), ptr_align.min(b_align))?; + self.memory.check_align(b_ptr.into(), b_align)?; // It is tempting to verify `b_offset` against `layout.fields.offset(1)`, // but that does not work: We could be a newtype around a pair, then the From cb8fa335721285809c5930cd0fded35899a05064 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 16 Nov 2018 11:48:48 +0100 Subject: [PATCH 29/34] tidy --- src/librustc_mir/interpret/memory.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 1cb9543b36631..4250023764555 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -659,7 +659,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { self.check_align(src, src_align)?; self.check_align(dest, dest_align)?; if size.bytes() == 0 { - // Nothing to do for ZST, other than checking alignment and non-NULLness which already happened. + // Nothing to do for ZST, other than checking alignment and + // non-NULLness which already happened. return Ok(()); } let src = src.to_ptr()?; From 972d798881cb470aa7a75b9ed7fa6c37117492e0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 16 Nov 2018 16:25:15 +0100 Subject: [PATCH 30/34] Document `Allocation` --- src/librustc/mir/interpret/allocation.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 2ecd5eb5a6269..296c2e2dd6b3d 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -170,6 +170,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// Reading and writing impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { + /// Reads bytes until a `0` is encountered. Will error if the end of the allocation is reached + /// before a `0` is found. pub fn read_c_str( &self, cx: &impl HasDataLayout, @@ -188,6 +190,9 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { } } + /// Validates that `ptr.offset` and `ptr.offset + size` do not point to the middle of a + /// relocation. If `allow_ptr_and_undef` is `false`, also enforces that the memory in the + /// given range contains neither relocations nor undef bytes. pub fn check_bytes( &self, cx: &impl HasDataLayout, @@ -195,7 +200,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { size: Size, allow_ptr_and_undef: bool, ) -> EvalResult<'tcx> { - // Check bounds, align and relocations on the edges + // Check bounds and relocations on the edges self.get_bytes_with_undef_and_ptr(cx, ptr, size)?; // Check undef and ptr if !allow_ptr_and_undef { @@ -205,6 +210,9 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(()) } + /// Writes `src` to the memory starting at `ptr.offset`. + /// + /// Will do bounds checks on the allocation. pub fn write_bytes( &mut self, cx: &impl HasDataLayout, @@ -216,6 +224,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(()) } + /// Sets `count` bytes starting at `ptr.offset` with `val`. Basically `memset`. pub fn write_repeat( &mut self, cx: &impl HasDataLayout, @@ -236,6 +245,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// * byteorder cannot work with zero element buffers /// * in oder to obtain a `Pointer` we need to check for ZSTness anyway due to integer pointers /// being valid for ZSTs + /// + /// Note: This function does not do *any* alignment checks, you need to do these before calling pub fn read_scalar( &self, cx: &impl HasDataLayout, @@ -270,6 +281,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(ScalarMaybeUndef::Scalar(Scalar::from_uint(bits, size))) } + /// Note: This function does not do *any* alignment checks, you need to do these before calling pub fn read_ptr_sized( &self, cx: &impl HasDataLayout, @@ -284,6 +296,8 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { /// * byteorder cannot work with zero element buffers /// * in oder to obtain a `Pointer` we need to check for ZSTness anyway due to integer pointers /// being valid for ZSTs + /// + /// Note: This function does not do *any* alignment checks, you need to do these before calling pub fn write_scalar( &mut self, cx: &impl HasDataLayout, @@ -330,6 +344,7 @@ impl<'tcx, Tag: Copy, Extra: AllocationExtra> Allocation { Ok(()) } + /// Note: This function does not do *any* alignment checks, you need to do these before calling pub fn write_ptr_sized( &mut self, cx: &impl HasDataLayout, @@ -357,7 +372,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { self.relocations.range(Size::from_bytes(start)..end) } - /// Check that there ar eno relocations overlapping with the given range. + /// Check that there are no relocations overlapping with the given range. #[inline(always)] fn check_relocations( &self, From 22872196f55d93d7af6b3c96e34bef3579f0c3fc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 17 Nov 2018 13:36:55 +0100 Subject: [PATCH 31/34] Factor out mplace offsetting into its own method --- src/librustc_mir/interpret/place.rs | 53 +++++++++++++++++------------ 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index d93aca7f4e1d9..0fd1a993cbd90 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -159,6 +159,19 @@ impl MemPlace { Some(meta) => Immediate::ScalarPair(self.ptr.into(), meta.into()), } } + + pub fn offset( + self, + offset: Size, + meta: Option>, + cx: &impl HasDataLayout, + ) -> EvalResult<'tcx, Self> { + Ok(MemPlace { + ptr: self.ptr.ptr_offset(offset, cx)?, + align: self.align.restrict_for_offset(offset), + meta, + }) + } } impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { @@ -174,6 +187,19 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { } } + pub fn offset( + self, + offset: Size, + meta: Option>, + layout: TyLayout<'tcx>, + cx: &impl HasDataLayout, + ) -> EvalResult<'tcx, Self> { + Ok(MPlaceTy { + mplace: self.mplace.offset(offset, meta, cx)?, + layout, + }) + } + #[inline] fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self { MPlaceTy { mplace: MemPlace::from_ptr(ptr, layout.align.abi), layout } @@ -367,13 +393,9 @@ where (None, offset) }; - let ptr = base.ptr.ptr_offset(offset, self)?; - let align = base.align - // We do not look at `base.layout.align` nor `field_layout.align`, unlike - // codegen -- mostly to see if we can get away with that - .restrict_for_offset(offset); // must be last thing that happens - - Ok(MPlaceTy { mplace: MemPlace { ptr, align, meta }, layout: field_layout }) + // We do not look at `base.layout.align` nor `field_layout.align`, unlike + // codegen -- mostly to see if we can get away with that + base.offset(offset, meta, field_layout, self) } // Iterates over all fields of an array. Much more efficient than doing the @@ -391,14 +413,7 @@ where }; let layout = base.layout.field(self, 0)?; let dl = &self.tcx.data_layout; - Ok((0..len).map(move |i| { - let ptr = base.ptr.ptr_offset(i * stride, dl)?; - let align = base.align.restrict_for_offset(i * stride); - Ok(MPlaceTy { - mplace: MemPlace { ptr, align, meta: None }, - layout - }) - })) + Ok((0..len).map(move |i| base.offset(i * stride, None, layout, dl))) } pub fn mplace_subslice( @@ -417,8 +432,6 @@ where stride * from, _ => bug!("Unexpected layout of index access: {:#?}", base.layout), }; - let ptr = base.ptr.ptr_offset(from_offset, self)?; - let align = base.align.restrict_for_offset(from_offset); // Compute meta and new layout let inner_len = len - to - from; @@ -435,11 +448,7 @@ where bug!("cannot subslice non-array type: `{:?}`", base.layout.ty), }; let layout = self.layout_of(ty)?; - - Ok(MPlaceTy { - mplace: MemPlace { ptr, align, meta }, - layout - }) + base.offset(from_offset, meta, layout, self) } pub fn mplace_downcast( From 3220c0ce1afcb7b47671d6efe8fc38ab12d3d806 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 17 Nov 2018 16:18:05 +0100 Subject: [PATCH 32/34] Explain why vtable generation needs no alignment checks --- src/librustc_mir/interpret/traits.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 37979c8ee664c..bda585b8eda34 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -59,6 +59,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let drop = ::monomorphize::resolve_drop_in_place(*tcx, ty); let drop = self.memory.create_fn_alloc(drop).with_default_tag(); + // no need to do any alignment checks on the memory accesses below, because we know the + // allocation is correctly aligned as we created it above. Also we're only offsetting by + // multiples of `ptr_align`, which means that it will stay aligned to `ptr_align`. self.memory .get_mut(vtable.alloc_id)? .write_ptr_sized(tcx, vtable, Scalar::Ptr(drop).into())?; From 360f9888bc143f6d7b2c09f723e255121bf49f8d Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 17 Nov 2018 17:52:25 +0100 Subject: [PATCH 33/34] update miri submodule --- src/tools/miri | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri b/src/tools/miri index 06758c48bd7a7..dd7f545a69e4b 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit 06758c48bd7a77bb5aa43fc50cf344540ba5afef +Subproject commit dd7f545a69e4b720407e458bf4ade0b207bbf9ee From b853252bcdb2ded2b049d833c51a993fe0ed40f8 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 20 Nov 2018 11:04:07 +0100 Subject: [PATCH 34/34] Rebase fallout --- src/librustc/mir/interpret/allocation.rs | 13 ++---------- src/librustc/mir/interpret/pointer.rs | 19 +++++++++++++++++- src/librustc_mir/interpret/memory.rs | 25 +++++++++++++++++++----- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/interpret/validity.rs | 2 +- src/tools/miri | 2 +- 6 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 296c2e2dd6b3d..c612d6ad1bb24 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -57,23 +57,14 @@ pub struct Allocation { impl<'tcx, Tag, Extra> Allocation { /// 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. `check` indicates whether we - /// additionally require the pointer to be pointing to a *live* (still allocated) - /// allocation. + /// in-bounds! This follows C's/LLVM's rules. /// If you want to check bounds before doing a memory access, better use `check_bounds`. pub fn check_bounds_ptr( &self, ptr: Pointer, ) -> EvalResult<'tcx> { let allocation_size = self.bytes.len() as u64; - if ptr.offset.bytes() > allocation_size { - return err!(PointerOutOfBounds { - ptr: ptr.erase_tag(), - check: InboundsCheck::Live, - allocation_size: Size::from_bytes(allocation_size), - }); - } - Ok(()) + ptr.check_in_alloc(Size::from_bytes(allocation_size), InboundsCheck::Live) } /// Check if the memory range beginning at `ptr` and of size `Size` is "in-bounds". diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index 969f2c0e8376a..a046825f088bb 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -2,7 +2,7 @@ use mir; use ty::layout::{self, HasDataLayout, Size}; use super::{ - AllocId, EvalResult, + AllocId, EvalResult, InboundsCheck, }; //////////////////////////////////////////////////////////////////////////////// @@ -148,4 +148,21 @@ impl<'tcx, Tag> Pointer { pub fn erase_tag(self) -> Pointer { Pointer { alloc_id: self.alloc_id, offset: self.offset, tag: () } } + + #[inline(always)] + pub fn check_in_alloc( + self, + allocation_size: Size, + check: InboundsCheck, + ) -> EvalResult<'tcx, ()> { + if self.offset > allocation_size { + err!(PointerOutOfBounds { + ptr: self.erase_tag(), + check, + allocation_size, + }) + } else { + Ok(()) + } + } } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 4250023764555..c673b57a66f5f 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -28,9 +28,9 @@ use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use syntax::ast::Mutability; use super::{ - Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InboundsCheck, + Pointer, AllocId, Allocation, GlobalId, AllocationExtra, EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, AllocationExtra, + Machine, AllocMap, MayLeak, ErrorHandled, InboundsCheck, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -251,9 +251,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Scalar::Ptr(ptr) => { // check this is not NULL -- which we can ensure only if this is in-bounds // of some (potentially dead) allocation. - self.check_bounds_ptr(ptr, InboundsCheck::MaybeDead)?; - // data required for alignment check - let (_, align) = self.get_size_and_align(ptr.alloc_id); + let align = self.check_bounds_ptr_maybe_dead(ptr)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { @@ -284,6 +282,23 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, '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. + /// This function also works for deallocated allocations. + /// Use `.get(ptr.alloc_id)?.check_bounds_ptr(ptr)` if you want to force the allocation + /// to still be live. + /// If you want to check bounds before doing a memory access, better first obtain + /// an `Allocation` and call `check_bounds`. + pub fn check_bounds_ptr_maybe_dead( + &self, + ptr: Pointer, + ) -> EvalResult<'tcx, Align> { + let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id); + ptr.check_in_alloc(allocation_size, InboundsCheck::MaybeDead)?; + Ok(align) + } } /// Allocation accessors diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 9588a931c4a35..539bc6d965fd6 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -19,7 +19,7 @@ use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerEx use rustc::mir::interpret::{ GlobalId, AllocId, ConstValue, Pointer, Scalar, - EvalResult, EvalErrorKind, InboundsCheck, + EvalResult, EvalErrorKind, }; use super::{EvalContext, Machine, MemPlace, MPlaceTy, MemoryKind}; pub use rustc::mir::interpret::ScalarMaybeUndef; @@ -647,7 +647,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> ScalarMaybeUndef::Scalar(Scalar::Ptr(ptr)) => { // The niche must be just 0 (which an inbounds pointer value never is) let ptr_valid = niche_start == 0 && variants_start == variants_end && - self.memory.check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok(); + self.memory.check_bounds_ptr_maybe_dead(ptr).is_ok(); if !ptr_valid { return err!(InvalidDiscriminant(raw_discr.erase_tag())); } diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8ce5a0365cf65..ed4cb65ea74b1 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -17,7 +17,7 @@ use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf, VariantIdx}; use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc::mir::interpret::{ - Scalar, AllocType, EvalResult, EvalErrorKind, InboundsCheck, + Scalar, AllocType, EvalResult, EvalErrorKind, }; use super::{ diff --git a/src/tools/miri b/src/tools/miri index dd7f545a69e4b..32e93ed7762e5 160000 --- a/src/tools/miri +++ b/src/tools/miri @@ -1 +1 @@ -Subproject commit dd7f545a69e4b720407e458bf4ade0b207bbf9ee +Subproject commit 32e93ed7762e5aa1a721636096848fc3c7bc7218