diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index b7abf89e61c..49ecfec74bb 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -6,22 +6,13 @@ use super::{ use crate::ty::layout::{Size, Align}; use syntax::ast::Mutability; -use std::{iter, fmt::{self, Display}}; +use std::iter; use crate::mir; use std::ops::{Range, Deref, DerefMut}; use rustc_data_structures::sorted_map::SortedMap; -use rustc_macros::HashStable; use rustc_target::abi::HasDataLayout; use std::borrow::Cow; -/// Used by `check_bounds` to indicate whether the pointer needs to be just inbounds -/// or also inbounds of a *live* allocation. -#[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] -pub enum InboundsCheck { - Live, - MaybeDead, -} - #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] pub struct Allocation { /// The actual bytes of the allocation. diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 4b1a69404dd..1b294250aa3 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -17,10 +17,7 @@ pub use self::error::{ pub use self::value::{Scalar, ScalarMaybeUndef, RawConst, ConstValue}; -pub use self::allocation::{ - InboundsCheck, Allocation, AllocationExtra, - Relocations, UndefMask, -}; +pub use self::allocation::{Allocation, AllocationExtra, Relocations, UndefMask}; pub use self::pointer::{Pointer, PointerArithmetic, CheckInAllocMsg}; diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index f5c5469fe1b..a17bc1f6728 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::fmt::{self, Display}; use crate::mir; use crate::ty::layout::{self, HasDataLayout, Size}; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 05d110a4372..197e067f674 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -19,7 +19,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InterpResult, Scalar, InterpError, GlobalAlloc, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, InboundsCheck, + Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, InterpError::ValidationFailure, }; @@ -44,6 +44,17 @@ impl MayLeak for MemoryKind { } } +/// Used by `get_size_and_align` to indicate whether the allocation needs to be live. +#[derive(Debug, Copy, Clone)] +pub enum AllocCheck { + /// Allocation must be live and not a function pointer. + Dereferencable, + /// Allocations needs to be live, but may be a function pointer. + Live, + /// Allocation may be dead. + MaybeDead, +} + // `Memory` has to depend on the `Machine` because some of its operations // (e.g., `get`) call a `Machine` hook. pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -248,66 +259,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Ok(()) } - /// Checks that the pointer is aligned AND non-NULL. This supports ZSTs in two ways: - /// You can pass a scalar, and a `Pointer` does not have to actually still be allocated. - fn check_align( - &self, - ptr: Scalar, - required_align: Align - ) -> InterpResult<'tcx> { - // Check non-NULL/Undef, extract offset - let (offset, alloc_align) = match ptr.to_bits_or_ptr(self.pointer_size(), self) { - Err(ptr) => { - // check this is not NULL -- which we can ensure only if this is in-bounds - // of some (potentially dead) allocation. - let align = self.check_ptr_bounds(ptr, InboundsCheck::MaybeDead, - CheckInAllocMsg::NullPointerTest)?; - (ptr.offset.bytes(), align) - } - Ok(data) => { - // check this is not NULL - if data == 0 { - return err!(InvalidNullPointerUsage); - } - // the "base address" is 0 and hence always aligned - (data as u64, required_align) - } - }; - // Check alignment - if alloc_align.bytes() < required_align.bytes() { - return err!(AlignmentCheckFailed { - has: alloc_align, - required: required_align, - }); - } - if offset % required_align.bytes() == 0 { - Ok(()) - } else { - let has = offset % required_align.bytes(); - err!(AlignmentCheckFailed { - has: Align::from_bytes(has).unwrap(), - required: required_align, - }) - } - } - - /// Checks if the pointer is "in-bounds" of *some* (live or dead) allocation. 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. - /// `liveness` can be used to rule out dead allocations. Testing in-bounds with a dead - /// allocation is useful e.g. to exclude the possibility of this pointer being NULL. - /// If you want to check bounds before doing a memory access, call `check_ptr_access`. - fn check_ptr_bounds( - &self, - ptr: Pointer, - liveness: InboundsCheck, - msg: CheckInAllocMsg, - ) -> InterpResult<'tcx, Align> { - let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, liveness)?; - ptr.check_in_alloc(allocation_size, msg)?; - Ok(align) - } - /// Check if the given scalar is allowed to do a memory access of given `size` /// and `align`. On success, returns `None` for zero-sized accesses (where /// nothing else is left to do) and a `Pointer` to use for the actual access otherwise. @@ -350,18 +301,35 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { None } Err(ptr) => { - // Test bounds. - self.check_ptr_bounds( - ptr.offset(size, self)?, - InboundsCheck::Live, - CheckInAllocMsg::MemoryAccessTest, - )?; - // Test align and non-NULL. - self.check_align(ptr.into(), align)?; - // FIXME: Alignment check is too strict, depending on the base address that - // got picked we might be aligned even if this check fails. - // We instead have to fall back to converting to an integer and checking - // the "real" alignment. + let (allocation_size, alloc_align) = + self.get_size_and_align(ptr.alloc_id, AllocCheck::Dereferencable)?; + // Test bounds. This also ensures non-NULL. + // It is sufficient to check this for the end pointer. The addition + // checks for overflow. + let end_ptr = ptr.offset(size, self)?; + end_ptr.check_in_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?; + // Test align. Check this last; if both bounds and alignment are violated + // we want the error to be about the bounds. + if alloc_align.bytes() < align.bytes() { + // The allocation itself is not aligned enough. + // FIXME: Alignment check is too strict, depending on the base address that + // got picked we might be aligned even if this check fails. + // We instead have to fall back to converting to an integer and checking + // the "real" alignment. + return err!(AlignmentCheckFailed { + has: alloc_align, + required: align, + }); + } + let offset = ptr.offset.bytes(); + if offset % align.bytes() != 0 { + // The offset os not aligned enough. + let has = offset % align.bytes(); + return err!(AlignmentCheckFailed { + has: Align::from_bytes(has).unwrap(), + required: align, + }) + } // We can still be zero-sized in this branch, in which case we have to // return `None`. @@ -375,8 +343,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { &self, ptr: Pointer, ) -> bool { - self.check_ptr_bounds(ptr, InboundsCheck::MaybeDead, CheckInAllocMsg::NullPointerTest) - .is_err() + let (size, _align) = self.get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead) + .expect("alloc info with MaybeDead cannot fail"); + ptr.check_in_alloc(size, CheckInAllocMsg::NullPointerTest).is_err() } } @@ -515,13 +484,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } - /// Obtain the size and alignment of an allocation, even if that allocation has been deallocated + /// Obtain the size and alignment of an allocation, even if that allocation has + /// been deallocated. /// - /// If `liveness` is `InboundsCheck::MaybeDead`, this function always returns `Ok` + /// If `liveness` is `AllocCheck::MaybeDead`, this function always returns `Ok`. pub fn get_size_and_align( &self, id: AllocId, - liveness: InboundsCheck, + liveness: AllocCheck, ) -> InterpResult<'static, (Size, Align)> { if let Ok(alloc) = self.get(id) { return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); @@ -531,7 +501,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let alloc = self.tcx.alloc_map.lock().get(id); // Could also be a fn ptr or extern static match alloc { - Some(GlobalAlloc::Function(..)) => Ok((Size::ZERO, Align::from_bytes(1).unwrap())), + Some(GlobalAlloc::Function(..)) => { + if let AllocCheck::Dereferencable = liveness { + // The caller requested no function pointers. + err!(DerefFunctionPointer) + } else { + Ok((Size::ZERO, Align::from_bytes(1).unwrap())) + } + } // `self.get` would also work, but can cause cycles if a static refers to itself Some(GlobalAlloc::Static(did)) => { // The only way `get` couldn't have worked here is if this is an extern static @@ -545,15 +522,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { if let Ok(alloc) = self.get(id) { return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); } - match liveness { - InboundsCheck::MaybeDead => { - // Must be a deallocated pointer - self.dead_alloc_map.get(&id).cloned().ok_or_else(|| - ValidationFailure("allocation missing in dead_alloc_map".to_string()) - .into() - ) - }, - InboundsCheck::Live => err!(DanglingPointerDeref), + if let AllocCheck::MaybeDead = liveness { + // Deallocated pointers are allowed, we should be able to find + // them in the map. + self.dead_alloc_map.get(&id).copied().ok_or_else(|| + ValidationFailure("allocation missing in dead_alloc_map".to_string()) + .into() + ) + } else { + err!(DanglingPointerDeref) } }, } diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 0293a8366d0..259bd6af0d5 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -24,7 +24,7 @@ pub use self::eval_context::{ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; -pub use self::memory::{Memory, MemoryKind}; +pub use self::memory::{Memory, MemoryKind, AllocCheck}; pub use self::machine::{Machine, AllocMap, MayLeak};