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

nightly ICE when building flexstr in release mode #127030

Closed
farnoy opened this issue Jun 27, 2024 · 8 comments
Closed

nightly ICE when building flexstr in release mode #127030

farnoy opened this issue Jun 27, 2024 · 8 comments
Assignees
Labels
C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@farnoy
Copy link
Contributor

farnoy commented Jun 27, 2024

Repro steps:

$ git clone https://github.com/nu11ptr/flexstr.git && cd flexstr
$ rustup override set nightly
$ rustup show
# ...
active toolchain
----------------

nightly-x86_64-pc-windows-msvc (directory override for 'C:\Projects\flexstr')
rustc 1.81.0-nightly (4bc39f028 2024-06-26)
$ cargo build --release
# ...
error: 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: please make sure that you have updated to the latest nightly

note: please attach the file at `C:\Projects\flexstr\rustc-ice-2024-06-27T09_40_04-37164.txt` to your bug report

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

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

query stack during panic:
#0 [evaluate_obligation] evaluating trait selection obligation `[core::mem::maybe_uninit::MaybeUninit<u8>; PAD2]: core::marker::Copy`
#1 [is_copy_raw] computing whether `[core::mem::maybe_uninit::MaybeUninit<u8>; PAD2]` is `Copy`
end of query stack

I did not bisect, but nightly-2024-04-15 does work just fine

