Skip to content

Commit

Permalink
Rollup merge of #122964 - joboet:pointer_expose, r=Amanieu
Browse files Browse the repository at this point in the history
Rename `expose_addr` to `expose_provenance`

`expose_addr` is a bad name, an address is just a number and cannot be exposed. The operation is actually about the provenance of the pointer.

This PR thus changes the name of the method to `expose_provenance` without changing its return type. There is sufficient precedence for returning a useful value from an operation that does something else without the name indicating such, e.g. [`Option::insert`](https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.insert) and [`MaybeUninit::write`](https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.write).

Returning the address is merely convenient, not a fundamental part of the operation. This is implied by the fact that integers do not have provenance since
```rust
let addr = ptr.addr();
ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
must behave exactly like
```rust
let addr = ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
as the result of `ptr.expose_provenance()` and `ptr.addr()` is the same integer. Therefore, this PR removes the `#[must_use]` annotation on the function and updates the documentation to reflect the important part.

~~An alternative name would be `expose_provenance`. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by `ptr::with_exposed_provenance`[^1] is the same as that what was exposed, which is not yet specified as such IIUC. IMHO `expose` does not make that connection.~~

A previous version of this PR suggested `expose` as name, libs-api [decided on](rust-lang/rust#122964 (comment)) `expose_provenance` to keep the symmetry with `with_exposed_provenance`.

CC `@RalfJung`
r? libs-api

[^1]: I'm using the new name for `from_exposed_addr` suggested by #122935 here.
  • Loading branch information
matthiaskrgr committed Apr 3, 2024
2 parents 8f012a3 + 34c6202 commit 00909fe
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/alloc_addresses/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use reuse_pool::ReusePool;

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum ProvenanceMode {
/// We support `expose_addr`/`with_exposed_provenance` via "wildcard" provenance.
/// However, we want on `with_exposed_provenance` to alert the user of the precision loss.
/// We support `expose_provenance`/`with_exposed_provenance` via "wildcard" provenance.
/// However, we warn on `with_exposed_provenance` to alert the user of the precision loss.
Default,
/// Like `Default`, but without the warning.
Permissive,
Expand Down
6 changes: 3 additions & 3 deletions src/shims/intrinsics/simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
dest.transmute(this.machine.layouts.uint(dest.layout.size).unwrap(), this)?;
this.write_int(res, &dest)?;
}
"cast" | "as" | "cast_ptr" | "expose_addr" | "with_exposed_provenance" => {
"cast" | "as" | "cast_ptr" | "expose_provenance" | "with_exposed_provenance" => {
let [op] = check_arg_count(args)?;
let (op, op_len) = this.operand_to_simd(op)?;
let (dest, dest_len) = this.mplace_to_simd(dest)?;
Expand All @@ -524,7 +524,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
let unsafe_cast = intrinsic_name == "cast";
let safe_cast = intrinsic_name == "as";
let ptr_cast = intrinsic_name == "cast_ptr";
let expose_cast = intrinsic_name == "expose_addr";
let expose_cast = intrinsic_name == "expose_provenance";
let from_exposed_cast = intrinsic_name == "with_exposed_provenance";

for i in 0..dest_len {
Expand Down Expand Up @@ -557,7 +557,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
this.ptr_to_ptr(&op, dest.layout)?,
// Ptr/Int casts
(ty::RawPtr(..), ty::Int(_) | ty::Uint(_)) if expose_cast =>
this.pointer_expose_address_cast(&op, dest.layout)?,
this.pointer_expose_provenance_cast(&op, dest.layout)?,
(ty::Int(_) | ty::Uint(_), ty::RawPtr(..)) if from_exposed_cast =>
this.pointer_with_exposed_provenance_cast(&op, dest.layout)?,
// Error otherwise
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/provenance/ptr_invalid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
fn main() {
let x = 42;
let xptr = &x as *const i32;
let xptr_invalid = std::ptr::without_provenance::<i32>(xptr.expose_addr());
let xptr_invalid = std::ptr::without_provenance::<i32>(xptr.expose_provenance());
let _val = unsafe { *xptr_invalid }; //~ ERROR: is a dangling pointer
}
2 changes: 1 addition & 1 deletion tests/fail/stacked_borrows/exposed_only_ro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
fn main() {
let mut x = 0;
let _fool = &mut x as *mut i32; // this would have fooled the old untagged pointer logic
let addr = (&x as *const i32).expose_addr();
let addr = (&x as *const i32).expose_provenance();
let ptr = std::ptr::with_exposed_provenance_mut::<i32>(addr);
unsafe { *ptr = 0 }; //~ ERROR: /write access using <wildcard> .* no exposed tags have suitable permission in the borrow stack/
}
2 changes: 1 addition & 1 deletion tests/pass/portable-simd-ptrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ use std::simd::prelude::*;
fn main() {
// Pointer casts
let _val: Simd<*const u8, 4> = Simd::<*const i32, 4>::splat(ptr::null()).cast();
let addrs = Simd::<*const i32, 4>::splat(ptr::null()).expose_addr();
let addrs = Simd::<*const i32, 4>::splat(ptr::null()).expose_provenance();
let _ptrs = Simd::<*const i32, 4>::with_exposed_provenance(addrs);
}
10 changes: 5 additions & 5 deletions tests/pass/ptr_int_from_exposed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn ptr_roundtrip_out_of_bounds() {
let x: i32 = 3;
let x_ptr = &x as *const i32;

let x_usize = x_ptr.wrapping_offset(128).expose_addr();
let x_usize = x_ptr.wrapping_offset(128).expose_provenance();

let ptr = ptr::with_exposed_provenance::<i32>(x_usize).wrapping_offset(-128);
assert_eq!(unsafe { *ptr }, 3);
Expand All @@ -24,8 +24,8 @@ fn ptr_roundtrip_confusion() {
let x_ptr = &x as *const i32;
let y_ptr = &y as *const i32;

let x_usize = x_ptr.expose_addr();
let y_usize = y_ptr.expose_addr();
let x_usize = x_ptr.expose_provenance();
let y_usize = y_ptr.expose_provenance();

let ptr = ptr::with_exposed_provenance::<i32>(y_usize);
let ptr = ptr.with_addr(x_usize);
Expand All @@ -37,7 +37,7 @@ fn ptr_roundtrip_imperfect() {
let x: u8 = 3;
let x_ptr = &x as *const u8;

let x_usize = x_ptr.expose_addr() + 128;
let x_usize = x_ptr.expose_provenance() + 128;

let ptr = ptr::with_exposed_provenance::<u8>(x_usize).wrapping_offset(-128);
assert_eq!(unsafe { *ptr }, 3);
Expand All @@ -48,7 +48,7 @@ fn ptr_roundtrip_null() {
let x = &42;
let x_ptr = x as *const i32;
let x_null_ptr = x_ptr.with_addr(0); // addr 0, but still the provenance of x
let null = x_null_ptr.expose_addr();
let null = x_null_ptr.expose_provenance();
assert_eq!(null, 0);

let x_null_ptr_copy = ptr::with_exposed_provenance::<i32>(null); // just a roundtrip, so has provenance of x (angelically)
Expand Down
4 changes: 2 additions & 2 deletions tests/pass/stacked-borrows/int-to-ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn example(variant: bool) {
unsafe {
fn not_so_innocent(x: &mut u32) -> usize {
let x_raw4 = x as *mut u32;
x_raw4.expose_addr()
x_raw4.expose_provenance()
}

let mut c = 42u32;
Expand All @@ -26,7 +26,7 @@ fn example(variant: bool) {
// stack: [..., Unique(1)]

let x_raw2 = x_unique1 as *mut u32;
let x_raw2_addr = x_raw2.expose_addr();
let x_raw2_addr = x_raw2.expose_provenance();
// stack: [..., Unique(1), SharedRW(2)]

let x_unique3 = &mut *x_raw2;
Expand Down
2 changes: 1 addition & 1 deletion tests/pass/stacked-borrows/unknown-bottom-gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn main() {

// Expose the allocation and use the exposed pointer, creating an unknown bottom
unsafe {
let p: *mut u8 = ptr::with_exposed_provenance::<u8>(ptr.expose_addr()) as *mut u8;
let p: *mut u8 = ptr::with_exposed_provenance::<u8>(ptr.expose_provenance()) as *mut u8;
*p = 1;
}

Expand Down

0 comments on commit 00909fe

Please sign in to comment.