Auto merge of #87224 - RalfJung:miri-ptr-oob, r=oli-obk

miri: better ptr-out-of-bounds errors

For offsets larger than `isize::MAX`, display them as negative offsets.

r? `@oli-obk`
This commit is contained in:
bors 2021-07-20 08:15:15 +00:00
commit 718d53b0cb
5 changed files with 65 additions and 28 deletions

View file

@ -240,12 +240,13 @@ pub enum UndefinedBehaviorInfo<'tcx> {
/// Dereferencing a dangling pointer after it got freed.
PointerUseAfterFree(AllocId),
/// Used a pointer outside the bounds it is valid for.
/// (If `ptr_size > 0`, determines the size of the memory range that was expected to be in-bounds.)
PointerOutOfBounds {
alloc_id: AllocId,
offset: Size,
size: Size,
alloc_size: Size,
ptr_offset: i64,
ptr_size: Size,
msg: CheckInAllocMsg,
allocation_size: Size,
},
/// Using an integer as a pointer in the wrong way.
DanglingIntPointer(u64, CheckInAllocMsg),
@ -318,24 +319,25 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
PointerUseAfterFree(a) => {
write!(f, "pointer to {} was dereferenced after this allocation got freed", a)
}
PointerOutOfBounds { alloc_id, offset, size: Size::ZERO, msg, allocation_size } => {
PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, ptr_size: Size::ZERO, msg } => {
write!(
f,
"{}{} has size {}, so pointer at offset {} is out-of-bounds",
"{}{alloc_id} has size {alloc_size}, so pointer at offset {ptr_offset} is out-of-bounds",
msg,
alloc_id,
allocation_size.bytes(),
offset.bytes(),
alloc_id = alloc_id,
alloc_size = alloc_size.bytes(),
ptr_offset = ptr_offset,
)
}
PointerOutOfBounds { alloc_id, offset, size, msg, allocation_size } => write!(
PointerOutOfBounds { alloc_id, alloc_size, ptr_offset, ptr_size, msg } => write!(
f,
"{}{} has size {}, so pointer to {} bytes starting at offset {} is out-of-bounds",
"{}{alloc_id} has size {alloc_size}, so pointer to {ptr_size} byte{ptr_size_p} starting at offset {ptr_offset} is out-of-bounds",
msg,
alloc_id,
allocation_size.bytes(),
size.bytes(),
offset.bytes(),
alloc_id = alloc_id,
alloc_size = alloc_size.bytes(),
ptr_size = ptr_size.bytes(),
ptr_size_p = pluralize!(ptr_size.bytes()),
ptr_offset = ptr_offset,
),
DanglingIntPointer(0, CheckInAllocMsg::InboundsTest) => {
write!(f, "null pointer is not a valid pointer for this operation")

View file

@ -36,6 +36,20 @@ pub trait PointerArithmetic: HasDataLayout {
i64::try_from(max_isize_plus_1 - 1).unwrap()
}
#[inline]
fn machine_usize_to_isize(&self, val: u64) -> i64 {
let val = val as i64;
// Now clamp into the machine_isize range.
if val > self.machine_isize_max() {
// This can only happen the the ptr size is < 64, so we know max_usize_plus_1 fits into
// i64.
let max_usize_plus_1 = 1u128 << self.pointer_size().bits();
val - i64::try_from(max_usize_plus_1).unwrap()
} else {
val
}
}
/// Helper function: truncate given value-"overflowed flag" pair to pointer size and
/// update "overflowed flag" if there was an overflow.
/// This should be called by all the other methods before returning!

View file

@ -372,7 +372,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
)
}
/// Check if the given pointerpoints to live memory of given `size` and `align`
/// Check if the given pointer points to live memory of given `size` and `align`
/// (ignoring `M::enforce_alignment`). The caller can control the error message for the
/// out-of-bounds case.
#[inline(always)]
@ -451,11 +451,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
None
}
Ok((alloc_id, offset, ptr)) => {
let (allocation_size, alloc_align, ret_val) = alloc_size(alloc_id, offset, ptr)?;
let (alloc_size, alloc_align, ret_val) = alloc_size(alloc_id, offset, ptr)?;
// Test bounds. This also ensures non-null.
// It is sufficient to check this for the end pointer. Also check for overflow!
if offset.checked_add(size, &self.tcx).map_or(true, |end| end > allocation_size) {
throw_ub!(PointerOutOfBounds { alloc_id, offset, size, allocation_size, msg })
if offset.checked_add(size, &self.tcx).map_or(true, |end| end > alloc_size) {
throw_ub!(PointerOutOfBounds {
alloc_id,
alloc_size,
ptr_offset: self.machine_usize_to_isize(offset.bytes()),
ptr_size: size,
msg,
})
}
// Test align. Check this last; if both bounds and alignment are violated
// we want the error to be about the bounds.

View file

@ -13,6 +13,7 @@ pub const OVERFLOW: *const u16 = unsafe { [0u16; 1].as_ptr().offset(isize::MAX)
pub const UNDERFLOW: *const u16 = unsafe { [0u16; 1].as_ptr().offset(isize::MIN) }; //~NOTE
pub const OVERFLOW_ADDRESS_SPACE: *const u8 = unsafe { (usize::MAX as *const u8).offset(2) }; //~NOTE
pub const UNDERFLOW_ADDRESS_SPACE: *const u8 = unsafe { (1 as *const u8).offset(-2) }; //~NOTE
pub const NEGATIVE_OFFSET: *const u8 = unsafe { [0u8; 1].as_ptr().wrapping_offset(-2).offset(-2) }; //~NOTE
pub const ZERO_SIZED_ALLOC: *const u8 = unsafe { [0u8; 0].as_ptr().offset(1) }; //~NOTE
pub const DANGLING: *const u8 = unsafe { ptr::NonNull::<u8>::dangling().as_ptr().offset(4) }; //~NOTE

View file

@ -102,13 +102,27 @@ error[E0080]: evaluation of constant value failed
LL | unsafe { intrinsics::offset(self, count) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| pointer arithmetic failed: allocN has size 0, so pointer to 1 bytes starting at offset 0 is out-of-bounds
| pointer arithmetic failed: allocN has size 1, so pointer to 2 bytes starting at offset -4 is out-of-bounds
| inside `ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/offset_ub.rs:17:50
::: $DIR/offset_ub.rs:16:49
|
LL | pub const NEGATIVE_OFFSET: *const u8 = unsafe { [0u8; 1].as_ptr().wrapping_offset(-2).offset(-2) };
| ------------------------------------------------ inside `NEGATIVE_OFFSET` at $DIR/offset_ub.rs:16:49
error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
LL | unsafe { intrinsics::offset(self, count) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| pointer arithmetic failed: allocN has size 0, so pointer to 1 byte starting at offset 0 is out-of-bounds
| inside `ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/offset_ub.rs:18:50
|
LL | pub const ZERO_SIZED_ALLOC: *const u8 = unsafe { [0u8; 0].as_ptr().offset(1) };
| --------------------------- inside `ZERO_SIZED_ALLOC` at $DIR/offset_ub.rs:17:50
| --------------------------- inside `ZERO_SIZED_ALLOC` at $DIR/offset_ub.rs:18:50
error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
@ -119,10 +133,10 @@ LL | unsafe { intrinsics::offset(self, count) as *mut T }
| 0x1 is not a valid pointer
| inside `ptr::mut_ptr::<impl *mut u8>::offset` at $SRC_DIR/core/src/ptr/mut_ptr.rs:LL:COL
|
::: $DIR/offset_ub.rs:18:42
::: $DIR/offset_ub.rs:19:42
|
LL | pub const DANGLING: *const u8 = unsafe { ptr::NonNull::<u8>::dangling().as_ptr().offset(4) };
| ------------------------------------------------- inside `DANGLING` at $DIR/offset_ub.rs:18:42
| ------------------------------------------------- inside `DANGLING` at $DIR/offset_ub.rs:19:42
error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
@ -133,10 +147,10 @@ LL | unsafe { intrinsics::offset(self, count) }
| pointer arithmetic failed: 0x0 is not a valid pointer
| inside `ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/offset_ub.rs:21:50
::: $DIR/offset_ub.rs:22:50
|
LL | pub const NULL_OFFSET_ZERO: *const u8 = unsafe { ptr::null::<u8>().offset(0) };
| --------------------------- inside `NULL_OFFSET_ZERO` at $DIR/offset_ub.rs:21:50
| --------------------------- inside `NULL_OFFSET_ZERO` at $DIR/offset_ub.rs:22:50
error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
@ -147,11 +161,11 @@ LL | unsafe { intrinsics::offset(self, count) }
| 0x7f..f is not a valid pointer
| inside `ptr::const_ptr::<impl *const u8>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/offset_ub.rs:24:47
::: $DIR/offset_ub.rs:25:47
|
LL | pub const UNDERFLOW_ABS: *const u8 = unsafe { (usize::MAX as *const u8).offset(isize::MIN) };
| -------------------------------------------- inside `UNDERFLOW_ABS` at $DIR/offset_ub.rs:24:47
| -------------------------------------------- inside `UNDERFLOW_ABS` at $DIR/offset_ub.rs:25:47
error: aborting due to 11 previous errors
error: aborting due to 12 previous errors
For more information about this error, try `rustc --explain E0080`.