Skip to content

Optimize miri checking of integer array/slices #53903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,6 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
ReadBytesAsPointer |
ReadForeignStatic |
InvalidPointerMath |
ReadUndefBytes |
DeadLocal |
StackFrameLimitReached |
OutOfTls |
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ pub enum EvalErrorKind<'tcx, O> {
ReadBytesAsPointer,
ReadForeignStatic,
InvalidPointerMath,
ReadUndefBytes,
ReadUndefBytes(Size),
DeadLocal,
InvalidBoolOp(mir::BinOp),
Unimplemented(String),
Expand Down Expand Up @@ -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",
Expand Down
21 changes: 14 additions & 7 deletions src/librustc/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
22 changes: 8 additions & 14 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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> {
/// 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> {
let alloc = self.get(ptr.alloc_id)?;
Ok(alloc.undef_mask.is_range_defined(
alloc.undef_mask.is_range_defined(
ptr.offset,
ptr.offset + size,
))
}

#[inline]
fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
if self.is_defined(ptr, size)? {
Ok(())
} else {
err!(ReadUndefBytes)
}
).or_else(|idx| err!(ReadUndefBytes(idx)))
}

pub fn mark_definedness(
Expand Down
57 changes: 53 additions & 4 deletions src/librustc_mir/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
| InvalidMemoryLockRelease { .. }
| DeallocatedLockedMemory { .. }
| InvalidPointerMath
| ReadUndefBytes
| ReadUndefBytes(_)
| DeadLocal
| InvalidBoolOp(_)
| DerefFunctionPointer
Expand Down