[validate-mir] validate that all accessed locals are initialized

[jonas-schievink]

This uses the MaybeInitializedLocals dataflow analysis to validate that every use of a local happens while that local is initialized.

Currently this has to ignore moves out of locals due to bugs in other passes (ie. still treat moved-out-of locals as initialized). These issues are tracked in #72797 and #72800.

cc @RalfJung

[rust-highfive]

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

[RalfJung]

This uses the MaybeInitializedLocals dataflow analysis to validate that every use of a local happens while that local is initialized.

The name of that pass doesn't sound like it is suited for this? We need a DefinitelyInitializedLocals for that, don't we?

[RalfJung]

Note that even with this check, unsafe code can still do things like

let x = vec![1,2,3];
let xptr = &x as *const _;
move_elsewhere(x);
unsafe { /* look at *xptr. */ }

So, "this local was moved out of" does not mean that its value is unobservable by the program!

[jonas-schievink]

We need a DefinitelyInitializedLocals for that, don't we?

If we wanted this to be more precise here, yes. But part of the motivation for this is to make sure that MaybeInitializedLocals computes the right thing (for generators and destination propagation).

So, "this local was moved out of" does not mean that its value is unobservable by the program!

Hmm, didn't we want to make that UB?

(I don't think that impacts this check though, since the raw pointer will be its own local and accesses through that don't count as a use of x)

[RalfJung]

If we wanted this to be more precise here, yes. But part of the motivation for this is to make sure that MaybeInitializedLocals computes the right thing (for generators and destination propagation).

Please define "right thing". Naively I would expect that it is correct for MaybeInitializedLocals to always say "yes this is maybe initialized". If stronger guarantees are required/provided, are they documented anywhere?

Hmm, didn't we want to make that UB?

Some people want to make it UB. However:

• Even if that is UB, whether or not storage gets reused still is observable via pointer comparison. Because of this, so far nobody has been able to demonstrate a (realistic and useful) optimization that would actually benefit from this being UB.
• The proposals for making this UB so far have had some nasty aspects as well, such as making reads actually mutate memory (which makes them harder to reorder).

See rust-lang/unsafe-code-guidelines#188 for more discussion, and #61849 for a related proposal that doesn't make this UB but achieves something stronger through extra StorageDead/StorageLive.

[jonas-schievink]

Please define "right thing". Naively I would expect that it is correct for MaybeInitializedLocals to always say "yes this is maybe initialized".

Yes, that is correct, and then we couldn't use it for this validation. But since it does actually try to be a bit more precise here, it is still useful here.

The docs for the pass are here, but they just refer to the MaybeInitializedPlaces analysis for borrowck:

rust/src/librustc_mir/dataflow/impls/init_locals.rs

Lines 1 to 3 in 4b1f86a

	//! A less precise version of `MaybeInitializedPlaces` whose domain is entire locals.
	//!
	//! A local will be maybe initialized if *any* projections of that local might be initialized.

[matthewjasper]

Does this handle something like

fn main() {
     let x: String;
}

before drop elaboration runs?

[jonas-schievink]

Oh, this needs to be removed

[jonas-schievink]

@matthewjasper yeah that looks like trouble

    bb0: {
        StorageLive(_1);                 // scope 0 at main.rs:2:10: 2:11
        _0 = const ();                   // scope 0 at main.rs:1:11: 3:2
                                         // ty::Const
                                         // + ty: ()
                                         // + val: Value(Scalar(<ZST>))
                                         // mir::Constant
                                         // + span: main.rs:1:11: 3:2
                                         // + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
        drop(_1) -> bb1;                 // scope 0 at main.rs:3:1: 3:2
    }

For some reason it doesn't ICE due to this though? I think it's better to land this after the passes have been fixed anyways, that way it doesn't require changes to MaybeInitializedLocals.

[bors]

☔ The latest upstream changes (presumably #73081) made this pull request unmergeable. Please resolve the merge conflicts.

[Muirrum]

@jonas-schievink This is a triage bump.