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

enums with repr(align) generate Scalar layout with padding (ICE: miri: field access on non aggregate <uninitialized> ) #96185

Closed
matthiaskrgr opened this issue Apr 18, 2022 · 12 comments · Fixed by #96814
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-miri Area: The miri tool C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Apr 18, 2022

Code

run this in miri to reproduce the ice:

#[allow(dead_code)]
#[repr(align(8))]
enum Aligned {
    Zero = 0,
    One = 1,
}

fn main() {
    let aligned = Aligned::Zero;
    assert_eq!(aligned as u8, 0);
}

Meta

rustc --version --verbose:

rustc 1.62.0-nightly (ec77f2524 2022-04-17)
binary: rustc
commit-hash: ec77f252434a532fdb5699ae4f21a3072d211edd
commit-date: 2022-04-17
host: x86_64-unknown-linux-gnu
release: 1.62.0-nightly
LLVM version: 14.0.0

Error output

error: internal compiler error: /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/compiler/rustc_const_eval/src/interpret/operand.rs:385:39: field access on non aggregate <uninitialized>, TyAndLayout {
                                    ty: Aligned,
                                    layout: Layout {
                                        fields: Arbitrary {
                                            offsets: [
                                                Size {
                                                    raw: 0,
                                                },
                                            ],
                                            memory_index: [
                                                0,
                                            ],
                                        },
                                        variants: Multiple {
                                            tag: Initialized {
                                                value: Int(
                                                    I8,
                                                    false,
                                                ),
                                                valid_range: 0..=1,
                                            },
                                            tag_encoding: Direct,
                                            tag_field: 0,
                                            variants: [
                                                Layout {
                                                    fields: Arbitrary {
                                                        offsets: [],
                                                        memory_index: [],
                                                    },
                                                    variants: Single {
                                                        index: 0,
                                                    },
                                                    abi: Aggregate {
                                                        sized: true,
                                                    },
                                                    largest_niche: None,
                                                    align: AbiAndPrefAlign {
                                                        abi: Align {
                                                            pow2: 3,
                                                        },
                                                        pref: Align {
                                                            pow2: 3,
                                                        },
                                                    },
                                                    size: Size {
                                                        raw: 8,
                                                    },
                                                },
                                                Layout {
                                                    fields: Arbitrary {
                                                        offsets: [],
                                                        memory_index: [],
                                                    },
                                                    variants: Single {
                                                        index: 1,
                                                    },
                                                    abi: Aggregate {
                                                        sized: true,
                                                    },
                                                    largest_niche: None,
                                                    align: AbiAndPrefAlign {
                                                        abi: Align {
                                                            pow2: 3,
                                                        },
                                                        pref: Align {
                                                            pow2: 3,
                                                        },
                                                    },
                                                    size: Size {
                                                        raw: 8,
                                                    },
                                                },
                                            ],
                                        },
                                        abi: Scalar(
                                            Initialized {
                                                value: Int(
                                                    I8,
                                                    false,
                                                ),
                                                valid_range: 0..=1,
                                            },
                                        ),
                                        largest_niche: Some(
                                            Niche {
                                                offset: Size {
                                                    raw: 0,
                                                },
                                                value: Int(
                                                    I8,
                                                    false,
                                                ),
                                                valid_range: 0..=1,
                                            },
                                        ),
                                        align: AbiAndPrefAlign {
                                            abi: Align {
                                                pow2: 3,
                                            },
                                            pref: Align {
                                                pow2: 3,
                                            },
                                        },
                                        size: Size {
                                            raw: 8,
                                        },
                                    },
                                }
  --> src/main.rs:10:20
   |
10 |     println!("{}", aligned as u8);
   |                    ^^^^^^^
Backtrace

thread 'rustc' panicked at 'Box<dyn Any>', /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/compiler/rustc_errors/src/lib.rs:1289:9
stack backtrace:
   0:     0x7f76b349d83d - std::backtrace_rs::backtrace::libunwind::trace::hdc1f21fb71c845ee
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x7f76b349d83d - std::backtrace_rs::backtrace::trace_unsynchronized::h825695a1eac7e4dc
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x7f76b349d83d - std::sys_common::backtrace::_print_fmt::h201cc1fed0ce4598
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/sys_common/backtrace.rs:66:5
   3:     0x7f76b349d83d - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h4c86a2fdb08a33f9
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/sys_common/backtrace.rs:45:22
   4:     0x7f76b34f8f1c - core::fmt::write::hee99b3f0c63f5ca9
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/core/src/fmt/mod.rs:1194:17
   5:     0x7f76b348f031 - std::io::Write::write_fmt::hfa56cdeea03f4b89
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/io/mod.rs:1655:15
   6:     0x7f76b34a0555 - std::sys_common::backtrace::_print::hbd75018aef718598
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/sys_common/backtrace.rs:48:5
   7:     0x7f76b34a0555 - std::sys_common::backtrace::print::hc594da4af1e8ead7
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/sys_common/backtrace.rs:35:9
   8:     0x7f76b34a0555 - std::panicking::default_hook::{{closure}}::h0602100d469ef01e
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/panicking.rs:295:22
   9:     0x7f76b34a01c9 - std::panicking::default_hook::hc7ef6a8909090411
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/panicking.rs:314:9
  10:     0x7f76b3cd0521 - rustc_driver[8e3c698622bece67]::DEFAULT_HOOK::{closure#0}::{closure#0}
  11:     0x7f76b34a0d26 - std::panicking::rust_panic_with_hook::hb9245fe7222caa6e
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/panicking.rs:702:17
  12:     0x55a6325ed2d3 - std[7eb92d5d5059ff02]::panicking::begin_panic::<rustc_errors[dbef8fecc23abfce]::ExplicitBug>::{closure#0}
  13:     0x55a6325ecbc6 - std[7eb92d5d5059ff02]::sys_common::backtrace::__rust_end_short_backtrace::<std[7eb92d5d5059ff02]::panicking::begin_panic<rustc_errors[dbef8fecc23abfce]::ExplicitBug>::{closure#0}, !>
  14:     0x55a63245f68f - std[7eb92d5d5059ff02]::panicking::begin_panic::<rustc_errors[dbef8fecc23abfce]::ExplicitBug>
  15:     0x55a6325dd1c6 - std[7eb92d5d5059ff02]::panic::panic_any::<rustc_errors[dbef8fecc23abfce]::ExplicitBug>
  16:     0x55a6325dcf93 - <rustc_errors[dbef8fecc23abfce]::HandlerInner>::span_bug::<rustc_span[e15e523c5f5ffbde]::span_encoding::Span, &alloc[8d468ea801fd4f9]::string::String>
  17:     0x55a6325dcc80 - <rustc_errors[dbef8fecc23abfce]::Handler>::span_bug::<rustc_span[e15e523c5f5ffbde]::span_encoding::Span, &alloc[8d468ea801fd4f9]::string::String>
  18:     0x55a632519f0e - rustc_middle[c02e249562e6ab88]::ty::context::tls::with_opt::<rustc_middle[c02e249562e6ab88]::util::bug::opt_span_bug_fmt<rustc_span[e15e523c5f5ffbde]::span_encoding::Span>::{closure#0}, ()>
  19:     0x55a632519c69 - rustc_middle[c02e249562e6ab88]::util::bug::opt_span_bug_fmt::<rustc_span[e15e523c5f5ffbde]::span_encoding::Span>
  20:     0x55a6324606d3 - rustc_middle[c02e249562e6ab88]::util::bug::span_bug_fmt::<rustc_span[e15e523c5f5ffbde]::span_encoding::Span>
  21:     0x55a63255b9b9 - <rustc_const_eval[afe50d724bd7fc79]::interpret::eval_context::InterpCx<miri[ada871e22c039dff]::machine::Evaluator>>::operand_field
  22:     0x55a6325f43f5 - <alloc[8d468ea801fd4f9]::vec::Vec<core[f08f6a517a2b1407]::result::Result<rustc_const_eval[afe50d724bd7fc79]::interpret::operand::OpTy<miri[ada871e22c039dff]::machine::Tag>, rustc_middle[c02e249562e6ab88]::mir::interpret::error::InterpErrorInfo>> as alloc[8d468ea801fd4f9]::vec::spec_from_iter::SpecFromIter<core[f08f6a517a2b1407]::result::Result<rustc_const_eval[afe50d724bd7fc79]::interpret::operand::OpTy<miri[ada871e22c039dff]::machine::Tag>, rustc_middle[c02e249562e6ab88]::mir::interpret::error::InterpErrorInfo>, core[f08f6a517a2b1407]::iter::adapters::map::Map<core[f08f6a517a2b1407]::ops::range::Range<usize>, <rustc_const_eval[afe50d724bd7fc79]::interpret::validity::ValidityVisitor<miri[ada871e22c039dff]::machine::Evaluator> as rustc_const_eval[afe50d724bd7fc79]::interpret::visitor::ValueVisitor<miri[ada871e22c039dff]::machine::Evaluator>>::walk_value::{closure#0}>>>::from_iter
  23:     0x55a632605ba5 - <rustc_const_eval[afe50d724bd7fc79]::interpret::validity::ValidityVisitor<miri[ada871e22c039dff]::machine::Evaluator> as rustc_const_eval[afe50d724bd7fc79]::interpret::visitor::ValueVisitor<miri[ada871e22c039dff]::machine::Evaluator>>::walk_value
  24:     0x55a63254ee1f - <rustc_const_eval[afe50d724bd7fc79]::interpret::eval_context::InterpCx<miri[ada871e22c039dff]::machine::Evaluator>>::validate_operand_internal
  25:     0x55a63254ca54 - <rustc_const_eval[afe50d724bd7fc79]::interpret::eval_context::InterpCx<miri[ada871e22c039dff]::machine::Evaluator>>::statement
  26:     0x55a6324f745b - miri[ada871e22c039dff]::eval::eval_entry
  27:     0x55a63246aa22 - <rustc_interface[b4fe925d1e8786a0]::passes::QueryContext>::enter::<<miri[e50af0efed857ec5]::MiriCompilerCalls as rustc_driver[8e3c698622bece67]::Callbacks>::after_analysis::{closure#0}, ()>
  28:     0x55a63246358e - <miri[e50af0efed857ec5]::MiriCompilerCalls as rustc_driver[8e3c698622bece67]::Callbacks>::after_analysis
  29:     0x7f76b5d05db5 - <rustc_interface[b4fe925d1e8786a0]::interface::Compiler>::enter::<rustc_driver[8e3c698622bece67]::run_compiler::{closure#1}::{closure#2}, core[f08f6a517a2b1407]::result::Result<core[f08f6a517a2b1407]::option::Option<rustc_interface[b4fe925d1e8786a0]::queries::Linker>, rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>
  30:     0x7f76b5d2eeef - rustc_span[e15e523c5f5ffbde]::with_source_map::<core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>, rustc_interface[b4fe925d1e8786a0]::interface::create_compiler_and_run<core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>, rustc_driver[8e3c698622bece67]::run_compiler::{closure#1}>::{closure#1}>
  31:     0x7f76b5d069b7 - <scoped_tls[ed206aee8c9203a6]::ScopedKey<rustc_span[e15e523c5f5ffbde]::SessionGlobals>>::set::<rustc_interface[b4fe925d1e8786a0]::interface::run_compiler<core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>, rustc_driver[8e3c698622bece67]::run_compiler::{closure#1}>::{closure#0}, core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>
  32:     0x7f76b5d1bf0f - std[7eb92d5d5059ff02]::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface[b4fe925d1e8786a0]::util::run_in_thread_pool_with_globals<rustc_interface[b4fe925d1e8786a0]::interface::run_compiler<core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>, rustc_driver[8e3c698622bece67]::run_compiler::{closure#1}>::{closure#0}, core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>::{closure#0}, core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>
  33:     0x7f76b5d1c049 - <<std[7eb92d5d5059ff02]::thread::Builder>::spawn_unchecked_<rustc_interface[b4fe925d1e8786a0]::util::run_in_thread_pool_with_globals<rustc_interface[b4fe925d1e8786a0]::interface::run_compiler<core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>, rustc_driver[8e3c698622bece67]::run_compiler::{closure#1}>::{closure#0}, core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>::{closure#0}, core[f08f6a517a2b1407]::result::Result<(), rustc_errors[dbef8fecc23abfce]::ErrorGuaranteed>>::{closure#1} as core[f08f6a517a2b1407]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
  34:     0x7f76b34aabd3 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::hd2056160a42acd52
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/alloc/src/boxed.rs:1866:9
  35:     0x7f76b34aabd3 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h99c5b980edefbf69
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/alloc/src/boxed.rs:1866:9
  36:     0x7f76b34aabd3 - std::sys::unix::thread::Thread::new::thread_start::h42961cfa146716e3
                               at /rustc/ec77f252434a532fdb5699ae4f21a3072d211edd/library/std/src/sys/unix/thread.rs:108:17
  37:     0x7f76b32835c2 - start_thread
  38:     0x7f76b3308584 - __clone
  39:                0x0 - <unknown>

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.62.0-nightly (ec77f2524 2022-04-17) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type bin -C embed-bitcode=no -C debuginfo=2 -C incremental -C target-cpu=native

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

@matthiaskrgr matthiaskrgr added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Apr 18, 2022
@bjorn3 bjorn3 added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Apr 18, 2022
@RalfJung
Copy link
Member

Looks like it tries to access the discriminant (field 0 of Aligned), but then this case does not apply:

_ if offset.bytes() == 0 && field_layout.size == op.layout.size => *base,

The case does not apply because Aligned has size 8, but the field only has size 1. I didn't know Scalar types can have padding in them... in fact this is a type with Scalar layout where the size of the layout is bigger than the size of the scalar. This has me slightly worried about our ScalarPair code path which has less strict sanity checks. Is it possible to have a ScalarPair layout with one field where that one field covers both components of the pair and yes is smaller than the entire type? Miri (and CTFE) would go completely wrong on that.

@RalfJung
Copy link
Member

Oh, turns out codegen uses the same logic:

if field.size == self.layout.size =>

So I think a similar bug can somehow be triggered in codegen. I am just not sure how to make it call extract_field in the right way...

@RalfJung
Copy link
Member

RalfJung commented Apr 18, 2022

I am still missing something, the bug doesn't reproduce as often as it should... e.g., this one doesn't ICE

#[allow(dead_code)]
#[repr(align(8))]
enum Aligned {
    Zero = 0,
    One = 1,
}

fn main() {
    let aligned = Aligned::Zero;
    let _x = match aligned { Aligned::Zero => 0, Aligned::One => 1 };
}

but this one does

#[allow(dead_code)]
#[repr(align(8))]
enum Aligned {
    Zero = 0,
    One = 1,
}

fn id(a: Aligned) -> Aligned { a }

fn main() {
    let aligned = id(Aligned::Zero);
    let _x = match aligned { Aligned::Zero => 0, Aligned::One => 1 };
}

Also I noticed that this type

#[repr(align(8))]
struct Aligned(NonZeroU8);

has Aggregate ABI. @oli-obk do you know why struct lose their newtype Scalar layout optimizations with repr(align) but enum do not? Should we really give repr(align) enums a Scalar layout? Or is "Scalars do not have padding" part of the contract of layout computation?

@RalfJung
Copy link
Member

RalfJung commented Apr 19, 2022

FWIW, if I put this kind of sanity check

(OperandValue::Pair(a_llval, b_llval), Abi::ScalarPair(a, b)) => {

also into Miri, things explode spectacularly. This type from memchr

enum Shift {
    Small { period: usize },
    Large { shift: usize },
}

ends up with a ScalarPair immediate while having Aggregate ABI. I suspect this is because even when the enum has ScalarPair layout, its variant (after a downcast) can have Aggregate layout. That sounds like yet another bug in layout computation to me.

EDIT: Also, just fixing the logic in operand_field will not be enough. We'd still be loading an 8-byte scalar which is partially uninit (yielding an entirely uninit scalar in Miri) and then try to get an integer out of that, which would raise UB. The assumption that scalars do not have padding is laced throughout Miri.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 19, 2022

do you know why struct lose their newtype Scalar layout optimizations with repr(align) but enum do not?

Not really a why in the sense you're asking for, but the layout computation makes the struct decision right here:

if offsets[i].bytes() == 0 && align.abi == field.align.abi && size == field.size

For fieldless enums,

if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) {
is def missing an alignment check

@RalfJung
Copy link
Member

So this is just a bug in enum layout computation then, and we are okay with repr(align) enums losing their Scalar layout (and hence their niche)?

That would be a big relief. ;)

@RalfJung
Copy link
Member

ends up with a ScalarPair immediate while having Aggregate ABI. I suspect this is because even when the enum has ScalarPair layout, its variant (after a downcast) can have Aggregate layout. That sounds like yet another bug in layout computation to me.

I made that a separate issue: #96221.

So this issue now is about some enums violating the invariant that Scalar layout types do not have padding.

@RalfJung RalfJung changed the title ICE: miri: field access on non aggregate <uninitialized> enums with repr(align) generate Scalar layout with padding (ICE: miri: field access on non aggregate <uninitialized> ) Apr 19, 2022
@RalfJung
Copy link
Member

RalfJung commented May 7, 2022

@oli-obk

is def missing an alignment check

size here is already rounded up to the alignment, so this is not the place where the alignment check is missing.

I am not sure where is the right place though...

However, I also cannot see any code in the enum path that would take into account repr().align, so I am probably missing something.

@RalfJung
Copy link
Member

RalfJung commented May 7, 2022

Actually the problem is the fix (#92932) for #92464 (the same code as this problem^^): that introduced the || variants.iter().all(|layout| layout.is_empty()) branch, which uses scalar layout even when it should not be used.

@RalfJung
Copy link
Member

RalfJung commented May 7, 2022

This is quite interesting, and I am not sure what to do.
The problem is that

  • on the one hand, Aligned must have Aggregate layout since it has padding.
  • on the other hand, since this enum has explicit discriminants, parts of the compiler expect that they can read_immediate on it, which means it must have Scalar layout.

It is impossible to satisfy both of these constraints together. @oli-obk @eddyb any ideas for what to do here?

@RalfJung
Copy link
Member

RalfJung commented May 7, 2022

(Digging a bit deeper) it's not [just] about having explicit discriminants, but rather that enums can be the input to a cast and cast values are expected to be represented as immediates.

Allowing repr(aligned) on enums looks like a mistake... (we disallow repr(packed), FWIW)

RalfJung added a commit to RalfJung/rust that referenced this issue May 10, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue May 11, 2022
tighten sanity checks around Scalar and ScalarPair

While investigating rust-lang#96185 I noticed codegen has tighter sanity checks here than Miri does, so I added some more assertions. Strangely, some of them fail, so I also needed to add a HACK... that is probably worth looking into.

This does not fix that issue, but it changes the ICE messages, making it quite clear that we have a scalar whose size is not the same as that of the surrounding layout.

r? `@oli-obk`
@RalfJung
Copy link
Member

FWIW, here's basically the same bug during CTFE, though for some reason it doesn't cause an ICE:

#[allow(dead_code)]
#[repr(align(8))]
enum Aligned {
    Zero = 0,
    One = 1,
}

const X: () = {
    let aligned = Aligned::Zero;
    let _val = &(aligned as u8);
};

So we can have this as a testcase in rustc.

@bors bors closed this as completed in 3e802d7 Jul 6, 2022
bors added a commit to rust-lang/miri that referenced this issue Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-miri Area: The miri tool C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ 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.

4 participants