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

Should Miri report MIR validation failures? #2840

Closed
cbeuw opened this issue Apr 12, 2023 · 4 comments · Fixed by #3486
Closed

Should Miri report MIR validation failures? #2840

cbeuw opened this issue Apr 12, 2023 · 4 comments · Fixed by #3486

Comments

@cbeuw
Copy link
Contributor

cbeuw commented Apr 12, 2023

rustc contains a MIR validation pass which detects ill-formed MIR. Currently Miri does not report MIR validation failures, but should it?

For instance, in runtime MIR, Deref must the the first projection if one exists. This code (rust-lang/rust#110228) performs a dereference after a field projection, so it fails MIR validation, but Miri runs this without any complaint

#![feature(custom_mir, core_intrinsics)]
extern crate core;
use core::intrinsics::mir::*;
#[custom_mir(dialect = "runtime", phase = "optimized")]
pub fn fn0() {
    mir! {
        let x: i32;
        let tuple: (*mut i32,);
        {
            tuple.0 = core::ptr::addr_of_mut!(x);
            *(tuple.0) = 1;
            Return()
        }
    }
}
pub fn main() {
    fn0();
}
@RalfJung
Copy link
Member

I am not entirely sure why that validation pass is skipped in Miri. @oli-obk is this some accident in MIR pass management?

@oli-obk
Copy link
Contributor

oli-obk commented Apr 12, 2023

That's really odd... I looked into it and it makes no sense. We fetch optimized_mir, which must run a validation pass. I'll need to actually compile a rustc and miri with debug asserts and look at some logs

@matthiaskrgr
Copy link
Member

I had a look since I already have a rustc+miri with debug assertions laying around and I didn't see anything interesting 🤔

MIRIFLAGS="-Zvalidate-mir" RUSTFLAGS="-Zvalidate-mir" ~/.cargo/bin/cargo +local-debug-assertions miri run

Preparing a sysroot for Miri (target: x86_64-unknown-linux-gnu)... done
WARNING: Ignoring `RUSTC_WRAPPER` environment variable, Miri does not support wrapping.
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `/home/matthias/.rustup/toolchains/local-debug-assertions/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/miritest`

@RalfJung
Copy link
Member

If I try this with today's Miri, I do get the expected ICE.

@bors bors closed this as completed in 4373a8e Apr 18, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this issue Apr 20, 2024
add test checking that we do run MIR validation

Fixes rust-lang/miri#2840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants