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 combining const (fn) and unions #47788

Closed
jonas-schievink opened this issue Jan 26, 2018 · 2 comments
Closed

ICE when combining const (fn) and unions #47788

jonas-schievink opened this issue Jan 26, 2018 · 2 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) 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

@jonas-schievink
Copy link
Contributor

This code posted by @sfleischman105 in #24111 currently compiles:

#![feature(const_fn)]

use std::time::Instant;

pub struct GlobalTimer {
    time: Instant,
}

union DummyUnion {
    time: Instant,
    junk: u32
}

impl GlobalTimer {
    pub const fn init() -> GlobalTimer {
        const DUMMY: DummyUnion = DummyUnion {
            junk: 0
        };
        
        const UNINIT: Instant = unsafe {
            DUMMY.time
        };
        
        // Ta-da!
        GlobalTimer {
            time: UNINIT,
        }
    }
}

fn main() {
    //GlobalTimer::init();
}

With this suspicious (and wrong?) warning:

warning: constant evaluation error: nonexistent struct field
  --> src/main.rs:20:33
   |
20 |           const UNINIT: Instant = unsafe {
   |  _________________________________^
21 | |             DUMMY.time
22 | |         };
   | |_________^
   |
   = note: #[warn(const_err)] on by default

However, trying to use the const fn by uncommenting the line in main causes an ICE:

error: internal compiler error: librustc_trans/type_of.rs:386: TyLayout::llvm_field_index(TyLayout { ty: DummyUnion, details: LayoutDetails { variants: Single { index: 0 }, fields: Union(2), abi: Aggregate { sized: true }, align: Align { abi: 3, pref: 3 }, size: Size { raw: 16 } } }): not applicable
@etaoins
Copy link
Contributor

etaoins commented Jan 26, 2018

The ICE can be reduced to:

union DummyUnion {
    field1: u32,
    field2: u32,
}

pub fn init() -> u32 {
    const UNION: DummyUnion = DummyUnion { field1: 0 };
    const FIELD: u32 = unsafe { UNION.field1 };
    FIELD
}

fn main() {
    init();
}

Note that this does not require const_fn. This also looks like a regression from stable to nightly.

@pietroalbini pietroalbini 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. A-const-fn C-bug Category: This is a bug. labels Jan 26, 2018
@etaoins
Copy link
Contributor

etaoins commented Jan 27, 2018

The non-existent field warning can be reduced to:

union DummyUnion {
    field1: i32,
    field2: i32,
}

const UNION: DummyUnion = DummyUnion { field1: 1 };

fn main() {
    assert_eq!(unsafe { UNION.field2 }, 1);
}

It looks like referencing any field except the first will trigger the warning.

bors added a commit that referenced this issue Jan 28, 2018
…r=eddyb

Fix ICE on const eval of union field

MIR's `Const::get_field()` attempts to retrieve the value for a given field in a constant. In the case of a union constant it was falling through to a generic `const_get_elt` based on the field index. As union fields don't have an index this caused an ICE in `llvm_field_index`.

Fix by simply returning the current value when accessing any field in a union. This works because all union fields start at byte offset 0.

The added test uses `const_fn` it ensure the field is extracted using MIR's const evaluation. The crash is reproducible without it, however.

Fixes #47788

r? @eddyb
MaloJaffre pushed a commit to MaloJaffre/rust that referenced this issue Feb 1, 2018
MIR's `Const::get_field()` attempts to retrieve the value for a given
field in a constant. In the case of a union constant it was falling
through to a generic `const_get_elt` based on the field index. As union
fields don't have an index this caused an ICE in `llvm_field_index`.

Fix by simply returning the current value when accessing any field in a
union. This works because all union fields start at byte offset 0.

The added test uses `const_fn` it ensure the field is extracted using
MIR's const evaluation. The crash is reproducible without it, however.

Fixes rust-lang#47788
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
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, ...) 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

No branches or pull requests

4 participants