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

Inconsistent behavior in irrefutable or-patterns due to UB-removing MIR opt #4237

Open
theemathas opened this issue Mar 27, 2025 · 2 comments · May be fixed by rust-lang/rust#139042
Open

Inconsistent behavior in irrefutable or-patterns due to UB-removing MIR opt #4237

theemathas opened this issue Mar 27, 2025 · 2 comments · May be fixed by rust-lang/rust#139042

Comments

@theemathas
Copy link

theemathas commented Mar 27, 2025

I ran the following code under miri, and it reported no errors:

use std::mem::MaybeUninit;
fn main() {
    let uninit: MaybeUninit<i32> = MaybeUninit::uninit();
    let bad_ref: &i32 = unsafe { uninit.assume_init_ref() };
    let &(0 | _) = bad_ref;
}

However, changing the 0 to 0.. causes Miri to report undefined behavior:

use std::mem::MaybeUninit;
fn main() {
    let uninit: MaybeUninit<i32> = MaybeUninit::uninit();
    let bad_ref: &i32 = unsafe { uninit.assume_init_ref() };
    let &(0.. | _) = bad_ref;
}

The error reported for the second version of the code:

error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
 --> src/main.rs:5:11
  |
5 |     let &(0.. | _) = bad_ref;
  |           ^^^ using uninitialized data, but this operation requires initialized memory
  |
  = 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
  = note: BACKTRACE:
  = note: inside `main` at src/main.rs:5:11: 5:14

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

I expected both versions of the code to behave the same.

Discovered while experimenting with rust-lang/rust#138973
cc @cyrgani @meithecatte

Reproducible on the playground with version 1.87.0-nightly (2025-03-26 a2e63569fd6702ac5dd0)

@compiler-errors
Copy link
Member

This seems to be due to SimplifyCfg pass removing a read on the built MIR:

use std::mem::MaybeUninit;
fn main() {
    let uninit: MaybeUninit<i32> = MaybeUninit::uninit();
    let bad_ref: &i32 = unsafe { uninit.assume_init_ref() };
    let &(0 | _) = bad_ref;
}

which originally has:

// Other stuff...

bb2: {
    // Other stuff...
    switchInt(copy (*_2)) -> [1: bb4, otherwise: bb3];
}

bb3: {
    goto -> bb7;
}

bb4: {
    goto -> bb7;
}

The read of *_2 is eliminated in the pass and the terminator is replaced with a goto to bb7. I think at least for the purposes of miri, the SimplifyCfg pass could preserve this read somehow?

@RalfJung
Copy link
Member

RalfJung commented Mar 27, 2025

Miri disables all MIR optimizations. So if the compiler then still performs optimizations that remove UB, I think that's a bug in the compiler, yes. mir-opt-level 0 must preserve all UB.

@RalfJung RalfJung changed the title Inconsistent behavior in irrefutable or-patterns Inconsistent behavior in irrefutable or-patterns due to UB-removing MIR opt Mar 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 28, 2025
…chint, r=<try>

Do not remove trivial `SwitchInt` with mir-opt-level=0

When mir-opt-level=0, do not optimize out `SwitchInt` terminators that all have the same terminator since it may remove a read which affects miri's ability to detect UB on that operand.

cc `@RalfJung`

Fixes rust-lang/miri#4237

This affects some tests... I guess I could mark them as `mir-opt-level=1`? Not sure.
bors added a commit to rust-lang-ci/rust that referenced this issue Mar 30, 2025
…chint, r=<try>

Do not remove trivial `SwitchInt` with mir-opt-level=0

When mir-opt-level=0, do not optimize out `SwitchInt` terminators that all have the same terminator since it may remove a read which affects miri's ability to detect UB on that operand.

cc `@RalfJung`

Fixes rust-lang/miri#4237

This affects some tests... I guess I could mark them as `mir-opt-level=1`? Not sure.
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.

3 participants