rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.49k stars 12.6k forks source link

unstable fingerprints for optimized_mir #80336

Closed KalitaAlexey closed 3 years ago

KalitaAlexey commented 3 years ago

Code

git clone -b v2.0.0 --depth 1 https://github.com/substrate-developer-hub/substrate-node-template

Steps

rustup target add wasm32-unknown-unknown --toolchain nightly-2020-10-05
export RUSTFLAGS="-Z incremental-info=yes -Z incremental-verify-ich=yes"
cargo build
Add a comment (modify a file with no actual impact)
cargo build

Meta

rustc --version --verbose:

rustc 1.49.0-nightly (beb5ae474 2020-10-04)
binary: rustc
commit-hash: beb5ae474d2835962ebdf7416bd1c9ad864fe101
commit-date: 2020-10-04
host: x86_64-apple-darwin
release: 1.49.0-nightly
LLVM version: 11.0

Error output

thread 'rustc' panicked at 'found unstable fingerprints for optimized_mir(node_template_runtime[3b47]::_::{impl#0}::decode)', /rustc/beb5ae474d2835962ebdf7416bd1c9ad864fe101/compiler/rustc_query_system/src/query/plumbing.rs:555:5
Backtrace

``` stack backtrace: 0: _rust_begin_unwind 1: std::panicking::begin_panic_fmt 2: rustc_query_system::query::plumbing::incremental_verify_ich 3: rustc_query_system::query::plumbing::load_from_disk_and_cache_in_memory 4: rustc_data_structures::stack::ensure_sufficient_stack 5: rustc_query_system::query::plumbing::get_query_impl 6: rustc_metadata::rmeta::encoder::EncodeContext::encode_optimized_mir 7: ::visit_item 8: rustc_hir::hir::Crate::visit_all_item_likes 9: rustc_metadata::rmeta::encoder::EncodeContext::encode_crate_root 10: rustc_metadata::rmeta::encoder::encode_metadata_impl 11: rustc_data_structures::sync::join 12: rustc_metadata::rmeta::decoder::cstore_impl::::encode_metadata 13: rustc_middle::ty::context::TyCtxt::encode_metadata 14: rustc_interface::passes::QueryContext::enter 15: rustc_interface::queries::Queries::ongoing_codegen 16: rustc_interface::queries::::enter 17: rustc_span::with_source_map 18: rustc_interface::interface::create_compiler_and_run 19: scoped_tls::ScopedKey::set ```

camelid commented 3 years ago

Meta

rustc --version --verbose:

rustc 1.49.0-nightly (beb5ae474 2020-10-04)
binary: rustc
commit-hash: beb5ae474d2835962ebdf7416bd1c9ad864fe101
commit-date: 2020-10-04
host: x86_64-apple-darwin
release: 1.49.0-nightly
LLVM version: 11.0

@KalitaAlexey Can you please try with a recent nightly and see if it works? E.g. try with 2020-12-22.

KalitaAlexey commented 3 years ago

@camelid

rustc --version --verbose
rustc 1.50.0-nightly (7f9c43cf9 2020-12-23)
binary: rustc
commit-hash: 7f9c43cf98cfe1c369045399929cb098155b8374
commit-date: 2020-12-23
host: x86_64-pc-windows-msvc
release: 1.50.0-nightly
git clone https://github.com/paritytech/substrate.git
cd substrate/bin/node-template/node
export RUSTFLAGS="-Z incremental-info=yes -Z incremental-verify-ich=yes"
cargo build
// Make a change to src/lib.rs
cargo build
cargo build
   Compiling node-template v2.0.0 (D:\Projects\Rust\substrate\bin\node-template\node)
[incremental] session directory: 259 files hard-linked
[incremental] session directory: 0 files copied
thread 'rustc' panicked at 'found unstable fingerprints for evaluate_obligation(537eb1fcc2ac3041-cafb908c05fc9c89)', /rustc/7f9c43cf98cfe1c369045399929cb098155b8374\compiler\rustc_query_system\src\query\plumbing.rs:566:5
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/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md
note: rustc 1.50.0-nightly (7f9c43cf9 2020-12-23) running on x86_64-pc-windows-msvc
note: compiler flags: -Z incremental-info=yes -Z incremental-verify-ich=yes -C embed-bitcode=no -C debuginfo=2 -C incremental --crate-type lib
note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [evaluate_obligation] evaluating trait selection obligation `sc_service::Arc<dyn sp_state_machine::trie_backend_essence::Storage<sp_runtime::traits::BlakeTwo256>>: sp_state_machine::trie_backend_essence::TrieBackendStorage<sp_runtime::traits::BlakeTwo256>`
#1 [normalize_projection_ty] normalizing `Canonical { max_universe: U0, variables: [], value: ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: All }, value: ProjectionTy { substs: [sc_client_db::storage_cache::SyncingCachingState<sc_client_db::RefTrackingState<sp_runtime::generic::Block<sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>, sp_runtime::OpaqueExtrinsic>>, sp_runtime::generic::Block<sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>, sp_runtime::OpaqueExtrinsic>>, sp_runtime::traits::BlakeTwo256], item_def_id: DefId(195:26 ~ sp_state_machine[4107]::backend::Backend::Transaction) } } }`
end of query stack
error: could not compile `node-template`
Aaron1011 commented 3 years ago

This looks like a legitimate issue with evaluate_obligation. In the initial successful run, we get the following result (I added some extra logging locally):

DEBUG rustc_traits::evaluate_obligation evaluate_obligation(Canonical {
    max_universe: U0,
    variables: [
        CanonicalVarInfo {
            kind: Region(
                U0,
            ),
        },
    ],
    value: ParamEnvAnd {
        param_env: ParamEnv {
            caller_bounds: [],
            reveal: All,
        },
        value: TraitPredicate(<sc_service::Arc<dyn sp_state_machine::trie_backend_essence::Storage<sp_runtime::traits::BlakeTwo256>> as sp_state_machine::trie_backend_essence::TrieBackendStorage<sp_runtime::traits::BlakeTwo256>>),
    },
}) = Ok(EvaluatedToOk)

On the second (modified incremental) run, we get this result:

DEBUG rustc_traits::evaluate_obligation evaluate_obligation(Canonical {
    max_universe: U0,
    variables: [
        CanonicalVarInfo {
            kind: Region(
                U0,
            ),
        },
    ],
    value: ParamEnvAnd {
        param_env: ParamEnv {
            caller_bounds: [],
            reveal: All,
        },
        value: TraitPredicate(<sc_service::Arc<dyn sp_state_machine::trie_backend_essence::Storage<sp_runtime::traits::BlakeTwo256>> as sp_state_machine::trie_backend_essence::TrieBackendStorage<sp_runtime::traits::BlakeTwo256>>),
    },
}) = Ok(EvaluatedToOkModuloRegions)

The return value of evaluate_obligation on the same input differs between compilation sessions - we get EvaluatedToOk with an empty incremental cache, but we get EvaluatedToOkModuloRegions with a partially outdated cache.

Aaron1011 commented 3 years ago

The issue is caused by the way that we handle regions in the evaluation cache. When we insert a result into the evaluation cache (either infcx.evaluation_cache or tcx.evaluation_cache, we use a 'freshened' version of the original TraitPredicate as the key. The 'freshened' TraitPredicate has all non-late-bound regions erased.

Unfortunately, this can lead to issues if we try to evaluate the following two predicates:

impl SomeTrait for SomeType<'static>

SomeType<'static> as SomeTrait
SomeType<'#_r> as SomeTrait

When we evaluate SomeType<'static> as SomeTrait, we'll get EvaluatedToOk, since the region parameter in our trait ref is known to match the impl. We will then cache the result as <SomeType<'erased> as SomeTrait> -> EvaluatedToOk.

If we later try to evaluate SomeType<'#_r> as SomeTrait, we will end up matching the evaluation cache entry, giving us a result of EvaluatedToOk. However, we should have gotten a result of EvaluatedToOkModuloRegions, since we don't know that '#_r == 'static holds.

This is really difficult to observe in practice, for a number of reasons:

As a result, I haven't been able to minimize this.

Aaron1011 commented 3 years ago

cc @matthewjasper

apiraino commented 3 years ago

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

apiraino commented 3 years ago

@rustbot ping cleanup

might be interesting to follow up on the great @Aaron1011 analysis and see how can be reproduced on a smaller scale, possibly checking if it's a WASM target only issue and if it also happen on other than nightlies

rustbot commented 3 years ago

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good "Cleanup ICE-breaking candidate". In case it's useful, here are some instructions for tackling these sorts of bugs. Maybe take a look? Thanks! <3

cc @AminArria @camelid @chrissimpkins @contrun @DutchGhost @elshize @ethanboxx @h-michael @HallerPatrick @hdhoang @hellow554 @imtsuki @JamesPatrickGill @kanru @KarlK90 @LeSeulArtichaut @MAdrianMattocks @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

matthiaskrgr commented 3 years ago

I was able to reproduce this on x86 in the rust-analyzer repo with -Zincremental-verify-ich=yes, so probably unrelated to wasm.

in the rust analyzer repo:

git checkout ca76a77791d2933770ecf9479eea407bfe5a9946
RUSTFLAGS="-Zincremental-verify-ich=yes" cargo build
git checkout 99ec2f623
RUSTFLAGS="-Zincremental-verify-ich=yes" cargo build
=> ICE

I am able to reproduce on 1.48.0 (7eac88abb 2020-11-16) 1.50.0-beta.1 (05b602367 2020-12-29) and 1.51.0-nightly (158f8d034 2020-12-29) if I enforce the -Zincremental-verify-ich=yes flag.

edit: also reproduces with 1.47 and 1.46

LeSeulArtichaut commented 3 years ago

Adjusted the labels and title according to Aaron's analysis, the big question regarding this issue being:

is it possible to weaponize this into extending a lifetime?

matthiaskrgr commented 3 years ago

Much smaller repro:

git clone https://github.com/rust-lang/regex/
cd regex
git checkout fc3e6aa 
RUSTFLAGS="-Zincremental-verify-ich=yes" cargo build
git checkout 0e96af4 
RUSTFLAGS="-Zincremental-verify-ich=yes" cargo build
thread 'rustc' panicked at 'found unstable fingerprints for optimized_mir(regex[ea4b]::vector::avx2::{impl#0}::new)', /rustc/e2267046859c9ceb932abc983561d53a117089f6/compiler/rustc_query_system/src/query/plumbing.rs:566:5
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/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.51.0-nightly (e22670468 2020-12-30) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z incremental-verify-ich=yes -C embed-bitcode=no -C debuginfo=2 -C incremental --crate-type lib

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

query stack during panic:
#0 [optimized_mir] optimizing MIR for `vector::avx2::AVX2VectorBuilder::new`
#1 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: could not compile `regex`
matthiaskrgr commented 3 years ago

With the sample above, I can reproduce the ICE by inserting an empty line in the project inside AVX2VectorBuilder::new() :thinking: ( check out regex fc3e6aa19aaaf929fd89395ee976456b5b3a8720 , add the line and build with incr verification)

diff --git a/src/vector/avx2.rs b/src/vector/avx2.rs
index 5228c60..b7d10d5 100644
--- a/src/vector/avx2.rs
+++ b/src/vector/avx2.rs
@@ -12,6 +12,7 @@ impl AVX2VectorBuilder {
         if is_x86_feature_detected!("avx2") {
             Some(AVX2VectorBuilder(()))
         } else {
+
             None
         }
     }
matthiaskrgr commented 3 years ago

reduced as lib.rs

#![allow(dead_code)]

use std::arch::x86_64::*;

pub struct AVX2VectorBuilder(());

impl AVX2VectorBuilder {
    pub fn new() -> Option<AVX2VectorBuilder> {
        if is_x86_feature_detected!("avx2") {
            Some(AVX2VectorBuilder(()))
        } else {
            None
        }
    }
}

#[repr(transparent)]
pub struct A {
    a: i32
}

adding empty lines to this file will cause the ICE

matthiaskrgr commented 3 years ago

Reduced even further

pub fn f() {
     let _ = is_x86_feature_detected!("avx2");
}
Aaron1011 commented 3 years ago

It looks like there are actually two different issues going on here. The original issue is a bug with caching in evaluate_obligation, described in https://github.com/rust-lang/rust/issues/80336#issuecomment-751311417

The other is a bug that shows up as a changed optimized_mir result. This appears to be completely unrelated, as the caching bug described above shouldn't result in any changes to the MIR of a function. I think this bug is actually due to the way we handle hygiene information in incremental compilation - it goes away if is_x86_feature_detected is replaced with the result from cargo expand. I came across a very similar bug in some other work on incremental compilation, and I'll continue to investigate.

Aaron1011 commented 3 years ago

Let's continue the discussion of the trait evaluation issue in https://github.com/rust-lang/rust/issues/80691

guotie commented 3 years ago

   Compiling substrate-test-runtime v2.0.0 (/home/thales/substrate/test-utils/runtime)
thread 'rustc' panicked at 'found unstable fingerprints for evaluate_obligation(f4ab1f83ea5f30bc-7172c6b02018f3b6): Ok(EvaluatedToOk)', /rustc/88f19c6dab716c6281af7602e30f413e809c5974/compiler/rustc_query_system/src/query/plumbing.rs:593:5
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/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.52.0 (88f19c6da 2021-05-03) running on x86_64-unknown-linux-gnu

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

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

query stack during panic:
#0 [evaluate_obligation] evaluating trait selection obligation `pallet_transaction_payment::ChargeTransactionPayment<Runtime>: REWARD_CURVE::_sp_runtime::traits::SignedExtension`
#1 [normalize_projection_ty] normalizing `Canonical { max_universe: U0, variables: [], value: ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: All }, value: ProjectionTy { substs: [REWARD_CURVE::_sp_runtime::generic::Block<REWARD_CURVE::_sp_runtime::generic::Header<u32, REWARD_CURVE::_sp_runtime::traits::BlakeTwo256>, REWARD_CURVE::_sp_runtime::generic::UncheckedExtrinsic<REWARD_CURVE::_sp_runtime::MultiAddress<REWARD_CURVE::_sp_runtime::AccountId32, u32>, Call, REWARD_CURVE::_sp_runtime::MultiSignature, (frame_system::CheckSpecVersion<Runtime>, frame_system::CheckTxVersion<Runtime>, frame_system::CheckGenesis<Runtime>, frame_system::CheckMortality<Runtime>, frame_system::CheckNonce<Runtime>, frame_system::CheckWeight<Runtime>, pallet_transaction_payment::ChargeTransactionPayment<Runtime>)>>], item_def_id: DefId(203:1528 ~ sp_runtime[653f]::traits::Block::Extrinsic) } } }`
end of query stack
error: could not compile `node-runtime`

cpuinfo:

processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 85
model name      : Intel(R) Xeon(R) Platinum 8269CY CPU @ 2.50GHz
stepping        : 7
microcode       : 0x1
cpu MHz         : 2499.998
cache size      : 36608 KB
physical id     : 0
siblings        : 2
core id         : 0
cpu cores       : 1
apicid          : 0
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 22
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid tsc_known_freq pni pclmulqdq monitor ssse3 fma cx16 pcid sse4_1 sse4_2 x2apic movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx avx512f avx512dq rdseed adx smap avx512cd avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves arat avx512_vnni
bugs            : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf mds swapgs taa itlb_multihit
bogomips        : 4999.99
clflush size    : 64
cache_alignment : 64
address sizes   : 46 bits physical, 48 bits virtual
power management:

processor       : 1
vendor_id       : GenuineIntel
cpu family      : 6
model           : 85
model name      : Intel(R) Xeon(R) Platinum 8269CY CPU @ 2.50GHz
stepping        : 7
microcode       : 0x1
cpu MHz         : 2499.998
cache size      : 36608 KB
physical id     : 0
siblings        : 2
core id         : 0
cpu cores       : 1
apicid          : 1
initial apicid  : 1
fpu             : yes
fpu_exception   : yes
cpuid level     : 22
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ss ht syscall nx pdpe1gb rdtscp lm constant_tsc rep_good nopl nonstop_tsc cpuid tsc_known_freq pni pclmulqdq monitor ssse3 fma cx16 pcid sse4_1 sse4_2 x2apic movbe popcnt aes xsave avx f16c rdrand hypervisor lahf_lm abm 3dnowprefetch invpcid_single pti fsgsbase tsc_adjust bmi1 hle avx2 smep bmi2 erms invpcid rtm mpx avx512f avx512dq rdseed adx smap avx512cd avx512bw avx512vl xsaveopt xsavec xgetbv1 xsaves arat avx512_vnni
bugs            : cpu_meltdown spectre_v1 spectre_v2 spec_store_bypass l1tf mds swapgs taa itlb_multihit
bogomips        : 4999.99
clflush size    : 64
cache_alignment : 64
address sizes   : 46 bits physical, 48 bits virtual
power management: