Skip to content

Commit

Permalink
Auto merge of #117658 - RalfJung:ptr-dangling, r=m-ou-se
Browse files Browse the repository at this point in the history
rename ptr::invalid -> ptr::without_provenance

It has long bothered me that `ptr::invalid` returns a pointer that is actually valid for zero-sized memory accesses. In general, it doesn't even make sense to ask "is this pointer valid", you have to ask "is this pointer valid for a given memory access". We could say that a pointer is invalid if it is not valid for *any* memory access, but [the way this FCP is going](rust-lang/unsafe-code-guidelines#472), it looks like *all* pointers will be valid for zero-sized memory accesses.

Two possible alternative names emerged as people's favorites:
1. Something involving `dangling`, in analogy to `NonNull::dangling`. To avoid inconsistency with the `NonNull` method, the address-taking method could be called `dangling_at(addr: usize) -> *const T`.
2. `without_provenance`, to be symmetric with the inverse operation `ptr.addr_without_provenance()` (currently still called `ptr.addr()` but probably going to be renamed)

I have no idea which one of these is better. I read [this comment](rust-lang/rust#117658 (comment)) as expressing a slight preference for something like the second option, so I went for that. I'm happy to go with `dangling_at` as well.

Cc `@rust-lang/opsem`
  • Loading branch information
bors committed Feb 21, 2024
2 parents 3e52033 + fb82ee0 commit 8b0443e
Show file tree
Hide file tree
Showing 11 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion tests/fail/dangling_pointers/deref_dangling_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::ptr::{self, addr_of_mut};
// (This test relies on the `deref_copy` pass that lowers `**ptr` to materialize the intermediate pointer.)

fn main() {
let mut inner = ptr::invalid::<i32>(24);
let mut inner = ptr::without_provenance::<i32>(24);
let outer = addr_of_mut!(inner).cast::<Box<i32>>();
// Now `outer` is a pointer to a dangling reference.
// Deref'ing that should be UB.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/dangling_pointers/deref_dangling_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::ptr::{self, addr_of_mut};
// (This test relies on the `deref_copy` pass that lowers `**ptr` to materialize the intermediate pointer.)

fn main() {
let mut inner = ptr::invalid::<i32>(24);
let mut inner = ptr::without_provenance::<i32>(24);
let outer = addr_of_mut!(inner).cast::<&'static mut i32>();
// Now `outer` is a pointer to a dangling reference.
// Deref'ing that should be UB.
Expand Down
4 changes: 2 additions & 2 deletions tests/fail/provenance/ptr_invalid.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#![feature(strict_provenance, exposed_provenance)]

// Ensure that a `ptr::invalid` ptr is truly invalid.
// Ensure that a `ptr::without_provenance` ptr is truly invalid.
fn main() {
let x = 42;
let xptr = &x as *const i32;
let xptr_invalid = std::ptr::invalid::<i32>(xptr.expose_addr());
let xptr_invalid = std::ptr::without_provenance::<i32>(xptr.expose_addr());
let _val = unsafe { *xptr_invalid }; //~ ERROR: is a dangling pointer
}
2 changes: 1 addition & 1 deletion tests/fail/provenance/ptr_invalid_offset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
fn main() {
let x = 22;
let ptr = &x as *const _ as *const u8;
let roundtrip = std::ptr::invalid::<u8>(ptr as usize);
let roundtrip = std::ptr::without_provenance::<u8>(ptr as usize);
// Not even offsetting this is allowed.
let _ = unsafe { roundtrip.offset(1) }; //~ERROR: is a dangling pointer
}
8 changes: 4 additions & 4 deletions tests/pass-dep/shims/mmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ fn test_mmap<Offset: Default>(

let ptr = unsafe {
mmap(
ptr::invalid_mut(page_size * 64),
ptr::without_provenance_mut(page_size * 64),
page_size,
libc::PROT_READ | libc::PROT_WRITE,
// We don't support MAP_FIXED
Expand Down Expand Up @@ -114,13 +114,13 @@ fn test_mmap<Offset: Default>(
assert_eq!(ptr, libc::MAP_FAILED);

// We report an error when trying to munmap an address which is not a multiple of the page size
let res = unsafe { libc::munmap(ptr::invalid_mut(1), page_size) };
let res = unsafe { libc::munmap(ptr::without_provenance_mut(1), page_size) };
assert_eq!(res, -1);
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);

// We report an error when trying to munmap a length that cannot be rounded up to a multiple of
// the page size.
let res = unsafe { libc::munmap(ptr::invalid_mut(page_size), usize::MAX - 1) };
let res = unsafe { libc::munmap(ptr::without_provenance_mut(page_size), usize::MAX - 1) };
assert_eq!(res, -1);
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);
}
Expand Down Expand Up @@ -156,7 +156,7 @@ fn test_mremap() {
// Test all of our error conditions
// Not aligned
let ptr =
unsafe { libc::mremap(ptr::invalid_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE) };
unsafe { libc::mremap(ptr::without_provenance_mut(1), page_size, page_size, libc::MREMAP_MAYMOVE) };
assert_eq!(ptr, libc::MAP_FAILED);
assert_eq!(Error::last_os_error().raw_os_error().unwrap(), libc::EINVAL);

Expand Down
4 changes: 2 additions & 2 deletions tests/pass-dep/shims/posix_memalign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn main() {

// Non-power of 2 align
unsafe {
let mut ptr: *mut libc::c_void = ptr::invalid_mut(0x1234567);
let mut ptr: *mut libc::c_void = ptr::without_provenance_mut(0x1234567);
let align = 15;
let size = 8;
assert_eq!(libc::posix_memalign(&mut ptr, align, size), libc::EINVAL);
Expand All @@ -70,7 +70,7 @@ fn main() {

// Too small align (smaller than ptr)
unsafe {
let mut ptr: *mut libc::c_void = ptr::invalid_mut(0x1234567);
let mut ptr: *mut libc::c_void = ptr::without_provenance_mut(0x1234567);
let align = std::mem::size_of::<usize>() / 2;
let size = 8;
assert_eq!(libc::posix_memalign(&mut ptr, align, size), libc::EINVAL);
Expand Down
2 changes: 1 addition & 1 deletion tests/pass/align_offset_symbolic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn huge_align() {
#[cfg(target_pointer_width = "16")]
const SIZE: usize = 1 << 13;
struct HugeSize(#[allow(dead_code)] [u8; SIZE - 1]);
let _ = std::ptr::invalid::<HugeSize>(SIZE).align_offset(SIZE);
let _ = std::ptr::without_provenance::<HugeSize>(SIZE).align_offset(SIZE);
}

// This shows that we cannot store the promised alignment info in `AllocExtra`,
Expand Down
6 changes: 3 additions & 3 deletions tests/pass/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,15 @@ fn atomic_ptr() {

let ptr = AtomicPtr::<i32>::new(ptr::null_mut());
assert!(ptr.load(Relaxed).addr() == 0);
ptr.store(ptr::invalid_mut(13), SeqCst);
ptr.store(ptr::without_provenance_mut(13), SeqCst);
assert!(ptr.swap(x, Relaxed).addr() == 13);
unsafe { assert!(*ptr.load(Acquire) == 0) };

// comparison ignores provenance
assert_eq!(
ptr.compare_exchange(
(&mut 0 as *mut i32).with_addr(x.addr()),
ptr::invalid_mut(0),
ptr::without_provenance_mut(0),
SeqCst,
SeqCst
)
Expand All @@ -156,7 +156,7 @@ fn atomic_ptr() {
assert_eq!(
ptr.compare_exchange(
(&mut 0 as *mut i32).with_addr(x.addr()),
ptr::invalid_mut(0),
ptr::without_provenance_mut(0),
SeqCst,
SeqCst
)
Expand Down
8 changes: 4 additions & 4 deletions tests/pass/ptr_raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ fn assign_overlapping() {
fn deref_invalid() {
unsafe {
// `addr_of!(*ptr)` is never UB.
let _val = addr_of!(*ptr::invalid::<i32>(0));
let _val = addr_of!(*ptr::invalid::<i32>(1)); // not aligned
let _val = addr_of!(*ptr::without_provenance::<i32>(0));
let _val = addr_of!(*ptr::without_provenance::<i32>(1)); // not aligned

// Similarly, just mentioning the place is fine.
let _ = *ptr::invalid::<i32>(0);
let _ = *ptr::invalid::<i32>(1);
let _ = *ptr::without_provenance::<i32>(0);
let _ = *ptr::without_provenance::<i32>(1);
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/pass/slices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn slice_of_zst() {

// In a slice of zero-size elements the pointer is meaningless.
// Ensure iteration still works even if the pointer is at the end of the address space.
let slice: &[()] = unsafe { slice::from_raw_parts(ptr::invalid(-5isize as usize), 10) };
let slice: &[()] = unsafe { slice::from_raw_parts(ptr::without_provenance(-5isize as usize), 10) };
assert_eq!(slice.len(), 10);
assert_eq!(slice.iter().count(), 10);

Expand All @@ -43,7 +43,7 @@ fn slice_of_zst() {

// Test mutable iterators as well
let slice: &mut [()] =
unsafe { slice::from_raw_parts_mut(ptr::invalid_mut(-5isize as usize), 10) };
unsafe { slice::from_raw_parts_mut(ptr::without_provenance_mut(-5isize as usize), 10) };
assert_eq!(slice.len(), 10);
assert_eq!(slice.iter_mut().count(), 10);

Expand Down Expand Up @@ -263,7 +263,7 @@ fn test_for_invalidated_pointers() {
fn large_raw_slice() {
let size = isize::MAX as usize;
// Creating a raw slice of size isize::MAX and asking for its size is okay.
let s = std::ptr::slice_from_raw_parts(ptr::invalid::<u8>(1), size);
let s = std::ptr::slice_from_raw_parts(ptr::without_provenance::<u8>(1), size);
assert_eq!(size, unsafe { std::mem::size_of_val_raw(s) });
}

Expand Down
4 changes: 2 additions & 2 deletions tests/pass/underscore_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fn invalid_match() {

fn dangling_let() {
unsafe {
let ptr = ptr::invalid::<bool>(0x40);
let ptr = ptr::without_provenance::<bool>(0x40);
let _ = *ptr;
}
}
Expand All @@ -54,7 +54,7 @@ fn invalid_let() {
// Adding a type annotation used to change how MIR is generated, make sure we cover both cases.
fn dangling_let_type_annotation() {
unsafe {
let ptr = ptr::invalid::<bool>(0x40);
let ptr = ptr::without_provenance::<bool>(0x40);
let _: bool = *ptr;
}
}
Expand Down

0 comments on commit 8b0443e

Please sign in to comment.