From e5245ef1eb2bacb07f7e2473b845d86c3bdd1b01 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 Jun 2022 20:47:06 -0400 Subject: [PATCH] interpret: unify offset_from check with offset check --- .../src/interpret/intrinsics.rs | 137 +++++++++--------- .../const-ptr/forbidden_slices.32bit.stderr | 4 +- .../const-ptr/forbidden_slices.64bit.stderr | 4 +- src/test/ui/consts/offset_from_ub.rs | 12 +- src/test/ui/consts/offset_from_ub.stderr | 14 +- 5 files changed, 87 insertions(+), 84 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index c5770f505262e..e51c51cf45e5e 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -313,78 +313,82 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let a = self.read_pointer(&args[0])?; let b = self.read_pointer(&args[1])?; - // Special case: if both scalars are *equal integers* - // and not null, we pretend there is an allocation of size 0 right there, - // and their offset is 0. (There's never a valid object at null, making it an - // exception from the exception.) - // This is the dual to the special exception for offset-by-0 - // in the inbounds pointer offset operation (see `ptr_offset_inbounds` below). - match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) { - (Err(a), Err(b)) if a == b && a != 0 => { - // Both are the same non-null integer. - self.write_scalar(Scalar::from_machine_isize(0, self), dest)?; - } - (Err(offset), _) | (_, Err(offset)) => { - throw_ub!(DanglingIntPointer(offset, CheckInAllocMsg::OffsetFromTest)); - } - (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => { - // Both are pointers. They must be into the same allocation. - if a_alloc_id != b_alloc_id { - throw_ub_format!( - "{} cannot compute offset of pointers into different allocations.", - intrinsic_name, - ); + let usize_layout = self.layout_of(self.tcx.types.usize)?; + let isize_layout = self.layout_of(self.tcx.types.isize)?; + + // Get offsets for both that are at least relative to the same base. + let (a_offset, b_offset) = + match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) { + (Err(a), Err(b)) => { + // Neither poiner points to an allocation. + // If these are inequal or null, this *will* fail the deref check below. + (a, b) } - // And they must both be valid for zero-sized accesses ("in-bounds or one past the end"). - self.check_ptr_access_align( - a, - Size::ZERO, - Align::ONE, - CheckInAllocMsg::OffsetFromTest, - )?; - self.check_ptr_access_align( - b, - Size::ZERO, - Align::ONE, - CheckInAllocMsg::OffsetFromTest, - )?; - - if intrinsic_name == sym::ptr_offset_from_unsigned && a_offset < b_offset { + (Err(_), _) | (_, Err(_)) => { + // We managed to find a valid allocation for one pointer, but not the other. + // That means they are definitely not pointing to the same allocation. throw_ub_format!( - "{} cannot compute a negative offset, but {} < {}", - intrinsic_name, - a_offset.bytes(), - b_offset.bytes(), + "{} called on pointers into different allocations", + intrinsic_name ); } - - // Compute offset. - let usize_layout = self.layout_of(self.tcx.types.usize)?; - let isize_layout = self.layout_of(self.tcx.types.isize)?; - let ret_layout = if intrinsic_name == sym::ptr_offset_from { - isize_layout - } else { - usize_layout - }; - - // The subtraction is always done in `isize` to enforce - // the "no more than `isize::MAX` apart" requirement. - let a_offset = ImmTy::from_uint(a_offset.bytes(), isize_layout); - let b_offset = ImmTy::from_uint(b_offset.bytes(), isize_layout); - let (val, overflowed, _ty) = - self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?; - if overflowed { - throw_ub_format!("Pointers were too far apart for {}", intrinsic_name); + (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => { + // Found allocation for both. They must be into the same allocation. + if a_alloc_id != b_alloc_id { + throw_ub_format!( + "{} called on pointers into different allocations", + intrinsic_name + ); + } + // Use these offsets for distance calculation. + (a_offset.bytes(), b_offset.bytes()) } - - let pointee_layout = self.layout_of(substs.type_at(0))?; - // This re-interprets an isize at ret_layout, but we already checked - // that if ret_layout is usize, then the result must be non-negative. - let val = ImmTy::from_scalar(val, ret_layout); - let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout); - self.exact_div(&val, &size, dest)?; + }; + + // Compute distance. + let distance = { + // The subtraction is always done in `isize` to enforce + // the "no more than `isize::MAX` apart" requirement. + let a_offset = ImmTy::from_uint(a_offset, isize_layout); + let b_offset = ImmTy::from_uint(b_offset, isize_layout); + let (val, overflowed, _ty) = + self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?; + if overflowed { + throw_ub_format!("pointers were too far apart for {}", intrinsic_name); } + val.to_machine_isize(self)? + }; + + // Check that the range between them is dereferenceable ("in-bounds or one past the + // end of the same allocation"). This is like the check in ptr_offset_inbounds. + let min_ptr = if distance >= 0 { b } else { a }; + self.check_ptr_access_align( + min_ptr, + Size::from_bytes(distance.unsigned_abs()), + Align::ONE, + CheckInAllocMsg::OffsetFromTest, + )?; + + if intrinsic_name == sym::ptr_offset_from_unsigned && distance < 0 { + throw_ub_format!( + "{} called when first pointer has smaller offset than second: {} < {}", + intrinsic_name, + a_offset, + b_offset, + ); } + + // Perform division by size to compute return value. + let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned { + usize_layout + } else { + isize_layout + }; + let pointee_layout = self.layout_of(substs.type_at(0))?; + // If ret_layout is unsigned, we checked that so is the distance, so we are good. + let val = ImmTy::from_int(distance, ret_layout); + let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout); + self.exact_div(&val, &size, dest)?; } sym::transmute => { @@ -575,11 +579,10 @@ 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.unsigned_abs(); // This call handles checking for integer/null pointers. self.check_ptr_access_align( min_ptr, - Size::from_bytes(size), + Size::from_bytes(offset_bytes.unsigned_abs()), Align::ONE, CheckInAllocMsg::PointerArithmeticTest, )?; diff --git a/src/test/ui/const-ptr/forbidden_slices.32bit.stderr b/src/test/ui/const-ptr/forbidden_slices.32bit.stderr index 5b0b9aa9666e5..023d5ea34d871 100644 --- a/src/test/ui/const-ptr/forbidden_slices.32bit.stderr +++ b/src/test/ui/const-ptr/forbidden_slices.32bit.stderr @@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + | ptr_offset_from_unsigned called on pointers into different allocations | inside `ptr::const_ptr::::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL @@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + | ptr_offset_from_unsigned called on pointers into different allocations | inside `ptr::const_ptr::::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL diff --git a/src/test/ui/const-ptr/forbidden_slices.64bit.stderr b/src/test/ui/const-ptr/forbidden_slices.64bit.stderr index b75e6c5024667..ed89c253d532c 100644 --- a/src/test/ui/const-ptr/forbidden_slices.64bit.stderr +++ b/src/test/ui/const-ptr/forbidden_slices.64bit.stderr @@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + | ptr_offset_from_unsigned called on pointers into different allocations | inside `ptr::const_ptr::::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL @@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + | ptr_offset_from_unsigned called on pointers into different allocations | inside `ptr::const_ptr::::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ::: $SRC_DIR/core/src/slice/raw.rs:LL:COL diff --git a/src/test/ui/consts/offset_from_ub.rs b/src/test/ui/consts/offset_from_ub.rs index f604f57e39d98..10e368ba13396 100644 --- a/src/test/ui/consts/offset_from_ub.rs +++ b/src/test/ui/consts/offset_from_ub.rs @@ -15,7 +15,7 @@ pub const DIFFERENT_ALLOC: usize = { let uninit2 = std::mem::MaybeUninit::::uninit(); let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct; let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed - //~| ptr_offset_from cannot compute offset of pointers into different allocations. + //~| pointers into different allocations offset as usize }; @@ -41,7 +41,7 @@ pub const DIFFERENT_INT: isize = { // offset_from with two different integers: l let ptr1 = 8 as *const u8; let ptr2 = 16 as *const u8; unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed - //~| 0x10 is not a valid pointer + //~| 0x8 is not a valid pointer }; const OUT_OF_BOUNDS_1: isize = { @@ -50,7 +50,7 @@ const OUT_OF_BOUNDS_1: isize = { let end_ptr = (start_ptr).wrapping_add(length); // First ptr is out of bounds unsafe { ptr_offset_from(end_ptr, start_ptr) } //~ERROR evaluation of constant value failed - //~| pointer at offset 10 is out-of-bounds + //~| pointer to 10 bytes starting at offset 0 is out-of-bounds }; const OUT_OF_BOUNDS_2: isize = { @@ -59,7 +59,7 @@ const OUT_OF_BOUNDS_2: isize = { let end_ptr = (start_ptr).wrapping_add(length); // Second ptr is out of bounds unsafe { ptr_offset_from(start_ptr, end_ptr) } //~ERROR evaluation of constant value failed - //~| pointer at offset 10 is out-of-bounds + //~| pointer to 10 bytes starting at offset 0 is out-of-bounds }; const OUT_OF_BOUNDS_SAME: isize = { @@ -76,7 +76,7 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = { let uninit2 = std::mem::MaybeUninit::::uninit(); let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct; let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed - //~| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + //~| pointers into different allocations offset as usize }; @@ -84,7 +84,7 @@ const WRONG_ORDER_UNSIGNED: usize = { let a = ['a', 'b', 'c']; let p = a.as_ptr(); unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } //~ERROR evaluation of constant value failed - //~| cannot compute a negative offset, but 0 < 8 + //~| first pointer has smaller offset than second: 0 < 8 }; fn main() {} diff --git a/src/test/ui/consts/offset_from_ub.stderr b/src/test/ui/consts/offset_from_ub.stderr index 4c98fd72cacc3..eb7f1d7a6b252 100644 --- a/src/test/ui/consts/offset_from_ub.stderr +++ b/src/test/ui/consts/offset_from_ub.stderr @@ -2,7 +2,7 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:17:27 | LL | let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from cannot compute offset of pointers into different allocations. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from called on pointers into different allocations error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -10,7 +10,7 @@ error[E0080]: evaluation of constant value failed LL | unsafe { intrinsics::ptr_offset_from(self, origin) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | out-of-bounds offset_from: 0x2a is not a valid pointer + | ptr_offset_from called on pointers into different allocations | inside `ptr::const_ptr::::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | ::: $DIR/offset_from_ub.rs:23:14 @@ -34,19 +34,19 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:43:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x10 is not a valid pointer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x8 is not a valid pointer error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:52:14 | LL | unsafe { ptr_offset_from(end_ptr, start_ptr) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer at offset 10 is out-of-bounds + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:61:14 | LL | unsafe { ptr_offset_from(start_ptr, end_ptr) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer at offset 10 is out-of-bounds + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:69:14 @@ -58,13 +58,13 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:78:27 | LL | let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute offset of pointers into different allocations. + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called on pointers into different allocations error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:86:14 | LL | unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute a negative offset, but 0 < 8 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called when first pointer has smaller offset than second: 0 < 8 error: aborting due to 10 previous errors