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.

Sign up for GitHub

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

Jump to bottom

interpret: unify offset_from check with offset check #97960

Merged
merged 1 commit into from
Jun 13, 2022
Merged

interpret: unify offset_from check with offset check #97960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
137 changes: 70 additions & 67 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -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,
)?;
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/const-ptr/forbidden_slices.32bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly the error does not even say that this is UB. It just says "could not evaluate static initializer". Maybe as part of rust-lang/miri#2200 we should also improve the const-eval errors.

| inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
Expand All @@ -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::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/const-ptr/forbidden_slices.64bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
Expand All @@ -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::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/consts/offset_from_ub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub const DIFFERENT_ALLOC: usize = {
let uninit2 = std::mem::MaybeUninit::<Struct>::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
};

Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -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 = {
Expand All @@ -76,15 +76,15 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
let uninit2 = std::mem::MaybeUninit::<Struct>::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
};

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() {}
14 changes: 7 additions & 7 deletions src/test/ui/consts/offset_from_ub.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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
|
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::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
|
::: $DIR/offset_from_ub.rs:23:14
Expand All @@ -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
Expand All @@ -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

Expand Down