Skip to content

Commit

Permalink
Rollup merge of rust-lang#97960 - RalfJung:offset-from, r=oli-obk
Browse files Browse the repository at this point in the history 
interpret: unify offset_from check with offset check

`offset` does the check with a single `check_ptr_access` call while `offset_from` used two calls. Make them both just one one call.

I originally intended to actually factor this into a common function, but I am no longer sure if that makes a lot of sense... the two functions start with pretty different precondition (e.g. `offset` *knows* that the 2nd pointer has the same provenance).

I also reworded the UB messages a little. Saying it "cannot" do something is not how we usually phrase UB (as far as I know). Instead it's not *allowed* to do that.

r? ````@oli-obk````
  • Loading branch information
@Dylan-DPC
Dylan-DPC committed Jun 13, 2022
2 parents 7c710f4 + e5245ef commit 33086c8
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 84 deletions.
137 changes: 70 additions & 67 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
View file
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
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
4 changes: 2 additions & 2 deletions src/test/ui/const-ptr/forbidden_slices.64bit.stderr
View file
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
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
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

0 comments on commit 33086c8

Please sign in to comment.