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

ICE when calling mem::uninitialized for an enum with 256 variants. #1456

Closed
lcnr opened this issue Jun 22, 2020 · 21 comments · Fixed by rust-lang/rust#74059
Closed

ICE when calling mem::uninitialized for an enum with 256 variants. #1456

lcnr opened this issue Jun 22, 2020 · 21 comments · Fixed by rust-lang/rust#74059
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE

Comments

@lcnr
Copy link
Contributor

lcnr commented Jun 22, 2020

Running MIRI on the following code causes an ICE:

error: internal compiler error: /rustc/a8cf3991177f30694200002cd9479ffbbe6d9a1a/src/librustc_middle/macros.rs:7:9: Unexpected error during validation: using uninitialized data, but this operation requires initialized memory

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:916:9
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:78
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:59
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1076
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1537
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:62
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:49
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:217
  10: rustc_driver::report_ice
  11: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:477
  12: std::panicking::begin_panic
  13: rustc_errors::HandlerInner::bug
  14: rustc_errors::Handler::bug
  15: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
  16: rustc_middle::ty::context::tls::with_opt::{{closure}}
  17: rustc_middle::ty::context::tls::with_opt
  18: rustc_middle::util::bug::opt_span_bug_fmt
  19: rustc_middle::util::bug::bug_fmt
  20: rustc_mir::interpret::validity::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::validate_operand_internal
  21: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_rvalue_into_place
  22: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::statement
  23: miri::eval::eval_main
  24: rustc_middle::ty::context::tls::enter_global
  25: <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis
  26: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  27: rustc_span::with_source_map
  28: rustc_interface::interface::run_compiler_in_existing_thread_pool
  29: scoped_tls::ScopedKey<T>::set

code:

#![allow(unused)]

enum A {
    A0,
    A1,
    A2,
    A3,
    A4,
    A5,
    A6,
    A7,
    A8,
    A9,
    A10,
    A11,
    A12,
    A13,
    A14,
    A15,
    A16,
    A17,
    A18,
    A19,
    A20,
    A21,
    A22,
    A23,
    A24,
    A25,
    A26,
    A27,
    A28,
    A29,
    A30,
    A31,
    A32,
    A33,
    A34,
    A35,
    A36,
    A37,
    A38,
    A39,
    A40,
    A41,
    A42,
    A43,
    A44,
    A45,
    A46,
    A47,
    A48,
    A49,
    A50,
    A51,
    A52,
    A53,
    A54,
    A55,
    A56,
    A57,
    A58,
    A59,
    A60,
    A61,
    A62,
    A63,
    A64,
    A65,
    A66,
    A67,
    A68,
    A69,
    A70,
    A71,
    A72,
    A73,
    A74,
    A75,
    A76,
    A77,
    A78,
    A79,
    A80,
    A81,
    A82,
    A83,
    A84,
    A85,
    A86,
    A87,
    A88,
    A89,
    A90,
    A91,
    A92,
    A93,
    A94,
    A95,
    A96,
    A97,
    A98,
    A99,
    A100,
    A101,
    A102,
    A103,
    A104,
    A105,
    A106,
    A107,
    A108,
    A109,
    A110,
    A111,
    A112,
    A113,
    A114,
    A115,
    A116,
    A117,
    A118,
    A119,
    A120,
    A121,
    A122,
    A123,
    A124,
    A125,
    A126,
    A127,
    A128,
    A129,
    A130,
    A131,
    A132,
    A133,
    A134,
    A135,
    A136,
    A137,
    A138,
    A139,
    A140,
    A141,
    A142,
    A143,
    A144,
    A145,
    A146,
    A147,
    A148,
    A149,
    A150,
    A151,
    A152,
    A153,
    A154,
    A155,
    A156,
    A157,
    A158,
    A159,
    A160,
    A161,
    A162,
    A163,
    A164,
    A165,
    A166,
    A167,
    A168,
    A169,
    A170,
    A171,
    A172,
    A173,
    A174,
    A175,
    A176,
    A177,
    A178,
    A179,
    A180,
    A181,
    A182,
    A183,
    A184,
    A185,
    A186,
    A187,
    A188,
    A189,
    A190,
    A191,
    A192,
    A193,
    A194,
    A195,
    A196,
    A197,
    A198,
    A199,
    A200,
    A201,
    A202,
    A203,
    A204,
    A205,
    A206,
    A207,
    A208,
    A209,
    A210,
    A211,
    A212,
    A213,
    A214,
    A215,
    A216,
    A217,
    A218,
    A219,
    A220,
    A221,
    A222,
    A223,
    A224,
    A225,
    A226,
    A227,
    A228,
    A229,
    A230,
    A231,
    A232,
    A233,
    A234,
    A235,
    A236,
    A237,
    A238,
    A239,
    A240,
    A241,
    A242,
    A243,
    A244,
    A245,
    A246,
    A247,
    A248,
    A249,
    A250,
    A251,
    A252,
    A253,
    A254,
    A255,
}

