1
Fork 0

Optimize miri checking of integer array/slices

Instead of checking every element, we can check the whole memory
range at once.
This commit is contained in:
Gabriel Majeri 2018-09-02 17:22:46 +03:00
parent 4efc0a7811
commit 82cde902c5
8 changed files with 82 additions and 32 deletions

View file

@ -523,7 +523,6 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
ReadBytesAsPointer |
ReadForeignStatic |
InvalidPointerMath |
ReadUndefBytes |
DeadLocal |
StackFrameLimitReached |
OutOfTls |
@ -550,6 +549,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
GeneratorResumedAfterReturn |
GeneratorResumedAfterPanic |
InfiniteLoop => {}
ReadUndefBytes(offset) => offset.hash_stable(hcx, hasher),
InvalidDiscriminant(val) => val.hash_stable(hcx, hasher),
Panic { ref msg, ref file, line, col } => {
msg.hash_stable(hcx, hasher);

View file

@ -205,7 +205,7 @@ pub enum EvalErrorKind<'tcx, O> {
ReadBytesAsPointer,
ReadForeignStatic,
InvalidPointerMath,
ReadUndefBytes,
ReadUndefBytes(Size),
DeadLocal,
InvalidBoolOp(mir::BinOp),
Unimplemented(String),
@ -331,7 +331,7 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
InvalidPointerMath =>
"attempted to do invalid arithmetic on pointers that would leak base addresses, \
e.g. comparing pointers into different allocations",
ReadUndefBytes =>
ReadUndefBytes(_) =>
"attempted to read undefined bytes",
DeadLocal =>
"tried to access a dead local variable",

View file

@ -640,16 +640,23 @@ impl UndefMask {
}
/// Check whether the range `start..end` (end-exclusive) is entirely defined.
pub fn is_range_defined(&self, start: Size, end: Size) -> bool {
///
/// Returns `Ok(())` if it's defined. Otherwise returns the index of the byte
/// at which the first undefined access begins.
#[inline]
pub fn is_range_defined(&self, start: Size, end: Size) -> Result<(), Size> {
if end > self.len {
return false;
return Err(self.len);
}
for i in start.bytes()..end.bytes() {
if !self.get(Size::from_bytes(i)) {
return false;
}
let idx = (start.bytes()..end.bytes())
.map(|i| Size::from_bytes(i))
.find(|&i| !self.get(i));
match idx {
Some(idx) => Err(idx),
None => Ok(())
}
true
}
pub fn set_range(&mut self, start: Size, end: Size, new_state: bool) {

View file

@ -359,7 +359,7 @@ impl<'tcx> ScalarMaybeUndef {
pub fn not_undef(self) -> EvalResult<'static, Scalar> {
match self {
ScalarMaybeUndef::Scalar(scalar) => Ok(scalar),
ScalarMaybeUndef::Undef => err!(ReadUndefBytes),
ScalarMaybeUndef::Undef => err!(ReadUndefBytes(Size::from_bytes(0))),
}
}

View file

@ -511,7 +511,7 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
ReadBytesAsPointer => ReadBytesAsPointer,
ReadForeignStatic => ReadForeignStatic,
InvalidPointerMath => InvalidPointerMath,
ReadUndefBytes => ReadUndefBytes,
ReadUndefBytes(offset) => ReadUndefBytes(offset),
DeadLocal => DeadLocal,
InvalidBoolOp(bop) => InvalidBoolOp(bop),
Unimplemented(ref s) => Unimplemented(s.clone()),

View file

@ -453,7 +453,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
}
relocations.push((i, target_id));
}
if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)) {
if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)).is_ok() {
// this `as usize` is fine, since `i` came from a `usize`
write!(msg, "{:02x} ", alloc.bytes[i.bytes() as usize]).unwrap();
} else {
@ -789,7 +789,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
)?;
// Undef check happens *after* we established that the alignment is correct.
// We must not return Ok() for unaligned pointers!
if !self.is_defined(ptr, size)? {
if self.check_defined(ptr, size).is_err() {
// this inflates undefined bytes to the entire scalar, even if only a few
// bytes are undefined
return Ok(ScalarMaybeUndef::Undef);
@ -991,21 +991,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
Ok(())
}
fn is_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx, bool> {
let alloc = self.get(ptr.alloc_id)?;
Ok(alloc.undef_mask.is_range_defined(
ptr.offset,
ptr.offset + size,
))
}
/// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes`
/// error which will report the first byte which is undefined.
#[inline]
fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
if self.is_defined(ptr, size)? {
Ok(())
} else {
err!(ReadUndefBytes)
}
let alloc = self.get(ptr.alloc_id)?;
alloc.undef_mask.is_range_defined(
ptr.offset,
ptr.offset + size,
).or_else(|idx| err!(ReadUndefBytes(idx)))
}
pub fn mark_definedness(

View file

@ -290,7 +290,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Ok(val) => val,
Err(err) => match err.kind {
EvalErrorKind::PointerOutOfBounds { .. } |
EvalErrorKind::ReadUndefBytes =>
EvalErrorKind::ReadUndefBytes(_) =>
return validation_failure!(
"uninitialized or out-of-bounds memory", path
),
@ -333,16 +333,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
// The fields don't need to correspond to any bit pattern of the union's fields.
// See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
},
layout::FieldPlacement::Array { .. } if !dest.layout.is_zst() => {
layout::FieldPlacement::Array { stride, .. } if !dest.layout.is_zst() => {
let dest = dest.to_mem_place(); // non-ZST array/slice/str cannot be immediate
// Special handling for strings to verify UTF-8
match dest.layout.ty.sty {
// Special handling for strings to verify UTF-8
ty::Str => {
match self.read_str(dest) {
Ok(_) => {},
Err(err) => match err.kind {
EvalErrorKind::PointerOutOfBounds { .. } |
EvalErrorKind::ReadUndefBytes =>
EvalErrorKind::ReadUndefBytes(_) =>
// The error here looks slightly different than it does
// for slices, because we do not report the index into the
// str at which we are OOB.
@ -356,6 +356,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
}
}
}
// Special handling for arrays/slices of builtin integer types
ty::Array(tys, ..) | ty::Slice(tys) if {
// This optimization applies only for integer types
match tys.sty {
ty::Int(..) | ty::Uint(..) => true,
_ => false,
}
} => {
// This is the length of the array/slice.
let len = dest.len(self)?;
// Since primitive types are naturally aligned and tightly packed in arrays,
// we can use the stride to get the size of the integral type.
let ty_size = stride.bytes();
// This is the size in bytes of the whole array.
let size = Size::from_bytes(ty_size * len);
match self.memory.read_bytes(dest.ptr, size) {
// In the happy case, we needn't check anything else.
Ok(_) => {},
// Some error happened, try to provide a more detailed description.
Err(err) => {
// For some errors we might be able to provide extra information
match err.kind {
EvalErrorKind::ReadUndefBytes(offset) => {
// Some byte was undefined, determine which
// element that byte belongs to so we can
// provide an index.
let i = (offset.bytes() / ty_size) as usize;
path.push(PathElem::ArrayElem(i));
return validation_failure!(
"undefined bytes", path
)
},
EvalErrorKind::PointerOutOfBounds { allocation_size, .. } => {
// If the array access is out-of-bounds, the first
// undefined access is the after the end of the array.
let i = (allocation_size.bytes() * ty_size) as usize;
path.push(PathElem::ArrayElem(i));
},
_ => (),
}
return validation_failure!(
"uninitialized or out-of-bounds memory", path
)
}
}
},
_ => {
// This handles the unsized case correctly as well, as well as
// SIMD an all sorts of other array-like types.

View file

@ -181,7 +181,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
| InvalidMemoryLockRelease { .. }
| DeallocatedLockedMemory { .. }
| InvalidPointerMath
| ReadUndefBytes
| ReadUndefBytes(_)
| DeadLocal
| InvalidBoolOp(_)
| DerefFunctionPointer