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

Miscompilation when awaiting generator containing an enum with niche at least [0, 1] #90038

Closed
tmandry opened this issue Oct 19, 2021 · 8 comments · Fixed by #90040
Closed

Miscompilation when awaiting generator containing an enum with niche at least [0, 1] #90038

tmandry opened this issue Oct 19, 2021 · 8 comments · Fixed by #90040
Assignees
Labels
C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@tmandry
Copy link
Member

tmandry commented Oct 19, 2021

Code

This code: (playground)

#[repr(u32)]
pub enum Key {
    // Any number greater than 1 works.
    A = 2,
}

async fn foo() {}
async fn bar(_key: Key) {}

#[tokio::main]
async fn main() {
    let (_, _) = futures::future::join(
        foo(),
        bar(Key::A),
    )
    .await;
}

Panics with the message MaybeDone polled after value taken from inside futures-rs. The code driving this particular object is also in futures-rs inside the Join combinator, but this code couldn't possibly result in the panic that we see. That would require the Join future to get polled after it was completed, but that future is only awaited, so this should never happen.

This is a regression from stable to beta, bisected to PR #87794. It still exists in nightly.

Backtrace

Backtrace

thread 'main' panicked at 'MaybeDone polled after value taken', /playground/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-0.3.17/src/future/maybe_done.rs:99:36
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/d4647278cb2948e76b51e8cd7aa7d31ba6478a08/library/std/src/panicking.rs:543:12
   1: <futures_util::future::maybe_done::MaybeDone<Fut> as core::future::future::Future>::poll
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-0.3.17/src/future/maybe_done.rs:99:36
   2: <futures_util::future::join::Join<Fut1,Fut2> as core::future::future::Future>::poll
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/futures-util-0.3.17/src/future/join.rs:55:33
   3: playground::main::{{closure}}
             at ./src/main.rs:11:18
   4: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/d4647278cb2948e76b51e8cd7aa7d31ba6478a08/library/core/src/future/mod.rs:80:19
   5: tokio::park::thread::CachedParkThread::block_on::{{closure}}
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/park/thread.rs:263:54
   6: tokio::coop::with_budget::{{closure}}
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/coop.rs:106:9
   7: std::thread::local::LocalKey<T>::try_with
             at /rustc/d4647278cb2948e76b51e8cd7aa7d31ba6478a08/library/std/src/thread/local.rs:399:16
   8: std::thread::local::LocalKey<T>::with
             at /rustc/d4647278cb2948e76b51e8cd7aa7d31ba6478a08/library/std/src/thread/local.rs:375:9
   9: tokio::coop::with_budget
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/coop.rs:99:5
  10: tokio::coop::budget
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/coop.rs:76:5
  11: tokio::park::thread::CachedParkThread::block_on
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/park/thread.rs:263:31
  12: tokio::runtime::enter::Enter::block_on
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/runtime/enter.rs:151:13
  13: tokio::runtime::thread_pool::ThreadPool::block_on
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/runtime/thread_pool/mod.rs:72:9
  14: tokio::runtime::Runtime::block_on
             at ./.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.11.0/src/runtime/mod.rs:459:43
  15: playground::main
             at ./src/main.rs:11:5
  16: core::ops::function::FnOnce::call_once
             at /rustc/d4647278cb2948e76b51e8cd7aa7d31ba6478a08/library/core/src/ops/function.rs:227:5

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

@tmandry tmandry added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Oct 19, 2021
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. and removed regression-untriaged Untriaged performance or correctness regression. labels Oct 19, 2021
@JohnTitor JohnTitor added A-async-await Area: Async & Await T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 19, 2021
@tmandry
Copy link
Member Author

tmandry commented Oct 19, 2021

Note that this regression is in the newly-promoted-to-beta 1.57, not 1.56 which is being released as stable this week.

@nbdd0121
Copy link
Contributor

nbdd0121 commented Oct 19, 2021

Minimized:

#[repr(u32)]
pub enum Foo {
    A = 2,
}

pub enum Bar {
	A(Foo),
	B,
    C,
}

fn main() {
    match Bar::A(Foo::A) {
        Bar::A(_) => (),
        _ => unreachable!(),
    }
}

@nbdd0121
Copy link
Contributor

@rustbot claim

@nbdd0121
Copy link
Contributor

@rustbot label: -A-async-await

@rustbot rustbot removed the A-async-await Area: Async & Await label Oct 19, 2021
@JohnTitor
Copy link
Member

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@JohnTitor JohnTitor added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 19, 2021
@oli-obk oli-obk added beta-nominated Nominated for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Oct 19, 2021
@eddyb
Copy link
Member

eddyb commented Oct 19, 2021

(Posted in the fix PR as well for redundancy)

For the record, while I may have seen #87794 (comment) I do not recall ever reviewing the change it links to (52f4be9), which is where the bug was introduced (I was tipped off by the closures which did not exist in the version of that PR I reviewed).

@bors bors closed this as completed in d45ed75 Oct 19, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Oct 19, 2021

the fix still needs a backport

@m-ou-se
Copy link
Member

m-ou-se commented Oct 26, 2021

This was backported in #90151.

@m-ou-se m-ou-se closed this as completed Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants