Skip to content

Commit 82cde90

Browse files
committed
Optimize miri checking of integer array/slices
Instead of checking every element, we can check the whole memory range at once.
1 parent 4efc0a7 commit 82cde90

File tree

8 files changed

+81
-31
lines changed

8 files changed

+81
-31
lines changed

src/librustc/ich/impls_ty.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,6 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
523523
ReadBytesAsPointer |
524524
ReadForeignStatic |
525525
InvalidPointerMath |
526-
ReadUndefBytes |
527526
DeadLocal |
528527
StackFrameLimitReached |
529528
OutOfTls |
@@ -550,6 +549,7 @@ for ::mir::interpret::EvalErrorKind<'gcx, O> {
550549
GeneratorResumedAfterReturn |
551550
GeneratorResumedAfterPanic |
552551
InfiniteLoop => {}
552+
ReadUndefBytes(offset) => offset.hash_stable(hcx, hasher),
553553
InvalidDiscriminant(val) => val.hash_stable(hcx, hasher),
554554
Panic { ref msg, ref file, line, col } => {
555555
msg.hash_stable(hcx, hasher);

src/librustc/mir/interpret/error.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ pub enum EvalErrorKind<'tcx, O> {
205205
ReadBytesAsPointer,
206206
ReadForeignStatic,
207207
InvalidPointerMath,
208-
ReadUndefBytes,
208+
ReadUndefBytes(Size),
209209
DeadLocal,
210210
InvalidBoolOp(mir::BinOp),
211211
Unimplemented(String),
@@ -331,7 +331,7 @@ impl<'tcx, O> EvalErrorKind<'tcx, O> {
331331
InvalidPointerMath =>
332332
"attempted to do invalid arithmetic on pointers that would leak base addresses, \
333333
e.g. comparing pointers into different allocations",
334-
ReadUndefBytes =>
334+
ReadUndefBytes(_) =>
335335
"attempted to read undefined bytes",
336336
DeadLocal =>
337337
"tried to access a dead local variable",

src/librustc/mir/interpret/mod.rs

+14-7
Original file line numberDiff line numberDiff line change
@@ -640,16 +640,23 @@ impl UndefMask {
640640
}
641641

642642
/// Check whether the range `start..end` (end-exclusive) is entirely defined.
643-
pub fn is_range_defined(&self, start: Size, end: Size) -> bool {
643+
///
644+
/// Returns `Ok(())` if it's defined. Otherwise returns the index of the byte
645+
/// at which the first undefined access begins.
646+
#[inline]
647+
pub fn is_range_defined(&self, start: Size, end: Size) -> Result<(), Size> {
644648
if end > self.len {
645-
return false;
649+
return Err(self.len);
646650
}
647-
for i in start.bytes()..end.bytes() {
648-
if !self.get(Size::from_bytes(i)) {
649-
return false;
650-
}
651+
652+
let idx = (start.bytes()..end.bytes())
653+
.map(|i| Size::from_bytes(i))
654+
.find(|&i| !self.get(i));
655+
656+
match idx {
657+
Some(idx) => Err(idx),
658+
None => Ok(())
651659
}
652-
true
653660
}
654661

655662
pub fn set_range(&mut self, start: Size, end: Size, new_state: bool) {

src/librustc/mir/interpret/value.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ impl<'tcx> ScalarMaybeUndef {
359359
pub fn not_undef(self) -> EvalResult<'static, Scalar> {
360360
match self {
361361
ScalarMaybeUndef::Scalar(scalar) => Ok(scalar),
362-
ScalarMaybeUndef::Undef => err!(ReadUndefBytes),
362+
ScalarMaybeUndef::Undef => err!(ReadUndefBytes(Size::from_bytes(0))),
363363
}
364364
}
365365

src/librustc/ty/structural_impls.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ impl<'a, 'tcx, O: Lift<'tcx>> Lift<'tcx> for interpret::EvalErrorKind<'a, O> {
511511
ReadBytesAsPointer => ReadBytesAsPointer,
512512
ReadForeignStatic => ReadForeignStatic,
513513
InvalidPointerMath => InvalidPointerMath,
514-
ReadUndefBytes => ReadUndefBytes,
514+
ReadUndefBytes(offset) => ReadUndefBytes(offset),
515515
DeadLocal => DeadLocal,
516516
InvalidBoolOp(bop) => InvalidBoolOp(bop),
517517
Unimplemented(ref s) => Unimplemented(s.clone()),

src/librustc_mir/interpret/memory.rs

+8-14
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
453453
}
454454
relocations.push((i, target_id));
455455
}
456-
if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)) {
456+
if alloc.undef_mask.is_range_defined(i, i + Size::from_bytes(1)).is_ok() {
457457
// this `as usize` is fine, since `i` came from a `usize`
458458
write!(msg, "{:02x} ", alloc.bytes[i.bytes() as usize]).unwrap();
459459
} else {
@@ -789,7 +789,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
789789
)?;
790790
// Undef check happens *after* we established that the alignment is correct.
791791
// We must not return Ok() for unaligned pointers!
792-
if !self.is_defined(ptr, size)? {
792+
if self.check_defined(ptr, size).is_err() {
793793
// this inflates undefined bytes to the entire scalar, even if only a few
794794
// bytes are undefined
795795
return Ok(ScalarMaybeUndef::Undef);
@@ -991,21 +991,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
991991
Ok(())
992992
}
993993

994-
fn is_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx, bool> {
994+
/// Checks that a range of bytes is defined. If not, returns the `ReadUndefBytes`
995+
/// error which will report the first byte which is undefined.
996+
#[inline]
997+
fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
995998
let alloc = self.get(ptr.alloc_id)?;
996-
Ok(alloc.undef_mask.is_range_defined(
999+
alloc.undef_mask.is_range_defined(
9971000
ptr.offset,
9981001
ptr.offset + size,
999-
))
1000-
}
1001-
1002-
#[inline]
1003-
fn check_defined(&self, ptr: Pointer, size: Size) -> EvalResult<'tcx> {
1004-
if self.is_defined(ptr, size)? {
1005-
Ok(())
1006-
} else {
1007-
err!(ReadUndefBytes)
1008-
}
1002+
).or_else(|idx| err!(ReadUndefBytes(idx)))
10091003
}
10101004

10111005
pub fn mark_definedness(

src/librustc_mir/interpret/validity.rs

+53-4
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
290290
Ok(val) => val,
291291
Err(err) => match err.kind {
292292
EvalErrorKind::PointerOutOfBounds { .. } |
293-
EvalErrorKind::ReadUndefBytes =>
293+
EvalErrorKind::ReadUndefBytes(_) =>
294294
return validation_failure!(
295295
"uninitialized or out-of-bounds memory", path
296296
),
@@ -333,16 +333,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
333333
// The fields don't need to correspond to any bit pattern of the union's fields.
334334
// See https://github.com/rust-lang/rust/issues/32836#issuecomment-406875389
335335
},
336-
layout::FieldPlacement::Array { .. } if !dest.layout.is_zst() => {
336+
layout::FieldPlacement::Array { stride, .. } if !dest.layout.is_zst() => {
337337
let dest = dest.to_mem_place(); // non-ZST array/slice/str cannot be immediate
338-
// Special handling for strings to verify UTF-8
339338
match dest.layout.ty.sty {
339+
// Special handling for strings to verify UTF-8
340340
ty::Str => {
341341
match self.read_str(dest) {
342342
Ok(_) => {},
343343
Err(err) => match err.kind {
344344
EvalErrorKind::PointerOutOfBounds { .. } |
345-
EvalErrorKind::ReadUndefBytes =>
345+
EvalErrorKind::ReadUndefBytes(_) =>
346346
// The error here looks slightly different than it does
347347
// for slices, because we do not report the index into the
348348
// str at which we are OOB.
@@ -356,6 +356,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
356356
}
357357
}
358358
}
359+
// Special handling for arrays/slices of builtin integer types
360+
ty::Array(tys, ..) | ty::Slice(tys) if {
361+
// This optimization applies only for integer types
362+
match tys.sty {
363+
ty::Int(..) | ty::Uint(..) => true,
364+
_ => false,
365+
}
366+
} => {
367+
// This is the length of the array/slice.
368+
let len = dest.len(self)?;
369+
// Since primitive types are naturally aligned and tightly packed in arrays,
370+
// we can use the stride to get the size of the integral type.
371+
let ty_size = stride.bytes();
372+
// This is the size in bytes of the whole array.
373+
let size = Size::from_bytes(ty_size * len);
374+
375+
match self.memory.read_bytes(dest.ptr, size) {
376+
// In the happy case, we needn't check anything else.
377+
Ok(_) => {},
378+
// Some error happened, try to provide a more detailed description.
379+
Err(err) => {
380+
// For some errors we might be able to provide extra information
381+
match err.kind {
382+
EvalErrorKind::ReadUndefBytes(offset) => {
383+
// Some byte was undefined, determine which
384+
// element that byte belongs to so we can
385+
// provide an index.
386+
let i = (offset.bytes() / ty_size) as usize;
387+
path.push(PathElem::ArrayElem(i));
388+
389+
return validation_failure!(
390+
"undefined bytes", path
391+
)
392+
},
393+
EvalErrorKind::PointerOutOfBounds { allocation_size, .. } => {
394+
// If the array access is out-of-bounds, the first
395+
// undefined access is the after the end of the array.
396+
let i = (allocation_size.bytes() * ty_size) as usize;
397+
path.push(PathElem::ArrayElem(i));
398+
},
399+
_ => (),
400+
}
401+
402+
return validation_failure!(
403+
"uninitialized or out-of-bounds memory", path
404+
)
405+
}
406+
}
407+
},
359408
_ => {
360409
// This handles the unsized case correctly as well, as well as
361410
// SIMD an all sorts of other array-like types.

src/librustc_mir/transform/const_prop.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
181181
| InvalidMemoryLockRelease { .. }
182182
| DeallocatedLockedMemory { .. }
183183
| InvalidPointerMath
184-
| ReadUndefBytes
184+
| ReadUndefBytes(_)
185185
| DeadLocal
186186
| InvalidBoolOp(_)
187187
| DerefFunctionPointer

0 commit comments

Comments
 (0)