Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

offset_from: do not require pointers to be inbounds #112712

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions compiler/rustc_const_eval/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,6 @@ const_eval_nullary_intrinsic_fail =

const_eval_offset_from_overflow =
`{$name}` called when first pointer is too far ahead of second

const_eval_offset_from_test = out-of-bounds `offset_from`
const_eval_offset_from_underflow =
`{$name}` called when first pointer is too far before second

Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ fn bad_pointer_message(msg: CheckInAllocMsg, handler: &Handler) -> String {
CheckInAllocMsg::DerefTest => const_eval_deref_test,
CheckInAllocMsg::MemoryAccessTest => const_eval_memory_access_test,
CheckInAllocMsg::PointerArithmeticTest => const_eval_pointer_arithmetic_test,
CheckInAllocMsg::OffsetFromTest => const_eval_offset_from_test,
CheckInAllocMsg::InboundsTest => const_eval_in_bounds_test,
};

Expand Down
28 changes: 13 additions & 15 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_hir::def_id::DefId;
use rustc_middle::mir::{
self,
interpret::{
Allocation, ConstAllocation, ConstValue, GlobalId, InterpResult, PointerArithmetic, Scalar,
Allocation, ConstAllocation, ConstValue, GlobalId, InterpResult, PointerArithmetic,
Provenance, Scalar,
},
BinOp, NonDivergingIntrinsic,
};
Expand Down Expand Up @@ -313,7 +314,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
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.
// If these are inequal or null, this *will* fail the deref check below.
(a, b)
}
(Err(_), _) | (_, Err(_)) => {
Expand All @@ -332,12 +332,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
name = intrinsic_name,
);
}
// Use these offsets for distance calculation.
(a_offset.bytes(), b_offset.bytes())
if M::Provenance::OFFSET_IS_ADDR {
// Use the absolute address for an accurate overflow check.
(a.addr().bytes(), b.addr().bytes())
} else {
// Use the relative offsets. We can't know where the "edge of the
// address space" is so we can't do a fully accurate overflow check.
// We basically put that edge at the beginning of the allocation.
(a_offset.bytes(), b_offset.bytes())
}
}
};

// Compute distance.
// Compute distance. The overflow check is special because there is an extra requirement that
// the pointers be no more than `isize::MAX` apart.
let dist = {
// Addresses are unsigned, so this is a `usize` computation. We have to do the
// overflow check separately anyway.
Expand Down Expand Up @@ -381,16 +389,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, '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_align(
min_ptr,
Size::from_bytes(dist.unsigned_abs()),
Align::ONE,
CheckInAllocMsg::OffsetFromTest,
)?;

