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

inlining produces broken MIR by changing whether types can be normalized #112332

Open
ahcm opened this issue Jun 6, 2023 · 12 comments
Open

inlining produces broken MIR by changing whether types can be normalized #112332

ahcm opened this issue Jun 6, 2023 · 12 comments
Labels
A-mir-opt-inlining Area: MIR inlining 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

@ahcm
Copy link

ahcm commented Jun 6, 2023

Meta

cargo +stable does not show this.

rustc --version --verbose:

rustc 1.72.0-nightly (101fa903b 2023-06-04)

Error output

   Compiling rkyv v0.6.7
   Compiling plotters v0.3.4
error: internal compiler error: no errors encountered even though `delay_span_bug` issued

error: internal compiler error: broken MIR in Item(DefId(0:1502 ~ rkyv[9515]::std_impl::{impl#47}::deserialize)) (after phase change to runtime-optimized) at bb0[27]:
                                Field projection `(*_19).1` specified type `u32`, but actual type is `<[<T as Archive>::Archived] as ArchivePointee>::ArchivedMetadata`
   --> /home/andy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rkyv-0.6.7/src/lib.rs:765:77
    |
765 |         ptr_meta::from_raw_parts(self.raw_ptr.as_ptr(), T::pointer_metadata(&self.metadata))
    |                                                                             ^^^^^^^^^^^^^^
    |
    = note: delayed at compiler/rustc_const_eval/src/transform/validate.rs:354:30
               0: <rustc_errors::HandlerInner>::emit_diagnostic
               1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, alloc::string::String>
               2: <rustc_const_eval::transform::validate::TypeChecker>::fail::<alloc::string::String>
               3: <rustc_const_eval::transform::validate::TypeChecker as rustc_middle::mir::visit::Visitor>::visit_place
               4: <rustc_const_eval::transform::validate::Validator as rustc_middle::mir::MirPass>::run_pass
               5: rustc_mir_transform::optimized_mir
               6: rustc_query_impl::plumbing::__rust_begin_short_backtrace::<rustc_query_impl::query_impl::optimized_mir::dynamic_query::{closure#2}::{closure#0}, rustc_middle::query::erase::Erased<[u8; 8]>>
               7: <rustc_query_impl::query_impl::optimized_mir::dynamic_query::{closure#2} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, rustc_span::def_id::DefId)>>::call_once
               8: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::DefaultCache<rustc_span::def_id::DefId, rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, false>
               9: rustc_query_impl::query_impl::optimized_mir::get_query_non_incr::__rust_end_short_backtrace
              10: <rustc_metadata::rmeta::encoder::EncodeContext>::encode_crate_root
              11: rustc_metadata::rmeta::encoder::encode_metadata_impl
              12: rustc_metadata::rmeta::encoder::encode_metadata
              13: rustc_metadata::fs::encode_and_write_metadata
              14: rustc_interface::passes::start_codegen
              15: <rustc_middle::ty::context::GlobalCtxt>::enter::<<rustc_interface::queries::Queries>::ongoing_codegen::{closure#0}::{closure#0}, core::result::Result<alloc::boxed::Box<dyn core::any::Any>, rustc_span::ErrorGuaranteed>>
              16: <rustc_interface::queries::Queries>::ongoing_codegen
              17: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_span::ErrorGuaranteed>>
              18: std::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{c
losure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
              19: <<std::thread::Builder>::spawn_unchecked_<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::
result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
              20: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/alloc/src/boxed.rs:1985:9
              21: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/alloc/src/boxed.rs:1985:9
              22: std::sys::unix::thread::Thread::new::thread_start
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/std/src/sys/unix/thread.rs:108:17
              23: <unknown>
              24: <unknown>


error: internal compiler error: broken MIR in Item(DefId(0:1502 ~ rkyv[9515]::std_impl::{impl#47}::deserialize)) (after phase change to runtime-optimized) at bb3[29]:
                                Field projection `(*_37).1` specified type `u32`, but actual type is `<[<T as Archive>::Archived] as ArchivePointee>::ArchivedMetadata`
   --> /home/andy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rkyv-0.6.7/src/lib.rs:765:77
    |
765 |         ptr_meta::from_raw_parts(self.raw_ptr.as_ptr(), T::pointer_metadata(&self.metadata))
    |                                                                             ^^^^^^^^^^^^^^
    |
    = note: delayed at compiler/rustc_const_eval/src/transform/validate.rs:354:30
               0: <rustc_errors::HandlerInner>::emit_diagnostic
               1: <rustc_errors::Handler>::delay_span_bug::<rustc_span::span_encoding::Span, alloc::string::String>
               2: <rustc_const_eval::transform::validate::TypeChecker>::fail::<alloc::string::String>
               3: <rustc_const_eval::transform::validate::TypeChecker as rustc_middle::mir::visit::Visitor>::visit_place
               4: <rustc_const_eval::transform::validate::Validator as rustc_middle::mir::MirPass>::run_pass
               5: rustc_mir_transform::optimized_mir
               6: rustc_query_impl::plumbing::__rust_begin_short_backtrace::<rustc_query_impl::query_impl::optimized_mir::dynamic_query::{closure#2}::{closure#0}, rustc_middle::query::erase::Erased<[u8; 8]>>
               7: <rustc_query_impl::query_impl::optimized_mir::dynamic_query::{closure#2} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, rustc_span::def_id::DefId)>>::call_once
               8: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::DefaultCache<rustc_span::def_id::DefId, rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, false>
               9: rustc_query_impl::query_impl::optimized_mir::get_query_non_incr::__rust_end_short_backtrace
              10: <rustc_metadata::rmeta::encoder::EncodeContext>::encode_crate_root
              11: rustc_metadata::rmeta::encoder::encode_metadata_impl
              12: rustc_metadata::rmeta::encoder::encode_metadata
              13: rustc_metadata::fs::encode_and_write_metadata
              14: rustc_interface::passes::start_codegen
              15: <rustc_middle::ty::context::GlobalCtxt>::enter::<<rustc_interface::queries::Queries>::ongoing_codegen::{closure#0}::{closure#0}, core::result::Result<alloc::boxed::Box<dyn core::any::Any>, rustc_span::ErrorGuaranteed>>
              16: <rustc_interface::queries::Queries>::ongoing_codegen
              17: <rustc_interface::interface::Compiler>::enter::<rustc_driver_impl::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_span::ErrorGuaranteed>>
              18: std::sys_common::backtrace::__rust_begin_short_backtrace::<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{c
losure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>
              19: <<std::thread::Builder>::spawn_unchecked_<rustc_interface::util::run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_span::ErrorGuaranteed>, rustc_driver_impl::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#0}::{closure#0}, core::
result::Result<(), rustc_span::ErrorGuaranteed>>::{closure#1} as core::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
              20: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/alloc/src/boxed.rs:1985:9
              21: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/alloc/src/boxed.rs:1985:9
              22: std::sys::unix::thread::Thread::new::thread_start
                         at /rustc/101fa903bb9209d270086da279247625a2869211/library/std/src/sys/unix/thread.rs:108:17
              23: <unknown>
              24: <unknown>


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.72.0-nightly (101fa903b 2023-06-04) running on x86_64-unknown-linux-gnu

note: compiler flags: --crate-type lib -C opt-level=3 -C embed-bitcode=no

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

query stack during panic:
end of query stack
error: could not compile `rkyv` (lib)
warning: build failed, waiting for other jobs to finish...

@ahcm ahcm added 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. labels Jun 6, 2023
@saethlin
Copy link
Member

saethlin commented Jun 6, 2023

I bisected on a download of rkyv v0.6.7 with this:

RUSTFLAGS="-Zmir-opt-level=0 -Zinline-mir -Zinline-mir-threshold=0 -Zinline-mir-hint-threshold=1000 -Cdebug-assertions=no" cargo test

This manages to immune the bisection to about 5 different mir-opt changes over the past 7 months.

Regression in nightly-2022-10-27

found 9 bors merge commits in the specified range
  commit[0] 2022-10-25: Auto merge of #102340 - JakobDegen:pass-manager-simplification, r=oli-obk
  commit[1] 2022-10-25: Auto merge of #102903 - compiler-errors:region-var-leak, r=jackh726
  commit[2] 2022-10-26: Auto merge of #103158 - Bryanskiy:resolve_perf, r=petrochenkov
  commit[3] 2022-10-26: Auto merge of #103279 - compiler-errors:normalize-hack-back, r=lcnr
  commit[4] 2022-10-26: Auto merge of #103562 - Dylan-DPC:rollup-sheepp5, r=Dylan-DPC
  commit[5] 2022-10-26: Auto merge of #103284 - compiler-errors:const-sad, r=oli-obk
  commit[6] 2022-10-26: Auto merge of #103572 - Dylan-DPC:rollup-a8bnxrw, r=Dylan-DPC
  commit[7] 2022-10-26: Auto merge of #103492 - ehuss:highfive-triagebot, r=Mark-Simulacrum
  commit[8] 2022-10-26: Auto merge of #103571 - RalfJung:miri, r=RalfJung

I feel like this might be normalization-related? cc @compiler-errors

@Noratrieb
Copy link
Member

Noratrieb commented Jun 6, 2023

export RUSTFLAGS="-Zmir-opt-level=0 -Zinline-mir -Zinline-mir-threshold=0 -Zinline-mir-hint-threshold=1000 -Cdebug-assertions=no -Zvalidate-mir"
pub trait InnerProj {
    type Assoc;
}

pub trait AmbigProj {
    type AlwaysU32;
}

struct InnerWrap<T: AmbigProj + ?Sized> {
    metadata: T::AlwaysU32,
}

pub struct Wrapper<T>(InnerWrap<[T]>);

#[inline]
fn as_slice<T>(v: &InnerWrap<[T]>) {
    let _a: <[T] as AmbigProj>::AlwaysU32 = v.metadata;
}

pub fn deserialize<T: InnerProj>(a: &Wrapper<T::Assoc>)
where
    [T::Assoc]: AmbigProj,
{
    as_slice(&a.0);
}

impl<T> AmbigProj for [T] {
    type AlwaysU32 = u32;
}
error: internal compiler error: broken MIR in Item(DefId(0:16 ~ uwu[d045]::deserialize)) (after pass Inline) at bb0[6]:
                                Field projection `(*_3).0` specified type `u32`, but actual type is `<[<T as InnerProj>::Assoc] as AmbigProj>::AlwaysU32`
  --> uwu.rs:17:45
   |
17 |     let _a: <[T] as AmbigProj>::AlwaysU32 = v.metadata;
   |                                             ^^^^^^^^^^
   |
   = note: delayed at compiler/rustc_const_eval/src/transform/validate.rs:354:30 - disabled backtrace

@compiler-errors
Copy link
Member

@saethlin its unlikely any of my PRs in that list.

more likely #102340 perhaps -- https://github.com/rust-lang/rust/pull/102340/files#diff-deb9513f9d3c0f87f924c627700124bed199d659cb223b0796a6509198139cd0R148 touches MIR validation right? cc @JakobDegen

@Noratrieb
Copy link
Member

with -Zvalidate-mir the regression predates 2021-01-01. it ICEs after Inline

@Noratrieb Noratrieb added the A-mir-opt-inlining Area: MIR inlining label Jun 6, 2023
@Noratrieb
Copy link
Member

Noratrieb commented Jun 6, 2023

I have a suspicion about the issue here.
The code fails to normalize <[<T as InnerProj>::Assoc] as AmbigProj>::AlwaysU32. But type checking can normalize it just fine. The issue reveals itself when we inline it manually

pub fn deserialize<T: InnerProj>(a: &Wrapper<T::Assoc>)
where
    [T::Assoc]: AmbigProj,
{
    let _a: <[T] as AmbigProj>::AlwaysU32 = a.0.metadata;
    as_slice(&a.0);
}

It now errors during type checking.

error[E0308]: mismatched types
  --> uwu.rs:24:45
   |
24 |     let _a: <[T] as AmbigProj>::AlwaysU32 = a.0.metadata;
   |             -----------------------------   ^^^^^^^^^^^^ expected `u32`, found associated type
   |             |
   |             expected due to this
   |
   = note:         expected type `u32`
           found associated type `<[<T as InnerProj>::Assoc] as AmbigProj>::AlwaysU32`
   = help: consider constraining the associated type `<[<T as InnerProj>::Assoc] as AmbigProj>::AlwaysU32` to `u32`

We have a blanket impl that always sets the associated type to u32, so we would expect it to normalize and succeed.

impl<T> AmbigProj for [T] {
    type AlwaysU32 = u32;
}

Except that isn't the only candidate for <[T] as AmbigProj> in scope. We also have a where clause, where [T::Assoc]: AmbigProj. If we pick the candidate from the where clause, then we cannot normalize it as we do not know the associated type.

And that's what we do! Candidates from the where clauses are perferred over impls. So inside the inner function, we pick the impl candidate and normalize it just fine. But after inlining, we now pick the where clause candidate and cannot normalize it.

I don't know how to best fix this.

@Noratrieb
Copy link
Member

cc @cjgillot maybe you have some insight

@compiler-errors
Copy link
Member

That analysis is correct.

I don't think this is easy to fix in general, and not sure how hard it would be to implement additional inlineability checks to avoid doing inlining in cases like this (also it would be a shame if the inliner skips valid inlining candidates just because of the trait solver being dumb).

This is fixed afaict in the new trait solver. I wonder if we should use the new solver in mir validation as one of the first targets -- #112122 already does some of the work of making this possible.

@JakobDegen
Copy link
Contributor

@compiler-errors if we disable this check in Mir validation, would we see an ICE in codegen/fullfillment? Depending on how widespread this is, doing less validation is always an option

@compiler-errors
Copy link
Member

No, this would probably not ICE in codegen.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 21, 2023
…-errors

Substitute types before checking inlining compatibility.

Addresses rust-lang#112332 and rust-lang#113781

I don't have a minimal test, but I this seems to remove the ICE locally.

This whole pre-inlining validation mirrors the "real" MIR validation pass to verify that inlined MIR will still pass validation.
The debuginfo loop is added because MIR validation check projections in debuginfo.
Likewise, MIR validation only checks `is_subtype`, so there is no reason for a stronger check.

The types were not being substituted in `check_equal`, so we were not bailing out of inlining if the substituted MIR callee body would not pass validation.
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jul 22, 2023
Substitute types before checking inlining compatibility.

Addresses rust-lang/rust#112332 and rust-lang/rust#113781

I don't have a minimal test, but I this seems to remove the ICE locally.

This whole pre-inlining validation mirrors the "real" MIR validation pass to verify that inlined MIR will still pass validation.
The debuginfo loop is added because MIR validation check projections in debuginfo.
Likewise, MIR validation only checks `is_subtype`, so there is no reason for a stronger check.

The types were not being substituted in `check_equal`, so we were not bailing out of inlining if the substituted MIR callee body would not pass validation.
@RalfJung
Copy link
Member

#113802 works around this, right? I guess we're keeping the issue open since it's not a proper fix, just disabling inlining?

@Haaroon
Copy link

Haaroon commented Nov 16, 2023

i am also having this issue as we are tring to build our project with maturin develop below was the error which i am trying to fix.

  error: internal compiler error: no errors encountered even though `delay_span_bug` issued


  error: internal compiler error: broken MIR in Item(DefId(0:7815 ~ tantivy[41bc]::store::reader::{impl#3}::iter_raw::{closure#2})) (after phase change to runtime-optimized) at bb2[6]:
                                  Alias(Opaque, AliasTy { args: [ReErased], def_id: DefId(0:18126 ~ tantivy[41bc]::store::reader::{impl#3}::block_checkpoints::{opaque#0}) }) does not have fields
   --> /rustc/79e9716c980570bfd1f666e3b16ac583f0168962/library/core/src/iter/adapters/flatten.rs:52:9
    |

@saethlin
Copy link
Member

That looks to me like a different error, please open a separate issue.

@RalfJung RalfJung changed the title error: internal compiler error: broken MIR inlining produces broken MIR by changing whether types can be normalized Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt-inlining Area: MIR inlining 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

7 participants