fn main() {
    let _: A = unsafe { std::mem::uninitialized() };
}
@RalfJung
Copy link
Member

Interesting, does it really take a 256-variant enum to trigger this? Good catch!

@RalfJung RalfJung added A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. labels Jun 22, 2020
@lcnr
Copy link
Contributor Author

lcnr commented Jun 22, 2020

jup, it probably causes a failure earlier in case the enum discriminant is not valid for all its possible values.

@lcnr
Copy link
Contributor Author

lcnr commented Jun 22, 2020

with 255 variants:

error: abnormal termination: the evaluated program aborted execution: attempted to leave type `A` uninitialized, which is invalid
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/mem/mod.rs:657:5
    |
657 |     intrinsics::assert_uninit_valid::<T>();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the evaluated program aborted execution: attempted to leave type `A` uninitialized, which is invalid
    |
    = note: inside `std::mem::uninitialized::<A>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/mem/mod.rs:657:5
note: inside `main` at src/main.rs:262:25
   --> src/main.rs:262:25
    |
262 |     let _: A = unsafe { std::mem::uninitialized() };
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:34
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:73
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@std::rt::lang_start_internal::{{closure}}#0::{{closure}}#0 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:130:5
    = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:52:13
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:296:40
    = note: inside `std::panicking::r#try::<i32, [closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panicking.rs:273:15
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{{closure}}#0 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/panic.rs:394:14
    = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/rt.rs:67:5

@RalfJung
Copy link
Member

Ah yes, our scalar bounds verification code actually special-cases "all values valid" to not complain about code until we have a decision in rust-lang/unsafe-code-guidelines#71, so with this big enum you end up causing an uninit read later when determining the discriminant.

it is probably possible to cause this in CTFE from rustc (and it needs a fix in rustc).

@RalfJung
Copy link
Member

That is strange, why does this not error?

@RalfJung
Copy link
Member

Possible it ends up in this branch but I did not think we would ever get there for enums...

@RalfJung
Copy link
Member

Ah, I think we are traversing the enum's fields before we read the discriminant, and the field actually has appropriate integer type and as such CTFE doesn't allow it to remain uninitialized. So this is indeed Miri-only.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 22, 2020

That is strange, why does this not error?

it does error in validation, did you expect an earlier error?

@RalfJung
Copy link
Member

I expected the same ICE as in the OP. But I figured out why it does not happen.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 22, 2020

So... did you mean CTFE does allow it to remain uninitialized?

Isn't the difference between the reported issue and the CTFE repro just that ctfe doesn't validate locals at all?

@RalfJung
Copy link
Member

RalfJung commented Jun 22, 2020

CTFE validation of the final value shows a proper error instead of an ICE. I wanted to understand why.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 22, 2020

Ah, that part I can help with: https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/validity.rs#L497-L504

we explicitly ignore uninitialized integers, so when you read the tag and validate it (which is of integer type), you get a validation error, but miri doesn't, so it continues and does the actual variant validation. To do that, it needs to know the discriminant, at which point the discriminant computation from the tag fails because the tag is uninitialized.

which... is basically what you wrote above, I just didn't grok it :D

@RalfJung RalfJung added the I-ICE Impact: makes Miri crash with some ICE label Jun 24, 2020
@niluxv
Copy link
Contributor

niluxv commented Jul 3, 2020

I'm getting this error in a rustbreak with serde_yaml serialiser/deserialiser (and not with any other serialiser/deserialiser).
I could find a mem::uninitialized call here in linked-hash-map, but I don't think there is such a large enum involved in this case.

