Skip to content

Commit

Permalink
Auto merge of #117329 - RalfJung:offset-by-zero, r=oli-obk,scottmcm
Browse files Browse the repository at this point in the history
offset: allow zero-byte offset on arbitrary pointers

As per prior `@rust-lang/opsem` [discussion](rust-lang/opsem-team#10) and [FCP](rust-lang/unsafe-code-guidelines#472 (comment)):

- Zero-sized reads and writes are allowed on all sufficiently aligned pointers, including the null pointer
- Inbounds-offset-by-zero is allowed on all pointers, including the null pointer
- `offset_from` on two pointers derived from the same allocation is always allowed when they have the same address

This removes surprising UB (in particular, even C++ allows "nullptr + 0", which we currently disallow), and it brings us one step closer to an important theoretical property for our semantics ("provenance monotonicity": if operations are valid on bytes without provenance, then adding provenance can't make them invalid).

The minimum LLVM we require (v17) includes https://reviews.llvm.org/D154051, so we can finally implement this.

The `offset_from` change is needed to maintain the equivalence with `offset`: if `let ptr2 = ptr1.offset(N)` is well-defined, then `ptr2.offset_from(ptr1)` should be well-defined and return N. Now consider the case where N is 0 and `ptr1` dangles: we want to still allow offset_from here.

I think we should change offset_from further, but that's a separate discussion.

Fixes rust-lang/rust#65108
[Tracking issue](rust-lang/rust#117945) | [T-lang summary](rust-lang/rust#117329 (comment))

Cc `@nikic`
  • Loading branch information
bors committed May 22, 2024
2 parents a79ce97 + c9e9916 commit 388606b
Show file tree
Hide file tree
Showing 28 changed files with 64 additions and 327 deletions.
10 changes: 0 additions & 10 deletions tests/fail/dangling_pointers/dangling_zst_deref.rs

This file was deleted.

25 changes: 0 additions & 25 deletions tests/fail/dangling_pointers/dangling_zst_deref.stderr

This file was deleted.

5 changes: 0 additions & 5 deletions tests/fail/dangling_pointers/maybe_null_pointer_deref_zst.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/dangling_pointers/maybe_null_pointer_deref_zst.stderr

This file was deleted.

8 changes: 0 additions & 8 deletions tests/fail/dangling_pointers/maybe_null_pointer_write_zst.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/dangling_pointers/maybe_null_pointer_write_zst.stderr

This file was deleted.

5 changes: 0 additions & 5 deletions tests/fail/dangling_pointers/null_pointer_deref_zst.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/dangling_pointers/null_pointer_deref_zst.stderr

This file was deleted.

8 changes: 0 additions & 8 deletions tests/fail/dangling_pointers/null_pointer_write_zst.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/dangling_pointers/null_pointer_write_zst.stderr

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/intrinsics/copy_null.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/intrinsics/copy_null.stderr

This file was deleted.

9 changes: 0 additions & 9 deletions tests/fail/intrinsics/ptr_offset_0_plus_0.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/intrinsics/ptr_offset_0_plus_0.stderr

This file was deleted.

7 changes: 0 additions & 7 deletions tests/fail/intrinsics/ptr_offset_from_oob.rs

This file was deleted.

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

This file was deleted.

7 changes: 0 additions & 7 deletions tests/fail/intrinsics/ptr_offset_ptr_plus_0.rs

This file was deleted.

20 changes: 0 additions & 20 deletions tests/fail/intrinsics/ptr_offset_ptr_plus_0.stderr

This file was deleted.

10 changes: 0 additions & 10 deletions tests/fail/intrinsics/write_bytes_null.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/intrinsics/write_bytes_null.stderr

This file was deleted.

12 changes: 0 additions & 12 deletions tests/fail/zst2.rs

This file was deleted.

25 changes: 0 additions & 25 deletions tests/fail/zst2.stderr

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/zst3.rs

This file was deleted.

20 changes: 0 additions & 20 deletions tests/fail/zst3.stderr

This file was deleted.

File renamed without changes.
4 changes: 2 additions & 2 deletions tests/fail/zst1.stderr → tests/fail/zst_local_oob.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error: Undefined Behavior: memory access failed: ALLOC has size 0, so pointer to 1 byte starting at offset 0 is out-of-bounds
--> $DIR/zst1.rs:LL:CC
--> $DIR/zst_local_oob.rs:LL:CC
|
LL | let _val = unsafe { *x };
| ^^ memory access failed: ALLOC has size 0, so pointer to 1 byte starting at offset 0 is out-of-bounds
|
= 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
= note: BACKTRACE:
= note: inside `main` at $DIR/zst1.rs:LL:CC
= note: inside `main` at $DIR/zst_local_oob.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

Expand Down
7 changes: 3 additions & 4 deletions tests/pass/align_offset_symbolic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,9 @@ fn vtable() {
let parts: (*const (), *const u8) = unsafe { mem::transmute(ptr) };
let vtable = parts.1;
let offset = vtable.align_offset(mem::align_of::<TWOPTR>());
let _vtable_aligned = vtable.wrapping_add(offset) as *const [TWOPTR; 0];
// FIXME: we can't actually do the access since vtable pointers act like zero-sized allocations.
// Enable the next line once https://github.com/rust-lang/rust/issues/117945 is implemented.
//let _place = unsafe { &*vtable_aligned };
let vtable_aligned = vtable.wrapping_add(offset) as *const [TWOPTR; 0];
// Zero-sized deref, so no in-bounds requirement.
let _place = unsafe { &*vtable_aligned };
}

fn main() {
Expand Down
Loading

0 comments on commit 388606b

Please sign in to comment.