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

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.

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

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).
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)

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.

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: ***@***.***>

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.

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). ↩

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

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?"

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

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

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.

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.

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.

...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.

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.

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).

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.

There was a proposal to allow this, though I can't find it right now. It's a fairly natural language extension that we shouldn't prevent: basically, just treat initialization entirely per-field, and once all fields are initialized, consider the entire value initialized.

Also, even just the single write that we already permit to let bindings is problematic enough to make this highly non-trivial to specify:

let x: i32;
if b { x = 13; }

Also, for MIR semantics we definitely want to permit deaggregation, where this

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

gets transformed into

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

I think that "erroneous behavior" classification is worth exploring more. It seems nice to have a way to forbid things in a way that the compiler can insert checks for, instead of the "I assume it never happens and generate code that way" nasal demons of UB. Like, "this is forbidden and I'm going to check it when I can" sounds way better for things where the reason it's forbidden is to make code more intelligible, instead of more optimizable.

What would the performance impact be of placing fully initialized let places in read-only memory, anyway?
Maybe this can be done by default in debug mode.

What would the performance impact be of placing fully initialized let places in read-only memory, anyway?
Maybe this can be done by default in debug mode.

Note that it would specifically have to be put into a static - there's no (easy or cheap) way to just make part of the stack read only. I think the AAAA solution tells us that it's fine for objects to share an address, as long as they don't interfere with each other, so there wouldn't be an issue with promoting an immutable let binding with a constant initializer to a static in theory.

Since I don't see it mentioned yet, the Rust Reference currently documents that "Mutating immutable bytes" is UB, and that

The bytes owned by an immutable binding are immutable, unless those bytes are part of an UnsafeCell<U>.

Note however that the Reference is explicitly non-normative and subject to change.

Bart Jacobs makes a good point on Zulip: an immutable local can be mutated more than once using destructors.

struct S(i32);

impl Drop for S {
  fn drop(&mut self) {
    self.0 = 42;
  }
}

let s: S;
s = S(0); // first mutation
// there may be a "variable now fully initialized" marker here, it doesn't help
// the implicit drop-in-place is the second mutation

I think that breaks the schemes suggested above for how an operational semantics could treat non-mut let variables as "immutable"?

It doesn't really break my proposal that &raw const be special-cased in lowering to minirust on an immutable binding. Also doesn't really interfere with the opts I'd like to do (they mostly only apply to types that aren't Drop).

Drop is funny in general.

It's not clear to me which proposal you are referring to. Where have you sketched the op.sem for that?

I'm not sure there would be op-sem for that proposal.
The idea is when lowering source rust to minirust, we split &raw const x specifically - when x is a place expression that is based on a local variable (such a local variable, followed by not declared mut, we'd lower to a &x retag (followed by a conversion to *const T), and in all other cases, we'd lower to a straight &raw const x retag. This has the effect of yielding a frozen tag in the particular case.

No different Minirust semantics are required, only a different lowering Rust->Minirust.

Ah, I see. I don't remember seeing this proposal before, but I think I understand.

My personal opinion is that I am not a fan of non-compositional lowerings so I'd rather not have such a special case.

I'm going to withdraw my concern re. closure captures.
Since the capture-elision (or copy-replacement) only applies to by-immutable-ref captures that are Copy (currently excluding UnsafeCell - if an interior mutability type ever becomes Copy the rule can be adjusted accordingly without breaking ABI) and exclusively used by-value, the Frozen tag would be invalidated if the local was written to, the closure could not be called after that point and observe the mutated value (since that would read via a now-invalid Frozen tag).

Auto merge of #142170 - RalfJung:sb-raw-retag, r=<try>

Navigation Menu

Repository navigation

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

41 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

chorman0773 commented on May 14, 2023

RalfJung commented on May 14, 2023

chorman0773 commented on May 14, 2023

CAD97 commented on Jun 18, 2023

Footnotes

RalfJung commented on Jun 18, 2023

workingjubilee commented on May 12, 2024

RalfJung commented on May 12, 2024

workingjubilee commented on May 12, 2024

RalfJung commented on May 12, 2024

workingjubilee commented on May 12, 2024

workingjubilee commented on May 12, 2024

workingjubilee commented on May 12, 2024

RalfJung commented on May 12, 2024

Calvin304 commented on May 12, 2024

RalfJung commented on May 12, 2024

41 remaining items

Monadic-Cat commented on May 15, 2024

chorman0773 commented on May 15, 2024

zachs18 commented on May 26, 2024

RalfJung commented on Dec 7, 2024

chorman0773 commented on Dec 7, 2024

RalfJung commented on Dec 12, 2024

chorman0773 commented on Dec 12, 2024

RalfJung commented on Dec 12, 2024

chorman0773 commented on Dec 16, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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