From d8ddb47fcebf2c73e8d92d65925c4ed72d22171f Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 19 Dec 2018 16:26:46 +0100 Subject: [PATCH 1/2] Allow testing pointers for inboundedness while forbidding dangling pointers --- src/librustc_mir/interpret/memory.rs | 10 ++++------ src/librustc_mir/interpret/operand.rs | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 420fe26426321..de7ad1651c166 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -262,7 +262,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. - let align = self.check_bounds_ptr_maybe_dead(ptr)?; + let align = self.check_bounds_ptr(ptr, InboundsCheck::MaybeDead)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { @@ -297,17 +297,15 @@ 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( + pub fn check_bounds_ptr( &self, ptr: Pointer, + liveness: InboundsCheck, ) -> EvalResult<'tcx, Align> { let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id); - ptr.check_in_alloc(allocation_size, InboundsCheck::MaybeDead)?; + ptr.check_in_alloc(allocation_size, liveness)?; Ok(align) } } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 83ceadada65ce..76f851a958c59 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -17,7 +17,7 @@ use rustc::mir; use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx}; use rustc::mir::interpret::{ - GlobalId, AllocId, + GlobalId, AllocId, InboundsCheck, ConstValue, Pointer, Scalar, EvalResult, EvalErrorKind, }; @@ -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_maybe_dead(ptr).is_ok(); + self.memory.check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok(); if !ptr_valid { return err!(InvalidDiscriminant(raw_discr.erase_tag())); } From c8bcac56649b21ae291e9d224c7e1ddb2f5c62a0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 20 Dec 2018 15:14:25 +0100 Subject: [PATCH 2/2] Reintroduce the original `check_bounds_ptr` checks --- src/librustc_mir/interpret/memory.rs | 32 ++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index de7ad1651c166..e5245c24c8c6a 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -304,7 +304,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ptr: Pointer, liveness: InboundsCheck, ) -> EvalResult<'tcx, Align> { - let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id); + let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, liveness)?; ptr.check_in_alloc(allocation_size, liveness)?; Ok(align) } @@ -427,27 +427,37 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } - pub fn get_size_and_align(&self, id: AllocId) -> (Size, Align) { + /// Obtain the size and alignment of an allocation, even if that allocation has been deallocated + /// + /// If `liveness` is `InboundsCheck::Dead`, this function always returns `Ok` + pub fn get_size_and_align( + &self, + id: AllocId, + liveness: InboundsCheck, + ) -> EvalResult<'static, (Size, Align)> { if let Ok(alloc) = self.get(id) { - return (Size::from_bytes(alloc.bytes.len() as u64), alloc.align); + return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); } // Could also be a fn ptr or extern static match self.tcx.alloc_map.lock().get(id) { - Some(AllocKind::Function(..)) => (Size::ZERO, Align::from_bytes(1).unwrap()), + Some(AllocKind::Function(..)) => Ok((Size::ZERO, Align::from_bytes(1).unwrap())), Some(AllocKind::Static(did)) => { // The only way `get` couldn't have worked here is if this is an extern static assert!(self.tcx.is_foreign_item(did)); // Use size and align of the type let ty = self.tcx.type_of(did); let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); - (layout.size, layout.align.abi) - } - _ => { - // Must be a deallocated pointer - *self.dead_alloc_map.get(&id).expect( - "allocation missing in dead_alloc_map" - ) + Ok((layout.size, layout.align.abi)) } + _ => match liveness { + InboundsCheck::MaybeDead => { + // Must be a deallocated pointer + Ok(*self.dead_alloc_map.get(&id).expect( + "allocation missing in dead_alloc_map" + )) + }, + InboundsCheck::Live => err!(DanglingPointerDeref), + }, } }