log
error: internal compiler error: /rustc/3503f565e1fb7296983757d2716346f48a4a262b/src/librustc_middle/macros.rs:7:9: Unexpected error during validation: using uninitialized data, but this operation requires initialized memory

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:916:9
stack backtrace:
   0:     0x7fd25522af05 - backtrace::backtrace::libunwind::trace::h34afbfad7fd770fc
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/libunwind.rs:86
   1:     0x7fd25522af05 - backtrace::backtrace::trace_unsynchronized::h460d522b1619a600
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.46/src/backtrace/mod.rs:66
   2:     0x7fd25522af05 - std::sys_common::backtrace::_print_fmt::ha45fac10086813b4
                               at src/libstd/sys_common/backtrace.rs:78
   3:     0x7fd25522af05 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hde84f63fcfd0e6de
                               at src/libstd/sys_common/backtrace.rs:59
   4:     0x7fd25526849c - core::fmt::write::h540ac4a6a1232abc
                               at src/libcore/fmt/mod.rs:1076
   5:     0x7fd25521cd72 - std::io::Write::write_fmt::hc344eafd6e850b4d
                               at src/libstd/io/mod.rs:1537
   6:     0x7fd25522fe30 - std::sys_common::backtrace::_print::h4db88ff15cb5d61d
                               at src/libstd/sys_common/backtrace.rs:62
   7:     0x7fd25522fe30 - std::sys_common::backtrace::print::h5fc39e1b1f610bd3
                               at src/libstd/sys_common/backtrace.rs:49
   8:     0x7fd25522fe30 - std::panicking::default_hook::{{closure}}::h59e55edacb1d974a
                               at src/libstd/panicking.rs:198
   9:     0x7fd25522fb7c - std::panicking::default_hook::heee4c8016dfbf328
                               at src/libstd/panicking.rs:217
  10:     0x7fd25599ba73 - rustc_driver::report_ice::h26eb523a40d729cf
  11:     0x7fd2552305a8 - std::panicking::rust_panic_with_hook::h8405b6301c79fb5a
                               at src/libstd/panicking.rs:524
  12:     0x7fd258541f43 - std::panicking::begin_panic::hbb5ec578e87d718c
  13:     0x7fd258578cb0 - rustc_errors::HandlerInner::bug::h0a156bac90c3bedd
  14:     0x7fd258577740 - rustc_errors::Handler::bug::hc7c77e7d63267087
  15:     0x7fd25810b564 - rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}::h2f54a27b24ae47f3
  16:     0x7fd258109e0b - rustc_middle::ty::context::tls::with_opt::{{closure}}::hfa05e53a82c505b4
  17:     0x7fd258109db2 - rustc_middle::ty::context::tls::with_opt::hf26ab64f8d428088
  18:     0x7fd25810b489 - rustc_middle::util::bug::opt_span_bug_fmt::h22a68da700c88d22
  19:     0x7fd25810b3fe - rustc_middle::util::bug::bug_fmt::h53f5cef7d1dc611d
  20:     0x5654d28f97c4 - rustc_mir::interpret::validity::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::validate_operand_internal::h3a5b6a03cb41d02b
  21:     0x5654d28de28f - rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::eval_rvalue_into_place::h82259394daaa6110
  22:     0x5654d28e08db - rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpCx<M>>::statement::h95b81d83b2af38c3
  23:     0x5654d282a75a - miri::eval::eval_main::h88dce33b8235b1f4
  24:     0x5654d26f5f5b - rustc_middle::ty::context::tls::enter_global::hf0012a6dc5ad9128
  25:     0x5654d26fe6be - <miri::MiriCompilerCalls as rustc_driver::Callbacks>::after_analysis::h3d61f5ec33a70dde
  26:     0x7fd255956430 - rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter::hb3ecfe94ca03e6c6
  27:     0x7fd2559f6343 - rustc_span::with_source_map::h21eed361cfe861de
  28:     0x7fd255957b16 - rustc_interface::interface::run_compiler_in_existing_thread_pool::h902d0e3e1f552acf
  29:     0x7fd2559811bd - scoped_tls::ScopedKey<T>::set::hbc88e7d76dd9a901
  30:     0x7fd2559a781a - std::sys_common::backtrace::__rust_begin_short_backtrace::h0f0c30d1296961bc
  31:     0x7fd2559645be - core::ops::function::FnOnce::call_once{{vtable.shim}}::h36b2cf3218c61088
  32:     0x7fd25523fdaa - <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::h13d34828db364579
                               at /rustc/3503f565e1fb7296983757d2716346f48a4a262b/src/liballoc/boxed.rs:1081
  33:     0x7fd25523fdaa - <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once::hd51b619e0f884abf
                               at /rustc/3503f565e1fb7296983757d2716346f48a4a262b/src/liballoc/boxed.rs:1081
  34:     0x7fd25523fdaa - std::sys::unix::thread::Thread::new::thread_start::h02c6e34c2c73f344
                               at src/libstd/sys/unix/thread.rs:87
  35:     0x7fd255179609 - start_thread
                               at /build/glibc-YYA7BZ/glibc-2.31/nptl/pthread_create.c:477
  36:     0x7fd255085103 - __clone
  37:                0x0 - <unknown>

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.46.0-nightly (3503f565e 2020-07-02) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
end of query stack
error: aborting due to previous error

