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

Bad MIR: StorageLive on a local that was already live with opt-level=4 #107511

Closed
RalfJung opened this issue Jan 31, 2023 · 3 comments · Fixed by #107524
Closed

Bad MIR: StorageLive on a local that was already live with opt-level=4 #107511

RalfJung opened this issue Jan 31, 2023 · 3 comments · Fixed by #107524
Assignees
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jan 31, 2023

Consider this code:

fn main() {
    let mut sum = 0;
    let a = [0, 10, 20, 30];
    for i in 0..a.len() {
        sum += a[i];
    }
    println!("{sum}");
}

When running this in Miri with -Zmir-opt-level=4 -O, we get an error:

error: Undefined Behavior: StorageLive on a local that was already live
 --> tests/pass/loops.rs:4:9
  |
4 |     for i in 0..a.len() {
  |         ^ StorageLive on a local that was already live
  |
  = 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 tests/pass/loops.rs:4:9: 4:10

This is a fairly recent regression (ad48c10 was good, a322848 is bad; commit range: ad48c10...a322848).

Cc @rust-lang/wg-mir-opt

@RalfJung
Copy link
Member Author

#106908 landed in that range, seems like the most likely culprit? Cc @cjgillot

@cjgillot cjgillot self-assigned this Jan 31, 2023
@bjorn3 bjorn3 added the A-mir-opt Area: MIR optimizations label Jan 31, 2023
@Noratrieb Noratrieb added the C-bug Category: This is a bug. label Jan 31, 2023
@matthiaskrgr
Copy link
Member

@RalfJung what does the -O flag do? I couldn't find it mentioned in the readme. :/

@RalfJung
Copy link
Member Author

RalfJung commented Jan 31, 2023

That's the regular rustc -O, turning on optimizations. Miri inherits all rustc flags.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 2, 2023
Remove both StorageLive and StorageDead in CopyProp.

Fixes rust-lang#107511

rust-lang#106908 removed StorageDead without the accompanying StorageLive. In loops, execution would see repeated StorageLive, without any StorageDead, which is UB.

So when removing storage statements, we have to remove both StorageLive and StorageDead.

~I also added a MIR validation pass for StorageLive. It may be a bit overzealous.~
@bors bors closed this as completed in 6917040 Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt Area: MIR optimizations C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants