evaluation panics on layout computation cycle

[matthiaskrgr]

Should miri be able to bail out more gracefully in this scenario?

use std::mem;

pub struct S<T: Tr> {
    pub f: <T as Tr>::I,
}

pub trait Tr {
   type I: Tr;
}

impl<T: Tr> Tr for S<T> {
    type I = S<S<T>>;
}

impl Tr for () {
    type I = ();
}

fn foo<T: Tr>() -> usize {
    mem::size_of::<S<T>>()
}

fn main() {
    println!("{}", foo::<S<()>>());
}

rustc:

error[E0391]: cycle detected when computing layout of `S<S<()>>`
  |
  = note: ...which requires computing layout of `<S<()> as Tr>::I`...
  = note: ...which again requires computing layout of `S<S<()>>`, completing the cycle

For more information about this error, try `rustc --explain E0391`.

miri:

error[E0391]: cycle detected when computing layout of `S<S<()>>`
  |
  = note: ...which requires computing layout of `<S<()> as Tr>::I`...
  = note: ...which again requires computing layout of `S<S<()>>`, completing the cycle


Miri caused an ICE during evaluation. Here's the interpreter backtrace at the time of the panic:
note: the place in the program where the ICE was triggered
   --> /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:313:5
    |
313 |     intrinsics::size_of::<T>()
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: inside `std::mem::size_of::<S<S<()>>>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:313:5: 313:31
note: inside `foo::<S<()>>`
   --> src/main.rs:20:5
    |
20  |     mem::size_of::<S<T>>()
    |     ^^^^^^^^^^^^^^^^^^^^^^
note: inside `main`
   --> src/main.rs:24:20
    |
24  |     println!("{}", foo::<S<()>>());
    |                    ^^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5: 250:71
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:134:18: 134:21
    = note: inside closure at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:166:18: 166:82
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:284:13: 284:31
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:485:40: 485:43
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:449:19: 449:81
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
    = note: inside closure at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:148:48: 148:73
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:485:40: 485:43
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:449:19: 449:81
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:140:14: 140:33
    = note: inside `std::rt::lang_start_internal` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:148:20: 148:98
    = note: inside `std::rt::lang_start::<()>` at /home/matthias/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:165:17: 170:6

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.

[RalfJung]

Seems like a duplicate of #2608 -- we should just stop after that error message and not go on interpreting.

[oli-obk]

It's actually a different issue, as this error occurs during interpretation, not before, so we need to check on every step (or properly bubble out this error to the interpreter)

[RalfJung]

We already propagate layout errors though, shouldn't that subsume this case?

@matthiaskrgr I can't see the actual ICE message in the output above, is there some other output that you cut from the issue?

[oli-obk]

We already propagate layout errors though, shouldn't that subsume this case?

This is a cycle error, which emits an error and then should abort, but somehow miri subverts this abort. I am investigating how to bubble up the "a cycle occurred" from the layout error

[RalfJung]

somehow miri subverts this abort

Oh I see. Might be worth figuring out how to avoid this (as an alternative to fundamentally changing the cycle error treatment).

[oli-obk]

If we want to generally avoid miri reporting fatal errors as ICEs, we need to stop catching unwinding of a Fatal error. I don't know if this is generally doable, but we could match on the panic message in the worst case.

Alternatively we can wait for more reports and look into removing those fatal errors

[RalfJung]

Yeah I didn't realize that there are cases where rustc is expected to unwind.

Using a different unwinding payload type for ICEs vs regular fatal errors is not an option, or is it?

[oli-obk]

Using a different unwinding payload type for ICEs vs regular fatal errors is not an option, or is it?

I think that already happens but I am not sure. I will investigate. But that's kinda what I mean. Even if it is possible, do we want to?

[bjorn3]

Looks like we use FatalError for fatal errors and ExplicitBug or DelayedBugPanic for ICE.

[RalfJung]

If we can check the payload type before printing the Miri ICE message, that seems like a clean solution?

[RalfJung]

This now prints a much nicer error, without an ICE:

error[E0391]: cycle detected when computing layout of `S<S<()>>`
  |
  = note: ...which requires computing layout of `<S<()> as Tr>::I`...
  = note: ...which again requires computing layout of `S<S<()>>`, completing the cycle
  = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: post-monomorphization error: a cycle occurred during layout computation
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:313:5
    |
313 |     intrinsics::size_of::<T>()
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ a cycle occurred during layout computation
    |
    = note: BACKTRACE:
    = note: inside `std::mem::size_of::<S<S<()>>>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:313:5: 313:31
note: inside `foo::<S<()>>`
   --> src/main.rs:20:5
    |
20  |     mem::size_of::<S<T>>()
    |     ^^^^^^^^^^^^^^^^^^^^^^
note: inside `main`
   --> src/main.rs:24:20
    |
24  |     println!("{}", foo::<S<()>>());
    |                    ^^^^^^^^^^^^^^