rust-lang / rust

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

Missing StorageLive and StorageDead Statements for some locals #67400

Open Skasselbard opened 4 years ago

Skasselbard commented 4 years ago

It seems that the StorageLive and StorageDead statements are not generated for every local of a function.

I tried this code:

pub fn main() {
    let mut x = 5;
    x = call(x);
}

fn call(i: usize) -> usize {
    i * 2
}

I expected to see something like this happen:

fn  call(_1: usize) -> usize {
    let mut _0: usize;
    let mut _2: usize;
    let mut _3: (usize, bool);

    bb0: {
        StorageLive(_2);
        _2 = _1;
        StorageLive(_3);  // ADDED
        _3 = CheckedMul(move _2, const 2usize);
        assert(!move (_3.1: bool), "attempt to multiply with overflow") -> bb1;
    }

    bb1: {
        _0 = move (_3.0: usize);
        StorageDead(_2);
        StorageDead(_3);  // ADDED
        return;
    }
}

Instead, this happened:

fn  call(_1: usize) -> usize {
    let mut _0: usize;
    let mut _2: usize;
    let mut _3: (usize, bool);

    bb0: {
        StorageLive(_2);
        _2 = _1;
        _3 = CheckedMul(move _2, const 2usize);
        assert(!move (_3.1: bool), "attempt to multiply with overflow") -> bb1;
    }

    bb1: {
        _0 = move (_3.0: usize);
        StorageDead(_2);
        return;
    }
}

Meta

rustc --version --verbose: rustc 1.41.0-nightly (25d8a9494 2019-11-29) binary: rustc commit-hash: 25d8a9494ca6d77361e47c1505ecf640b168819e commit-date: 2019-11-29 host: x86_64-unknown-linux-gnu release: 1.41.0-nightly LLVM version: 9.0

Reproducable on [Rust Playgound](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&code=pub%20fn%20main()%20%7B%0A%20%20%20%20let%20mut%20x%20%3D%205%3B%0A%20%20%20%20x%20%3D%20call(x)%3B%0A%7D%0A%0Afn%20call(i%3A%20usize)%20-%3E%20usize%20%7B%0A%20%20%20%20i%20*%202%0A%7D%0A) in stable and nightly channel

RalfJung commented 4 years ago

As far as I know this is deliberate. Cc @rust-lang/wg-mir-opt for some MIR experts.

eddyb commented 4 years ago

This is arguably a bug, although for codegen this doesn't usually matter since _3 is never on the stack.

That's not a reason to not have Storage{Live,Dead} but rather why this hasn't caused enough problems to be changed. IIUC, only miri would take advantage of Storage{Live,Dead} for _3.

Skasselbard commented 4 years ago

I saw something similar in other places. If I remember correctly the return place of intrinsic and/or foreign functions (like calls to the pthread library in mutexes) missed the storage statements as well. Something like the _x in this pseudo mir:

bbn:
    // statements
    _x = ffcall() -> somebb
}
Skasselbard commented 4 years ago

From my point of view, the mir graph is a nice intermediate representation that can act as source for a translation to other targets. Especially because all code generation (macros and generics) was done in previous compiler phases and because there are only few language constructs that have to be translated (mainly statements and terminators). I used it as source to translate into Petri-Nets for verification purposes.

If you see mir as a simple machine-readable source representation as base for all kind of translations (and not just as a component of the compiler), it makes sense to stay consistent (or document a rule that describes this behaviour). Even though it might not impact the compilation process.