Miri ICE: InvalidProgram(ConstPropNonsense): uninhabited variant in project_downcast #115145
Labels
A-const-eval
Area: constant evaluation (mir interpretation)
A-coroutines
Area: Coroutines
A-layout
Area: Memory layout of types
C-bug
Category: This is a bug.
I-ICE
Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
I-unsound
Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
P-high
High priority
S-has-mcve
Status: A Minimal Complete and Verifiable Example has been found for this issue
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
saethlin
added
I-ICE
rustbot
added
the
needs-triage
compiler-errors
added
E-needs-mcve
|
Can you run this with MIRI_BACKTRACE=1 so we learn where in the code the error is triggered? Also, Miri should print an interpreter backtrace of where in the crate it was when the error happened; please show that as well. |
|
I set that environment variable and I do not get any additional output. There is also no interpreter backtrace. |
|
Uh, that is strange. Does MIRI_BACKTRACE=immediate show anything? If not, what about RUSTC_CTFE_BACKTRACE=immediate?
|
|
Both of those flags print the same backtrace before the ICE: |
|
Oh interesting, it seems to be triggering this: rust/compiler/rustc_const_eval/src/interpret/projection.rs Lines 141 to 145 in 4354192 So there's something funny going on with an enum. Hopefully #115272 will let us capture an interpreter backtrace. |
|
Yep, that PR seems to work: |
|
Oh, this may be a downcast on a generator. I had not considered those. Hm... I think currently we guarantee that even uninhabited variants have sufficient space allocated to them, so maybe we can just remove that check? Still would be nice to have a small example. |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Aug 30, 2023
miri/diagnostics: don't forget to print_backtrace when ICEing on unexpected errors This should fix the missing output encountered [here](rust-lang#115145 (comment)). r? `@saethlin`
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this issue
Aug 31, 2023
miri/diagnostics: don't forget to print_backtrace when ICEing on unexpected errors This should fix the missing output encountered [here](rust-lang/rust#115145 (comment)). r? `@saethlin`
|
I've minimized use deadpool_runtime::Runtime;
use futures_lite::future::block_on;
use std::time::Duration;
enum TimeoutType {
Create,
}
async fn apply_timeout(
timeout_type: TimeoutType,
) -> Result<(), TimeoutType> {
match None::<Runtime> {
None => {
let _ = create().await;
Ok(())
}
Some(r) => {
let _ = r.timeout(Duration::from_secs(1), create()).await;
Err(timeout_type)
}
}
}
async fn create() -> Result<(), TimeoutType> {
Ok(())
}
fn main() {
let _ = block_on(apply_timeout(TimeoutType::Create));
} |
RalfJung
changed the title
|
Further reduced by @zachs18 to something we can put in a test case: #![feature(noop_waker)]
use std::future::Future;
enum Runtime {}
async fn run(_: Runtime) {}
async fn apply_timeout() {
let c = async {};
match None::<Runtime> {
None => {
c.await;
},
Some(r) => {
run(r).await;
}
}
}
fn main() {
let waker = std::task::Waker::noop();
let mut ctx = std::task::Context::from_waker(&waker);
let fut = std::pin::pin!(apply_timeout());
let _ = fut.poll(&mut ctx);
} |
saethlin
added
S-has-mcve
|
I can't say I understand why we are projecting to an uninhabited variant here... but I guess the point is, the lowering is picking any variant that has the field we need, and there's no logic that would avoid using an uninhabited variant for that? |
|
I don't understand either, and I think at this point it would be good for me to understand exactly what about the generator lowering is problematic here. I've mostly ignored the MIR because there's quite a lot. |
I started looking into this question but then realized that since all our lowering code is working on polymorphic MIR I'm not certain this situation is even possible to avoid in lowering. |
|
Hm... I would think that there's always a "current" state in generator lowering and one can use that to access the field, and we know that the current state has all fields inhabited because otherwise it couldn't even become the current state. But I'm not sure how the lowering actually works so this is probably naive. Also, even for enums, we might want to lower code like this enum Option2 {
Some(i32, !),
None
}
fn panic() -> ! { panic!() }
let x = Some(42, panic());into MIR like and then we would downcast to an uninhabited variant. So I'm perfectly fine with allowing such downcasts, the reason I added the assertion was so that we'd know when they actually happen. Would be good to have something like the above as a custom MIR test. |
|
The fact that the coroutine variant has an uninhabited ABI in the first place The prefix layout already uses Ty::new_maybe_uninit. The same should presumably |
Can it? So far we haven't even be able to turn this into UB, let alone a miscompilation. Causing UB would require a generator to yield when it is in one of these variants that is considered uninhabited. Is that possible? |
Introduce a local that is alive across yield, but make the initialization conditional (the fact that a prefix part is handled correctly also complicates the situation somewhat). $ rustc --edition=2021 a.rs && ./a
Illegal instructiona.rs#![feature(never_type)]
#![feature(noop_waker)]
#![feature(rustc_attrs)]
use std::pin::Pin;
use std::future::Future;
use std::mem::ManuallyDrop;
use std::task::*;
pub async fn f<T>(x: bool, y: bool) {
if y {
let b : ManuallyDrop<T>;
let c : *const ManuallyDrop<T>;
if x {
b = make::<ManuallyDrop<T>>();
c = &b;
}
Wait.await;
} else {
let b : ManuallyDrop<T>;
let c : *const ManuallyDrop<T>;
if x {
b = make::<ManuallyDrop<T>>();
c = &b;
}
Wait.await;
}
}
pub struct Wait;
impl Future for Wait {
type Output = ();
fn poll(self: Pin<&mut Self>, _: &mut Context<'_>) -> Poll<()> {
Poll::Pending
}
}
pub fn make<T>() -> T {
loop {}
}
fn main() {
let waker = std::task::Waker::noop();
let mut ctx = std::task::Context::from_waker(&waker);
let f = std::pin::pin!(f::<!>(false, true));
let _ = f.poll(&mut ctx);
} |
tmiasko
added
I-unsound
rustbot
added
the
I-prioritize
|
WG-prioritization assigning priority (Zulip discussion). @rustbot label -I-prioritize +P-high |
rustbot
added
P-high
|
The incorrect layout will be fixed by #118871. |
Wow, good catch! @saethlin, what does Miri with your PR do on that example? I wonder where the |
It is an llvm.trap generated for SetDiscriminant when switching to an uninhabited variant. |
|
Ah, I see. That's good then, Miri also explicitly checks that and triggers UB. |
|
With the assertion removed indeed, we get this: |
|
Great. :) We should definitely have this as a testcase once #118871 lands. I assume the ICE will also be gone then. We could still decide to remove the check in Miri, since arguably the example I gave here should work -- but we can also keep it for now since after all it has found a real bug... |
saethlin
added a commit
to saethlin/rust
that referenced
this issue
Dec 14, 2023
|
With #118871 having landed, the example no longer ICEs. |
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
Dec 14, 2023
…thlin interpret: extend comment on the inhabitedness check in downcast Cc rust-lang#115145 r? `@saethlin`
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Dec 14, 2023
…thlin interpret: extend comment on the inhabitedness check in downcast Cc rust-lang#115145 r? ``@saethlin``
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Dec 14, 2023
Rollup merge of rust-lang#118935 - RalfJung:interpret-downcast, r=saethlin interpret: extend comment on the inhabitedness check in downcast Cc rust-lang#115145 r? ``@saethlin``
|
ICE is fixed and regression tests have landed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reduced example: #115145 (comment)
https://github.com/axodotdev/axoasset at
10e2f44ee1bccaa5ae0dca28ee5061af04c0ae8dError output
Backtrace
searched nightlies: from nightly-2023-01-01 to nightly-2023-08-14
regressed nightly: nightly-2023-07-26
searched commit range: 31395ec...864bdf7
regressed commit: 4fc6b33
bisected with cargo-bisect-rustc v0.6.6
Host triple: x86_64-unknown-linux-gnu
Reproduce with:
The text was updated successfully, but these errors were encountered: