From 725199ce6512cc946d5b16c4ef3a845a2926cb09 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 2 Apr 2019 05:58:25 +0300 Subject: [PATCH 01/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 70 +++++++++++++++++++----- src/librustc/mir/interpret/error.rs | 12 ++-- src/librustc/mir/interpret/mod.rs | 2 +- src/librustc/mir/interpret/pointer.rs | 6 +- src/librustc_mir/hair/pattern/_match.rs | 6 +- src/librustc_mir/interpret/memory.rs | 20 +++---- src/librustc_mir/interpret/operand.rs | 4 +- src/librustc_mir/interpret/validity.rs | 4 +- 8 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 80fef910cc718..f00da804e9535 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -7,7 +7,7 @@ use super::{ use crate::ty::layout::{Size, Align}; use syntax::ast::Mutability; -use std::iter; +use std::{iter, fmt::{self, Display}}; use crate::mir; use std::ops::{Deref, DerefMut}; use rustc_data_structures::sorted_map::SortedMap; @@ -22,6 +22,44 @@ pub enum InboundsCheck { MaybeDead, } +/// Used by `check_in_alloc` 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 CheckInAllocMsg { + ReadCStr, + CheckBytes, + WriteBytes, + WriteRepeat, + ReadScalar, + WriteScalar, + SlicePatCoveredByConst, + ReadDiscriminant, + CheckAlign, + ReadBytes, + CopyRepeatedly, + CheckBounds, +} + +impl Display for CheckInAllocMsg { + + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", match *self { + CheckInAllocMsg::ReadCStr => "read C str", + CheckInAllocMsg::CheckBytes => "check bytes", + CheckInAllocMsg::WriteBytes => "write bytes", + CheckInAllocMsg::WriteRepeat => "write repeat", + CheckInAllocMsg::ReadScalar => "read scalar", + CheckInAllocMsg::WriteScalar => "write scalar", + CheckInAllocMsg::SlicePatCoveredByConst => "slice pat covered by const", + CheckInAllocMsg::ReadDiscriminant => "read discriminant", + CheckInAllocMsg::CheckAlign => "check align", + CheckInAllocMsg::ReadBytes => "read bytes", + CheckInAllocMsg::CopyRepeatedly => "copy repeatedly", + CheckInAllocMsg::CheckBounds => "check bounds", + }) + } +} + #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] pub struct Allocation { /// The actual bytes of the allocation. @@ -140,9 +178,10 @@ impl<'tcx, Tag, Extra> Allocation { fn check_bounds_ptr( &self, ptr: Pointer, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx> { let allocation_size = self.bytes.len() as u64; - ptr.check_in_alloc(Size::from_bytes(allocation_size), InboundsCheck::Live) + ptr.check_in_alloc(Size::from_bytes(allocation_size), msg) } /// Checks if the memory range beginning at `ptr` and of size `Size` is "in-bounds". @@ -152,9 +191,10 @@ impl<'tcx, Tag, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx> { // if ptr.offset is in bounds, then so is ptr (because offset checks for overflow) - self.check_bounds_ptr(ptr.offset(size, cx)?) + self.check_bounds_ptr(ptr.offset(size, cx)?, msg) } } @@ -173,11 +213,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation { ptr: Pointer, size: Size, check_defined_and_ptr: bool, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &[u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.check_bounds(cx, ptr, size)?; + self.check_bounds(cx, ptr, size, msg)?; if check_defined_and_ptr { self.check_defined(ptr, size)?; @@ -201,11 +242,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &[u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, true) + self.get_bytes_internal(cx, ptr, size, true, msg) } /// It is the caller's responsibility to handle undefined and pointer bytes. @@ -216,11 +258,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &[u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, false) + self.get_bytes_internal(cx, ptr, size, false, msg) } /// Just calling this already marks everything as defined and removes relocations, @@ -230,12 +273,13 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &mut [u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_bounds(cx, ptr, size)?; + self.check_bounds(cx, ptr, size, msg)?; self.mark_definedness(ptr, size, true)?; self.clear_relocations(cx, ptr, size)?; @@ -269,7 +313,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // Go through `get_bytes` for checks and AllocationExtra hooks. // We read the null, so we include it in the request, but we want it removed // from the result! - Ok(&self.get_bytes(cx, ptr, size_with_null)?[..size]) + Ok(&self.get_bytes(cx, ptr, size_with_null, CheckInAllocMsg::ReadCStr)?[..size]) } None => err!(UnterminatedCString(ptr.erase_tag())), } @@ -289,7 +333,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // Check bounds and relocations on the edges - self.get_bytes_with_undef_and_ptr(cx, ptr, size)?; + self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::CheckBytes)?; // Check undef and ptr if !allow_ptr_and_undef { self.check_defined(ptr, size)?; @@ -310,7 +354,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?; + let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64), CheckInAllocMsg::WriteBytes)?; bytes.clone_from_slice(src); Ok(()) } @@ -326,7 +370,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, count)?; + let bytes = self.get_bytes_mut(cx, ptr, count, CheckInAllocMsg::WriteRepeat)?; for b in bytes { *b = val; } @@ -351,7 +395,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // get_bytes_unchecked tests relocation edges - let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size)?; + let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::ReadScalar)?; // 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() { @@ -428,7 +472,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { }; let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, ptr, type_size)?; + let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::WriteScalar)?; write_target_uint(endian, dst, bytes).unwrap(); // See if we have to also write a relocation diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index fc04c7672db68..a3acc03fb6318 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -8,7 +8,7 @@ use crate::ty::layout::{Size, Align, LayoutError}; use rustc_target::spec::abi::Abi; use rustc_macros::HashStable; -use super::{RawConst, Pointer, InboundsCheck, ScalarMaybeUndef}; +use super::{RawConst, Pointer, CheckInAllocMsg, ScalarMaybeUndef}; use backtrace::Backtrace; @@ -243,7 +243,7 @@ pub enum EvalErrorKind<'tcx, O> { InvalidDiscriminant(ScalarMaybeUndef), PointerOutOfBounds { ptr: Pointer, - check: InboundsCheck, + msg: CheckInAllocMsg, allocation_size: Size, }, InvalidNullPointerUsage, @@ -460,13 +460,9 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for EvalErrorKind<'tcx, O> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use self::EvalErrorKind::*; match *self { - PointerOutOfBounds { ptr, check, allocation_size } => { + PointerOutOfBounds { ptr, msg, allocation_size } => { write!(f, "Pointer must be in-bounds{} at offset {}, but is outside bounds of \ - allocation {} which has size {}", - match check { - InboundsCheck::Live => " and live", - InboundsCheck::MaybeDead => "", - }, + allocation {} which has size {}", msg, ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes()) }, ValidationFailure(ref err) => { diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 0dd8316852780..c6881cfa99f3f 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -19,7 +19,7 @@ pub use self::value::{Scalar, ScalarMaybeUndef, RawConst, ConstValue}; pub use self::allocation::{ InboundsCheck, Allocation, AllocationExtra, - Relocations, UndefMask, + Relocations, UndefMask, CheckInAllocMsg, }; pub use self::pointer::{Pointer, PointerArithmetic}; diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index 9216cb494cef9..bf9694693aa17 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -3,7 +3,7 @@ use crate::ty::layout::{self, HasDataLayout, Size}; use rustc_macros::HashStable; use super::{ - AllocId, EvalResult, InboundsCheck, + AllocId, EvalResult, CheckInAllocMsg }; //////////////////////////////////////////////////////////////////////////////// @@ -157,12 +157,12 @@ impl<'tcx, Tag> Pointer { pub fn check_in_alloc( self, allocation_size: Size, - check: InboundsCheck, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, ()> { if self.offset > allocation_size { err!(PointerOutOfBounds { ptr: self.erase_tag(), - check, + msg, allocation_size, }) } else { diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 303ffcb3bfb3a..014c5ced37b25 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Scalar, truncate}; +use rustc::mir::interpret::{ConstValue, Scalar, truncate, CheckInAllocMsg}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -1418,7 +1418,7 @@ fn slice_pat_covered_by_const<'tcx>( return Ok(false); } let n = n.assert_usize(tcx).unwrap(); - alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() + alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::SlicePatCoveredByConst).unwrap() }, // a slice fat pointer to a zero length slice (ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => { @@ -1443,7 +1443,7 @@ fn slice_pat_covered_by_const<'tcx>( tcx.alloc_map .lock() .unwrap_memory(ptr.alloc_id) - .get_bytes(&tcx, ptr, Size::from_bytes(n)) + .get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::SlicePatCoveredByConst) .unwrap() }, _ => bug!( diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 6ea200d4e4fad..302bc84e65c4a 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -20,7 +20,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, EvalResult, Scalar, EvalErrorKind, AllocKind, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, InboundsCheck, + Machine, AllocMap, MayLeak, ErrorHandled, InboundsCheck, CheckInAllocMsg, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -252,7 +252,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(ptr, InboundsCheck::MaybeDead)?; + let align = self.check_bounds_ptr(ptr, CheckInAllocMsg::CheckAlign)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { @@ -292,10 +292,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn check_bounds_ptr( &self, ptr: Pointer, - liveness: InboundsCheck, + msg: CheckInAllocMsg, ) -> EvalResult<'tcx, Align> { - let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, liveness)?; - ptr.check_in_alloc(allocation_size, liveness)?; + let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, msg)?; + ptr.check_in_alloc(allocation_size, msg)?; Ok(align) } } @@ -423,7 +423,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn get_size_and_align( &self, id: AllocId, - liveness: InboundsCheck, + msg: CheckInAllocMsg, ) -> EvalResult<'static, (Size, Align)> { if let Ok(alloc) = self.get(id) { return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); @@ -439,7 +439,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); Ok((layout.size, layout.align.abi)) } - _ => match liveness { + _ => match msg { InboundsCheck::MaybeDead => { // Must be a deallocated pointer Ok(*self.dead_alloc_map.get(&id).expect( @@ -604,7 +604,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)?.get_bytes(self, ptr, size) + self.get(ptr.alloc_id)?.get_bytes(self, ptr, size, CheckInAllocMsg::ReadBytes) } } } @@ -729,10 +729,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // This checks relocation edges on the src. let src_bytes = self.get(src.alloc_id)? - .get_bytes_with_undef_and_ptr(&tcx, src, size)? + .get_bytes_with_undef_and_ptr(&tcx, src, size, CheckInAllocMsg::CopyRepeatedly)? .as_ptr(); let dest_bytes = self.get_mut(dest.alloc_id)? - .get_bytes_mut(&tcx, dest, size * length)? + .get_bytes_mut(&tcx, dest, size * length, CheckInAllocMsg::CopyRepeatedly)? .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 38a9371b92723..b90ec42de7ef0 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -7,7 +7,7 @@ use rustc::{mir, ty}; use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt, VariantIdx}; use rustc::mir::interpret::{ - GlobalId, AllocId, InboundsCheck, + GlobalId, AllocId, CheckInAllocMsg, ConstValue, Pointer, Scalar, EvalResult, EvalErrorKind, sign_extend, truncate, @@ -667,7 +667,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'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(ptr, CheckInAllocMsg::ReadDiscriminant).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 3323ec387bfd5..23c1681cbd879 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -7,7 +7,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, AllocKind, EvalResult, EvalErrorKind, + Scalar, AllocKind, EvalResult, EvalErrorKind, CheckInAllocMsg, }; use super::{ @@ -394,7 +394,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> try_validation!( self.ecx.memory .get(ptr.alloc_id)? - .check_bounds(self.ecx, ptr, size), + .check_bounds(self.ecx, ptr, size, CheckInAllocMsg::CheckBounds), "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination From 7b4bc6974a16df46e5d50a1191c124fc8c1f56ae Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 2 Apr 2019 06:16:11 +0300 Subject: [PATCH 02/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 3 +-- src/librustc_mir/interpret/memory.rs | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index f00da804e9535..a04316b719e83 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -22,8 +22,7 @@ pub enum InboundsCheck { MaybeDead, } -/// Used by `check_in_alloc` to indicate whether the pointer needs to be just inbounds -/// or also inbounds of a *live* allocation. +/// Used by `check_in_alloc` to indicate context of check #[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum CheckInAllocMsg { ReadCStr, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 302bc84e65c4a..80efafa655036 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -20,7 +20,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, EvalResult, Scalar, EvalErrorKind, AllocKind, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, InboundsCheck, CheckInAllocMsg, + Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -440,13 +440,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok((layout.size, layout.align.abi)) } _ => match msg { - InboundsCheck::MaybeDead => { + CheckInAllocMsg::CheckAlign | CheckInAllocMsg::ReadDiscriminant => { // Must be a deallocated pointer Ok(*self.dead_alloc_map.get(&id).expect( "allocation missing in dead_alloc_map" )) }, - InboundsCheck::Live => err!(DanglingPointerDeref), + _ => err!(DanglingPointerDeref), }, } } From 705d75ef1aa20c124975786cf1ad9764a82b12ef Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 2 Apr 2019 06:33:54 +0300 Subject: [PATCH 03/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 3 ++- src/librustc_mir/hair/pattern/_match.rs | 3 ++- src/librustc_mir/interpret/operand.rs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index a04316b719e83..50f5da77c0fb6 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -353,7 +353,8 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64), CheckInAllocMsg::WriteBytes)?; + let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64), + CheckInAllocMsg::WriteBytes)?; bytes.clone_from_slice(src); Ok(()) } diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 014c5ced37b25..561ac207b6bcb 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1418,7 +1418,8 @@ fn slice_pat_covered_by_const<'tcx>( return Ok(false); } let n = n.assert_usize(tcx).unwrap(); - alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::SlicePatCoveredByConst).unwrap() + alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), + CheckInAllocMsg::SlicePatCoveredByConst).unwrap() }, // a slice fat pointer to a zero length slice (ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index b90ec42de7ef0..7529ec0aee59c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -667,7 +667,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'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, CheckInAllocMsg::ReadDiscriminant).is_ok(); + self.memory.check_bounds_ptr(ptr, + CheckInAllocMsg::ReadDiscriminant).is_ok(); if !ptr_valid { return err!(InvalidDiscriminant(raw_discr.erase_tag())); } From 4c4dbb12d35c502c27b6cf284fcc4d0000661f8c Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 2 Apr 2019 18:24:57 +0300 Subject: [PATCH 04/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 50f5da77c0fb6..3f7730a8bbc07 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -40,7 +40,6 @@ pub enum CheckInAllocMsg { } impl Display for CheckInAllocMsg { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match *self { CheckInAllocMsg::ReadCStr => "read C str", From 2a738bb8edc7c0ed77bb12657a55db666cffeda3 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Mon, 8 Apr 2019 23:34:28 +0300 Subject: [PATCH 05/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 44 ++++++++---------------- src/librustc_mir/hair/pattern/_match.rs | 4 +-- src/librustc_mir/interpret/memory.rs | 10 +++--- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/validity.rs | 2 +- 5 files changed, 23 insertions(+), 39 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 3f7730a8bbc07..ca680187b37db 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -25,35 +25,19 @@ pub enum InboundsCheck { /// Used by `check_in_alloc` to indicate context of check #[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum CheckInAllocMsg { - ReadCStr, - CheckBytes, - WriteBytes, - WriteRepeat, - ReadScalar, - WriteScalar, - SlicePatCoveredByConst, - ReadDiscriminant, - CheckAlign, - ReadBytes, - CopyRepeatedly, - CheckBounds, + MemoryAccess, + NullPointer, + PointerArithmetic, + OutOfBounds, } impl Display for CheckInAllocMsg { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match *self { - CheckInAllocMsg::ReadCStr => "read C str", - CheckInAllocMsg::CheckBytes => "check bytes", - CheckInAllocMsg::WriteBytes => "write bytes", - CheckInAllocMsg::WriteRepeat => "write repeat", - CheckInAllocMsg::ReadScalar => "read scalar", - CheckInAllocMsg::WriteScalar => "write scalar", - CheckInAllocMsg::SlicePatCoveredByConst => "slice pat covered by const", - CheckInAllocMsg::ReadDiscriminant => "read discriminant", - CheckInAllocMsg::CheckAlign => "check align", - CheckInAllocMsg::ReadBytes => "read bytes", - CheckInAllocMsg::CopyRepeatedly => "copy repeatedly", - CheckInAllocMsg::CheckBounds => "check bounds", + CheckInAllocMsg::MemoryAccess => "memory access", + CheckInAllocMsg::NullPointer => "null pointer", + CheckInAllocMsg::PointerArithmetic => "pointer arithmetic", + CheckInAllocMsg::OutOfBounds => "out of bounds", }) } } @@ -311,7 +295,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // Go through `get_bytes` for checks and AllocationExtra hooks. // We read the null, so we include it in the request, but we want it removed // from the result! - Ok(&self.get_bytes(cx, ptr, size_with_null, CheckInAllocMsg::ReadCStr)?[..size]) + Ok(&self.get_bytes(cx, ptr, size_with_null, CheckInAllocMsg::NullPointer)?[..size]) } None => err!(UnterminatedCString(ptr.erase_tag())), } @@ -331,7 +315,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // Check bounds and relocations on the edges - self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::CheckBytes)?; + self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::OutOfBounds)?; // Check undef and ptr if !allow_ptr_and_undef { self.check_defined(ptr, size)?; @@ -353,7 +337,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64), - CheckInAllocMsg::WriteBytes)?; + CheckInAllocMsg::MemoryAccess)?; bytes.clone_from_slice(src); Ok(()) } @@ -369,7 +353,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, count, CheckInAllocMsg::WriteRepeat)?; + let bytes = self.get_bytes_mut(cx, ptr, count, CheckInAllocMsg::MemoryAccess)?; for b in bytes { *b = val; } @@ -394,7 +378,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // get_bytes_unchecked tests relocation edges - let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::ReadScalar)?; + let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::PointerArithmetic)?; // 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() { @@ -471,7 +455,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { }; let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::WriteScalar)?; + let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::PointerArithmetic)?; write_target_uint(endian, dst, bytes).unwrap(); // See if we have to also write a relocation diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 561ac207b6bcb..1e2ffa5db440b 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -1419,7 +1419,7 @@ fn slice_pat_covered_by_const<'tcx>( } let n = n.assert_usize(tcx).unwrap(); alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), - CheckInAllocMsg::SlicePatCoveredByConst).unwrap() + CheckInAllocMsg::OutOfBounds).unwrap() }, // a slice fat pointer to a zero length slice (ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => { @@ -1444,7 +1444,7 @@ fn slice_pat_covered_by_const<'tcx>( tcx.alloc_map .lock() .unwrap_memory(ptr.alloc_id) - .get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::SlicePatCoveredByConst) + .get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::OutOfBounds) .unwrap() }, _ => bug!( diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index b5e3f81869919..a911479a15a23 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -252,7 +252,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(ptr, CheckInAllocMsg::CheckAlign)?; + let align = self.check_bounds_ptr(ptr, CheckInAllocMsg::NullPointer)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { @@ -440,7 +440,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Ok((layout.size, layout.align.abi)) } _ => match msg { - CheckInAllocMsg::CheckAlign | CheckInAllocMsg::ReadDiscriminant => { + CheckInAllocMsg::NullPointer | CheckInAllocMsg::OutOfBounds => { // Must be a deallocated pointer Ok(*self.dead_alloc_map.get(&id).expect( "allocation missing in dead_alloc_map" @@ -604,7 +604,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)?.get_bytes(self, ptr, size, CheckInAllocMsg::ReadBytes) + self.get(ptr.alloc_id)?.get_bytes(self, ptr, size, CheckInAllocMsg::MemoryAccess) } } } @@ -729,10 +729,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // This checks relocation edges on the src. let src_bytes = self.get(src.alloc_id)? - .get_bytes_with_undef_and_ptr(&tcx, src, size, CheckInAllocMsg::CopyRepeatedly)? + .get_bytes_with_undef_and_ptr(&tcx, src, size, CheckInAllocMsg::MemoryAccess)? .as_ptr(); let dest_bytes = self.get_mut(dest.alloc_id)? - .get_bytes_mut(&tcx, dest, size * length, CheckInAllocMsg::CopyRepeatedly)? + .get_bytes_mut(&tcx, dest, size * length, CheckInAllocMsg::MemoryAccess)? .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 166e79c12d806..e7e22a12cd68f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -668,7 +668,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> // 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, - CheckInAllocMsg::ReadDiscriminant).is_ok(); + CheckInAllocMsg::OutOfBounds).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 37c880ccf72bf..965b2898cad53 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -394,7 +394,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> try_validation!( self.ecx.memory .get(ptr.alloc_id)? - .check_bounds(self.ecx, ptr, size, CheckInAllocMsg::CheckBounds), + .check_bounds(self.ecx, ptr, size, CheckInAllocMsg::OutOfBounds), "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination From 980db98b6440779ed714e454f544faf707b2ab9c Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 9 Apr 2019 00:01:08 +0300 Subject: [PATCH 06/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index ca680187b37db..19f28b96c3a70 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -378,7 +378,8 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // get_bytes_unchecked tests relocation edges - let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::PointerArithmetic)?; + let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, + CheckInAllocMsg::PointerArithmetic)?; // 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() { From e5b6fab5768d3e2079c3a06e4206d3a199507be4 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 9 Apr 2019 00:39:57 +0300 Subject: [PATCH 07/18] Improve miri's error reporting in check_in_alloc --- src/librustc_mir/interpret/memory.rs | 15 ++++++++------- src/librustc_mir/interpret/operand.rs | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index a911479a15a23..6475eac41e3a0 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -20,7 +20,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, EvalResult, Scalar, InterpError, AllocKind, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, + Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, InboundsCheck, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -252,7 +252,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(ptr, CheckInAllocMsg::NullPointer)?; + let align = self.check_bounds_ptr(ptr, InboundsCheck::MaybeDead, CheckInAllocMsg::NullPointer)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { @@ -292,9 +292,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { pub fn check_bounds_ptr( &self, ptr: Pointer, + liveness: InboundsCheck, msg: CheckInAllocMsg, ) -> EvalResult<'tcx, Align> { - let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, msg)?; + let (allocation_size, align) = self.get_size_and_align(ptr.alloc_id, liveness)?; ptr.check_in_alloc(allocation_size, msg)?; Ok(align) } @@ -419,11 +420,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// 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` + /// If `liveness` is `InboundsCheck::MaybeDead`, this function always returns `Ok` pub fn get_size_and_align( &self, id: AllocId, - msg: CheckInAllocMsg, + liveness: InboundsCheck, ) -> EvalResult<'static, (Size, Align)> { if let Ok(alloc) = self.get(id) { return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); @@ -439,8 +440,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); Ok((layout.size, layout.align.abi)) } - _ => match msg { - CheckInAllocMsg::NullPointer | CheckInAllocMsg::OutOfBounds => { + _ => match liveness { + InboundsCheck::MaybeDead => { // Must be a deallocated pointer Ok(*self.dead_alloc_map.get(&id).expect( "allocation missing in dead_alloc_map" diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index e7e22a12cd68f..38fe1caa41f60 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -9,7 +9,7 @@ use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerEx use rustc::mir::interpret::{ GlobalId, AllocId, CheckInAllocMsg, ConstValue, Pointer, Scalar, - EvalResult, InterpError, + EvalResult, InterpError, InboundsCheck, sign_extend, truncate, }; use super::{ @@ -667,7 +667,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'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, + self.memory.check_bounds_ptr(ptr, InboundsCheck::MaybeDead, CheckInAllocMsg::OutOfBounds).is_ok(); if !ptr_valid { return err!(InvalidDiscriminant(raw_discr.erase_tag())); From 11464e714a670c6304cd00d20c0e31cc66290866 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 9 Apr 2019 00:42:45 +0300 Subject: [PATCH 08/18] Improve miri's error reporting in check_in_alloc --- 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 6475eac41e3a0..3c7cded5ba604 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -252,7 +252,8 @@ 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(ptr, InboundsCheck::MaybeDead, CheckInAllocMsg::NullPointer)?; + let align = self.check_bounds_ptr(ptr, InboundsCheck::MaybeDead, + CheckInAllocMsg::NullPointer)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { From 32ba4bda7aa1b8e66714090d5a8e7253c83f8161 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 9 Apr 2019 01:11:02 +0300 Subject: [PATCH 09/18] Improve miri's error reporting in check_in_alloc --- 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 3c7cded5ba604..cc8bed770c460 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -448,7 +448,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { "allocation missing in dead_alloc_map" )) }, - _ => err!(DanglingPointerDeref), + InboundsCheck::Live => err!(DanglingPointerDeref), }, } } From 9147e26fcb8d0f140d5c7f97a6063758ff5aeac4 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 9 Apr 2019 12:38:14 +0300 Subject: [PATCH 10/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 9 ++++----- src/librustc_mir/hair/pattern/_match.rs | 7 +++---- src/librustc_mir/interpret/memory.rs | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 19f28b96c3a70..61f77171ce354 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -224,12 +224,11 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &[u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, true, msg) + self.get_bytes_internal(cx, ptr, size, true, CheckInAllocMsg::MemoryAccess) } /// It is the caller's responsibility to handle undefined and pointer bytes. @@ -295,7 +294,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // Go through `get_bytes` for checks and AllocationExtra hooks. // We read the null, so we include it in the request, but we want it removed // from the result! - Ok(&self.get_bytes(cx, ptr, size_with_null, CheckInAllocMsg::NullPointer)?[..size]) + Ok(&self.get_bytes(cx, ptr, size_with_null)?[..size]) } None => err!(UnterminatedCString(ptr.erase_tag())), } @@ -379,7 +378,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { { // get_bytes_unchecked tests relocation edges let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, - CheckInAllocMsg::PointerArithmetic)?; + CheckInAllocMsg::MemoryAccess)?; // 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() { @@ -456,7 +455,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { }; let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::PointerArithmetic)?; + let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::MemoryAccess)?; write_target_uint(endian, dst, bytes).unwrap(); // See if we have to also write a relocation diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1e2ffa5db440b..303ffcb3bfb3a 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Scalar, truncate, CheckInAllocMsg}; +use rustc::mir::interpret::{ConstValue, Scalar, truncate}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -1418,8 +1418,7 @@ fn slice_pat_covered_by_const<'tcx>( return Ok(false); } let n = n.assert_usize(tcx).unwrap(); - alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), - CheckInAllocMsg::OutOfBounds).unwrap() + alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() }, // a slice fat pointer to a zero length slice (ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => { @@ -1444,7 +1443,7 @@ fn slice_pat_covered_by_const<'tcx>( tcx.alloc_map .lock() .unwrap_memory(ptr.alloc_id) - .get_bytes(&tcx, ptr, Size::from_bytes(n), CheckInAllocMsg::OutOfBounds) + .get_bytes(&tcx, ptr, Size::from_bytes(n)) .unwrap() }, _ => bug!( diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index cc8bed770c460..b9e2b9d499e55 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -606,7 +606,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)?.get_bytes(self, ptr, size, CheckInAllocMsg::MemoryAccess) + self.get(ptr.alloc_id)?.get_bytes(self, ptr, size) } } } From 30a96267a6050c55e1f2bb1c540458a88fdb9567 Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Wed, 10 Apr 2019 04:18:19 +0300 Subject: [PATCH 11/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 18 +++++++----------- src/librustc_mir/interpret/memory.rs | 4 ++-- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 61f77171ce354..433c105231e01 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -239,12 +239,11 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &[u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, false, msg) + self.get_bytes_internal(cx, ptr, size, false, CheckInAllocMsg::MemoryAccess) } /// Just calling this already marks everything as defined and removes relocations, @@ -254,13 +253,12 @@ impl<'tcx, Tag: Copy, Extra> Allocation { cx: &impl HasDataLayout, ptr: Pointer, size: Size, - msg: CheckInAllocMsg, ) -> EvalResult<'tcx, &mut [u8]> // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_bounds(cx, ptr, size, msg)?; + self.check_bounds(cx, ptr, size, CheckInAllocMsg::MemoryAccess)?; self.mark_definedness(ptr, size, true)?; self.clear_relocations(cx, ptr, size)?; @@ -314,7 +312,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // Check bounds and relocations on the edges - self.get_bytes_with_undef_and_ptr(cx, ptr, size, CheckInAllocMsg::OutOfBounds)?; + self.get_bytes_with_undef_and_ptr(cx, ptr, size)?; // Check undef and ptr if !allow_ptr_and_undef { self.check_defined(ptr, size)?; @@ -335,8 +333,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64), - CheckInAllocMsg::MemoryAccess)?; + let bytes = self.get_bytes_mut(cx, ptr, Size::from_bytes(src.len() as u64))?; bytes.clone_from_slice(src); Ok(()) } @@ -352,7 +349,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - let bytes = self.get_bytes_mut(cx, ptr, count, CheckInAllocMsg::MemoryAccess)?; + let bytes = self.get_bytes_mut(cx, ptr, count)?; for b in bytes { *b = val; } @@ -377,8 +374,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { // get_bytes_unchecked tests relocation edges - let bytes = self.get_bytes_with_undef_and_ptr(cx, ptr, size, - CheckInAllocMsg::MemoryAccess)?; + 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() { @@ -455,7 +451,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { }; let endian = cx.data_layout().endian; - let dst = self.get_bytes_mut(cx, ptr, type_size, CheckInAllocMsg::MemoryAccess)?; + let dst = self.get_bytes_mut(cx, ptr, type_size)?; write_target_uint(endian, dst, bytes).unwrap(); // See if we have to also write a relocation diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index b9e2b9d499e55..e8ae7ab579b93 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -731,10 +731,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // This checks relocation edges on the src. let src_bytes = self.get(src.alloc_id)? - .get_bytes_with_undef_and_ptr(&tcx, src, size, CheckInAllocMsg::MemoryAccess)? + .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, CheckInAllocMsg::MemoryAccess)? + .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 From 0d97ad383412970b39275226bdfde7393d73867f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Apr 2019 19:24:28 +0300 Subject: [PATCH 12/18] Update src/librustc/mir/interpret/allocation.rs Co-Authored-By: LooMaclin --- src/librustc/mir/interpret/allocation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 433c105231e01..35b8a5fda4feb 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -26,7 +26,7 @@ pub enum InboundsCheck { #[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum CheckInAllocMsg { MemoryAccess, - NullPointer, + NullPointerTest, PointerArithmetic, OutOfBounds, } From 15d50deeb457d5761802dd1066f7bfbeaf71c2cb Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Fri, 19 Apr 2019 02:10:59 +0300 Subject: [PATCH 13/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 14 +++++++------- src/librustc/mir/interpret/error.rs | 2 +- src/librustc_mir/interpret/memory.rs | 2 +- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/validity.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 35b8a5fda4feb..79ef81e81eb1f 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -25,19 +25,19 @@ pub enum InboundsCheck { /// Used by `check_in_alloc` to indicate context of check #[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum CheckInAllocMsg { - MemoryAccess, + MemoryAccessTest, NullPointerTest, - PointerArithmetic, - OutOfBounds, + PointerArithmeticTest, + InboundsTest, } impl Display for CheckInAllocMsg { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", match *self { - CheckInAllocMsg::MemoryAccess => "memory access", - CheckInAllocMsg::NullPointer => "null pointer", - CheckInAllocMsg::PointerArithmetic => "pointer arithmetic", - CheckInAllocMsg::OutOfBounds => "out of bounds", + CheckInAllocMsg::MemoryAccessTest => "Memory access", + CheckInAllocMsg::NullPointerTest => "Null pointer", + CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic", + CheckInAllocMsg::InboundsTest => "Inbounds", }) } } diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 23a47fb1a35ea..2fe6982fdf821 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -461,7 +461,7 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for InterpError<'tcx, O> { use self::InterpError::*; match *self { PointerOutOfBounds { ptr, msg, allocation_size } => { - write!(f, "Pointer must be in-bounds{} at offset {}, but is outside bounds of \ + write!(f, "{} test failed: pointer must be in-bounds at offset {}, but is outside bounds of \ allocation {} which has size {}", msg, ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes()) }, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index e8ae7ab579b93..b176a0ac61f3a 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { // 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(ptr, InboundsCheck::MaybeDead, - CheckInAllocMsg::NullPointer)?; + CheckInAllocMsg::NullPointerTest)?; (ptr.offset.bytes(), align) } Scalar::Bits { bits, size } => { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 38fe1caa41f60..8d9550e8f07be 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -668,7 +668,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> // 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, - CheckInAllocMsg::OutOfBounds).is_ok(); + CheckInAllocMsg::NullPointerTest).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 965b2898cad53..ebba704e4f4d6 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -394,7 +394,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> try_validation!( self.ecx.memory .get(ptr.alloc_id)? - .check_bounds(self.ecx, ptr, size, CheckInAllocMsg::OutOfBounds), + .check_bounds(self.ecx, ptr, size, CheckInAllocMsg::InboundsTest), "dangling (not entirely in bounds) reference", self.path); } // Check if we have encountered this pointer+layout combination From a54e3cc9e7aa938ef6e4aa3ba00864bcefc717df Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Fri, 19 Apr 2019 02:26:57 +0300 Subject: [PATCH 14/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 6 +++--- src/librustc/mir/interpret/error.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 79ef81e81eb1f..84733f9af6d58 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -228,7 +228,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, true, CheckInAllocMsg::MemoryAccess) + self.get_bytes_internal(cx, ptr, size, true, CheckInAllocMsg::MemoryAccessTest) } /// It is the caller's responsibility to handle undefined and pointer bytes. @@ -243,7 +243,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { // FIXME: Working around https://github.com/rust-lang/rust/issues/56209 where Extra: AllocationExtra { - self.get_bytes_internal(cx, ptr, size, false, CheckInAllocMsg::MemoryAccess) + self.get_bytes_internal(cx, ptr, size, false, CheckInAllocMsg::MemoryAccessTest) } /// Just calling this already marks everything as defined and removes relocations, @@ -258,7 +258,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation { where Extra: AllocationExtra { assert_ne!(size.bytes(), 0, "0-sized accesses should never even get a `Pointer`"); - self.check_bounds(cx, ptr, size, CheckInAllocMsg::MemoryAccess)?; + self.check_bounds(cx, ptr, size, CheckInAllocMsg::MemoryAccessTest)?; self.mark_definedness(ptr, size, true)?; self.clear_relocations(cx, ptr, size)?; diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 2fe6982fdf821..cf2d12b59359b 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -461,9 +461,9 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for InterpError<'tcx, O> { use self::InterpError::*; match *self { PointerOutOfBounds { ptr, msg, allocation_size } => { - write!(f, "{} test failed: pointer must be in-bounds at offset {}, but is outside bounds of \ - allocation {} which has size {}", msg, - ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes()) + write!(f, "{} test failed: pointer must be in-bounds at offset {}, \ + but is outside bounds of allocation {} which has size {}", + msg, ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes()) }, ValidationFailure(ref err) => { write!(f, "type validation failed: {}", err) From fc7ffa670c1eeea4999aa84d131c4fd5358da43b Mon Sep 17 00:00:00 2001 From: LooMaclin Date: Tue, 23 Apr 2019 03:15:27 +0300 Subject: [PATCH 15/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 4 +++- src/librustc/mir/interpret/error.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 84733f9af6d58..bc2dbfcbf388f 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -32,8 +32,10 @@ pub enum CheckInAllocMsg { } impl Display for CheckInAllocMsg { + /// When this printed as an error the context looks like this + /// "{test name} test failed: pointer must be in-bounds at offset..." fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", match *self { + write!(f, "{} test", match *self { CheckInAllocMsg::MemoryAccessTest => "Memory access", CheckInAllocMsg::NullPointerTest => "Null pointer", CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic", diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index cf2d12b59359b..ce281cf0d3400 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -461,7 +461,7 @@ impl<'tcx, O: fmt::Debug> fmt::Debug for InterpError<'tcx, O> { use self::InterpError::*; match *self { PointerOutOfBounds { ptr, msg, allocation_size } => { - write!(f, "{} test failed: pointer must be in-bounds at offset {}, \ + write!(f, "{} failed: pointer must be in-bounds at offset {}, \ but is outside bounds of allocation {} which has size {}", msg, ptr.offset.bytes(), ptr.alloc_id, allocation_size.bytes()) }, From b1c829b6379568b4355e9759981b6ad96d08c9c7 Mon Sep 17 00:00:00 2001 From: Loo Maclin Date: Tue, 23 Apr 2019 12:12:16 +0300 Subject: [PATCH 16/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index bc2dbfcbf388f..2910deadb9e43 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -32,7 +32,7 @@ pub enum CheckInAllocMsg { } impl Display for CheckInAllocMsg { - /// When this printed as an error the context looks like this + /// When this is printed as an error the context looks like this /// "{test name} test failed: pointer must be in-bounds at offset..." fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{} test", match *self { From ffd0dc79dab71743cbb5b345c3d0400b4cb0e645 Mon Sep 17 00:00:00 2001 From: loomaclin Date: Sun, 26 May 2019 13:26:24 +0300 Subject: [PATCH 17/18] Improve miri's error reporting in check_in_alloc --- src/librustc/mir/interpret/allocation.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 2910deadb9e43..718d9046f9240 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -33,13 +33,13 @@ pub enum CheckInAllocMsg { impl Display for CheckInAllocMsg { /// When this is printed as an error the context looks like this - /// "{test name} test failed: pointer must be in-bounds at offset..." + /// "{test name} failed: pointer must be in-bounds at offset..." fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} test", match *self { + write!(f, "{}", match *self { CheckInAllocMsg::MemoryAccessTest => "Memory access", - CheckInAllocMsg::NullPointerTest => "Null pointer", - CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic", - CheckInAllocMsg::InboundsTest => "Inbounds", + CheckInAllocMsg::NullPointerTest => "Null pointer test", + CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic test", + CheckInAllocMsg::InboundsTest => "Inbounds test", }) } } From 9e643e6792474cb0ae49873feb3ca65243469506 Mon Sep 17 00:00:00 2001 From: Rust Date: Sun, 26 May 2019 14:44:58 +0300 Subject: [PATCH 18/18] Improve miri's error reporting in check_in_alloc Co-Authored-By: Ralf Jung --- src/librustc/mir/interpret/allocation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 718d9046f9240..6b48db2cf85fb 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -38,7 +38,7 @@ impl Display for CheckInAllocMsg { write!(f, "{}", match *self { CheckInAllocMsg::MemoryAccessTest => "Memory access", CheckInAllocMsg::NullPointerTest => "Null pointer test", - CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic test", + CheckInAllocMsg::PointerArithmeticTest => "Pointer arithmetic", CheckInAllocMsg::InboundsTest => "Inbounds test", }) }