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

Regression in Servo: "internal compiler error: mutable allocation in constant" #67601

Closed
SimonSapin opened this issue Dec 25, 2019 · 10 comments · Fixed by #67603
Closed

Regression in Servo: "internal compiler error: mutable allocation in constant" #67601

SimonSapin opened this issue Dec 25, 2019 · 10 comments · Fixed by #67603
Assignees
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@SimonSapin
Copy link
Contributor

https://community-tc.services.mozilla.com/tasks/bdytAYuMTdWK1kbybZ6FWA/runs/0/logs/https%3A%2F%2Fcommunity-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FbdytAYuMTdWK1kbybZ6FWA%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log#L1943

   --> /repo/target/debug/build/script-a3d7e9e184a37789/out/Bindings/ANGLEInstancedArraysBinding.rs:852:61
    |
852 |   const sMethods_specs: &'static [&'static[JSFunctionSpec]] = &[
    |  _____________________________________________________________^
853 | | &[
854 | |     JSFunctionSpec {
855 | |         name: b"drawArraysInstancedANGLE\0" as *const u8 as *const libc::c_char,
...   |
882 | |
883 | | ];
    | |_^

(Many other occurrences)

error: internal compiler error: mutable allocation in constant
    --> /repo/target/debug/build/script-a3d7e9e184a37789/out/Bindings/XRWebGLLayerBinding.rs:1088:64
     |
1088 |   const sAttributes_specs: &'static [&'static[JSPropertySpec]] = &[
     |  ________________________________________________________________^
1089 | | &[
1090 | |     JSPropertySpec {
1091 | |         name: b"context\0" as *const u8 as *const libc::c_char,
...    |
1203 | |
1204 | | ];
     | |_^
thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:349:17
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:77
   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:1057
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1426
   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:195
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:215
  10: rustc_driver::report_ice
  11: <alloc::boxed::Box<F> as core::ops::function::Fn<A>>::call
             at /rustc/a9c1c04e986dbf610be8cbe6a8107f90b4db61ce/src/liballoc/boxed.rs:1029
  12: proc_macro::bridge::client::<impl proc_macro::bridge::Bridge>::enter::{{closure}}::{{closure}}
             at /rustc/a9c1c04e986dbf610be8cbe6a8107f90b4db61ce/src/libproc_macro/bridge/client.rs:305
  13: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:476
  14: std::panicking::begin_panic
  15: <rustc_errors::HandlerInner as core::ops::drop::Drop>::drop
  16: core::ptr::real_drop_in_place
  17: core::ptr::real_drop_in_place
  18: core::ptr::real_drop_in_place
  19: rustc_interface::interface::run_compiler_in_existing_thread_pool
  20: std::thread::local::LocalKey<T>::with
  21: scoped_tls::ScopedKey<T>::set
  22: syntax::with_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: internal compiler error: unexpected panic
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.42.0-nightly (a9c1c04e9 2019-12-24) running on x86_64-unknown-linux-gnu
note: compiler flags: -C debuginfo=2 -C link-args=-fuse-ld=gold --crate-type rlib
note: some of the compiler flags provided by cargo are hidden
query stack during panic:
end of query stack
error: could not compile `script`.

The crate uses #![feature(const_fn)].

Example of one of the affected const items:

const sMethods_specs: &'static [&'static[JSFunctionSpec]] = &[
&[
    JSFunctionSpec {
        name: b"getViewport\0" as *const u8 as *const libc::c_char,
        call: JSNativeWrapper { 
            // `generic_method` is a free function
            op: Some(generic_method),
            // getViewport_methodinfo is a const item
            info: &getViewport_methodinfo as *const _ as *const JSJitInfo
        },
        nargs: 1,
        flags: (JSPROP_ENUMERATE) as u16,
        selfHostedName: 0 as *const libc::c_char
    },
    JSFunctionSpec {
        name: 0 as *const libc::c_char,
        call: JSNativeWrapper { 
            op: None, 
            info: 0 as *const JSJitInfo
        },
        nargs: 0,
        flags: 0,
        selfHostedName: 0 as *const libc::c_char
    }
]
];

Definitions of types involved:

pub struct JSFunctionSpec {
    pub name: *const i8,
    pub call: JSNativeWrapper,
    pub nargs: u16,
    pub flags: u16,
    pub selfHostedName: *const i8,
}
pub struct JSNativeWrapper {
    pub op: Option<unsafe extern "C" fn(*mut JSContext, u32, *mut Value) -> bool>,
    pub info: *const JSJitInfo,
}
pub struct JSJitInfo {
    pub call: *const c_void,
    pub protoID: u16,
    pub depth: u16,
    pub _bitfield_1: u32,
}

Regression range:

git log --merges --oneline 9ae6ced...a9c1c04
a9c1c04 Auto merge of #67241 - mark-i-m:simplify-borrow_check-3, r=matthewjasper
f6dca76 Auto merge of #66221 - ohadravid:doc-constants, r=Dylan-DPC
84d8f9d Auto merge of #67579 - RalfJung:miri, r=RalfJung
6253754 Auto merge of #67575 - Centril:rollup-feikoir, r=Centril
a76d67f Rollup merge of #67572 - aidanhs:aphs-choco-direct-cdn, r=Mark-Simulacrum
20d5df9 Rollup merge of #67569 - Mark-Simulacrum:opt-char-encode, r=oli-obk
75b27ef Rollup merge of #67561 - euclio:remove-description, r=jonas-schievink
a75968a Rollup merge of #67551 - ldm0:E0627, r=Dylan-DPC
d130e8d Rollup merge of #67547 - GuillaumeGomez:cleanup-err-codes, r=Dylan-DPC
07effe1 Rollup merge of #67543 - JohnTitor:regression-tests, r=Centril
1ac8fc7 Rollup merge of #67337 - oli-obk:no_mut_static_ref_from_const, r=RalfJung
a4cd03d Auto merge of #66296 - Centril:bindings_after_at-init, r=pnkfelix

The error message appears to be from #67337, cc @oli-obk

If this (modulo ICE vs proper diagnostic) is not a rustc bug, I don’t understand what change we’re supposed to make to this code. I don’t see any heap allocation or mutability happening.

@SimonSapin SimonSapin added I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. A-const-eval Area: Constant evaluation (MIR interpretation) labels Dec 25, 2019
@Centril Centril added requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated P-medium Medium priority E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Dec 25, 2019
@Centril
Copy link
Contributor

Centril commented Dec 25, 2019

If possible, providing a self-contained example that fits into the playground would make minimization and finding the cause of the ICE easier.

@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Dec 25, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Dec 25, 2019

Repro extracted from example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=d437e043d28ec84f02dd5467c36fc645

minifying now

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Dec 25, 2019

Reduced to:

pub const FOO:  &'static *const u32 = &(&BAR as _);
pub const BAR: u32 = 0;

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error: internal compiler error: mutable allocation in constant
 --> src/lib.rs:1:39
  |
1 | pub const FOO:  &'static *const u32 = &(&BAR as _);
  |                                       ^^^^^^^^^^^^

thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:349:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

error: internal compiler error: unexpected panic

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.42.0-nightly (a9c1c04e9 2019-12-24) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type lib

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

error: could not compile `playground`.

To learn more, run the command again with --verbose.

@Centril Centril removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-nominated labels Dec 25, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Dec 25, 2019

This code is 100% reasonable and should not be ICEing or erroring. I think the reason it is ICEing is the raw pointer causing the allocation to not get interned as immutable, even though it must be.

Basically the inner & does not get its own promoted:

pub const FOO:  &'static *const i32 = &(&0 as _);

gives the following MIR

// WARNING: This output format is intended for human consumers only
// and is subject to change without notice. Knock yourself out.
const  FOO: &*const i32 = {
    let mut _0: &*const i32;             // return place in scope 0 at src/lib.rs:1:17: 1:36
    let _1: &*const i32;                 // in scope 0 at src/lib.rs:1:39: 1:49

    bb0: {
        StorageLive(_1);                 // bb0[0]: scope 0 at src/lib.rs:1:39: 1:49
        _1 = &(promoted[0]: *const i32); // bb0[1]: scope 0 at src/lib.rs:1:39: 1:49
        _0 = _1;                         // bb0[2]: scope 0 at src/lib.rs:1:39: 1:49
        StorageDead(_1);                 // bb0[3]: scope 0 at src/lib.rs:1:48: 1:49
        return;                          // bb0[4]: scope 0 at src/lib.rs:1:1: 1:50
    }
}

promoted[0] in  FOO: *const i32 = {
    let mut _0: *const i32;              // return place in scope 0 at src/lib.rs:1:39: 1:49
    let mut _1: *const i32;              // in scope 0 at src/lib.rs:1:40: 1:49
    let mut _2: &i32;                    // in scope 0 at src/lib.rs:1:41: 1:43
    let mut _3: i32;                     // in scope 0 at src/lib.rs:1:42: 1:43

    bb0: {
        _3 = const 0i32;                 // bb0[0]: scope 0 at src/lib.rs:1:42: 1:43
                                         // ty::Const
                                         // + ty: i32
                                         // + val: Value(Scalar(0x00000000))
                                         // mir::Constant
                                         // + span: src/lib.rs:1:42: 1:43
                                         // + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
        _2 = &_3;                        // bb0[1]: scope 0 at src/lib.rs:1:41: 1:43
        _1 = &raw const (*_2);           // bb0[2]: scope 0 at src/lib.rs:1:41: 1:43
        _0 = move _1;                    // bb0[3]: scope 0 at src/lib.rs:1:39: 1:49
        return;                          // bb0[4]: scope 0 at src/lib.rs:1:39: 1:49
    }
}

cc @ecstatic-morse promoteds still contain the inner promoteds as just locals without StorageDead markers. It's probably fine, just wanted to let you know

@Centril
Copy link
Contributor

Centril commented Dec 25, 2019

( cc also @RalfJung & @eddyb )

@oli-obk
Copy link
Contributor

oli-obk commented Dec 25, 2019

I'll prep a fix PR.

@RalfJung
Copy link
Member

This code is 100% reasonable

Well I beg to disagree, this creates non-Sync shared allocations in constants, which is very much not reasonable (Cc #49206). But that's a separate issue.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 25, 2019

well... 100% reasonable from our current rule set, which does a const body based analysis instead of relying on our type system. I didn't say the rule set is 100% reasonable.

@SimonSapin
Copy link
Contributor Author

The ICE persists if we eliminate sharing of non-Sync values:

pub struct SyncPtr(*const u32);
unsafe impl Sync for SyncPtr {}
pub const FOO:  &'static SyncPtr = &SyncPtr(&BAR as _);
pub const BAR: u32 = 0;

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error: internal compiler error: mutable allocation in constant
 --> src/lib.rs:3:36
  |
3 | pub const FOO:  &'static SyncPtr = &SyncPtr(&BAR as _);
  |                                    ^^^^^^^^^^^^^^^^^^^

thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:349:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

error: internal compiler error: unexpected panic

@RalfJung
Copy link
Member

Yeah, that's what I meant by "separate issue". The ICE has nothing to do with Sync. But seeing such code in the wild just makes me wonder if we'll ever be able to fix the Sync thing. Sorry for the noise.

@oli-obk oli-obk added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed requires-nightly This issue requires a nightly compiler in some way. P-medium Medium priority labels Jan 8, 2020
@oli-obk oli-obk added the P-high High priority label Jan 8, 2020
@bors bors closed this as completed in faf45c5 Jan 15, 2020
bors-servo pushed a commit to servo/servo that referenced this issue Jan 16, 2020
Upgrade to rustc 1.42.0-nightly (3291ae339 2020-01-15)

This was unblocked by rust-lang/rust#67603 fixing rust-lang/rust#67601.
bors-servo pushed a commit to servo/servo that referenced this issue Jan 16, 2020
Upgrade to rustc 1.42.0-nightly (3291ae339 2020-01-15)

This was unblocked by rust-lang/rust#67603 fixing rust-lang/rust#67601.
bors-servo pushed a commit to servo/servo that referenced this issue Jan 16, 2020
Upgrade to rustc 1.42.0-nightly (3291ae339 2020-01-15)

This was unblocked by rust-lang/rust#67603 fixing rust-lang/rust#67601.
bors-servo pushed a commit to servo/servo that referenced this issue Jan 16, 2020
Upgrade to rustc 1.42.0-nightly (3291ae339 2020-01-15)

This was unblocked by rust-lang/rust#67603 fixing rust-lang/rust#67601.
bors-servo pushed a commit to servo/servo that referenced this issue Jan 16, 2020
Upgrade to rustc 1.42.0-nightly (3291ae339 2020-01-15)

This was unblocked by rust-lang/rust#67603 fixing rust-lang/rust#67601.
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 (MIR interpretation) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
5 participants