Skip to content

Commit 2de690b

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#97960 - RalfJung:offset-from, r=oli-obk
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`
2 parents d45eaeb + e5245ef commit 2de690b

File tree

5 files changed

+87
-84
lines changed

5 files changed

+87
-84
lines changed

‎compiler/rustc_const_eval/src/interpret/intrinsics.rs‎

Lines changed: 70 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -313,78 +313,82 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
313313
let a = self.read_pointer(&args[0])?;
314314
let b = self.read_pointer(&args[1])?;
315315

316-
// Special case: if both scalars are *equal integers*
317-
// and not null, we pretend there is an allocation of size 0 right there,
318-
// and their offset is 0. (There's never a valid object at null, making it an
319-
// exception from the exception.)
320-
// This is the dual to the special exception for offset-by-0
321-
// in the inbounds pointer offset operation (see `ptr_offset_inbounds` below).
322-
match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
323-
(Err(a), Err(b)) if a == b && a != 0 => {
324-
// Both are the same non-null integer.
325-
self.write_scalar(Scalar::from_machine_isize(0, self), dest)?;
326-
}
327-
(Err(offset), _) | (_, Err(offset)) => {
328-
throw_ub!(DanglingIntPointer(offset, CheckInAllocMsg::OffsetFromTest));
329-
}
330-
(Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
331-
// Both are pointers. They must be into the same allocation.
332-
if a_alloc_id != b_alloc_id {
333-
throw_ub_format!(
334-
"{} cannot compute offset of pointers into different allocations.",
335-
intrinsic_name,
336-
);
316+
let usize_layout = self.layout_of(self.tcx.types.usize)?;
317+
let isize_layout = self.layout_of(self.tcx.types.isize)?;
318+
319+
// Get offsets for both that are at least relative to the same base.
320+
let (a_offset, b_offset) =
321+
match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
322+
(Err(a), Err(b)) => {
323+
// Neither poiner points to an allocation.
324+
// If these are inequal or null, this *will* fail the deref check below.
325+
(a, b)
337326
}
338-
// And they must both be valid for zero-sized accesses ("in-bounds or one past the end").
339-
self.check_ptr_access_align(
340-
a,
341-
Size::ZERO,
342-
Align::ONE,
343-
CheckInAllocMsg::OffsetFromTest,
344-
)?;
345-
self.check_ptr_access_align(
346-
b,
347-
Size::ZERO,
348-
Align::ONE,
349-
CheckInAllocMsg::OffsetFromTest,
350-
)?;
351-
352-
if intrinsic_name == sym::ptr_offset_from_unsigned && a_offset < b_offset {
327+
(Err(_), _) | (_, Err(_)) => {
328+
// We managed to find a valid allocation for one pointer, but not the other.
329+
// That means they are definitely not pointing to the same allocation.
353330
throw_ub_format!(
354-
"{} cannot compute a negative offset, but {} < {}",
355-
intrinsic_name,
356-
a_offset.bytes(),
357-
b_offset.bytes(),
331+
"{} called on pointers into different allocations",
332+
intrinsic_name
358333
);
359334
}
360-
361-
// Compute offset.
362-
let usize_layout = self.layout_of(self.tcx.types.usize)?;
363-
let isize_layout = self.layout_of(self.tcx.types.isize)?;
364-
let ret_layout = if intrinsic_name == sym::ptr_offset_from {
365-
isize_layout
366-
} else {
367-
usize_layout
368-
};
369-
370-
// The subtraction is always done in `isize` to enforce
371-
// the "no more than `isize::MAX` apart" requirement.
372-
let a_offset = ImmTy::from_uint(a_offset.bytes(), isize_layout);
373-
let b_offset = ImmTy::from_uint(b_offset.bytes(), isize_layout);
374-
let (val, overflowed, _ty) =
375-
self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
376-
if overflowed {
377-
throw_ub_format!("Pointers were too far apart for {}", intrinsic_name);
335+
(Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
336+
// Found allocation for both. They must be into the same allocation.
337+
if a_alloc_id != b_alloc_id {
338+
throw_ub_format!(
339+
"{} called on pointers into different allocations",
340+
intrinsic_name
341+
);
342+
}
343+
// Use these offsets for distance calculation.
344+
(a_offset.bytes(), b_offset.bytes())
378345
}
379-
380-
let pointee_layout = self.layout_of(substs.type_at(0))?;
381-
// This re-interprets an isize at ret_layout, but we already checked
382-
// that if ret_layout is usize, then the result must be non-negative.
383-
let val = ImmTy::from_scalar(val, ret_layout);
384-
let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
385-
self.exact_div(&val, &size, dest)?;
346+
};
347+
348+
// Compute distance.
349+
let distance = {
350+
// The subtraction is always done in `isize` to enforce
351+
// the "no more than `isize::MAX` apart" requirement.
352+
let a_offset = ImmTy::from_uint(a_offset, isize_layout);
353+
let b_offset = ImmTy::from_uint(b_offset, isize_layout);
354+
let (val, overflowed, _ty) =
355+
self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?;
356+
if overflowed {
357+
throw_ub_format!("pointers were too far apart for {}", intrinsic_name);
386358
}
359+
val.to_machine_isize(self)?
360+
};
361+
362+
// Check that the range between them is dereferenceable ("in-bounds or one past the
363+
// end of the same allocation"). This is like the check in ptr_offset_inbounds.
364+
let min_ptr = if distance >= 0 { b } else { a };
365+
self.check_ptr_access_align(
366+
min_ptr,
367+
Size::from_bytes(distance.unsigned_abs()),
368+
Align::ONE,
369+
CheckInAllocMsg::OffsetFromTest,
370+
)?;
371+
372+
if intrinsic_name == sym::ptr_offset_from_unsigned && distance < 0 {
373+
throw_ub_format!(
374+
"{} called when first pointer has smaller offset than second: {} < {}",
375+
intrinsic_name,
376+
a_offset,
377+
b_offset,
378+
);
387379
}
380+
381+
// Perform division by size to compute return value.
382+
let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned {
383+
usize_layout
384+
} else {
385+
isize_layout
386+
};
387+
let pointee_layout = self.layout_of(substs.type_at(0))?;
388+
// If ret_layout is unsigned, we checked that so is the distance, so we are good.
389+
let val = ImmTy::from_int(distance, ret_layout);
390+
let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
391+
self.exact_div(&val, &size, dest)?;
388392
}
389393

390394
sym::transmute => {
@@ -575,11 +579,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
575579
// memory between these pointers must be accessible. Note that we do not require the
576580
// pointers to be properly aligned (unlike a read/write operation).
577581
let min_ptr = if offset_bytes >= 0 { ptr } else { offset_ptr };
578-
let size = offset_bytes.unsigned_abs();
579582
// This call handles checking for integer/null pointers.
580583
self.check_ptr_access_align(
581584
min_ptr,
582-
Size::from_bytes(size),
585+
Size::from_bytes(offset_bytes.unsigned_abs()),
583586
Align::ONE,
584587
CheckInAllocMsg::PointerArithmeticTest,
585588
)?;

‎src/test/ui/const-ptr/forbidden_slices.32bit.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer
243243
LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
244244
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
245245
| |
246-
| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
246+
| ptr_offset_from_unsigned called on pointers into different allocations
247247
| inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
248248
|
249249
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
@@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer
262262
LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
263263
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
264264
| |
265-
| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
265+
| ptr_offset_from_unsigned called on pointers into different allocations
266266
| inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
267267
|
268268
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL

‎src/test/ui/const-ptr/forbidden_slices.64bit.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ error[E0080]: could not evaluate static initializer
243243
LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
244244
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
245245
| |
246-
| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
246+
| ptr_offset_from_unsigned called on pointers into different allocations
247247
| inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
248248
|
249249
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL
@@ -262,7 +262,7 @@ error[E0080]: could not evaluate static initializer
262262
LL | unsafe { intrinsics::ptr_offset_from_unsigned(self, origin) }
263263
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
264264
| |
265-
| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
265+
| ptr_offset_from_unsigned called on pointers into different allocations
266266
| inside `ptr::const_ptr::<impl *const u32>::sub_ptr` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
267267
|
268268
::: $SRC_DIR/core/src/slice/raw.rs:LL:COL

‎src/test/ui/consts/offset_from_ub.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ pub const DIFFERENT_ALLOC: usize = {
1515
let uninit2 = std::mem::MaybeUninit::<Struct>::uninit();
1616
let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct;
1717
let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed
18-
//~| ptr_offset_from cannot compute offset of pointers into different allocations.
18+
//~| pointers into different allocations
1919
offset as usize
2020
};
2121

@@ -41,7 +41,7 @@ pub const DIFFERENT_INT: isize = { // offset_from with two different integers: l
4141
let ptr1 = 8 as *const u8;
4242
let ptr2 = 16 as *const u8;
4343
unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed
44-
//~| 0x10 is not a valid pointer
44+
//~| 0x8 is not a valid pointer
4545
};
4646

4747
const OUT_OF_BOUNDS_1: isize = {
@@ -50,7 +50,7 @@ const OUT_OF_BOUNDS_1: isize = {
5050
let end_ptr = (start_ptr).wrapping_add(length);
5151
// First ptr is out of bounds
5252
unsafe { ptr_offset_from(end_ptr, start_ptr) } //~ERROR evaluation of constant value failed
53-
//~| pointer at offset 10 is out-of-bounds
53+
//~| pointer to 10 bytes starting at offset 0 is out-of-bounds
5454
};
5555

5656
const OUT_OF_BOUNDS_2: isize = {
@@ -59,7 +59,7 @@ const OUT_OF_BOUNDS_2: isize = {
5959
let end_ptr = (start_ptr).wrapping_add(length);
6060
// Second ptr is out of bounds
6161
unsafe { ptr_offset_from(start_ptr, end_ptr) } //~ERROR evaluation of constant value failed
62-
//~| pointer at offset 10 is out-of-bounds
62+
//~| pointer to 10 bytes starting at offset 0 is out-of-bounds
6363
};
6464

6565
const OUT_OF_BOUNDS_SAME: isize = {
@@ -76,15 +76,15 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = {
7676
let uninit2 = std::mem::MaybeUninit::<Struct>::uninit();
7777
let field_ptr: *const Struct = &uninit2 as *const _ as *const Struct;
7878
let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) }; //~ERROR evaluation of constant value failed
79-
//~| ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
79+
//~| pointers into different allocations
8080
offset as usize
8181
};
8282

8383
const WRONG_ORDER_UNSIGNED: usize = {
8484
let a = ['a', 'b', 'c'];
8585
let p = a.as_ptr();
8686
unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } //~ERROR evaluation of constant value failed
87-
//~| cannot compute a negative offset, but 0 < 8
87+
//~| first pointer has smaller offset than second: 0 < 8
8888
};
8989

9090
fn main() {}

‎src/test/ui/consts/offset_from_ub.stderr‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ error[E0080]: evaluation of constant value failed
22
--> $DIR/offset_from_ub.rs:17:27
33
|
44
LL | let offset = unsafe { ptr_offset_from(field_ptr, base_ptr) };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from cannot compute offset of pointers into different allocations.
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from called on pointers into different allocations
66

77
error[E0080]: evaluation of constant value failed
88
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
99
|
1010
LL | unsafe { intrinsics::ptr_offset_from(self, origin) }
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1212
| |
13-
| out-of-bounds offset_from: 0x2a is not a valid pointer
13+
| ptr_offset_from called on pointers into different allocations
1414
| inside `ptr::const_ptr::<impl *const u8>::offset_from` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
1515
|
1616
::: $DIR/offset_from_ub.rs:23:14
@@ -34,19 +34,19 @@ error[E0080]: evaluation of constant value failed
3434
--> $DIR/offset_from_ub.rs:43:14
3535
|
3636
LL | unsafe { ptr_offset_from(ptr2, ptr1) }
37-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x10 is not a valid pointer
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: 0x8 is not a valid pointer
3838

3939
error[E0080]: evaluation of constant value failed
4040
--> $DIR/offset_from_ub.rs:52:14
4141
|
4242
LL | unsafe { ptr_offset_from(end_ptr, start_ptr) }
43-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer at offset 10 is out-of-bounds
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc20 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds
4444

4545
error[E0080]: evaluation of constant value failed
4646
--> $DIR/offset_from_ub.rs:61:14
4747
|
4848
LL | unsafe { ptr_offset_from(start_ptr, end_ptr) }
49-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer at offset 10 is out-of-bounds
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds offset_from: alloc23 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds
5050

5151
error[E0080]: evaluation of constant value failed
5252
--> $DIR/offset_from_ub.rs:69:14
@@ -58,13 +58,13 @@ error[E0080]: evaluation of constant value failed
5858
--> $DIR/offset_from_ub.rs:78:27
5959
|
6060
LL | let offset = unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) };
61-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute offset of pointers into different allocations.
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called on pointers into different allocations
6262

6363
error[E0080]: evaluation of constant value failed
6464
--> $DIR/offset_from_ub.rs:86:14
6565
|
6666
LL | unsafe { ptr_offset_from_unsigned(p, p.add(2) ) }
67-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned cannot compute a negative offset, but 0 < 8
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ptr_offset_from_unsigned called when first pointer has smaller offset than second: 0 < 8
6868

6969
error: aborting due to 10 previous errors
7070

0 commit comments

Comments
 (0)