bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

miri: fix offset_from behavior on wildcard pointers

Browse source
This commit is contained in:
Ralf Jung 2024-07-27 17:18:35 +02:00
parent a526d7ce45
commit 5b38b149dc
11 changed files with 80 additions and 61 deletions
Download patch file Download diff file Expand all files Collapse all files

7
compiler/rustc_const_eval/messages.ftl
View file

@ -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

81
compiler/rustc_const_eval/src/interpret/intrinsics.rs
View file

@ -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,
);
// Neither pointer points to an allocation, so they are both absolute.
(a, b, /*is_addr*/ true)
}
// This will always return 0.
(a, b)
}
_ 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<Option<M::Provenance>>,
offset_bytes: i64,
) -> InterpResult<'tcx, Pointer<Option<M::Provenance>>> {
// 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::<T>()` many bytes from `*src` to `*dst`.

19
compiler/rustc_const_eval/src/interpret/memory.rs
View file

@ -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<Option<M::Provenance>>,
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.

2
src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs
View file

@ -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
}
}

4
src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr
View file

@ -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

3
src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.rs
View file

@ -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
}

4
src/tools/miri/tests/fail/intrinsics/ptr_offset_from_unsigned_neg.stderr
View file

@ -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

9
src/tools/miri/tests/pass/ptr_int_from_exposed.rs
View file

@ -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::<i32>(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();
}

2
tests/ui/consts/offset_from_ub.rs
View file

@ -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 = {

6
tests/ui/consts/offset_from_ub.stderr
View file

@ -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::<impl *const u8>::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::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL

2
tests/ui/consts/offset_ub.rs
View file

@ -1,6 +1,6 @@
use std::ptr;
//@ normalize-stderr-test: "0xf+" -> "0xf..f"
//@ normalize-stderr-test: "0x7f+" -> "0x7f..f"