From 71ef8414bd86cbd79b29f8b1a0145da96e2f2f09 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Tue, 26 May 2020 02:00:02 -0700 Subject: [PATCH] Add checks and tests for computing abs(offset_bytes) The previous code paniced if offset_bytes == i64::MIN. This commit: - Properly computes the absoulte value to avoid this panic - Adds a test for this edge case Signed-off-by: Joe Richey --- src/librustc_mir/interpret/intrinsics.rs | 5 +++-- src/test/ui/consts/offset_ub.rs | 3 +++ src/test/ui/consts/offset_ub.stderr | 17 ++++++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index b4748059262..239115076bc 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -7,7 +7,7 @@ use std::convert::TryFrom; use rustc_hir::def_id::DefId; use rustc_middle::mir::{ self, - interpret::{ConstValue, GlobalId, InterpResult, Scalar}, + interpret::{uabs, ConstValue, GlobalId, InterpResult, Scalar}, BinOp, }; use rustc_middle::ty; @@ -438,6 +438,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { pointee_ty: Ty<'tcx>, offset_count: i64, ) -> InterpResult<'tcx, Scalar> { + // 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 @@ -450,7 +451,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // memory between these pointers must be accessible. Note that we do not require the // pointers to be properly aligned (unlike a read/write operation). let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr }; - let size = offset_bytes.checked_abs().unwrap(); + let size: u64 = uabs(offset_bytes); // This call handles checking for integer/NULL pointers. self.memory.check_ptr_access_align( min_ptr, diff --git a/src/test/ui/consts/offset_ub.rs b/src/test/ui/consts/offset_ub.rs index ac120789694..4f943ed9ad1 100644 --- a/src/test/ui/consts/offset_ub.rs +++ b/src/test/ui/consts/offset_ub.rs @@ -19,4 +19,7 @@ pub const DANGLING: *const u8 = unsafe { ptr::NonNull::::dangling().as_ptr() // Right now, a zero offset from null is UB pub const NULL_OFFSET_ZERO: *const u8 = unsafe { ptr::null::().offset(0) }; //~NOTE +// Make sure that we don't panic when computing abs(offset*size_of::()) +pub const UNDERFLOW_ABS: *const u8 = unsafe { (usize::MAX as *const u8).offset(isize::MIN) }; //~NOTE + fn main() {} diff --git a/src/test/ui/consts/offset_ub.stderr b/src/test/ui/consts/offset_ub.stderr index b2b77586a50..e808939682f 100644 --- a/src/test/ui/consts/offset_ub.stderr +++ b/src/test/ui/consts/offset_ub.stderr @@ -150,5 +150,20 @@ LL | intrinsics::offset(self, count) LL | pub const NULL_OFFSET_ZERO: *const u8 = unsafe { ptr::null::().offset(0) }; | ------------------------------------------------------------------------------- -error: aborting due to 10 previous errors +error: any use of this value will cause an error + --> $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL + | +LL | intrinsics::offset(self, count) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | unable to turn bytes into a pointer + | inside `std::ptr::const_ptr::::offset` at $SRC_DIR/libcore/ptr/const_ptr.rs:LL:COL + | inside `UNDERFLOW_ABS` at $DIR/offset_ub.rs:23:47 + | + ::: $DIR/offset_ub.rs:23:1 + | +LL | pub const UNDERFLOW_ABS: *const u8 = unsafe { (usize::MAX as *const u8).offset(isize::MIN) }; + | --------------------------------------------------------------------------------------------- + +error: aborting due to 11 previous errors