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

Detect erroneous constants post-monomorphization #1382

Closed
RalfJung opened this issue May 1, 2020 · 5 comments · Fixed by #1504
Closed

Detect erroneous constants post-monomorphization #1382

RalfJung opened this issue May 1, 2020 · 5 comments · Fixed by #1504
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings) I-ICE Impact: makes Miri crash with some ICE

Comments

@RalfJung
Copy link
Member

RalfJung commented May 1, 2020

Codegen recently gained a loop to make sure that all constants required by the MIR actually error. Miri does not have anything like that yet, but we probably need that for soundness.

@RalfJung RalfJung added C-bug Category: This is a bug. A-interpreter Area: affects the core interpreter labels May 1, 2020
@RalfJung
Copy link
Member Author

RalfJung commented May 1, 2020

Here is a testcase that currently ICEs:

#![feature(const_panic)]
#![feature(never_type)]
#![warn(const_err)]
struct PrintName<T>(T);
impl<T> PrintName<T> {
    const VOID: ! = panic!();
}


fn no_codegen<T>() {
    let _ = PrintName::<T>::VOID;
}
fn main() {
    no_codegen::<i32>();
}

@RalfJung
Copy link
Member Author

RalfJung commented May 1, 2020

Even more interestingly, the same code is considered UB by Miri with -Zmir-opt-level=3:

error: Undefined Behavior: entering unreachable code
  --> const_err.rs:11:13
   |
11 |     let _ = PrintName::<T>::VOID;
   |             ^^^^^^^^^^^^^^^^^^^^ entering unreachable code
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

@RalfJung RalfJung added the I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings) label May 1, 2020
@oli-obk

This comment has been minimized.

@RalfJung

This comment has been minimized.

@oli-obk

This comment has been minimized.

@RalfJung RalfJung added the I-ICE Impact: makes Miri crash with some ICE label May 3, 2020
bors added a commit that referenced this issue Aug 10, 2020
accept some post-monomorphization errors

Miri test for rust-lang/rust#75339. We also need to allow `ReferencedConstant` post-monomorphization errors. The other post-monomorphization errors should still be impossible to trigger in Miri.

Fixes #1382.
bors added a commit that referenced this issue Aug 10, 2020
accept some post-monomorphization errors

For #1382, we also need to allow `ReferencedConstant` post-monomorphization errors. The other post-monomorphization errors should still be impossible to trigger in Miri. The fix is not complete though without rust-lang/rust#75339.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 10, 2020
evaluate required_consts when pushing stack frame in Miri engine

[Just like codegen](https://github.com/rust-lang/rust/pull/70820/files#diff-32c57af5c8e23eb048f55d1e955e5cd5R194), Miri needs to make sure all `required_consts` evaluate successfully, to catch post-monomorphization errors.

While at it I also moved the const_eval error reporting logic into rustc_mir::const_eval::error; there is no reason it should be in `rustc_middle`. I kept this in a separate commit for easier reviewing.

Helps with rust-lang/miri#1382. I will add a test on the Miri side (done now: rust-lang/miri#1504).
r? @oli-obk
@bors bors closed this as completed in 1bfb26d Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings) I-ICE Impact: makes Miri crash with some ICE
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants