Skip to content

Commit

Permalink
Improve miri's error reporting in check_in_alloc
Browse files Browse the repository at this point in the history
  • Loading branch information
LooMaclin committed Apr 8, 2019
1 parent 3449fa9 commit 2a738bb
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 39 deletions.
44 changes: 14 additions & 30 deletions src/librustc/mir/interpret/allocation.rs
Expand Up @@ -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",
})
}
}
Expand Down Expand Up @@ -311,7 +295,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, CheckInAllocMsg::ReadCStr)?[..size])
Ok(&self.get_bytes(cx, ptr, size_with_null, CheckInAllocMsg::NullPointer)?[..size])
}
None => err!(UnterminatedCString(ptr.erase_tag())),
}
Expand All @@ -331,7 +315,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, 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)?;
Expand All @@ -353,7 +337,7 @@ impl<'tcx, Tag: Copy, Extra> Allocation<Tag, Extra> {
where Extra: AllocationExtra<Tag, MemoryExtra>
{
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(())
}
Expand All @@ -369,7 +353,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, CheckInAllocMsg::WriteRepeat)?;
let bytes = self.get_bytes_mut(cx, ptr, count, CheckInAllocMsg::MemoryAccess)?;
for b in bytes {
*b = val;
}
Expand All @@ -394,7 +378,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, 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() {
Expand Down Expand Up @@ -471,7 +455,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, 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
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/hair/pattern/_match.rs
Expand Up @@ -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)) => {
Expand All @@ -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!(
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_mir/interpret/memory.rs
Expand Up @@ -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 } => {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/operand.rs
Expand Up @@ -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()));
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/validity.rs
Expand Up @@ -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
Expand Down

0 comments on commit 2a738bb

Please sign in to comment.