// Perform division by size to compute return value.
let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned {
assert!(0 <= dist && dist <= self.target_isize_max());
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ pub enum CheckInAllocMsg {
MemoryAccessTest,
/// We are doing pointer arithmetic.
PointerArithmeticTest,
/// We are doing pointer offset_from.
OffsetFromTest,
/// None of the above -- generic/unspecific inbounds test.
InboundsTest,
}
Expand Down
12 changes: 8 additions & 4 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,8 @@ impl<T: ?Sized> *const T {
/// If any of the following conditions are violated, the result is Undefined
/// Behavior:
///
/// * Both the starting and other pointer must be either in bounds or one
/// byte past the end of the same [allocated object].
///
/// * Both pointers must be *derived from* a pointer to the same object.
/// * Both pointers must be *derived from* a pointer to the same [allocated object],
/// or they must both be "invalid" pointers not derived from any object.
/// (See below for an example.)
///
/// * The distance between the pointers, in bytes, must be an exact multiple
Expand All @@ -644,6 +642,12 @@ impl<T: ?Sized> *const T {
/// (Note that [`offset`] and [`add`] also have a similar limitation and hence cannot be used on
/// such large allocations either.)
///
/// The requirement for pointers to be derived from the same allocated object is primarily
/// equired for `const`-compatibility: at compile-time, pointers into *different* allocated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// equired for `const`-compatibility: at compile-time, pointers into *different* allocated
/// required for `const`-compatibility: at compile-time, pointers into *different* allocated

😄

/// object do not have a known distance to each other. However, the requirement also exists at
/// runtime, and may be exploited by optimizations. You can use `(self as usize).sub(origin as
/// usize) / mem::size_of::<T>()` to avoid this requirement.
///
/// [`add`]: #method.add
/// [allocated object]: crate::ptr#allocated-object
///
Expand Down
13 changes: 8 additions & 5 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -786,11 +786,8 @@ impl<T: ?Sized> *mut T {
/// If any of the following conditions are violated, the result is Undefined
/// Behavior:
///
/// * Both the starting and other pointer must be either in bounds or one
/// byte past the end of the same [allocated object].
///
/// * Both pointers must be *derived from* a pointer to the same object.
/// (See below for an example.)
/// * Both pointers must be *derived from* a pointer to the same [allocated object],
/// or they must both be "invalid" pointers not derived from any object.
///
/// * The distance between the pointers, in bytes, must be an exact multiple
/// of the size of `T`.
Expand All @@ -816,6 +813,12 @@ impl<T: ?Sized> *mut T {
/// (Note that [`offset`] and [`add`] also have a similar limitation and hence cannot be used on
/// such large allocations either.)
///
/// The requirement for pointers to be derived from the same allocated object is primarily
/// equired for `const`-compatibility: at compile-time, pointers into *different* allocated
/// object do not have a known distance to each other. However, the requirement also exists at
/// runtime, and may be exploited by optimizations. You can use `(self as usize).sub(origin as
/// usize) / mem::size_of::<T>()` to avoid this requirement.
///
/// [`add`]: #method.add
/// [allocated object]: crate::ptr#allocated-object
///
Expand Down
7 changes: 0 additions & 7 deletions src/tools/miri/tests/fail/intrinsics/ptr_offset_from_oob.rs

This file was deleted.

15 changes: 0 additions & 15 deletions src/tools/miri/tests/fail/intrinsics/ptr_offset_from_oob.stderr

This file was deleted.

4 changes: 4 additions & 0 deletions src/tools/miri/tests/pass/ptr_offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,15 @@ fn test_offset_from() {
assert_eq!(x.offset_from(y), -12);
assert_eq!((y as *const u32).offset_from(x as *const u32), 12 / 4);
assert_eq!((x as *const u32).offset_from(y as *const u32), -12 / 4);
assert_eq!(y.wrapping_add(6).offset_from(x), 12+6); // out-of-bounds
assert_eq!(x.wrapping_sub(6).offset_from(y), -12-6); // out-of-bounds

let x = (((x as usize) * 2) / 2) as *const u8;
assert_eq!(y.offset_from(x), 12);
assert_eq!(y.sub_ptr(x), 12);
assert_eq!(x.offset_from(y), -12);
assert_eq!(y.wrapping_add(6).offset_from(x), 12+6); // out-of-bounds
assert_eq!(x.wrapping_sub(6).offset_from(y), -12-6); // out-of-bounds
}
}

Expand Down
30 changes: 15 additions & 15 deletions tests/ui/consts/offset_from_ub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub const DIFFERENT_ALLOC: usize = {
offset as usize
};

// Allowed.
pub const NOT_PTR: usize = {
unsafe { (42 as *const u8).offset_from(&5u8) as usize }
};
Expand All @@ -32,43 +33,45 @@ pub const NOT_MULTIPLE_OF_SIZE: isize = {
//~| 1_isize cannot be divided by 2_isize without remainder
};

// Allowed.
pub const OFFSET_FROM_NULL: isize = {
let ptr = 0 as *const u8;
unsafe { ptr_offset_from(ptr, ptr) } //~ERROR evaluation of constant value failed
//~| null pointer is a dangling pointer
unsafe { ptr_offset_from(ptr, ptr) }
};

pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC

// Allowed.
pub const DIFFERENT_INT: isize = {
let ptr1 = 8 as *const u8;
let ptr2 = 16 as *const u8;
unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed
//~| 0x8[noalloc] is a dangling pointer
unsafe { ptr_offset_from(ptr2, ptr1) }
};

// Allowed.
const OUT_OF_BOUNDS_1: isize = {
let start_ptr = &4 as *const _ as *const u8;
let length = 10;
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 to 10 bytes starting at offset 0 is out-of-bounds
unsafe { ptr_offset_from(end_ptr, start_ptr) }
};

// Allowed.
const OUT_OF_BOUNDS_2: isize = {
let start_ptr = &4 as *const _ as *const u8;
let length = 10;
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 to 10 bytes starting at offset 0 is out-of-bounds
unsafe { ptr_offset_from(start_ptr, end_ptr) }
};


// Allowed.
const OUT_OF_BOUNDS_SAME: isize = {
let start_ptr = &4 as *const _ as *const u8;
let length = 10;
let end_ptr = (start_ptr).wrapping_add(length);
unsafe { ptr_offset_from(end_ptr, end_ptr) } //~ERROR evaluation of constant value failed
//~| pointer at offset 10 is out-of-bounds
unsafe { ptr_offset_from(end_ptr, end_ptr) }
};

pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
Expand Down Expand Up @@ -107,19 +110,16 @@ pub const TOO_FAR_APART_UNSIGNED: usize = {
//~| too far ahead
};

// These do NOT complain that pointers are too far apart; they pass that check (to then fail the
// next one).
// These do NOT complain that pointers are too far apart.
pub const OFFSET_VERY_FAR1: isize = {
let ptr1 = ptr::null::<u8>();
let ptr2 = ptr1.wrapping_offset(isize::MAX);
unsafe { ptr2.offset_from(ptr1) }
//~^ inside
};
pub const OFFSET_VERY_FAR2: isize = {
let ptr1 = ptr::null::<u8>();
let ptr2 = ptr1.wrapping_offset(isize::MAX);
unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
//~^ inside
};

fn main() {}
72 changes: 8 additions & 64 deletions tests/ui/consts/offset_from_ub.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,103 +12,47 @@ error[E0080]: evaluation of constant value failed
note: inside `ptr::const_ptr::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `NOT_PTR`
--> $DIR/offset_from_ub.rs:24:14
--> $DIR/offset_from_ub.rs:25:14
|
LL | unsafe { (42 as *const u8).offset_from(&5u8) as usize }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:31:14
--> $DIR/offset_from_ub.rs:32:14
|
LL | unsafe { ptr_offset_from(field_ptr, base_ptr as *const u16) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ exact_div: 1_isize cannot be divided by 2_isize without remainder

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:37:14
|
LL | unsafe { ptr_offset_from(ptr, ptr) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance)

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:44:14
|
LL | unsafe { ptr_offset_from(ptr2, ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:53:14
|
LL | unsafe { ptr_offset_from(end_ptr, start_ptr) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: alloc17 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:62:14
|
LL | unsafe { ptr_offset_from(start_ptr, end_ptr) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:70:14
|
LL | unsafe { ptr_offset_from(end_ptr, end_ptr) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: alloc23 has size 4, so pointer at offset 10 is out-of-bounds

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:79:14
--> $DIR/offset_from_ub.rs:82:14
|
LL | unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called on pointers into different allocations

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:86:14
--> $DIR/offset_from_ub.rs:89:14
|
LL | unsafe { ptr_offset_from(ptr2, ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far ahead of second

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:92:14
--> $DIR/offset_from_ub.rs:95:14
|
LL | unsafe { ptr_offset_from(ptr1, ptr2) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far before second

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:99:14
--> $DIR/offset_from_ub.rs:102:14
|
LL | unsafe { ptr_offset_from_unsigned(p, p.add(2) ) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: 0 < 8

error[E0080]: evaluation of constant value failed
--> $DIR/offset_from_ub.rs:106:14
--> $DIR/offset_from_ub.rs:109:14
|
LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer is too far ahead of second

error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
= note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance)
|
note: inside `ptr::const_ptr::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `OFFSET_VERY_FAR1`
--> $DIR/offset_from_ub.rs:115:14
|
LL | unsafe { ptr2.offset_from(ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^

error[E0080]: evaluation of constant value failed
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
= note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance)
|
note: inside `ptr::const_ptr::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `OFFSET_VERY_FAR2`
--> $DIR/offset_from_ub.rs:121:14
|
LL | unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 15 previous errors
error: aborting due to 8 previous errors

For more information about this error, try `rustc --explain E0080`.