What about: StorageDead/StorageLive?

[RalfJung]

We should properly document the semantics of StroageDead/StorageLive. One interesting aspect here is that this includes documenting when they are emitted! In some sense, this is observable via UB, and as such subject to stabilization guarantees.

Related issues/posts:

• A blog post of mine about the topic.
• Open question around the semantics of StorageLive.
• drop implicitly does a StorageDead.
• an older discussion of the subject of StorageLive.

[JakobDegen]

Closing this issue. It's unclear what exactly this is tracking. The Mir semantics of storage statements are relatively fixed nowadays, which seems to be what this is mostly about.

[RalfJung]

I think there are still some open questions:

• When do we guarantee these to be emitted?
• Is it legal to do StorageDead on an already dead local? Currently rustc generates that but maybe we should fix that: StorageDead is not dominated by StorageLive rust#98896.
• Is it legal to do StorageLive on an already live local? Currently Miri says no, but that's inconsistent with the previous item. (Cc Should we allow StorageLive on a live local? rust#99160)

[RalfJung]

@JakobDegen you had objections yesterday against claiming that this is pending-design, that I didn't fully understand.

I think one thing we have to do, at least, is decide whether rust-lang/rust#98896 is a bug in MIR building or not. We have some consumers preferring if it was a bug (Prusti, MiniRust), but it is very possible that MIR optimizations would prefer this to be allowed. I also don't know how hard it would be to have MIR building not generate double-free of locals; that could also be prohibitive.

[chorman0773]

IDK about Jake, but my objection was that StorageDead/StorageLive generation in rustc MIR building is very much a rustc implementation detail, since how it's generated and it's rules are going to be very rustc-specific. Rustc could also change those rules as it sees fit.

If there's desire to translate MIR to MiniRust, the translation tool can handle that if the rules for rustc-mir-storagedead is different than the minirust-storagedead.

[JakobDegen]

@chorman0773 I don't think that's quite correct. Where exactly in control flow locals are allocated and deallocated is visible to users by looking at addresses. Although the particular details of storage statements might be rustc internals (see below), their general existence and semantics is definitely not.

That being said, @RalfJung , my concern is that it's not clear to me that these need to be minirust concerns. ie, why does the minirust answer to your question need to be consisent with the rustc answer?

[RalfJung]

IDK about Jake, but my objection was that StorageDead/StorageLive generation in rustc MIR building is very much a rustc implementation detail, since how it's generated and it's rules are going to be very rustc-specific. Rustc could also change those rules as it sees fit.

There is clearly something here that is not just an implementation detail. The following code has UB:

let x = {
  let y = 0;
  addr_of!(y)
};
x.read();

The reason this has UB, in my view, is that the lowering from surface Rust to the lower-level spec language (a la MiniRust) has a StorageDead(y) at the end of the scope of y. But where and when exactly are these StorageDead introduced? Clearly we need to spec this.

why does the minirust answer to your question need to be consisent with the rustc answer?

I guess it doesn't. Though so far MiniRust is very consistent with MIR (on the syntactic fragment they share), so any deviation needs justification IMO.

[tmiasko]

Making StorageLive on already live local not legal is also problematic for inlining (cc rust-lang/rust#117733).

Consider a callee located in a loop, and which might return while some locals are still live. Inlining would have to insert code that executes StorageDead for locals that are live when callee returns.

Or alternatively it invites a further restriction that makes it not legal to return while there are locals that are still live.

[RalfJung]

Or alternatively it invites a further restriction that makes it not legal to return while there are locals that are still live.

I had considered that for MiniRust, basically making everything fully explicit.

Good point about inlining though!

Personally I don't really care either way, I just don't like the current inconsistency where "StorageLive on live local" is forbidden but "StorageDead on dead local" is allowed. We do have some MIR consumers that would appreciate the strict semantics. I have no idea how hard it would be to get MIR building to always emit storage annotations in well-scoped pairs. Why does it currently sometimes have too many / too few StorageDead? Doesn't it have to have the scope information already anyway?

[tmiasko]

Personally, I think all those should be legal:

• StorageDead on already dead local
• StorageLive on already live local
• Return when they are still live locals

For the most part, most violations have been quite harmless. With restrictions they become critical issues.

Why does it currently sometimes have too many / too few StorageDead? Doesn't it have to have the scope information already anyway?

• StorageLive of live local when building tokio with -Zcross-crate-inline-threshold=always + high MIR inlining threshold rust#117733 - StateTransform forgot to insert StorageDead when transforming yields into returns.
• StorageDead is not dominated by StorageLive rust#98896 - cursory look suggest that for temporaries StorageLive is inserted when building particular expression, but StorageDead is inserted when leaving associated scope (so with non-trivial control flow StorageLive no longer dominates StorageDead).

[RalfJung]

Resolved by rust-lang/rust#126154