Must `let` variables (without `mut` or `UnsafeCell`) be read-only?

[RalfJung]

This came up in rust-lang/rust#111502 and overlaps with #257: should it be allowed to mutate let-bound variables, via code like this?

let x = 0i32;
let ptr = addr_of!(x) as *mut i32;
*ptr = 42;

Tree Borrows currently accepts this code since addr_of never generates a new tag, it just creates an alias to the original pointer. Stacked Borrows rejects it due to #257. There are other ways to reject this code while making *const and *mut equivalent, hence I opened this as a separate issue.

Personally I favor the Tree Borrows behavior here and don't think it is worth the extra effort to make these variables read-only.

[chorman0773]

I would certain like for it to be undefined behaviour for a few reasons:

1. I would like to be able to put xlang's const on lowered local variables for non-mutable bindings. This would allow the value of the local to be optimized past an opaque function call given addr_of!(x) rather than &x, which I would expect to be the case (thus allowing the value to be constant propagated, or potentially stored in a register that doesn't get spilled by the call).

2. Going to the Generator/Future and Closure questions, I would like to be able to do const-capture elision, and the best way to define it would be

If the nth capture does not have it's address taken by the closure body, and is a non-mutable binding initialized by a constant expression, the capture type Tn is (), and uses of the capture's name substitutes the value of the constant expression.

The definiton could rule out using addr_or!() on the binding before the closure, but that just gets complicated. See:

let x = true;

if false{
     unsafe{call_opaque_function(addr_of!(x))} // does this affect the closure layout?
}

(|| println!("{}",{x}))(); 

(loops do even more wacky things)

[RalfJung]

the value of the local to be optimized past an opaque function call given addr_of!(x) rather than &x

Right, that is the key optimization difference here (for the case where the opaque fn call takes that ptr-to-x as argument).

I just don't think this optimization is important enough to justify introducing a whole new kind of read-only allocation. Specifying these "read-only" locals will be tricky since of course they are written to, even more than once, to receive their initial value:

let x: (i32, i32);
x.0 = 0;
x.1 = 1;

I think we do want addr_of!/addr_of_mut! to not generate a fresh tag and just return an exact alias of the pointer they start with. This is pretty much required to fix #134. If we take that as a given then making addr_of!(local) not mutable cannot be done by the aliasing model, it has to be a property of the allocation itself. Moreover the allocation cannot be read-only, it would have to be something like write-once-per-location... or we need an explicit MIR statement saying "now mark this memory read-only" (because it has been initialized).

If the nth capture does not have it's address taken by the closure body, and is a non-mutable binding initialized by a constant expression

This issue here is only relevant for cases where the address is taken so this seems to be orthogonal.

Also I am fairly sure we don't want any rules that look anything like this in our op.sem. "Local has its address taken" is not a property that is stable under optimizations. Capture elision, like other optimizations, should fall out of the general properties of the model -- it should not itself be part of the spec.

[chorman0773]

On Sun, May 14, 2023 at 10:38 Ralf Jung ***@***.***> wrote: the value of the local to be optimized past an opaque function call given addr_of!(x) rather than &x Right, that is the key optimization difference here (for the case where the opaque fn call takes that ptr-to-x as argument). I just don't think this optimization is important enough to justify introducing a whole new kind of read-only allocation. Specifying these "read-only" locals will be tricky since of course they are written to, even more than once, to receive their initial value: let x: (i32, i32); x.0 = 0; x.1 = 1; I think we *do* want addr_of!/addr_of_mut! to *not* generate a fresh tag and just return an exact alias of the pointer they start with. This is pretty much required to fix #134 <#134>. If we take that as a given then making addr_of!(local) *not* mutable cannot be done by the aliasing model, it has to be a property of the allocation itself. Moreover the allocation cannot be read-only, it would have to be something like write-once... or we need an explicit MIR statement saying "now mark this memory read-only" (because it has been initialized).

Hmm... Yeah. Partial Init is a problem. For full init, the allocation itself could come into existence when it's initialized possibly. My main reasons I would like it is if it's initialized with a constant expression (or, at least, something I can evaluate as one), but separating that circumstance is definitely challenging.

If the nth capture does not have it's address taken by the closure body, and is a non-mutable binding initialized by a constant expression This issue here is only relevant for cases where the address is taken so this seems to be orthogonal.

Right, but the difference here is whether the address is taken outside of the closure or inside. I'd really like to be able to make the capture rules for layout purposes only care about the initial declaration of the binding and the body of the closure itself.

Also I am fairly sure we don't want any rules that look anything like this in our op.sem. "Local has its address taken" is *not* a property that is stable under optimizations. Capture elision, like other optimizations, should fall out of the general properties of the model -- it should not itself be part of the spec.

Yeah, I should have clarified, this is a potential rule in a future version of lccc's abi spec, not rust. It does apply it's decision making pre-optimization, since the point is that optimizations shouldn't affect something that can move across cgu's in an unstable way. I'd like to be able to justify having that rule in the first instance, since imo const-propagation capture ellision is a useful layout optimization, and that rule is a non-complex way of defining it.

…

— Reply to this email directly, view it on GitHub <#400 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGLD2ZOCFSHBIVIU7SNWE3XGDU53ANCNFSM6AAAAAAYBC4KBA> . You are receiving this because you commented.Message ID: ***@***.***>

[CAD97]

Another related but I think distinct potential reason to prohibit mutation of a mut-less binding is that it permits the value to be promoted to a static. A couple other things also need to be true for the transform to both be valid and potentially desirable¹, but this is certainly a potentially useful transformation to be able to apply.

Basically, being able to constant-propagate a static promotion as an optimization, rather than requiring the developer to notice it can be and ask for it to be const evaluated to get access to the potential promotion benefit.

Footnotes

1. Namely: at least that we don't guarantee (non-async) locals' address to be in the stack region, and some amount of relaxing address uniqueness (so multiple statically-immutable locals with the same value can be colocated). The transform is perhaps most interesting as a way to optimize constant values out of futures' witness tables (and into the static memory region). ↩

[RalfJung]

We can just not do that promotion if we see an addr_of!(var). This should be an easy analysis.

[workingjubilee]

As the maintainer, co-author, and also user of an unsettling and increasing amount of unsafe code that is expanded under cover of macros, I do not want the following code to ever complete:

let x = 5;
let stuff = construct!(x);
stuff.do_something();
assert_ne!(x, 5);
println!("if this code is reached, modular reasoning got axed.");

I would prefer the assert-not-equals line to always trigger its associated panic.

And I do pass (via pointer) a considerable number of values to FFI that are then left unmodified, and I would prefer that I not have to reason carefully about optimization rules when picking between &T and ptr::addr_of!. I prefer the much-more-boring reasoning of "did I want a pointer, or did I want a reference?" than asking questions like "what exciting nuances of the opsem model will bleed through into my code's compilation today?"

[RalfJung]

No unsafe code programming problem is made better by adding more UB. So I am not sure what you are arguing for here.

[workingjubilee]

Making it defined means promising to compile it which means people will start introducing things which rely on it working.

[RalfJung]

Making it undefined is worse, it turns this into non-debuggable bugs. UB exists for optimizations and nothing else. (And sometimes as a way to paper over platform differences that we haven't found a better way to explain away yet.) Using it for anything else is just the worst kind of bug amplification. We're talking at the level of validity invariants here, not safety invariants. If you write a library abstraction, the client's rules (safety invariant) are yours to define and default to "what can be done in safe code". Safe code cannot mutate these variables, so your libraries are not affected.

[workingjubilee]

In practice people do in fact notice and report "the compiler discarded a write" when they pass &something to FFI (when they should have passed &mut T or something). It might be non-debuggable according to the formalization that says the behavior is non-predictable at that point.

We're talking at the level of validity invariants here, not safety invariants. If you write a library abstraction, the client's rules (safety invariant) are yours to define and default to "what can be done in safe code". Safe code cannot mutate these variables, so your libraries are not affected.

And in practice, people do write macros that expand to unsafe {} blocks, including FFI calls, thus making it so that they appear safe to use, and then someone else has to audit that unsafe code for its correctness.

And programmers are not unwilling to brute-force-search for code that works.

[workingjubilee]

basically, in order for someone to reason "xyz var is never mutated", even if they know the monomorphic type, they must now prove a negative: no one ever used ptr::addr_of! on this ident. if the source is obfuscated by overly-clever constructions, this can be a very long search, especially if the source already has reasons to fling pointers around.

comparatively, the search for "is this let mut or is this type UnsafeCell?" is much shorter, in my experience, since I know exactly where to look for each.

[workingjubilee]

...I suppose they could also declare every single "no, really, it's immutable" variable as a constant but perhaps Rust programmers would rather not BE_CONSTANTLY_SCREAMING.

[RalfJung]

basically, in order for someone to reason [...]

We're discussing the reasoning principles permitted to the compiler here, not the reasoning principles permitted to programmers.

And programmers are not unwilling to brute-force-search for code that works.

That's a doomed approach in a language with UB, it makes zero sense to try to account for it. Furthermore, even if we did add the extra UB you are asking for, things would still seem to "work" in many cases when you don't want them to. So this argument doesn't even support (my understanding of) your position.

And in practice, people do write macros that expand to unsafe {} blocks, including FFI calls, thus making it so that they appear safe to use, and then someone else has to audit that unsafe code for its correctness.

These are true statements and I fail to see the relation to this discussion. People also write safe functions that contain unsafe blocks that cause UB -- people have bugs in their code. The notion of soundness for macros is a bit unclear, but certainly does not involve having to study the code the macro expands to, any more than the notion of soundness of a safe function involves having to study the code inside that function.

I honestly don't understand what kind of API you're even concerned about here. The code you sketched above doesn't help me. But as a general principle there are large priors against a soundness concern motivating more UB. That's like fixing someone's trouble with their door handle by just blowing up the entire car.

[Calvin304]

I just don't think this optimization is important enough to justify introducing a whole new kind of read-only allocation. Specifying these "read-only" locals will be tricky since of course they are written to, even more than once, to receive their initial value:

let x: (i32, i32);
x.0 = 0;
x.1 = 1;

I copied this into the playground and it doesn't compile (E0381 'partially assigned binding x isn't fully initialized'). As such it seems you can only write to an unitialized let binding once.

As far as I can tell, it is also impossible to take a pointer to an uninitialized let binding. If you cannot write to a let binding more than once as it's initialization and you cannot take a pointer to an uninitialized let binding, then any pointer to a let binding must have already be initialized and therefore allowing writing to it would be as surprising as allowing writes to an initialized let binding directly (eg allowing let x = 0; x = 1; to compile).