Improve miri's error reporting in check_in_alloc

This commit is contained in:
LooMaclin 2019-04-02 05:58:25 +03:00
parent 9ebf47851a
commit 725199ce65
8 changed files with 82 additions and 42 deletions

View File

@ -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<Tag=(),Extra=()> {
/// The actual bytes of the allocation.
@ -140,9 +178,10 @@ impl<'tcx, Tag, Extra> Allocation<Tag, Extra> {
fn check_bounds_ptr(
&self,
ptr: Pointer<Tag>,
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<Tag, Extra> {
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
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<Tag, Extra> {
ptr: Pointer<Tag>,
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<Tag, MemoryExtra>
{
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<Tag, Extra> {
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
msg: CheckInAllocMsg,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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<Tag, Extra> {
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
msg: CheckInAllocMsg,
) -> EvalResult<'tcx, &[u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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<Tag, Extra> {
cx: &impl HasDataLayout,
ptr: Pointer<Tag>,
size: Size,
msg: CheckInAllocMsg,
) -> EvalResult<'tcx, &mut [u8]>
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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<Tag, Extra> {
// 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<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra>
{
// 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<Tag, Extra> {
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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<Tag, Extra> {
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra>
{
// 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<Tag, Extra> {
};
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

View File

@ -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) => {

View File

@ -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};

View File

@ -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<Tag> {
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 {

View File

@ -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!(

View File

@ -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<M::PointerTag>,
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

View File

@ -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()));
}

View File

@ -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