ICE when reifying function pointers to copy / copy_nonoverlapping using an if

[5225225]

This happens on nightly only, I included a bisection at the bottom. (But it doesn't seem as if the bisected commit is the direct cause of this).

It only ICE's if you assign the result to a variable, assigning it to _ or not at all doesn't ICE.

The bisection for using core::intrinsics::copy is the same.

Code

fn main() {
    let _unused = if true {
        core::ptr::copy::<i32>
    } else {
        core::ptr::copy_nonoverlapping::<i32>
    };
}

Meta

rustc --version --verbose:

rustc 1.53.0-nightly (b0c818c5e 2021-04-16)
binary: rustc
commit-hash: b0c818c5e0fa37251d9fda2f656bf1041a2e1e1d
commit-date: 2021-04-16
host: x86_64-unknown-linux-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0

Error output

error: internal compiler error: compiler/rustc_mir/src/monomorphize/collector.rs:809:17: Instance { def: Intrinsic(DefId(2:1659 ~ core[ec89]::intrinsics::#1::copy)), substs: [i32] } being reified

thread 'rustc' panicked at 'Box<Any>', /rustc/b0c818c5e0fa37251d9fda2f656bf1041a2e1e1d/library/std/src/panic.rs:59:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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.53.0-nightly (b0c818c5e 2021-04-16) running on x86_64-unknown-linux-gnu

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

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

query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

error: could not compile `scratch`

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

Backtrace

   Compiling scratch v0.1.0 (/tmp/scratchl90UMMXXQ)
error: internal compiler error: compiler/rustc_mir/src/monomorphize/collector.rs:809:17: Instance { def: Intrinsic(DefId(2:1659 ~ core[ec89]::intrinsics::#1::copy)), substs: [i32] } being reified

thread 'rustc' panicked at 'Box<Any>', /rustc/b0c818c5e0fa37251d9fda2f656bf1041a2e1e1d/library/std/src/panic.rs:59:5
stack backtrace:
   0: std::panicking::begin_panic
   1: std::panic::panic_any
   2: rustc_errors::HandlerInner::bug
   3: rustc_errors::Handler::bug
   4: rustc_middle::ty::context::tls::with_opt
   5: rustc_middle::util::bug::opt_span_bug_fmt
   6: rustc_middle::util::bug::bug_fmt
   7: rustc_mir::monomorphize::collector::visit_instance_use
   8: <rustc_mir::monomorphize::collector::MirNeighborCollector as rustc_middle::mir::visit::Visitor>::visit_rvalue
   9: rustc_mir::monomorphize::collector::collect_neighbours
  10: rustc_mir::monomorphize::collector::collect_items_rec
  11: rustc_session::utils::<impl rustc_session::session::Session>::time
  12: rustc_mir::monomorphize::collector::collect_crate_mono_items
  13: rustc_mir::monomorphize::partitioning::collect_and_partition_mono_items
  14: rustc_query_impl::<impl rustc_query_system::query::config::QueryAccessors<rustc_query_impl::plumbing::QueryCtxt> for rustc_query_impl::queries::collect_and_partition_mono_items>::compute
  15: rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::with_deps
  16: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  17: rustc_data_structures::stack::ensure_sufficient_stack
  18: rustc_query_system::query::plumbing::force_query_with_job
  19: rustc_query_system::query::plumbing::get_query_impl
  20: <rustc_query_impl::Queries as rustc_middle::ty::query::QueryEngine>::collect_and_partition_mono_items
  21: rustc_codegen_ssa::base::codegen_crate
  22: <rustc_codegen_llvm::LlvmCodegenBackend as rustc_codegen_ssa::traits::backend::CodegenBackend>::codegen_crate
  23: rustc_interface::passes::QueryContext::enter
  24: rustc_interface::queries::Queries::ongoing_codegen
  25: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  26: rustc_span::with_source_map
  27: rustc_interface::interface::create_compiler_and_run
  28: scoped_tls::ScopedKey<T>::set
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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.53.0-nightly (b0c818c5e 2021-04-16) running on x86_64-unknown-linux-gnu

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

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

query stack during panic:
#0 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: aborting due to previous error

error: could not compile `scratch`

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

Bisection

searched nightlies: from nightly-2021-01-01 to nightly-2021-04-18
regressed nightly: nightly-2021-02-14
searched commits: from 3f5aee2 to 8e54a21
regressed commit: 8e54a21

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --test-dir=. --start=2021-01-01 --end=2021-04-18 --preserve 

[5225225]

This also happens for any intrinsic where the type signatures match up.

#![feature(core_intrinsics)]

fn main() {
    let _unused = if true {
        std::intrinsics::likely
    } else {
        std::intrinsics::unlikely
    };
}

And this variant will ICE on nightlies older than the one above, but it requires a nightly compiler and opting in to intrinsics.

Match statements also work.

Looks like a similar issue to #64104

[apiraino]

Assigning priority as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@rustbot label -I-prioritize +P-medium

[ghost]

It happens on beta too: https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=a922a77b8fb2361b488d151613cd48e4
(This compiles on stable 1.51.)

[pietroalbini]

#84404 needs to be backported to beta for this to be fixed. Adding the labels on that PR.

[tmiasko]

#81238 turned core::ptr::copy & core::ptr::copy_nonoverlapping functions into rexeports of intrinsics. This was a breaking change, because functions can be coerced into function pointers while intrinsics currently cannot be.

This change also exposed an ICE that might occur when an attempt is made to coerce intrinsics into a common type. The ICE was turned into error by #84404, but I would consider this to be of secondary importance.

If we actually want to avoid a breaking change here, in the short term the only solution is to revert #81238.

[RalfJung]

The fact that this ever worked is kind of an accident that was introduced by #57997...

but yes, if you think this needs to keep working, then #81238 needs to be reverted (and some more ptr functions should probably be changed to call the intrinsic directly).

[Mark-Simulacrum]

Bumping milestone to 1.53 as 1.52 should be fixed by the revert of #81238 in #84864. We may want to consider breaking this code, but I don't want to do that in the few days before a release; I suspect it's a hard sell to break working code here though.

[Mark-Simulacrum]

Re-classifying as P-critical, as this is observable on non-nightly builds. Nominating for T-lang; I'd like to get a read on whether there's something special about intrinsics that should prevent reification here (i.e., is this "just something that needs to be fixed" or something underlying).

fn main() {
    let _unused = if true {
        core::ptr::copy::<i32>
    } else {
        core::ptr::copy_nonoverlapping::<i32>
    };
}

[RalfJung]

whether there's something special about intrinsics that should prevent reification here

From a compiler implementation perspective, intrinsics are not real functions, they are more like "custom MIR statements". No function call code is ever generated when you "call" an intrinsic.

But in principle, for fully monomorphic uses of intrinsics I see no reason why we couldn't auto-generate little shim functions that call the intrinsic. The user can already write these functions manually, so this is no increase in expressivity, just more convenient.

[Mark-Simulacrum]

Yeah, I think that's true, but I want to check-in -- not sure if T-lang is the right set of folks either, maybe we'll leave it nominated for T-compiler. Certainly there's some overlap anyway.

I suspect the auto-generation of shim functions isn't something we'd be willing to backport, so likely we'll want to apply the revert to 1.53 (beta) and 1.54 (master), and then land the fix for intrinsic reification, and then land the original (set of) PRs.

[apiraino]

Probably a glitch of github didnt apply the new priority label as suggested by @Mark-Simulacrum

@rustbot label -label:P-medium label:P-critical

[rustbot]

Error: Parsing relabel command in comment failed: ...'bel -label' | error: a label delta at >| ':P-medium '...

Please let @rust-lang/release know if you're having trouble with this bot.

[apiraino]

Issue was mentioned during T-compiler meeting and discussion will happen in a dedicated Zulip topic

[pnkfelix]

Yeah, I think that's true, but I want to check-in -- not sure if T-lang is the right set of folks either, maybe we'll leave it nominated for T-compiler. Certainly there's some overlap anyway.

I suspect the auto-generation of shim functions isn't something we'd be willing to backport, so likely we'll want to apply the revert to 1.53 (beta) and 1.54 (master), and then land the fix for intrinsic reification, and then land the original (set of) PRs.

(Just an update: as mentioned in yesterday's compiler team meeting, and the plan outlined above is what we are going to do. I am doing it.)

[pnkfelix]

Also, the discussion about what to do in the long term is taking place on zulip

[pnkfelix]

We have now implemented the short term plan of again making copy and copy_nonoverlapping fn's rather than intrinsics. PR #86003 on mainline, PR #86203 on beta.

Closing as fixed.