Should we allow StorageLive on a live local?

[RalfJung]

Currently, we disallow StorageLive being called on a local that is already live (both in Miri, and in the MIR docs). This was originally motivated by some very vague wording in LLVM making it unclear whether LLVM would behave properly when there are multiple lifetime.start without intermediate lifetime.end.

However, LLVM has in the mean time clarified that such a redundant lifetime.start is fine, and just resets the contents of the storage to be uninitialized. So @JakobDegen is suggesting that we change our StorageLive semantics for the case where the local is already live: this could be be defined behavior, and it will re-allocate the backing local, potentially giving it a new address and resetting its contents to be uninitialized.

I do not have any fundamental objections to this, but I also don't know if this could cause other problems, or maybe even help in other situations. Having such a "reallocation" primitive has previously been suggested by @tmandry ; the proposed semantics would mean we don't need another primitive. But maybe some MIR opts would prefer the stricter interpretation of the storage annotations? #98896 sounds like @vakaras would prefer a stricter interpretation than what we currently implement for StorageDead, so making StorageLive more relaxed might be a problem for them.

So, let's collect all the potential benefits of the current semantics of StorageLive before deciding that we change it. (And also all benefits of the change, of course.)

Cc @rust-lang/wg-unsafe-code-guidelines @rust-lang/wg-mir-opt

[JakobDegen]

To be clear, for now I don't have a real reason for wanting this besides that it seems like unnecessary UB. I don't know of an optimization that is affected by this in either direction - will think about it though.

[vakaras]

Doing this would imply that the compiler consumers (for example, verifiers) would have to compute themselves whether StorageLive is allocation or reallocation. Compiler consumers can do that, but then they would not be able to check whether the compiler is doing the right thing (for example, I found #98896 just by trying to verify a simple example).

As a selfish guiding design principle, I personally would prefer for the compiler to be as explicit as possible about what it knows and what it does not know because this helps to ensure that compiler consumers interpret data in the same way as the compiler, which is important for the consumers used for quality assurance in critical fields.

[RalfJung]

To be clear, for now I don't have a real reason for wanting this besides that it seems like unnecessary UB. I don't know of an optimization that is affected by this in either direction - will think about it though.

Same here. I just find the current situation unsatisfying -- either we should require proper structure for both StorageLive and StorageDead, or for none of them. Right now we require it for StorageLive but not for StorageDead (that's #98896), and that is just inconsistent.

[jyn514]

@saethlin says "This is a MIR semantics question. We're not sure yet if we want to pin the semantics of Rust on MIR, but t-opsem is a good owner for this issue." I'm going to label MIR semantics issues as T-opsem going forward, feel free to speak up if you think isn't a good fit.

@rustbot label T-opsem

[RalfJung]

#119366 is an argument for allowing StorageLive on an already live local.

[RalfJung]

@rust-lang/opsem I propose that we allow StorageLive when the local is already live, with the semantics that the local is implicitly freed and then re-allocated (with fresh, uninitialized contents).
Cc @rust-lang/wg-mir-opt

Basically, StorageLive will implicitly first execute a StorageDead if needed. This is similar to how we treat StorageDead, which is a NOP when the local is already dead, which is equivalent to implicitly first executing a StorageLive if needed. We have to treat StorageDead that way due to #98896. mir-opts are also sometimes struggling with maintaining the subtle invariant that StorageLive on an already live local is UB. And in particular given the fact that we do allow return to serve as implicit StorageDead for all locals that are still live, the current rules are causing issues for inlining; that's what #119366 is about.

Some out-of-tree MIR consumers would like to use StorageLive/StorageDead as exact markers for the lifetime of local variables, but at this point that seems better served by a dedicated analysis than by trying to make all mir-opts ensure this non-trivial invariant.

@rfcbot close

[rfcbot]

Team member @RalfJung has proposed to close this. The next step is review by the rest of the tagged team members:

• @CAD97
• @JakobDegen
• @RalfJung
• @digama0
• @saethlin

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[digama0]

@RalfJung What is the impact of this change on inlining? Is it now simply a matter of doing nothing to the StorageLive/StorageDead calls or does the unsoundness issue persist?

[RalfJung]

I think doing nothing is sound under this proposal. @tmiasko thought about that more than I did, though.

[CAD97]

I don't fully grasp the extent of the implications on MIR, but this makes sense to me given the context Ralf provided.

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[celinval]

I have a question, in this case, will a StorageLive imply StorageDead of the initial variable? What happens to a raw pointer that holds the address of the previous local?

[digama0]

My understanding is that yes, a StorageLive implies a StorageDead of the initial variable, and this also means that a raw pointer holding the previous address is no longer pointing to a valid allocation. (I think we want the flexibility to take advantage of the re-initialization to be able to change the storage location.)

[RalfJung]

Yes, that's what I wrote:

with the semantics that the local is implicitly freed and then re-allocated (with fresh, uninitialized contents).

[rfcbot]

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.