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

Spurious "Undefined Behavior" Error #2580

Closed
jswrenn opened this issue Oct 4, 2022 · 3 comments
Closed

Spurious "Undefined Behavior" Error #2580

jswrenn opened this issue Oct 4, 2022 · 3 comments

Comments

@jswrenn
Copy link
Member

jswrenn commented Oct 4, 2022

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
Copy link
Member

RalfJung commented Oct 4, 2022

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
Copy link
Member

RalfJung commented Oct 4, 2022

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
Copy link
Member

RalfJung commented Oct 4, 2022

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.

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