From 7d5415b5a2faf5b8f76156eedfd66f94b91660a0 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Tue, 26 May 2020 23:14:55 -0700 Subject: [PATCH] Add additional checks for isize overflow We now perform the correct checks even if the pointer size differs between the host and target. Signed-off-by: Joe Richey --- src/librustc_middle/mir/interpret/pointer.rs | 15 +++++++++++++-- src/librustc_mir/interpret/intrinsics.rs | 5 ++--- src/test/ui/consts/offset_ub.stderr | 4 ++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/librustc_middle/mir/interpret/pointer.rs b/src/librustc_middle/mir/interpret/pointer.rs index 019c96bc511eb..ccad4f0a135a1 100644 --- a/src/librustc_middle/mir/interpret/pointer.rs +++ b/src/librustc_middle/mir/interpret/pointer.rs @@ -24,6 +24,12 @@ pub trait PointerArithmetic: HasDataLayout { u64::try_from(max_usize_plus_1 - 1).unwrap() } + #[inline] + fn machine_isize_min(&self) -> i64 { + let max_isize_plus_1 = 1i128 << (self.pointer_size().bits() - 1); + i64::try_from(-max_isize_plus_1).unwrap() + } + #[inline] fn machine_isize_max(&self) -> i64 { let max_isize_plus_1 = 1u128 << (self.pointer_size().bits() - 1); @@ -42,18 +48,23 @@ pub trait PointerArithmetic: HasDataLayout { #[inline] fn overflowing_offset(&self, val: u64, i: u64) -> (u64, bool) { + // We do not need to check if i fits in a machine usize. If it doesn't, + // either the wrapping_add will wrap or res will not fit in a pointer. let res = val.overflowing_add(i); self.truncate_to_ptr(res) } #[inline] fn overflowing_signed_offset(&self, val: u64, i: i64) -> (u64, bool) { + // We need to make sure that i fits in a machine isize. let n = uabs(i); if i >= 0 { - self.overflowing_offset(val, n) + let (val, over) = self.overflowing_offset(val, n); + (val, over || i > self.machine_isize_max()) } else { let res = val.overflowing_sub(n); - self.truncate_to_ptr(res) + let (val, over) = self.truncate_to_ptr(res); + (val, over || i < self.machine_isize_min()) } } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 239115076bc4b..55f254f573261 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -441,9 +441,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We cannot overflow i64 as a type's size must be <= isize::MAX. let pointee_size = i64::try_from(self.layout_of(pointee_ty)?.size.bytes()).unwrap(); // The computed offset, in bytes, cannot overflow an isize. - let offset_bytes = offset_count - .checked_mul(pointee_size) - .ok_or(err_ub_format!("inbounds pointer arithmetic: overflow computing offset"))?; + let offset_bytes = + offset_count.checked_mul(pointee_size).ok_or(err_ub!(PointerArithOverflow))?; // The offset being in bounds cannot rely on "wrapping around" the address space. // So, first rule out overflows in the pointer arithmetic. let offset_ptr = ptr.ptr_signed_offset(offset_bytes, self)?; diff --git a/src/test/ui/consts/offset_ub.stderr b/src/test/ui/consts/offset_ub.stderr index e808939682f30..0ab81cc0c5b31 100644 --- a/src/test/ui/consts/offset_ub.stderr +++ b/src/test/ui/consts/offset_ub.stderr @@ -51,7 +51,7 @@ error: any use of this value will cause an error LL | intrinsics::offset(self, count) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | inbounds pointer arithmetic: overflow computing offset + | overflowing in-bounds pointer arithmetic | inside `std::ptr::const_ptr::::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL | inside `OVERFLOW` at $DIR/offset_ub.rs:11:43 | @@ -66,7 +66,7 @@ error: any use of this value will cause an error LL | intrinsics::offset(self, count) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | inbounds pointer arithmetic: overflow computing offset + | overflowing in-bounds pointer arithmetic | inside `std::ptr::const_ptr::::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL | inside `UNDERFLOW` at $DIR/offset_ub.rs:12:44 |