Improve miri's error reporting in check_in_alloc

This commit is contained in:
LooMaclin 2019-04-08 23:34:28 +03:00
parent 3449fa90f8
commit 2a738bb8ed
5 changed files with 23 additions and 39 deletions

View File

@ -25,35 +25,19 @@ pub enum InboundsCheck {
/// Used by `check_in_alloc` to indicate context of check /// Used by `check_in_alloc` to indicate context of check
#[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)] #[derive(Debug, Copy, Clone, RustcEncodable, RustcDecodable, HashStable)]
pub enum CheckInAllocMsg { pub enum CheckInAllocMsg {
ReadCStr, MemoryAccess,
CheckBytes, NullPointer,
WriteBytes, PointerArithmetic,
WriteRepeat, OutOfBounds,
ReadScalar,
WriteScalar,
SlicePatCoveredByConst,
ReadDiscriminant,
CheckAlign,
ReadBytes,
CopyRepeatedly,
CheckBounds,
} }
impl Display for CheckInAllocMsg { impl Display for CheckInAllocMsg {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", match *self { write!(f, "{}", match *self {
CheckInAllocMsg::ReadCStr => "read C str", CheckInAllocMsg::MemoryAccess => "memory access",
CheckInAllocMsg::CheckBytes => "check bytes", CheckInAllocMsg::NullPointer => "null pointer",
CheckInAllocMsg::WriteBytes => "write bytes", CheckInAllocMsg::PointerArithmetic => "pointer arithmetic",
CheckInAllocMsg::WriteRepeat => "write repeat", CheckInAllocMsg::OutOfBounds => "out of bounds",
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",
}) })
} }
} }
@ -311,7 +295,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
// Go through `get_bytes` for checks and AllocationExtra hooks. // 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 // We read the null, so we include it in the request, but we want it removed
// from the result! // 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())), None => err!(UnterminatedCString(ptr.erase_tag())),
} }
@ -331,7 +315,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra> where Extra: AllocationExtra<Tag, MemoryExtra>
{ {
// Check bounds and relocations on the edges // 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 // Check undef and ptr
if !allow_ptr_and_undef { if !allow_ptr_and_undef {
self.check_defined(ptr, size)?; self.check_defined(ptr, size)?;
@ -353,7 +337,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra> 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)?; CheckInAllocMsg::MemoryAccess)?;
bytes.clone_from_slice(src); bytes.clone_from_slice(src);
Ok(()) Ok(())
} }
@ -369,7 +353,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
// FIXME: Working around https://github.com/rust-lang/rust/issues/56209 // FIXME: Working around https://github.com/rust-lang/rust/issues/56209
where Extra: AllocationExtra<Tag, MemoryExtra> where Extra: AllocationExtra<Tag, MemoryExtra>
{ {
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 { for b in bytes {
*b = val; *b = val;
} }
@ -394,7 +378,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra> where Extra: AllocationExtra<Tag, MemoryExtra>
{ {
// get_bytes_unchecked tests relocation edges // 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. // Undef check happens *after* we established that the alignment is correct.
// We must not return Ok() for unaligned pointers! // We must not return Ok() for unaligned pointers!
if self.check_defined(ptr, size).is_err() { if self.check_defined(ptr, size).is_err() {
@ -471,7 +455,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
}; };
let endian = cx.data_layout().endian; 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(); write_target_uint(endian, dst, bytes).unwrap();
// See if we have to also write a relocation // See if we have to also write a relocation

View File

@ -1419,7 +1419,7 @@ fn slice_pat_covered_by_const<'tcx>(
} }
let n = n.assert_usize(tcx).unwrap(); let n = n.assert_usize(tcx).unwrap();
alloc.get_bytes(&tcx, ptr, Size::from_bytes(n), alloc.get_bytes(&tcx, ptr, Size::from_bytes(n),
CheckInAllocMsg::SlicePatCoveredByConst).unwrap() CheckInAllocMsg::OutOfBounds).unwrap()
}, },
// a slice fat pointer to a zero length slice // a slice fat pointer to a zero length slice
(ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => { (ConstValue::Slice(Scalar::Bits { .. }, 0), ty::Slice(t)) => {
@ -1444,7 +1444,7 @@ fn slice_pat_covered_by_const<'tcx>(
tcx.alloc_map tcx.alloc_map
.lock() .lock()
.unwrap_memory(ptr.alloc_id) .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() .unwrap()
}, },
_ => bug!( _ => bug!(

View File

@ -252,7 +252,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
Scalar::Ptr(ptr) => { Scalar::Ptr(ptr) => {
// check this is not NULL -- which we can ensure only if this is in-bounds // check this is not NULL -- which we can ensure only if this is in-bounds
// of some (potentially dead) allocation. // 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) (ptr.offset.bytes(), align)
} }
Scalar::Bits { bits, size } => { 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)) Ok((layout.size, layout.align.abi))
} }
_ => match msg { _ => match msg {
CheckInAllocMsg::CheckAlign | CheckInAllocMsg::ReadDiscriminant => { CheckInAllocMsg::NullPointer | CheckInAllocMsg::OutOfBounds => {
// Must be a deallocated pointer // Must be a deallocated pointer
Ok(*self.dead_alloc_map.get(&id).expect( Ok(*self.dead_alloc_map.get(&id).expect(
"allocation missing in dead_alloc_map" "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(&[]) Ok(&[])
} else { } else {
let ptr = ptr.to_ptr()?; 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. // This checks relocation edges on the src.
let src_bytes = self.get(src.alloc_id)? 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(); .as_ptr();
let dest_bytes = self.get_mut(dest.alloc_id)? 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(); .as_mut_ptr();
// SAFE: The above indexing would have panicked if there weren't at least `size` bytes // SAFE: The above indexing would have panicked if there weren't at least `size` bytes

View File

@ -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) // The niche must be just 0 (which an inbounds pointer value never is)
let ptr_valid = niche_start == 0 && variants_start == variants_end && let ptr_valid = niche_start == 0 && variants_start == variants_end &&
self.memory.check_bounds_ptr(ptr, self.memory.check_bounds_ptr(ptr,
CheckInAllocMsg::ReadDiscriminant).is_ok(); CheckInAllocMsg::OutOfBounds).is_ok();
if !ptr_valid { if !ptr_valid {
return err!(InvalidDiscriminant(raw_discr.erase_tag())); return err!(InvalidDiscriminant(raw_discr.erase_tag()));
} }

View File

@ -394,7 +394,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>>
try_validation!( try_validation!(
self.ecx.memory self.ecx.memory
.get(ptr.alloc_id)? .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); "dangling (not entirely in bounds) reference", self.path);
} }
// Check if we have encountered this pointer+layout combination // Check if we have encountered this pointer+layout combination