test mem_yaml ... 
error: could not compile `rustbreak`.

Hope it helps

@RalfJung
Copy link
Member

RalfJung commented Jul 4, 2020

@niluxv If there is no such big enum it is likely a different ICE... are you able to reduce this to a self-contained example?

If not, I hope next week-end I can get back to this problem; once it is fixed you can see if that also fixes your problem.

@niluxv
Copy link
Contributor

niluxv commented Jul 4, 2020

The errors and backtraces seem to be the same... But I would be surprised if enums with 256 variants are used.

I tried to pinpoint it a bit and it is indeed caused by linked-hash-map. Their test test_ser_de triggers this ICE in the assert_tokens call (inside assert_de_tokens around the deserialize_in_place part).

@niluxv
Copy link
Contributor

niluxv commented Jul 4, 2020

Smallest example I could get
use linked_hash_map::LinkedHashMap;
use serde::Derserialize;
use serde_test::Token;
use serde_test::de::Deserializer; // Note this is privat

#[test]
fn test_ser_de_simple() {
    assert_de_tokens::<'_,LinkedHashMap<char, usize>>(&[
        Token::Map { len: Some(1) },
            Token::Char('b'),
            Token::I32(20),
        Token::MapEnd,
    ]);
}

fn assert_de_tokens<'de, T>(tokens: &'de [Token])
where
    T: Deserialize<'de>,
{
    let mut de = de::Deserializer::new(tokens);
    let mut deserialized_val: T = T::deserialize(&mut de).unwrap();
    let mut de2 = de::Deserializer::new(tokens);
    let deserialized_val2: T = T::deserialize(&mut de2).unwrap();
    let reference = &mut deserialized_val;
    *reference = deserialized_val2; // ICE happens here
    println!("never reached");
}

@niluxv
Copy link
Contributor

niluxv commented Jul 5, 2020

Even simpler
use linked_hash_map::LinkedHashMap;

#[test]
fn test_ser_de_simpler() {
    let mut val = LinkedHashMap::new();
    val.insert('a', 0);
    let mut val2 = LinkedHashMap::new();
    val2.insert('a', 0);
    let reference = &mut val;
    *reference = val2; // ICE is triggered here
    println!("never reached");
}

The drop implementation of LinkedHashMap is required to trigger the ICE. Also when the char key is changed to a numeric key, the ICE isn't triggered anymore.

@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2020

Btw, do you know if this version of linked-hash-map is with or without contain-rs/linked-hash-map#100?

@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2020

@niluxv I think your bug is different, and probably should be tracked separately.
The ICE occurs in this line when loading the char from memory.

However, uninitialized char do not cause an ICE elsewhere, so I am not sure why this is happening.

@niluxv
Copy link
Contributor

niluxv commented Jul 5, 2020

Btw, do you know if this version of linked-hash-map is with or without contain-rs/linked-hash-map#100?

I used the master branch so it includes this patch.

@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2020

rust-lang/rust#74059 should fix this.

Manishearth added a commit to Manishearth/rust that referenced this issue Jul 6, 2020
…oli-obk

Miri value validation: fix handling of uninit memory

Fixes rust-lang/miri#1456
Fixes rust-lang/miri#1467

r? @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-ICE Impact: makes Miri crash with some ICE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants