Miri UB error from Fuse

[Nemo157]

Tested code (playground):

use std::pin::pin;
use std::future::Future;
use futures::{StreamExt, FutureExt, future::FusedFuture};

fn main() {
    async fn loopy() {
        let mut stream = pin!(futures::stream::pending());
        loop {
            let () = stream.next().await.unwrap();
        }
    }

    let mut future = pin!(async {
        let mut l = pin!(loopy().fuse());
        std::future::poll_fn(|cx| {
            l.is_terminated();
            l.as_mut().poll(cx)
        }).await;
    });

    future.as_mut().now_or_never();
    future.as_mut().now_or_never();
}

gives a miri error:

error: Undefined Behavior: trying to retag from <2953> for SharedReadWrite permission at alloc1164[0x8], but that tag does not exist in the borrow stack for this location
  --> /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/next.rs:32:9
   |
32 |         self.stream.poll_next_unpin(cx)
   |         ^^^^^^^^^^^
   |         |
   |         trying to retag from <2953> for SharedReadWrite permission at alloc1164[0x8], but that tag does not exist in the borrow stack for this location
   |         this error occurs as part of two-phase retag at alloc1164[0x8..0x10]
   |
   = 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: <2953> was created by a Unique retag at offsets [0x8..0x10]
  --> src/main.rs:9:22
   |
9  |             let () = stream.next().await.unwrap();
   |                      ^^^^^^^^^^^^^^^^^^^
help: <2953> was later invalidated at offsets [0x8..0x20] by a SharedReadOnly retag
  --> src/main.rs:16:13
   |
16 |             l.is_terminated();
   |             ^^^^^^^^^^^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `<futures::stream::Next<'_, std::pin::Pin<&mut futures::stream::Pending<()>>> as futures::Future>::poll` at /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/next.rs:32:9: 32:20
note: inside closure
  --> src/main.rs:9:36
   |
9  |             let () = stream.next().await.unwrap();
   |                                    ^^^^^
   = note: inside `<futures::future::Fuse<{async fn body of main::loopy()}> as futures::Future>::poll` at /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/future/future/fuse.rs:84:26: 84:38
note: inside closure
  --> src/main.rs:17:13
   |
17 |             l.as_mut().poll(cx)
   |             ^^^^^^^^^^^^^^^^^^^
   = note: inside `<std::future::PollFn<{closure@src/main.rs:15:30: 15:34}> 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:151:9: 151:57
note: inside closure
  --> src/main.rs:18:12
   |
18 |         }).await;
   |            ^^^^^
   = note: inside `<std::pin::Pin<&mut {async block@src/main.rs:13:27: 13:32}> as futures::Future>::poll` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/future/future.rs:123:9: 123:61
   = note: inside `<std::pin::Pin<&mut {async block@src/main.rs:13:27: 13:32}> as futures::FutureExt>::now_or_never` at /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/future/future/mod.rs:601:15: 601:33
note: inside `main`
  --> src/main.rs:22:5
   |
22 |     future.as_mut().now_or_never();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This is the same pattern used in select!, so it is also impacted by it.

I'm assuming this is some kind of miri false positive related to the &Self reference used in FusedFuture, as the alternative seems to be that pin-project is unsound 😰.

[taiki-e]

I think this is rust-lang/miri#3780 / rust-lang/miri#3796 because Fuse uses Option<impl Future> which has the same issue as MaybeDone.

[Nemo157]

Yes, looks like it; my understanding: the Option discriminant in Fuse is getting niche-optimized somewhere inside loopy's future, so when calling is_terminated that is actually reading memory inside the future as read-only. If the async-lowering is fixed to place the loopy state into UnsafePinned fields that should block niche-optimization from going into them, leaving the discriminant in separate memory and fixing this.

Maybe it's worth doing something like https://github.com/tokio-rs/tokio/pull/6744/files to Fuse to avoid the issue for now.

[taiki-e]

To be honest, replacing the use of all Option's that might wrap futures with a custom repr-C enum does not sound like a realistic solutionworkaround.

There are at least 25 that can be easily searched.
https://github.com/search?q=repo%3Arust-lang%2Ffutures-rs+%2FOption%3CFut%3E%2F&type=code

(EDIT: Some streams contain future internally, such as ones generated by async-stream, so we may actually need to replace all Option<T>s whose T content is not known...)

[taiki-e]

Ralf opened a PR to work around this issue in rustc: rust-lang/rust#129313

Closing this issue in favor of upstream bug tracker.