Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code identical to Box::as_ptr has different behavior #3870

Closed
theemathas opened this issue Sep 8, 2024 · 2 comments
Closed

Code identical to Box::as_ptr has different behavior #3870

theemathas opened this issue Sep 8, 2024 · 2 comments

Comments

@theemathas
Copy link

Box::as_ptr has the following source code:

impl<T: ?Sized, A: Allocator> Box<T, A> {
    // snip
    #[unstable(feature = "box_as_ptr", issue = "129090")]
    #[rustc_never_returns_null_ptr]
    #[inline]
    pub fn as_ptr(b: &Self) -> *const T {
        // This is a primitive deref, not going through `DerefMut`, and therefore not materializing
        // any references.
        ptr::addr_of!(**b)
    }
    // snip
}

The documentation says that mutating the value via the returned raw pointer is disallowed. But I tried the following code in Miri anyway:

#![feature(box_as_ptr)]

fn main() {
    let mut x = Box::new(1);
    let x_ptr = (Box::as_ptr(&x)) as *mut i32;
    unsafe { x_ptr.write(4); }
}

The above code runs with Miri fine without any errors.

However, if I try to define a function with the same behavior as Box::as_ptr myself, as follows:

fn as_ptr(x: &Box<i32>) -> *const i32 {
    std::ptr::addr_of!(**x)
}

fn main() {
    let mut x = Box::new(1);
    let x_ptr = (as_ptr(&x)) as *mut i32;
    unsafe { x_ptr.write(4); }
}

The above code results in Miri reporting undefined behavior:

error: Undefined Behavior: attempting a write access using <2662> at alloc1235[0x0], but that tag only grants SharedReadOnly permission for this location
 --> src/main.rs:8:14
  |
8 |     unsafe { x_ptr.write(4); }
  |              ^^^^^^^^^^^^^^
  |              |
  |              attempting a write access using <2662> at alloc1235[0x0], but that tag only grants SharedReadOnly permission for this location
  |              this error occurs as part of an access at alloc1235[0x0..0x4]
  |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
  = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <2662> was created by a SharedReadOnly retag at offsets [0x0..0x4]
 --> src/main.rs:2:5
  |
2 |     std::ptr::addr_of!(**x)
  |     ^^^^^^^^^^^^^^^^^^^^^^^
  = note: BACKTRACE (of the first span):
  = note: inside `main` at src/main.rs:8:14: 8:28
  = note: this error originates in the macro `std::ptr::addr_of` (in Nightly builds, run with -Z macro-backtrace for more info)

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

error: aborting due to 1 previous error; 1 warning emitted

I expected both versions of the code to have the same behavior, but miri only reported undefined behavior on only one of them. Thus, I'm filing this issue.

The compiler version I tested is the playground nightly version "1.83.0-nightly (2024-09-05 9c01301c52df5d2d7b6f)"

@theemathas
Copy link
Author

theemathas commented Sep 8, 2024

For some reason, adding an Allocator type parameter results in Miri not detecting UB again:

#![feature(allocator_api)]

fn as_ptr<A: std::alloc::Allocator>(x: &Box<i32, A>) -> *const i32 {
    std::ptr::addr_of!(**x)
}

fn main() {
    let mut x = Box::new(1);
    let x_ptr = (as_ptr(&x)) as *mut i32;
    unsafe { x_ptr.write(4); }
}

The above code runs on Miri without any errors.

@RalfJung
Copy link
Member

RalfJung commented Sep 8, 2024

Thanks for the report!

Indeed, your code is not identical to the one in Box::as_ptr due to the alloc parameter. The alloc parameter was added hastily and without sufficient consideration for all the side-effects it has to change a language-primitive type like this. There's little Miri can do here, this needs to be cleaned up in rustc and the library first -- see rust-lang/rust#95453. Miri has a bunch of terrible hacks to work around this, and you managed to hit the edge case of when exactly the hacks do and do not apply.

You should treat this code as UB, but due to the alloc parameter sadly Miri cannot report this UB. To my knowledge, this does not make Miri miss any UB that actually can affect your code today, but this code may be declared UB in the future as the Rust aliasing model solidifies.

@RalfJung RalfJung closed this as completed Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants