rust-lang / rust

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

Improve diagnostics of `evaluation of constant value failed` errors #85155

Closed Herschel closed 1 year ago

Herschel commented 3 years ago

Given the following code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=0a24628f94a07753519f86993f792ddb

#[cfg(target_arch = "x86_64")]
fn main() {
    use std::arch::x86_64::*;
    unsafe {
        let a: __m128 = _mm_setzero_ps();
        let b: __m128 = _mm_setzero_ps();
        let _blended = _mm_blend_ps(a, b, 0x33);
    }
}

The current output is:

error[E0080]: evaluation of constant value failed
 --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/../../stdarch/crates/core_arch/src/macros.rs:8:17
  |
8 |         let _ = 1 / ((IMM >= MIN && IMM <= MAX) as usize);
  |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to divide `1_usize` by zero

error: internal compiler error: src/tools/miri/src/diagnostics.rs:94:21: This error should be impossible in Miri: encountered constants with type errors, stopping evaluation

Pointing to these lines inside std::arch: https://github.com/rust-lang/stdarch/blob/master/crates/core_arch/src/macros.rs#L5-L10

The error message only points to the final site where const evaluation failed in std::arch. Ideally the error would point higher to the calling code that caused the const evaluation. If this error occurs deep within the dependency graph of a project, there's no easy way to deduce which crate is making the problematic call and causing the build to fail. I resorted to grepping .cargo for arch:: and lots of trial and error. Neither --verbose nor RUST_BACKTRACE were helpful. Someone later mentioned that RUSTC_LOG=rustc_mir::interpret=info could help track it down, but this is user-unfriendly.

Impacting this issue in the wild: https://github.com/ejmahler/RustFFT/issues/74 https://github.com/rust-lang/stdarch/issues/1159

1.54.0-nightly

(2021-05-09 ca82264ec7556a6011b9)
Mark-Simulacrum commented 3 years ago

Possibly a regression as a result of the stdarch PR landing, tagging as such so we eventually investigate.

Amanieu commented 3 years ago

cc @rust-lang/wg-const-eval

apiraino commented 3 years ago

I've tried bisecting this error, seems to point out to PR https://github.com/rust-lang/rust/pull/83278

searched nightlies: from nightly-2021-04-01 to nightly-2021-05-11 regressed nightly: nightly-2021-05-09 searched commits: from https://github.com/rust-lang/rust/commit/770792ff8d1ec542e78e77876ac936f43ffb8e05 to https://github.com/rust-lang/rust/commit/881c1ac408d93bb7adaa3a51dabab9266e82eee8 regressed commit: https://github.com/rust-lang/rust/commit/881c1ac408d93bb7adaa3a51dabab9266e82eee8

bisected with cargo-bisect-rustc v0.6.0 Host triple: x86_64-unknown-linux-gnu Reproduce with: ```bash cargo bisect-rustc --start=2021-04-01 --regress error ```
oli-obk commented 3 years ago

Oh wow, that is the error when run with miri. The error in regular builds has no information beyond "const eval failed"

oli-obk commented 3 years ago

Ah, this is a post-monomorphization error. This is already being tracked in https://github.com/rust-lang/rust/issues/73961

While we want to improve post-monomorphization errors, the "right" way to do this is const_evaluatable_checked. Unfortunately that feature is nowhere near ready to even be used unstably.

apiraino commented 3 years ago

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

Amanieu commented 3 years ago

Just to clarify: stdarch was changed to reject invalid const operands to intrinsics which were previously accepted. This was done to align with the behavior of these intrinsics in C compilers.

However the method to do this involves post-monomorphization errors which unfortunately produce very poor error messages (just "const eval failed" with no line info).

oli-obk commented 3 years ago

cross post from zulip

I have an idea for how to create a backtrace for post monomorphization errors. It's not great, but better than the status quo: in the monomorphization collector, we already have that stack available, we just can't feed it into any queries or whatever is producing errors. So we wrap all possibly erroring calls with something that checks the error count before and after the call. If the count went up, print the stack mentioning that the last error occurred at this monomorphization stack. To reduce the noise, only do that if the current item is generic

oli-obk commented 3 years ago

I'd be happy to mentor someone who wants to implement that

lqd commented 3 years ago

@oli-obk I would like to help :)

bjorn3 commented 3 years ago

re https://github.com/rust-lang/rust/issues/85155#issuecomment-840715762: I think that would require evaluating the required consts of a MIR body inside the mono item collector.

Mark-Simulacrum commented 3 years ago

This was a regression but is no longer tracked for a particular release.

pnkfelix commented 1 year ago

Current output as of 1.66:

error[[E0080]](https://doc.rust-lang.org/stable/error-index.html#E0080): evaluation of `core::core_arch::macros::ValidateConstImm::<51, 0, 15>::VALID` failed
  |
  = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

note: the above error was encountered while instantiating `fn std::arch::x86_64::_mm_blend_ps::<51>`
 --> src/main.rs:7:24
  |
7 |         let _blended = _mm_blend_ps(a, b, 0x33);
  |                        ^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0080`.
error: could not compile `playground` due to previous error

This seems leaps and bounds better than it was when this was first filed...

pnkfelix commented 1 year ago

@lqd and @oli-obk: do you think we can close this? (and, if necessary, open a separate tracking issue outlining what further changes you think are necessary to improve error backtraces from const-eval?)

pnkfelix commented 1 year ago

(I suppose at minimum we should add a regression test, to ensure we catch if the output gets worse...)

oli-obk commented 1 year ago

Oh yea, this is fixed now

oli-obk commented 1 year ago

This was fixed in https://github.com/rust-lang/rust/pull/104449