thread 'rustc' panicked at compiler\rustc_middle\src\ty\sty.rs:360:36:
called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x7ff97cd4d5a3 - std::backtrace_rs::backtrace::dbghelp64::trace
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\..\..\backtrace\src\backtrace\dbghelp64.rs:91
   1:     0x7ff97cd4d5a3 - std::backtrace_rs::backtrace::trace_unsynchronized
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66
   2:     0x7ff97cd4d5a3 - std::backtrace::Backtrace::create
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\backtrace.rs:331
   3:     0x7ff97cd4d4ea - std::backtrace::Backtrace::force_capture
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\backtrace.rs:312
   4:     0x7ff950da0e41 - memchr
   5:     0x7ff97cd67207 - alloc::boxed::impl$50::call
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\alloc\src\boxed.rs:2078
   6:     0x7ff97cd67207 - std::panicking::rust_panic_with_hook
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\panicking.rs:804
   7:     0x7ff97cd66fdf - std::panicking::begin_panic_handler::closure$0
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\panicking.rs:663
   8:     0x7ff97cd644bf - std::sys::backtrace::__rust_end_short_backtrace<std::panicking::begin_panic_handler::closure_env$0,never$>
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\sys\backtrace.rs:171
   9:     0x7ff97cd66c76 - std::panicking::begin_panic_handler
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\panicking.rs:661
  10:     0x7ff97cdbcbc4 - core::panicking::panic_fmt
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\core\src\panicking.rs:74
  11:     0x7ff97cdbcc6d - core::panicking::panic
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\core\src\panicking.rs:148
  12:     0x7ff97cdbcb2e - core::option::unwrap_failed
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\core\src\option.rs:2013
  13:     0x7ff94f7cff01 - <rustc_middle[d84a46905b9833cf]::ty::sty::ParamConst>::find_ty_from_env
  14:     0x7ff9506bd5d7 - <rustc_trait_selection[ce8e559b29610d30]::traits::query::normalize::QueryNormalizer as rustc_type_ir[aefdc4761e7ea19c]::fold::FallibleTypeFolder<rustc_middle[d84a46905b9833cf]::ty::context::TyCtxt>>::try_fold_ty
  15:     0x7ff9506bf2fe - <rustc_trait_selection[ce8e559b29610d30]::traits::query::normalize::QueryNormalizer as rustc_type_ir[aefdc4761e7ea19c]::fold::FallibleTypeFolder<rustc_middle[d84a46905b9833cf]::ty::context::TyCtxt>>::try_fold_ty
  16:     0x7ff95061084e - <rustc_trait_selection[ce8e559b29610d30]::traits::select::SelectionContext>::evaluate_root_obligation
  17:     0x7ff94ff6cd57 - rustc_traits[8fc914267bf48580]::evaluate_obligation::evaluate_obligation
  18:     0x7ff95033d43e - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  19:     0x7ff95023db0f - rustc_ty_utils[7a4117f0c994b440]::ty::self_ty_of_trait_impl_enabling_order_dep_trait_object_hack
  20:     0x7ff95034fca0 - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  21:     0x7ff95067f7de - <rustc_infer[d17335e7fed5b4ff]::infer::InferCtxt as rustc_trait_selection[ce8e559b29610d30]::traits::query::evaluate_obligation::InferCtxtExt>::evaluate_obligation_no_overflow
  22:     0x7ff95062e392 - rustc_trait_selection[ce8e559b29610d30]::traits::type_known_to_meet_bound_modulo_regions
  23:     0x7ff9501b822b - rustc_ty_utils[7a4117f0c994b440]::common_traits::is_copy_raw
  24:     0x7ff95033c44b - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  25:     0x7ff95024b8cc - rustc_ty_utils[7a4117f0c994b440]::ty::self_ty_of_trait_impl_enabling_order_dep_trait_object_hack
  26:     0x7ff95034bab8 - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  27:     0x7ff950bb786b - <rustc_middle[d84a46905b9833cf]::ty::Ty>::is_copy_modulo_regions
  28:     0x7ff9501d1cb6 - rustc_ty_utils[7a4117f0c994b440]::needs_drop::needs_drop_raw
  29:     0x7ff95033c93b - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  30:     0x7ff95024b8b4 - rustc_ty_utils[7a4117f0c994b440]::ty::self_ty_of_trait_impl_enabling_order_dep_trait_object_hack
  31:     0x7ff95034be93 - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  32:     0x7ff94fbe4582 - rustc_hir_analysis[f30f03e5312332c1]::collect::predicates_defined_on
  33:     0x7ff94fbd9438 - rustc_hir_analysis[f30f03e5312332c1]::collect::predicates_defined_on
  34:     0x7ff94f268549 - <rustc_hir_analysis[f30f03e5312332c1]::variance::variance_of_opaque::{closure#0}::OpaqueTypeLifetimeCollector as rustc_type_ir[aefdc4761e7ea19c]::visit::TypeVisitor<rustc_middle[d84a46905b9833cf]::ty::context::TyCtxt>>::visit_ty
  35:     0x7ff94f27ec9e - rustc_mir_transform[3a1eaf4008c3052b]::shim::make_shim
  36:     0x7ff94f4af817 - <dyn std[85dfe2848fbb0331]::io::Write as nu_ansi_term[494721e700deaf73]::write::AnyWrite>::write_str
  37:     0x7ff94f498cb3 - <dyn std[85dfe2848fbb0331]::io::Write as nu_ansi_term[494721e700deaf73]::write::AnyWrite>::write_str
  38:     0x7ff94f401391 - rustc_ty_utils[7a4117f0c994b440]::ty::adt_sized_constraint
  39:     0x7ff94f4c2b00 - rustc_query_impl[16b8175ba1dd8f83]::query_system
  40:     0x7ff950c68ddc - <rustc_middle[d84a46905b9833cf]::ty::context::TyCtxt>::instance_mir
  41:     0x7ff94fc285dd - rustc_mir_transform[3a1eaf4008c3052b]::inline::cycle::mir_inliner_callees
  42:     0x7ff94fc26a56 - rustc_mir_transform[3a1eaf4008c3052b]::inline::cycle::mir_inliner_callees
  43:     0x7ff94fc26afd - rustc_mir_transform[3a1eaf4008c3052b]::inline::cycle::mir_inliner_callees
  44:     0x7ff94f27c780 - <rustc_mir_transform[3a1eaf4008c3052b]::inline::Inline as rustc_middle[d84a46905b9833cf]::mir::MirPass>::run_pass
  45:     0x7ff94fbf9cdc - <rustc_mir_transform[3a1eaf4008c3052b]::simplify::SimplifyCfg as rustc_middle[d84a46905b9833cf]::mir::MirPass>::run_pass
  46:     0x7ff94fcb6145 - rustc_mir_transform[3a1eaf4008c3052b]::optimized_mir
  47:     0x7ff95033c7a0 - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  48:     0x7ff95022a81a - rustc_ty_utils[7a4117f0c994b440]::ty::self_ty_of_trait_impl_enabling_order_dep_trait_object_hack
  49:     0x7ff950342b05 - rustc_query_impl[16b8175ba1dd8f83]::plumbing::query_key_hash_verify_all
  50:     0x7ff95203e0f2 - <cc[c04df6f83f42a58f]::AppleOs as core[8b3c6311f9756dee]::fmt::Debug>::fmt
  51:     0x7ff9508b6b75 - <rustc_metadata[d6eac6bb222208d8]::rmeta::encoder::EncodeContext as rustc_span[33560a3c1d01ec1a]::SpanEncoder>::encode_symbol
  52:     0x7ff94f69ebfa - rustc_metadata[d6eac6bb222208d8]::rmeta::encoder::encode_metadata
  53:     0x7ff94f6a6c88 - rustc_metadata[d6eac6bb222208d8]::fs::encode_and_write_metadata
  54:     0x7ff94cb0361a - <rustc_interface[7b147ced95828432]::queries::Queries>::codegen_and_build_linker
  55:     0x7ff94cab5c8b - _LNan_C
  56:     0x7ff94cab1c0f - _LNan_C
  57:     0x7ff94cabb239 - _LNan_C
  58:     0x7ff97cd7815d - alloc::boxed::impl$48::call_once
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\alloc\src\boxed.rs:2064
  59:     0x7ff97cd7815d - alloc::boxed::impl$48::call_once
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\alloc\src\boxed.rs:2064
  60:     0x7ff97cd7815d - std::sys::pal::windows::thread::impl$0::new::thread_start
                               at /rustc/4bc39f028d14c24b04dd17dc425432c6ec354536/library\std\src\sys\pal\windows\thread.rs:52
  61:     0x7ffa1779257d - BaseThreadInitThunk
  62:     0x7ffa1888af28 - RtlUserThreadStart


rustc version: 1.81.0-nightly (4bc39f028 2024-06-26)
platform: x86_64-pc-windows-msvc

query stack during panic:
#0 [evaluate_obligation] evaluating trait selection obligation `[core::mem::maybe_uninit::MaybeUninit<u8>; PAD2]: core::marker::Copy`
#1 [is_copy_raw] computing whether `[core::mem::maybe_uninit::MaybeUninit<u8>; PAD2]` is `Copy`
#2 [needs_drop_raw] computing whether `[core::mem::maybe_uninit::MaybeUninit<u8>; PAD2]` needs drop
#3 [mir_shims] generating MIR shim for `core::ptr::drop_in_place`
#4 [optimized_mir] optimizing MIR for `<impl at flexstr\src\lib.rs:207:1: 208:40>::drop`
end of query stack
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 27, 2024
@lqd
Copy link
Member

lqd commented Jun 27, 2024

Bisects to #125958, cc PR author @BoxyUwU -- ah but it's likely a duplicate of #126378 and so forth.

@Noratrieb
Copy link
Member

this is stable and doesn't use effects, so it's probably different

@Noratrieb Noratrieb 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. T-types Relevant to the types team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 27, 2024
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 27, 2024
@lqd
Copy link
Member

lqd commented Jun 27, 2024

@Nilstrieb I'm going from #126889 (comment)

@BoxyUwU BoxyUwU self-assigned this Jun 27, 2024
@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 27, 2024

I cannot reproduce this locally

edit: oh --release is required

@BoxyUwU BoxyUwU assigned compiler-errors and unassigned BoxyUwU Jun 27, 2024
@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 27, 2024

Assigning @compiler-errors since he said he'd take a look at this after I figured out that this is a mess to fix.

The tl;dr here is that pre-monomorphization we wind up trying to instance drop glue.

The logic for creating drop glue should be able to assume that it is in a post-monomorphization state where using an empty ParamEnv is fine. Because we try to instance drop glue with generic types (e.g. Foo<N>) this means that we need to actually be using the ParamEnv of the body that is instancing the drop glue instead. This turns out to be quite involved to fix and not as simple as other "pass the correct param env in" bug fixes.

The... good ish? bad? news ? is that we've been getting this wrong for much longer than my PR, my PR only surfaced this due to the fact that it relies on us providing the correct ParamEnv everywhere in the compiler which apparently is not the case :-)

edit: I suspect the principled solution here is to have two codepaths (that share logic) for generating mir shims. One for instancing stuff post mono that behaves how monomorphization should be behaving and can use an empty Reveal::All env. And one for when we want to generate the mir pre-mono and it's as if we're putting the mir inline into some other body in which cases we need to be using the ParamEnv of the body we're placing the shim's mir into`.

@BoxyUwU BoxyUwU added regression-from-stable-to-beta Performance or correctness regression from stable to beta. P-critical Critical priority and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 27, 2024
@BoxyUwU
Copy link
Member

BoxyUwU commented Jun 27, 2024

To avoid breaking stuff when beta gets bumped to stable we probably want to try the following:

  • Figure out exactly what's causing the drop glue generation to occur in --release and see if its viable to backport a change to stop doing that to mask the bug for a cycle
  • If thats not possible try to revert Const::ty removal on beta only
  • If that's not possible try land a hack that's simple to reason about and beta backport it

And then when that's done try to land the principled refactoring before the next beta cutoff

bors added a commit to rust-lang-ci/rust that referenced this issue Jun 28, 2024
Stall dropaStall computing instance for drop shim until it has no unsubstituted const params

Stall resolving the drop shim instance for types that still have unsubstituted const params.

## Why?

rust-lang#127030 ICEs because it tries to inline the drop shim for a type with an unsubstituted const param. In order to generate this shim, this requires calling the drop shim builder, which invokes the trait solver to compute whether constituent types need drop (since we compute if a type is copy to disqualify any `Drop` behavior):

https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_dataflow/src/elaborate_drops.rs#L378

However, since we don't keep the param-env of the instance we resolved the item for, we use the wrong param-env:
https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_transform/src/shim.rs#L278
(which is the param-env of `std::ptr::drop_in_place`)

This param-env is notably missing `ConstParamHasTy` predicates, and since we removed the type from consts in rust-lang#125958, we literally cannot prove these predicates in this (relatively) empty param-env. This currently happens in places like the MIR inliner, but may happen elsewhere such as in lints that resolve terminators.

## What?

We delay the resolution (`Instance::resolve`) of calls for `drop_in_place` for types that have unsubstituted const params. This should be OK, since all cases that deal with polymorphic code should handle `Instance::resolve` returning `None` gracefully.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 28, 2024
Stall computing instance for drop shim until it has no unsubstituted const params

Do not inline the drop shim for types that still have unsubstituted const params.

## Why?

rust-lang#127030 ICEs because it tries to inline the drop shim for a type with an unsubstituted const param. In order to generate this shim, this requires calling the drop shim builder, which invokes the trait solver to compute whether constituent types need drop (since we compute if a type is copy to disqualify any `Drop` behavior):

https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_dataflow/src/elaborate_drops.rs#L378

However, since we don't keep the param-env of the instance we resolved the item for, we use the wrong param-env:
https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_transform/src/shim.rs#L278
(which is the param-env of `std::ptr::drop_in_place`)

This param-env is notably missing `ConstParamHasTy` predicates, and since we removed the type from consts in rust-lang#125958, we literally cannot prove these predicates in this (relatively) empty param-env. This currently happens in places like the MIR inliner, but may happen elsewhere such as in lints that resolve terminators.

## What?

We force the inliner to not consider calls for `drop_in_place` for types that have unsubstituted const params.

## So what?

This may negatively affect MIR inlining, but I doubt this matters in practice, and fixes a beta regression, so let's fix it. I will look into approaches for fixing this in a more maintainable way, perhaps delaying the creation of drop shim bodies until codegen (like how intrinsics work).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 29, 2024
Rollup merge of rust-lang#127068 - compiler-errors:stall-drop, r=BoxyUwU

Stall computing instance for drop shim until it has no unsubstituted const params

Do not inline the drop shim for types that still have unsubstituted const params.

## Why?

rust-lang#127030 ICEs because it tries to inline the drop shim for a type with an unsubstituted const param. In order to generate this shim, this requires calling the drop shim builder, which invokes the trait solver to compute whether constituent types need drop (since we compute if a type is copy to disqualify any `Drop` behavior):

https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_dataflow/src/elaborate_drops.rs#L378

However, since we don't keep the param-env of the instance we resolved the item for, we use the wrong param-env:
https://github.com/rust-lang/rust/blob/9c3bc805dd9cb84019c124b9a50fdff1e62a7ec9/compiler/rustc_mir_transform/src/shim.rs#L278
(which is the param-env of `std::ptr::drop_in_place`)

This param-env is notably missing `ConstParamHasTy` predicates, and since we removed the type from consts in rust-lang#125958, we literally cannot prove these predicates in this (relatively) empty param-env. This currently happens in places like the MIR inliner, but may happen elsewhere such as in lints that resolve terminators.

## What?

We force the inliner to not consider calls for `drop_in_place` for types that have unsubstituted const params.

## So what?

This may negatively affect MIR inlining, but I doubt this matters in practice, and fixes a beta regression, so let's fix it. I will look into approaches for fixing this in a more maintainable way, perhaps delaying the creation of drop shim bodies until codegen (like how intrinsics work).
@jieyouxu jieyouxu added L-out_of_scope_macro_calls Lint: out_of_scope_macro_calls and removed L-out_of_scope_macro_calls Lint: out_of_scope_macro_calls labels Jun 29, 2024
@compiler-errors
Copy link
Member

This should be fixed on nightly, awaiting a beta backport

@compiler-errors
Copy link
Member

This has been beta-backported, should show up next time a beta build is released on rustup (next day or so?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants