rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.59k stars 346 forks source link

Spurious "Undefined Behavior" Error #2580

Closed jswrenn closed 2 years ago

jswrenn commented 2 years ago

Running this code snippet (playground) with miri:

use std::{
    future::{self, Future},
    pin::Pin,
    task::{Context, Poll},
};

fn main() {
    let mut future = trouble();

    let mut pinned = Box::pin(future::poll_fn(move |cx| {
        unsafe { Pin::new_unchecked(&mut future) }.poll(cx)
    }));

    let waker = futures::task::noop_waker();
    let mut cx = Context::from_waker(&waker);

    let _ = pinned.as_mut().poll(&mut cx);
    let _ = pinned.as_mut().poll(&mut cx);
}

async fn trouble() {
    let lucky_number = 42;
    let problematic_variable = &lucky_number;

    yield_now().await;

    // problematic dereference
    let _ = { *problematic_variable };
}

fn yield_now() -> impl Future {
    let mut yielded = false;
    future::poll_fn(move |_| {
        if core::mem::replace(&mut yielded, true) {
            Poll::Ready(())
        } else {
            Poll::Pending
        }
    })
}

...results in this diagnostic:

error: Undefined Behavior: attempting a read access using <3121> at alloc1549[0x8], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:28:15
   |
28 |     let _ = { *problematic_variable };
   |               ^^^^^^^^^^^^^^^^^^^^^
   |               |
   |               attempting a read access using <3121> at alloc1549[0x8], but that tag does not exist in the borrow stack for this location
   |               this error occurs as part of an access at alloc1549[0x8..0xc]
   |
   = 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: <3121> was created by a SharedReadWrite retag at offsets [0x0..0x10]
  --> src/main.rs:11:9
   |
11 |         unsafe { Pin::new_unchecked(&mut future) }.poll(cx)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <3121> was later invalidated at offsets [0x0..0x10] by a Unique retag
  --> src/main.rs:18:13
   |
18 |     let _ = pinned.as_mut().poll(&mut cx);
   |             ^^^^^^^^^^^^^^^
   = note: BACKTRACE:
   = note: inside closure at src/main.rs:28:15
   = note: inside `<std::future::from_generator::GenFuture<[static generator@src/main.rs:21:20: 29:2]> as futures::Future>::poll` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/mod.rs:91:19
note: inside closure at src/main.rs:11:9
  --> src/main.rs:11:9
   |
11 |         unsafe { Pin::new_unchecked(&mut future) }.poll(cx)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: inside `<std::future::PollFn<[closure@src/main.rs:10:47: 10:56]> as futures::Future>::poll` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/poll_fn.rs:61:9
note: inside `main` at src/main.rs:18:13
  --> src/main.rs:18:13
   |
18 |     let _ = pinned.as_mut().poll(&mut cx);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to previous error

I'm using the miri packaged with rustc 1.66.0-nightly (ce7f0f1aa 2022-09-28).

Discussion on Zulip here: https://rust-lang.zulipchat.com/#narrow/stream/269128-miri/topic/Diagnosing.20retag.20in.20.60.2Eawait.60

RalfJung commented 2 years ago

I get that this error is unexpected, but 'spurious' sounds like a claim that this is a false positive?

Here's a version that avoids unsafe, and it also makes Miri happy.

RalfJung commented 2 years ago

The original code is probably just buggy -- each time you write &mut future, that claims that this is the only pointer to future that will be used henceforth. And that claim is just wrong since the future is self-referential. Once it is pinned, you must never create a mutable reference directly to a future, since that would invalidate the self-referential references.

Though I admit I don't entirely understand why the !Unpin exception is insufficient.

RalfJung commented 2 years ago

I did some more investigation, see IRLO for details. But Miri is right, the original code is buggy -- pinned is in fact an Unpin future with a by-value self-referential generator, which is not kosher. A slight variant of this causes a use-after-free, so it's not just aliasing rules that are violated.