diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index cd269810741..a93911225e4 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -233,8 +233,6 @@ const_eval_nullary_intrinsic_fail = const_eval_offset_from_different_allocations = `{$name}` called on pointers into different allocations -const_eval_offset_from_different_integers = - `{$name}` called on different pointers without provenance (i.e., without an associated allocation) const_eval_offset_from_overflow = `{$name}` called when first pointer is too far ahead of second const_eval_offset_from_test = @@ -242,7 +240,10 @@ const_eval_offset_from_test = const_eval_offset_from_underflow = `{$name}` called when first pointer is too far before second const_eval_offset_from_unsigned_overflow = - `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: {$a_offset} < {$b_offset} + `ptr_offset_from_unsigned` called when first pointer has smaller {$is_addr -> + [true] address + *[false] offset + } than second: {$a_offset} < {$b_offset} const_eval_operator_non_const = cannot call non-const operator in {const_eval_const_context}s diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index b227565f8f9..0b4a21d972c 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -243,36 +243,22 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { 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) = + // With `OFFSET_IS_ADDR` this is trivial; without it we need either + // two integers or two pointers into the same allocation. + let (a_offset, b_offset, is_addr) = if M::Provenance::OFFSET_IS_ADDR { + (a.addr().bytes(), b.addr().bytes(), /*is_addr*/ true) + } else { match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) { (Err(a), Err(b)) => { - // Neither pointer points to an allocation. - // This is okay only if they are the same. - if a != b { - // We'd catch this below in the "dereferenceable" check, but - // show a nicer error for this particular case. - throw_ub_custom!( - fluent::const_eval_offset_from_different_integers, - name = intrinsic_name, - ); - } - // This will always return 0. - (a, b) + // Neither pointer points to an allocation, so they are both absolute. + (a, b, /*is_addr*/ true) } - _ if M::Provenance::OFFSET_IS_ADDR && a.addr() == b.addr() => { - // At least one of the pointers has provenance, but they also point to - // the same address so it doesn't matter; this is fine. `(0, 0)` means - // we pass all the checks below and return 0. - (0, 0) - } - // From here onwards, the pointers are definitely for different addresses - // (or we can't determine their absolute address). (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) if a_alloc_id == b_alloc_id => { // Found allocation for both, and it's the same. // Use these offsets for distance calculation. - (a_offset.bytes(), b_offset.bytes()) + (a_offset.bytes(), b_offset.bytes(), /*is_addr*/ false) } _ => { // Not into the same allocation -- this is UB. @@ -281,9 +267,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { name = intrinsic_name, ); } - }; + } + }; - // Compute distance. + // Compute distance: a - b. let dist = { // Addresses are unsigned, so this is a `usize` computation. We have to do the // overflow check separately anyway. @@ -300,6 +287,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { fluent::const_eval_offset_from_unsigned_overflow, a_offset = a_offset, b_offset = b_offset, + is_addr = is_addr, ); } // The signed form of the intrinsic allows this. If we interpret the @@ -328,14 +316,23 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } }; - // 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 dist >= 0 { b } else { a }; - self.check_ptr_access( - min_ptr, - Size::from_bytes(dist.unsigned_abs()), + // Check that the memory between them is dereferenceable at all, starting from the + // base pointer: `dist` is `a - b`, so it is based on `b`. + self.check_ptr_access_signed(b, dist, CheckInAllocMsg::OffsetFromTest)?; + // Then check that this is also dereferenceable from `a`. This ensures that they are + // derived from the same allocation. + self.check_ptr_access_signed( + a, + dist.checked_neg().unwrap(), // i64::MIN is impossible as no allocation can be that large CheckInAllocMsg::OffsetFromTest, - )?; + ) + .map_err(|_| { + // Make the error more specific. + err_ub_custom!( + fluent::const_eval_offset_from_different_allocations, + name = intrinsic_name, + ) + })?; // Perform division by size to compute return value. let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned { @@ -582,27 +579,19 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { } /// Offsets a pointer by some multiple of its type, returning an error if the pointer leaves its - /// allocation. For integer pointers, we consider each of them their own tiny allocation of size - /// 0, so offset-by-0 (and only 0) is okay -- except that null cannot be offset by _any_ value. + /// allocation. pub fn ptr_offset_inbounds( &self, ptr: Pointer>, offset_bytes: i64, ) -> InterpResult<'tcx, Pointer>> { - // 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.signed_offset(offset_bytes, self)?; - // ptr and offset_ptr must be in bounds of the same allocated object. This means all of the - // 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 }; - // This call handles checking for integer/null pointers. - self.check_ptr_access( - min_ptr, - Size::from_bytes(offset_bytes.unsigned_abs()), - CheckInAllocMsg::PointerArithmeticTest, - )?; - Ok(offset_ptr) + // We first compute the pointer with overflow checks, to get a specific error for when it + // overflows (though technically this is redundant with the following inbounds check). + let result = ptr.signed_offset(offset_bytes, self)?; + // The offset must be in bounds starting from `ptr`. + self.check_ptr_access_signed(ptr, offset_bytes, CheckInAllocMsg::PointerArithmeticTest)?; + // Done. + Ok(result) } /// Copy `count*size_of::()` many bytes from `*src` to `*dst`. diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 36fe8dfdd29..0d85e8ef664 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -414,6 +414,25 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { Ok(()) } + /// Check whether the given pointer points to live memory for a signed amount of bytes. + /// A negative amounts means that the given range of memory to the left of the pointer + /// needs to be dereferenceable. + pub fn check_ptr_access_signed( + &self, + ptr: Pointer>, + size: i64, + msg: CheckInAllocMsg, + ) -> InterpResult<'tcx> { + if let Ok(size) = u64::try_from(size) { + self.check_ptr_access(ptr, Size::from_bytes(size), msg) + } else { + // Compute the pointer at the beginning of the range, and do the standard + // dereferenceability check from there. + let begin_ptr = ptr.wrapping_signed_offset(size, self); + self.check_ptr_access(begin_ptr, Size::from_bytes(size.unsigned_abs()), msg) + } + } + /// Low-level helper function to check if a ptr is in-bounds and potentially return a reference /// to the allocation it points to. Supports both shared and mutable references, as the actual /// checking is offloaded to a helper closure. diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs index 57b4fd0dedb..c307dfddb6c 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs @@ -16,6 +16,6 @@ fn main() { let _ = p1.byte_offset_from(p1); // UB because different pointers. - let _ = p1.byte_offset_from(p2); //~ERROR: different pointers without provenance + let _ = p1.byte_offset_from(p2); //~ERROR: no provenance } } diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr index 6e9e5633fec..65f1161425f 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) +error: Undefined Behavior: out-of-bounds `offset_from`: 0xa[noalloc] is a dangling pointer (it has no provenance) --> $DIR/ptr_offset_from_different_ints.rs:LL:CC | LL | let _ = p1.byte_offset_from(p2); - | ^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + | ^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: 0xa[noalloc] is a dangling pointer (it has no provenance) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.rs index 06d13d9bdba..13eb5bfb342 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.rs +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.rs @@ -1,8 +1,9 @@ +//@normalize-stderr-test: "\d+ < \d+" -> "$$ADDR < $$ADDR" #![feature(ptr_sub_ptr)] fn main() { let arr = [0u8; 8]; let ptr1 = arr.as_ptr(); let ptr2 = ptr1.wrapping_add(4); - let _val = unsafe { ptr1.sub_ptr(ptr2) }; //~ERROR: first pointer has smaller offset than second: 0 < 4 + let _val = unsafe { ptr1.sub_ptr(ptr2) }; //~ERROR: first pointer has smaller address than second } diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.stderr index 0b4a9faf1b2..e436f9029d5 100644 --- a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.stderr +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: 0 < 4 +error: Undefined Behavior: `ptr_offset_from_unsigned` called when first pointer has smaller address than second: $ADDR < $ADDR --> $DIR/ptr_offset_from_unsigned_neg.rs:LL:CC | LL | let _val = unsafe { ptr1.sub_ptr(ptr2) }; - | ^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: 0 < 4 + | ^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer has smaller address than second: $ADDR < $ADDR | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information diff --git a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs index 5690d7865bb..589ef781a91 100644 --- a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs +++ b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs @@ -56,9 +56,18 @@ fn ptr_roundtrip_null() { assert_eq!(unsafe { *x_ptr_copy }, 42); } +fn ptr_roundtrip_offset_from() { + let arr = [0; 5]; + let begin = arr.as_ptr(); + let end = begin.wrapping_add(arr.len()); + let end_roundtrip = ptr::with_exposed_provenance::(end.expose_provenance()); + unsafe { end_roundtrip.offset_from(begin) }; +} + fn main() { ptr_roundtrip_out_of_bounds(); ptr_roundtrip_confusion(); ptr_roundtrip_imperfect(); ptr_roundtrip_null(); + ptr_roundtrip_offset_from(); } diff --git a/tests/ui/consts/offset_from_ub.rs b/tests/ui/consts/offset_from_ub.rs index 1506c212fba..95b3118f157 100644 --- a/tests/ui/consts/offset_from_ub.rs +++ b/tests/ui/consts/offset_from_ub.rs @@ -36,7 +36,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 - //~| different pointers without provenance + //~| dangling pointer }; const OUT_OF_BOUNDS_1: isize = { diff --git a/tests/ui/consts/offset_from_ub.stderr b/tests/ui/consts/offset_from_ub.stderr index 7b623126d54..0a860f42c64 100644 --- a/tests/ui/consts/offset_from_ub.stderr +++ b/tests/ui/consts/offset_from_ub.stderr @@ -27,7 +27,7 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:38:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: 0x8[noalloc] is a dangling pointer (it has no provenance) error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:47:14 @@ -80,7 +80,7 @@ LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + = note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance) | note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -93,7 +93,7 @@ LL | unsafe { ptr2.offset_from(ptr1) } error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + = note: `ptr_offset_from` called when first pointer is too far before second | note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL diff --git a/tests/ui/consts/offset_ub.rs b/tests/ui/consts/offset_ub.rs index ebc7019a75a..9a8f756749b 100644 --- a/tests/ui/consts/offset_ub.rs +++ b/tests/ui/consts/offset_ub.rs @@ -1,6 +1,6 @@ use std::ptr; - +//@ normalize-stderr-test: "0xf+" -> "0xf..f" //@ normalize-stderr-test: "0x7f+" -> "0x7f..f"