Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
RalfJung opened this issue May 14, 2023 · 59 comments
Open

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

RalfJung opened this issue May 14, 2023 · 59 comments

Comments

@RalfJung
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

RalfJung commented May 14, 2023

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
Copy link
Contributor

chorman0773 commented May 14, 2023 via email

@CAD97
Copy link

CAD97 commented Jun 18, 2023

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 desirable1, 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
Copy link
Member Author

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

@workingjubilee
Copy link

workingjubilee commented May 12, 2024

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
Copy link
Member Author

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

@workingjubilee
Copy link

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

@RalfJung
Copy link
Member Author

RalfJung commented May 12, 2024 via email

@workingjubilee
Copy link

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
Copy link

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
Copy link

...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
Copy link
Member Author

RalfJung commented May 12, 2024

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
Copy link

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

@RalfJung
Copy link
Member Author

RalfJung commented May 12, 2024

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;

@Calvin304
Copy link

Partial initialization is still kinda besides the point if you can only take a pointer to a let binding statically known to be fully initialized.

@RalfJung
Copy link
Member Author

No, taking a pointer is besides the point. ;) The allocation exists already before the pointer gets created. We're not going to have completely different operational semantics for let before and after they have the pointer taken, that would be a huge pain.

@Calvin304
Copy link

this issue has allowing

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

as its central question and i'm saying that allowing it would be as surprising as allowing

let x = 0i32;
x = 42;

and unitialized let bindings are already 'special'.

@Calvin304
Copy link

Calvin304 commented May 12, 2024

We're not going to have completely different operational semantics for let before and after they have the pointer taken, that would be a huge pain.

I'm saying that if a pointer can be taken then the weirdness with uninitialized let bindings is already over.

@chorman0773
Copy link
Contributor

FTR, in lccc currently I do distinguish between locals that have and have not had their address taken (though currently not for aggregates, I need to add an insertfield mir-expr, which is waiting on equivalent XIR). Before a local has an address taken, it's just purely held as an SSA var (which through magic, ends up as on the xlang value stack). Reassignments don't matter because they're just always new values. After the address is taken, alloca is used (which becomes a local variable in xlang), based on the Mutability of the binding in HIR. It would be nice if I could use a read-only allocation for alloca const <type>.

A similar model could theoretically be used in Minirust to achieve the required/desired semantics.

@RalfJung
Copy link
Member Author

RalfJung commented May 12, 2024

and unitialized let bindings are already 'special'.

You're thinking in terms of surface Rust, but that's not how it works in the operational semantics. In MIR, there isn't even a difference between let x = 0; and let x; x = 0;.

I'm saying that if a pointer can be taken then the weirdness with uninitialized let bindings is already over.

No idea which point you're trying to make here. We need one rule for what happens at a write to a place, treating all places uniformly. We don't even know we're writing to a local variable when we do the *ptr = 42;, we're just writing to a memory location that's part of some allocation. We could say that the allocation that backs a let is read-only, but then even the initial write would be forbidden. Whether or not a pointer is ever taken has nothing to do with this, the write x = 0 and *ptr = 0 look exactly the same to the memory model (assuming *ptr points to x).

I suggest making yourself familiar with MiniRust or a similar operational model. Just jumping into a deeply technical discussion can be confusing and cause confusion when you're not familiar with the technical background.

A similar model could theoretically be used in Minirust to achieve the required/desired semantics.

Miri uses such a model to be a bit faster.
But having such an optimization in the spec would IMO be a big mistake. The spec should be as clean and lean as possible. It's a bad idea to optimize the spec for performance. It risks issues like this where you accidentally change the spec.

@RalfJung
Copy link
Member Author

So far there has been one proposal for an op.sem that actually makes these writes to let-bound variables UB, and it's Stacked Borrows. This is also an aspect of Stacked Borrows that has caused a lot of raised eyebrows and confusion, as tracked in #257. I am convinced we want to fix #257, via something like what Tree Borrows does.

Absent that, the best model I can think of is to introduce a new statement in MIR that says "this variable is now initialized" and marks the corresponding memory allocation as read-only. But I don't see sufficient motivation to add such a statement, and I don't know how hard it would be to make MIR building emit such a statement.

@chorman0773
Copy link
Contributor

chorman0773 commented May 12, 2024

Fair enough that including the opt by-spec is potentially a pain.
Although, a simpler model is (in spec-prose)

If the operand to a raw-address expr is an immutable place expression that denotes a binding, the resulting tag is Frozen.

IE. addr_of!(local) would specifically generate a Frozen tag because local is a an immutable place expression that denotes a binding.

Edit: Though this has one hole: Primitive slice/array indexes.

@RalfJung
Copy link
Member Author

addr_of! doesn't generate any tags though, it's a raw pointer operation. So this sounds like a rather non-compositional special case. Even Stacked Borrows, when it does the retags for "this was just cast from a ref to a raw ptr", doesn't know the place expression that computed this raw ptr -- it just gets the raw ptr and does its retag. (And that's leading to all sorts of trouble like #257 so I want to get rid of it.)

@workingjubilee
Copy link

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

If you canonize this:

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

then programmers will be allowed to reason based on it. Indeed, they will be forced to.

@chorman0773
Copy link
Contributor

There's a lot of things that are awkward to reason about as programmers that is well-defined.

Enum variants and UnsafeCell (in every model under serious consideration) is a great example.

@workingjubilee
Copy link

Yes, knowing the type is required, and?

@workingjubilee
Copy link

Part of my complaint is that if this optimizes differently:

let x = 5;
ffi_call(&x);
let y = x + 5;

than this:

let x = 5;
ffi_call(ptr::addr_of!(x));
let y = x + 5;

then now I can't tell people they should just prefer to use ptr::addr_of! anymore, I have to justify it based on whether the program needs that optimization.

@Calvin304
Copy link

I suggest making yourself familiar with MiniRust or a similar operational model. Just jumping into a deeply technical discussion can be confusing and cause confusion when you're not familiar with the technical background.

Rude. I'm already familiar with MiniRust. But I suppose I must have been unclear.

No idea which point you're trying to make here. We need one rule for what happens at a write to a place, treating all places uniformly. We don't even know we're writing to a local variable when we do the *ptr = 42;, we're just writing to a memory location that's part of some allocation. We could say that the allocation that backs a let is read-only, but then even the initial write would be forbidden. Whether or not a pointer is ever taken has nothing to do with this, the write x = 0 and *ptr = 0 look exactly the same to the memory model (assuming *ptr points to x).

Assuming:

  • Writes to an uninitialized let binding are fully managed statically by surface rust (you cannot take a pointer, take a reference, call a function with, or return an uninitialized let binding in surface rust. you can only initialize the binding directly in a way already tracked by surface rust)
  • the chosen operational semantics allow for however uninitialized let bindings in surface rust are lowered

Then the choice of operational semantic can only possibly impact a programmer writing surface rust by deciding whether an initialized let binding is read-only or read-write.

I see a couple options for lowering let bindings:

  • Introduce write-once allocations. Write after initialize is UB with exact tracking in miri and MiniRust. This also might be useful for optimizing OnceCell/Lazy type things.
  • Emit some sort of special initialize/make-read-only operation directly after the initializing write. Write after initialize is UB with exact tracking in miri and MiniRust.
  • Emit some sort of special initialize operation as the write. Write after initialize is UB with exact tracking in miri and MiniRust.
  • Just use read-write semantics. Write after initialize is not UB to opsem but is still basically UB to everyone except those doing unsafe crimes. As write after initialize is "not UB" they cannot be optimized and have worse/no detection in miri and MiniRust. (easiest to implement)

The chosen semantics would be useful for verifying the lowering of surface rust or for allowing a easier path to new language features like partial initialization.

but right now, as uninitialized let bindings are fully encapsulated by surface rust, the only impact the chosen operational semantics can have on programmers writing surface rust is the behavior of code like this:

let x = 0;
let ptr = addr_of!(x);
unsafe { *ptr = 42 };

tldr: i don't care what specific operational semantic we choose, but we can and should make writing to an initialized let binding UB detectable by our existing tooling.

@Calvin304
Copy link

You're still harping on what I consider superficial syntactic coincidences of current Rust: that one can't do addr_of! of a variable that is declared but not yet initialized, and that one can't field-by-field initialize a tuple/struct. I think it would be a mistake to design an opsem that enshrines these limitations. Neither of them is fundamental, neither of them exists in MIR, I could imagine both of them being lifted in the future.

I am arguing that actually choosing a specific operational semantic for uninitialized let bindings doesn't have to happen until we have a way (or even just a proposal) to write surface rust that can 'care' about the specific operational semantic. Even without a specific operational semantic we can already guarantee for unsafe code authors that initialized let bindings are read-only.

I understand you want that, but what's the argument for why? You're saying we should explode program's in people's faces (aka make them UB) and you've now provided some mechanisms for how to do so (thanks for that), but you haven't explained why you think it's better for these programs to have the worst kind of bug a program can have rather than being "discouraged but well-defined unsafe code crimes".

Saying let bindings are read-only allows the implementation to choose what to do if writes occur, here are some options for the implementation:

  • Optimize on the assumption that let bindings are read only. This is the choice rust with optimizations enabled should use imo.
  • Just use read-write semantics, and nobody writing UB free code can tell the difference. This would simplify a 3rd party compiler dramatically (eg for mrustc).
  • Trap on illegal write. This is the strategy our sanitizers should use (eg miri). This strategy would be unsound/illegal if we decide let bindings have read-write semantics. This strategy is better for diagnosing (practically) unsound unsafe code than defining let bindings as read-write.

One of the nice things about let bindings is that they are local. This means that for you to take a pointer to a local directly (which is required to avoid &'s own read-only guarantee) you have to do so in the same function with the name of the local directly. As such any unsafe code authors would have a very easy switch to let mut and addr_of_mut if they want read-write semantics.

Imagine you are an unsafe code author you run the following excerpt in miri

fn main() {
    let x = 0i32;
    unsafe { call_library_function(addr_of!(x)); }
    if x != 0 {
        unsafe { unreachable_unchecked() }
    }
}

unsafe fn call_library_function(p: *const i32) {
    // potentially arbitrarily far down the call stack
    *(p as *mut i32) = 1;
}

with let binding read-write semantics you would see miri error at the unreachable_unchecked and then have to spend potentially some time trying to find the unexpected write. But with let binding read-only semantics you would see miri error at the write *(p as *mut i32) = 1;. At this point the unsafe code author would either add let mut and addr_of_mut if they want read-write semantics or they would remove the pointer write.

Emit some sort of special initialize/make-read-only operation directly after the initializing write. Write after initialize is UB with exact tracking in miri and MiniRust.

I mentioned this. It's the most plausible way to do this IMO, but requires MIR building to figure out when a variable is fully initialized which seems non-trivial.

Currently this tracking is already done in order for uninitialized let bindings to be safe in surface rust. And as far as I can tell this tracking is done lexically, not in MIR. This tracking is mentioned in the rust reference.

@zachs18
Copy link

zachs18 commented May 13, 2024

To echo Calvin and Jubille, I do not think it is worth the confusion to allow mutating initialized let bindings (via pointer or any other way aside from interior mutability). If one wants a local variable to be mutable, they can just declare it using let mut (and take its address using addr_of_mut!, (but I have less of an opinion on whether addr_of! should deny writes in general in cases where addr_of_mut! would not)).

(Slightly offtopic from the original question, but relevant to later discussion) I do not think that it is a "syntactic coincidence" that one cannot get a pointer to a partially initiailzed local in Rust; in safe code Rust guarantees that let bindings are written to at most once (and only lets you use then after they have been initialized), and I see no reason that unsafe code should deviate from that. If one needs to both have a partially initialized value and take a pointer to it, IMO let mut and MaybeUninit should be used, to explicitly tell the compiler "I am doing the initialization tracking for this". (I might compare this to how unsafe code still most uphold (most of) the invariants of & and &mut, and should instead use raw pointers to tell the compiler "I am doing the aliasing tracking for this".)

@chorman0773
Copy link
Contributor

chorman0773 commented May 13, 2024

Confusion of users isn't a justification for making things UB (on the contrary, it's typically a justification for making things DB). The justification is optimization or ease of implementation. A user being confused about a snippet of code doesn't necessitate giving every rust implementation unlimited license to do whatever it wants with that snippet. We don't need to teach that it's a good idea or even a correct operation to say "Yes, it's DB, compiler vendors no you don't get to delete code".

And certainly if there's no good way to specify that given code has undefined behaviour, the spec (neither minirust nor the prose version in rust-lang/spec) shouldn't be complicated because it's "hard to teach" that some code is considered valid by the abstract machine (even if we as programmers might not consider it "valid").

I do not think that it is a "syntactic coincidence" that one cannot get a pointer to a partially initiailzed local in Rust

I'm actually unsure this is true even now - you can get a pointer to the initialized field. Under TB provenance rules, I'd expect you can access the whole allocation with that pointer. Though, in that case, I'd expect the resulting pointer to behave as-if we took the whole pointer (so if the rules are that the entire object is immutable via pointer, then I'd expect the same here even for a partially-initialized object).

@chorman0773
Copy link
Contributor

addr_of! doesn't generate any tags though, it's a raw pointer operation.

It has to get the base pointer from somewhere (unless Minirust locals really are just SSA Vars carrying an alloca pointer like lccc-MIR after the first address is taken).

@RalfJung
Copy link
Member Author

RalfJung commented May 13, 2024

It has to get the base pointer from somewhere (unless Minirust locals really are just SSA Vars carrying an alloca pointer like lccc-MIR after the first address is taken).

Quoting MiniRust:

    /// For each live local, the location in memory where its value is stored.
    locals: Map<LocalName, Pointer<M::Provenance>>,

So yes, that's basically what they are -- except this isn't SSA, and there's no "after first address taken". On StorageLive, we reserve memory (and generate a tag) and store the pointer in locals; on StorageDead, we free the memory and remove the entry in locals. Very simple, and nicely modular -- each operation just does one thing, and there's just a single possible representation for a live local (not an "optimized" form and an "address was taken" form, as in Miri and apparently in your language).

@RalfJung
Copy link
Member Author

RalfJung commented May 13, 2024 via email

@RalfJung
Copy link
Member Author

RalfJung commented May 13, 2024 via email

@RalfJung
Copy link
Member Author

If the goal is to make Miri flag more things, then we should be talking about "erroneous behavior" (that's what C++ calls it), not Undefined or Defined behavior. Erroneous behavior means: when this happens it's a bug, and then either execution stops or it continues just fine, but no extra assumptions or optimizations can be made by the compiler. If the program doesn't abort, it continues in an entirely well-defined way.

You are making some valid arguments for why writing to non-interior-mutable let should be erroneous behavior. I am still not sure I agree with carrying all that extra state to implement write-once semantics, but that would explain why you are focusing so much on Miri checking this and so little on unsafe code authors having to prove it.

@workingjubilee
Copy link

You seem to have some concrete example in your mind that involves you providing a macro and someone else using it it the wrong way, or maybe I entirely misunderstood because I can't read your mind.

I am treating the author of the macro as potentially adversarial to the author of the code.

This can remain true even if they are the same person.

I find that it is often the case that I must adopt this quasi-adversarial stance in order to understand how to rectify questionably-written unsafe code until it is sound again. So I make my request knowing what can make it harder and easier for me to audit and rectify a gnarled and twisted codebase full of mazy indirections, obscuring code with poorly-documented preconditions, postconditions, and invariants. And macros offer a perfect ad-hoc solution for authors that want to provide an interface but don't want to reason quite so much about types, because they can "simply" parse the input and do codegen.

I can, without a doubt, guarantee you that there is more than one way to introduce a Heisenbug.

@RalfJung
Copy link
Member Author

RalfJung commented May 13, 2024

@workingjubilee I can just repeat asking you for a fully concrete example. I asked before, but you're still just talking in generalities that are impossible to confirm or refute. I can't look into your head to see what you think would be harder to easier to audit. Until I am given a counterexample, I maintain my claim that all else being equal, adding more UB can never make anything easier to audit. (I think that's a theorem: if P → Q, then proving Q can never be easier than proving P. P is "program well-defined with less UB" and Q is "program well-defined with more UB". Things get less monotonic when considering libraries, not whole programs, but then we're also considering safety invariants, not validity invariants.)

@workingjubilee
Copy link

I provided one, did I not?

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

The internal implementation of construct! and do_something are unimportant because they are arbitrarily non-trivial: as many thousands of lines long as necessary to make you flinch at the thought of having to sift an invocation of ptr::addr_of! from. Because they then do FFI with a million-line codebase.

Oh, and yes, they're libraries that produce libraries that do IPC.

@workingjubilee

This comment was marked as off-topic.

@RalfJung

This comment was marked as off-topic.

@workingjubilee

This comment was marked as off-topic.

@RalfJung

This comment was marked as off-topic.

@workingjubilee

This comment was marked as off-topic.

@RalfJung

This comment was marked as off-topic.

@workingjubilee
Copy link

@RalfJung "proc macros with mazy implementations that then expand into mazy code that are used as part of a library that is used to implement libraries that that get dlopened and do IPC over shmem" is an actual, no-shit, stone-faced description of what I work on.

@RalfJung
Copy link
Member Author

RalfJung commented May 13, 2024

I guess I can't help myself so I'll write a reply before likely unsubscribing to protect my sanity. Someone please ping me when things cooled down.

It's not entirely clear to me who's reasoning about what in your example. That's what I was asking you to explain. Making it an actual self-contained example that people not in your brain can understand. I was not asking you to explain how to conjure Cthulu, even if conjuring Cthulu is your dayjob.

I guess it's about construct! or do_something having a bug and mutating things when they shouldn't? You know, you could have just said that: where's a potential bug, who's doing what to find it. (You still haven't said the latter part.)

Let's see how this fares

Without let mutating being UB

The program has well-defined semantics so you can actually use a debugger to find this. Set a memory watcher on the variable to see where it gets mutated, voila. You still have to sift through all the abstractions but you have a starting point.

With let mutation being UB

Your assert_ne can still succeed! It can't succeed in any UB-free execution, but that means nothing here. You don't have a magic oracle that tells you whether there is UB. (You can't use Miri because as you said yourself, FFI is involved.) Nothing gets any easier in this world.
The code is still buggy, but now you additionally have to worry about the compiler miscompiling things so sometimes code may seem to work when it should not. You now have 2 problems: a buggy codebase, and a compiler that works against you.

It is orders of magnitude simpler to debug a "normal" bug than a UB bug. So for the sake of those that try to conjure Cthulu (and everyone else), we should have less UB, not more.

These are points I already made above.

@chorman0773
Copy link
Contributor

Attempting to get this conversation off of the above discussion, I'm going to remake my points actually in favour of having UB here, though more concise and specific:

  • During mir lowering, in lccc, local variables are kept in SSA until their address is taken. At that point, I'd like to generate alloca <mutability> (where mutability corresponds to the HIR Var declaration's mutability), and I'd like to lower alloca const to an immutable local variable in xlang,
  • I'd also like to have "immutable after initialization" as a property that can be derived solely from a variables declaration, so I can do const-capture elision without needing to spec full function analysis.

There should logically be a way to make "immutable after initialization" work in a reasonable way - I'd expect it would be simpler to modify addr_of's minirust lowering rather than the variable declaration itself.

@CAD97
Copy link

CAD97 commented May 14, 2024

This is a bit of an essay, sorry. I marked the most important takeaway, with the TL;DR being that I'm in support of the OP example being defined behavior.

There is a weird semantic space where mutating a non-mut let binding would live — behavior considered defined by the compiler, but still heavily discouraged, borderline still disallowed in any reasonable code.

To @workingjubilee's point, as I understand it:

let place: i32 = 0;
macro_from_upstream_library!(place);
assert_eq!(place, 0);

Answering the issue title's question as no and declaring the OP's example as defined behavior would mean that this assertion could panic in valid executions. However, if it did, the macro would be unsound. Macros are absolutely allowed to expand to internally encapsulated unsafe, but the unsoundness of the macro which makes this assertion panic isn't even a difficult to spot technicality; the counterexample is as simple as just writing macro_from_upstream_library!(*&place).

When reasoning about what other code (outside the safety boundary, whatever you've locally defined that to be) can do, the question isn't about language validity and what is or isn't UB, it's about safety invariants, which are much stricter than simple validity.

(Aside: I think I want to write (another) blog post on the idea of and separation between the concepts of validity and safety. It comes up and I've re-explained it frequently enough that I'd like to have a stable reference that I can point to that covers just that difference specifically.)

I am treating the author of the macro as potentially adversarial to the author of the code.

There is some level of cooperation which you have to assume, and that cooperation is that upstream is sound. If you can't assume that, upstream could just unreachable_unchecked and eat your laundry for free. Or just write the mutation through addr_of! anyway despite it being invalid instead of just unsound.

The correct model that Rust helps with is explicitly not an adversarial upstream. An ignorant and/or negligent upstream, sure, but upstream is always presumed to be competent, cognizant, and cooperative enough to maintain the soundness of the interface.

In fact, allowing mutation helps with the case of negligent upstream, because now if they use addr_of! instead of addr_of_mut! incorrectly when the value could potentially be written to, the result is merely surprising behavior instead of deleting your code (or worse, seeming like it works). In the specific case where it gets run under Miri, the perfect result might be to point at the errant write. But since pointers in public API are involved, it's much more likely that we can't use Miri because there's FFI involved, and in that case I'd much prefer that my debugger works and shows me that the value changed, instead of presenting some arbitrarily wrong picture of the world that's been corrupted by UB.

That is the primary issue with UB — you can't reliably debug it when it happens. The main problem with how we're understanding your position is that you're apparently claiming that UB is preferable to a defined but semantically wrong result. This is a very confusing position to hold, because while you can debug a wrong result, you fundamentally can't debug a UB.

Perhaps the convenience of Miri as a sanitizer works against us here. UB is not equivalent to diagnosed by Miri. UB should always be thought of as perhaps the exact opposite — daemonic nondeterminism that chooses between time traveling and changing previous control flow choices, the most sensitive data accessible by the machine, the "correct" result, or a crash, whichever is worst for you at the moment, non-exhaustively.

"Forbid it, diagnose it as forbidden whenever possible, but as a mitigation permit it when it can't be prevented" is a defensible position. "Eat my laundry if I dare make a mistake" less so.


To the second form of the point mentioned both by Jubilee and Calvin, between

lib_call(&place); // or
lib_call(addr_of!(place)); // where
// extern fn lib_call(_: *const Type);

I would still recommend using &place and reference-to-pointer decay in most cases, and in all cases place is an initialized let without mut. And if you want to be explicit that conversion to pointer is happening, ptr::from_ref(&place). The entire point of choosing to "always prefer addr_of!" when creating a pointer is that addr_of! has weaker semantics and permits more operations as valid; if the semantics (and optimization) you want are that of a reference, use a reference.

If a developer chooses to use addr_of!(place) and not ptr::from_ref(&place), they're explicitly requesting weaker semantics that don't include freezing place from potential mutation. Entirely ignoring that signal because the place was declared with let — potentially because the compiler told them the mut was unnecessary — seems needlessly adversarial of the compiler.

In fairness, I absolutely could see Miri in a conservative mode (e.g. under cargo-careful) choosing to warn when let-bound places are mutated. It's the same reasoning I gave behind considering shallow memory initialization requirements for reference retagging — it's "nicer" to "blame" the cause rather than the symptom.

If we had a formal classification for "forbidden but still with predictably defined behavior of the 'obvious' semantics or a crash," it could make sense to place this there. Exposed provenance is mostly in that bucket, I believe. But it isn't necessary to formally have that bucket in order to diagnose it. Miri isn't UBSAN which (in theory) accepts all valid programs and rejects most invalid; Miri (in theory) rejects all invalid programs and accepts a majority of valid ones.

This might be "erroneous behavior." It might be "safe misbehavior." Maybe we should try to build such a catorization as we stabilize the strict provenance APIs and start moving towards supporting CHERI as an experimental target. But making it UB will never make Jubilee's million-line maze easier to manage, and can only make it more difficult to debug.

I will even agree that any code written that deliberately relies on this being DB should be immediately fixed with priority equal to that of latent UB. Code which relies on the "wide" (or "infectious") model for shared mutability instead of wrapping things in UnsafeCell, which relies on the !Unpin hack instead of using UnsafePinned, or on moving containers without MaybeDangling not invalidating interior references is all in the same boat — should be fixed to not rely on these hidden details of the Rust AM as soon as possible.

None of these things are DB because we endorse doing them. They're DB because all else being equal, we prefer more things being DB. Just because you can doesn't mean you should, and we consistently make the nicer and more straightforward way to accomplish these things available at the same time as or before we canonicalize that the circuitous way is actually guaranteed to work instead of just working by happenstance.


To @Calvin304's primary point about initialization tracking:

I don't know at what point uninitialized bindings are currently diagnosed. However, with the shifts to use THIR unsafeck and MIR borrowck, the trend is to push these analyses later in the compilation pipeline.

MIR does actually know a difference between let and let mut; you can see this when dumping MIR built from surface Rust. But on the other hand, parameters in dumped MIR aren't ever marked mut, and the mir! macro just uses let and doesn't even parse let mut.

I intuitively expect that if/when the language is extended to support piecewise initialization of let, this analysis will happen on THIR; it must happen after types are known and name resolution has occurred in order to ensure no deref coercion happens on a partially uninitialized place, independently of whether we allow piecewise initialization to completely initialize a place.

Thus, once that analysis is done, it basically just becomes an implementation choice whether that information is validated in THIR then discarded or if it's passed along into MIR. We already have StorageLive and StorageDead, so a "StorageFreeze" for non-mut initialization of let is far from unreasonable.

The benefits of immutable-after-init let do exist (and aren't novel, since C++ compilers exploit them for const locals), but it's worth explicitly noting that these benefits would still apply to most let even if we say let can still validly be mutated. A non-mut place can only be mutated if it is used by a raw reference-of expression (addr_of! / &raw const), so it can still be optimized in the absence of that.

For such to exist in the TB model, the "root" tag for let allocation is still "Active", but any place mention (excluding direct place = assignment) doesn't use that tag directly, but instead a child tag, which is frozen for any non-mut bindings.


So, with that in mind, reducing the question of the OP to absurdity, it really does boil down to:

If, given some let place, a developer chooses to write addr_of!(place) instead of the stricter ptr::from_ref(&place) (maybe using coercion), does it still make sense to punish a mistaken write with arbitrarily disastrous UB?

My personal position and answer to the question is the following three-phase conditional:

  • If we ever allow addr_of!(place) for an uninitialized place, then we should permit writing through a such-derived pointer for the same reason we allow = assignment to an uninitialized place.
  • If we never allow addr_of!(place) for an uninitialized place (thus requiring the use of MaybeUninit for unsafe piecewise value initialization behind a pointer), then freezing let places is justifiable.
  • However, addr_of!(place) is a request for weaker semantics than &place, so it makes more sense to respect that, especially since any and all optimization benefits can be exactly recovered by using &place instead, without any1 subtle pointer validity differences.

The points/arguments I make elsewhere throughout this comment are why I agree with Ralf here that the example in the OP should be defined behavior. The most relevant point is that addr_of! is a non-mut place mention that nonetheless doesn't freeze the mentioned place (like a non-raw reference-of would), which I expand on a bit more below.

Separately, I also think an initial intuition can reasonably expect a binding's place mention to behave like *&mut binding (if a mut mention) or *&binding (if a non-mut mention). This doesn't work because of place projection (partial borrows), but up until that point integrates well with how auto(de)ref and deref/index place mutability inference work2. If a developer understands that the purpose of addr_of! is to avoid creating a reference, that same understanding covers why addr_of! shouldn't freeze the mentioned place.


While I absolutely agree with the sentiment of #2573, I do wonder whether the perception of addr_of!(place) and &raw const place might differ, since the latter has const in the syntax. Additionally, addr_of! is enough syntactic salt that it's IMHO a strong signal for "I want the weaker semantics," but &raw const place is so convenient that it loses some of that signal4 and is easier than writing ptr::from_ref(&place) even when the stronger semantics are wanted.

Drawing from a Rust background, I5 don't expect to be able to modify through an addr_of!(binding) pointer6, personally. Drawing from a C++ background, on the other hand, my understanding of when it's valid to use const_cast can suggest that addr_of!(binding).cast_mut().write(…) could be valid. But on the other side of that hand, const in C++ applied to the type of the storage place does actually make the place UB to mutate7.

So in (not particularly useful) C++ terms, the question is whether let place = init; in Rust has C++ semantics more like auto place = init; auto const&& place = (auto&&)place; (actually mutable) or more like auto const place = init; (actually immutable).

Perhaps more usefully (despite being less strictly correspondent), it depends on whether the C++ developer learning Rust sees let place: T and let mut place: T as the moral equivalents of const T place and T place (actually immutable) or of T place and mutable T place (actually mutable). That mut isn't permitted in struct fields should hopefully indicate that Rust mut is closer to the absence of C++ const than the presence of C++ mutable.

As one last wrinkle, though, I've only really considered addr_of!(binding) above, as that's what's relevant to the OP topic. Indirectly relevant, however, is computed places, e.g. addr_of!((*mut_ptr).field) or even addr_of!(*impl_deref). That gets unambiguously into #257 territory, so I won't touch on it further except to say that addr_of! regularly producing valid-for-writes pointers despite being treated (e.g. for borrowck and deref/index mutability inference) as a non-mut place mention makes the concept of addr_of!(let_binding) being valid-for-writes a lot easier to conceptualize. Under this model, mut is truly "just a lint" and only controls whether a mut place mention is allowed, and since addr_of! is a non-mut place mention that doesn't freeze (make immutable) the mentioned place, the writing through the pointer is still permitted unless a different operation has frozen the place.

IMPORTANT TAKEAWAY BEGIN

So the question finally becomes, after defining addr_of!: does assignment to a let binding without mut freeze the assigned-to place (until the place is dropped)? I fail to see any reason for it to (the locality of reasoning for the developer is maintained by safety, not by validity) and a very clear reason for it not to (an errant write still being debuggable behavior) so my vote's that the OP example should be DB.

If you want errors to be detectable, you want them to not be UB. Being DB does not mean you as a developer have to tolerate it; it means the compiler has to tolerate it. Being UB doesn't mean a rushed developer is any less prone to making mistakes; it means that determining that and how they did is miles more difficult.

IMPORTANT TAKEAWAY END


Post-script response to @chorman0773's point: (it took >6h to write this 🙃)

It's already fundamentally the case throughout Rust's semantics that simple inorder translation isn't sufficient. I don't think making one small part of the translation easier should inform any surface language visible decisions. E.g. type inference is a full-function process, so I don't see how capture elision could happen without having done full-function analysis already. Also, it seems like your eager SSA form lowering

Modifying addr_of! to get "immutable after initialization" requires one of two things — either addr_of! always generates a pointer without write permission (what SB does), or addr_of! needs to behave differently based on whether it's referencing a non-mut binding (or pure place projection thereof) or some other place (i.e. a computed place/value or a place that names a (projection from) mut binding). IMHO that seems much worse than the options of either:

  • Making non-moving binding place mention use a non-root borrow tag, which gets tagged Frozen for let bindings (of Freeze types); or
  • Making place assignments behave differently if a place isn't previously initialized, which is already the case because whether the old value is dropped depends on initialization.

The "proper" solution is for lccc to have an xlang optimization pass that replaces alloca with SSA form where possible and desirable. Given that needs to exist anyway if you do any optimization on the xlang form, it seems like the benefit of deferring alloca when lowering MIR to xlang is quite minimal. Especially with how eagerly surface Rust creates references.

The only real potential benefit I see is in capture elision, turning by-reference capture of let bindings into by-value (and by-constant-value) capture … except since the capture is by-ref, the by-ref capture freezes the place, so mutation of the place is excluded by the capture semantics, and doesn't need to be excluded by the place semantics.


Final afterthought: the above prompted me to think about how closure capture is dependent on mut annotation. This is just another case of addr_of! being a non-mut place mention, but capture rules result in that even if addr_of!(x) gets write-capable provenance, even if x is bound with mut, (|| addr_of!(x))() does not, because that's semantically equivalent to ({ let ref_x = &x; move || addr_of!(*ref_x))().

I don't think there's a fair alternative which still captures a borrow lifetime. That we do precise field capture for closures limits the impact (i.e. capturing for addr_of!(place.field) won't ever impact pointers to the rest of place), but it does feel kind of footgun-y. (FWIW, a pure place mention (e.g. _ = x) doesn't capture x at all.)

Footnotes

  1. The strongest argument for not using &place would be that the resulting pointer's pseudo-lifetime outlives the enforced borrow lifetime. (However, if the execution is valid, note that no lifetime extension occurs; the borrow lifetime and validity always die together for a shared borrow of a let binding… in the absence of any mutation as allowed by this, anyway.) Any project pedantic enough to care about this should probably be banning reference to pointer coercion and appreciate the syntactic noise (and semantic signal) of using ptr::from_ref.

  2. Example: the place v[i] is explained as sugar for *Index::index(&v, i) (or the mut version), which after mental inlining turns into essentially *&*<_>::as_ptr(&v).add(i). Sugared place mentions have *& semantics, so it's understandable to expect something similar when mentioning a let binding as a place.

  3. It remains extremely unfortunate that (potentially coerced) &mut place as *const _ produces a pointer that's illegal to write through imo, especially since there's no "unused mut" style lint emitted to say it could just be &place instead. The coercion "desugars" into &raw const (*&mut place), so IIUC SB errors and TB allows mutation, as SB uses a "raw" tag kind for &raw const but TB just uses the source's tag.

  4. Similar to using raw pointers even when they're known non-null. If the hypothetical "One True Pointer Type" doesn't distinguish "being" mut, then the OTPT version of &raw mut would exclusively determine whether the place mention is mut, making it easier to argue for read-only (i.e. if you want write permissions, use the mut syntax, which despite applying to raw pointers because of coercion, is a bit of a weaker argument when the types differ).

  5. This is about me personally, not a theoretical I. And probably I'm far from a typical user w.r.t. what I intuitively expect, given my involvement with T-opsem and that Rust's ownership model immediately made intuitive sense to me, allowing me to mostly skip the "fighting the borrow checker" phase when I was first learning Rust.

  6. Although, for full clarity, my "first guess" semantic also doesn't have addr_of! freeze the location, either; it's still weaker than ptr::from_ref(&place) and the place could be mutated through a separate pointer to the place. Additionally, for addr_of!(*mut_ptr), my "first guess" heuristic is entirely unsure whether to treat this differently from mut_ptr.cast_const().

  7. In the absence of mutable fields, essentially the C++ version of Rust's UnsafeCell.

@chorman0773
Copy link
Contributor

The "proper" solution is for lccc to have an xlang optimization pass that replaces alloca with SSA form where possible and desirable. Given that needs to exist anyway if you do any optimization on the xlang form, it seems like the benefit of deferring alloca when lowering MIR to xlang is quite minimal. Especially with how eagerly surface Rust creates references.

Note that the deference happens in the MIR level (lccc's rust frontend has its own MIR). It actually makes some reasoning (like init-tracking) easier as well, since the lowering just inherently will error on an uninit memory location. This is then lowered to using the xlang stack (note, not the call stack at the hardware level) through use of pivot and dup operations (this translation step is cursed, but useful). This translation is necessary to make sure parameters in the THIR are consistent with other bindings (xlang recieves all function parameters on the incoming stack of the first basic block, not as locals). The translation itself is not impacted by either resolution, but without them being "immutable after init", one of two things must be true:

  1. I can't lower alloca const T <val> to a define const at the xlang level, or
  2. I cannot blindly generate the appropriate alloca solely from the THIR Declaration Mutability, and need to take into account the usage of the place (which violates linearty, a property I rely on a lot in all of the lowering steps).

I'd personally choose the former, though I'd rather have neither constraint.

As to the capture rules, the main issue is I can't have local optimization-dependent type layout - and I need to define type layout very exactly, in terms of (post-mono, fully-inferred) surface rust (I cannot refer to dynamic-only semantics). Closures are no exception, and in fact, their layout rules for a backbone for most of the anonymous types, given that generators and async blocks/async functions use the same capture-for-layout-purposes rules.

@CAD97
Copy link

CAD97 commented May 15, 2024

Although I'm certain lccc isn't particularly enthused about it, since you're trying so hard to have a more stable ABI than Rustc provides, the semantics of Rust should not be beholden to implementations that want to provide stronger guarantees than provided by the language. If you'll forgive the hyperbole, this is how a lang gets into the pit that C and C++ find themselves in — the language provides no guarantees for ABI (has no concept of compiled objects, only linking based on surface language semantics), but can't make otherwise desirable changes because it might require a deliberately fragile compiler implementation to break ABI. Because Rust layout and ABI is unspecified, it's entirely okay if the language relies on that to be able to get reasonable performance characteristics.

And anyway, if you want to specify the captures of a witness table in terms of surface language semantics, the obviously correct answer is to just include each reference that gets captured. Attempting to do stably observable guaranteed promotion is a poor idea. Rustc does some of a similar promotion where &promotable can be extended to be 'static, and that we promote some #[rustc_promotable] tagged functions is generally seen as a mistake in retrospect. If a developer wants constant promotion, they can use const to request it instead of stack local semantics. Furthermore, I don't see promotion of captures of let bindings happening often. It's distinctly not sufficient to have "captured by-ref, has a constant, promotable value" and do promotion, because if the capture is referenced from within the closure and the address is inspected, that address had better be equal to the address of the captured binding, because those are the semantics of by-ref binding capture. You can promote any by-move captures, but in that case whether the binding is mutable is irrelevant.

In fact, I don't think rustc ever optimizes closure captures beyond the naive capture semantics, even for capturing every field of a binding independently. And while the size of async does get complained about (but more because of a lack of stack binding space optimization, not because of captures), I've never seen any complaints about the size of normal closures.

TL;DR: While alternative implementations can and should inform decisions, those alternative implementations choosing to tie their own feet more than necessary shouldn't restrict the main language from benefitting from the flexibility it deliberately reserves.

(A plea to "lccc Rust" — at least for anonymous closure/async types, don't stably specify the layout by default, and require annotation of e.g. #[repr(lcccv1)]. Even in the face of dynamic linking, only one compilation unit needs to fully know the layout — the one that defines it. Other compilation units only need the external shape to be predictable, not the monomorphic one.)

@workingjubilee
Copy link

workingjubilee commented May 15, 2024

@CAD97 I too drafted a much longer explanation, but the part of the problem is that I understand everything you're saying, and the actual communication gap is that scale is not an isolated quantity of the discussion, to be torn away so someone can refute it in isolation.

The problem is that in my experience, the code doesn't necessarily become more sound if the mutation is permitted, because later code may be relying on that immutability. For, yes, soundness reasons. "Everything is a big mesh of horrible, hard-to-understand, entangled state, with unclear invariants" is exactly why I have trouble explaining it. That is my point about scale, and about this behavior being so counterintuitive it can undermine the code around it. Allowing mutation lets the error become more nonlocal, thus harder to find, as the mutation means it can flow downstream until it hits something that has become unsound because values changed. For every source line in a toy example I provide, there may be hundreds separating them... if they're even in the same crate or repo... in the real thing.

People by and large won't set a breakpoint on the variable changing if they never imagine it can be changed.

I would genuinely rather the composited binaries try to do something that makes valgrind scream at the impossibility, immediately. Because Miri isn't the only game in town. As annoying as they may be, it's very easy to pop gdb and examine a core dump of a segfault, too (...the immediate backtrace and register dump, anyway, the rest can be somewhat "???").

@workingjubilee
Copy link

The only real "optimization" available that I am aware of in the separate compilation case...

...we did all agree that this is most likely to occur in the case of FFI, right?...

...where an address must be revealed to the world, and that must have a legible value at that location, or the program simply cannot work, neither as the abstract machine understands it nor as the programmer understands it... is to move the data into read-only memory. Attempts to write to that location will generally violate virtual memory protection and immediately trigger SIGSEGV or the equivalent... Windows still calls it EXCEPTION_ACCESS_VIOLATION, right?... which then reveals the error immediately, usually including the relevant address.

Unconditionally permitting the mutation does not allow this transformation, allowing errors to be latent in the actual binary.

@chorman0773
Copy link
Contributor

chorman0773 commented May 15, 2024

(A plea to "lccc Rust" — at least for anonymous closure/async types, don't stably specify the layout by default, and require annotation of e.g. #[repr(lcccv1)]. Even in the face of dynamic linking, only one compilation unit needs to fully know the layout — the one that defines it. Other compilation units only need the external shape to be predictable, not the monomorphic one.)

As a note, I can think of at least one place where two different translation units may need to come up with the layout of an anonymous type de novo, that being inlining/generics. This is because now two (or more) different compilation units can contain the definition, and need to agree in order to be interconvertible. Especially in the generics case, because the captures' list can be built partially, but not entirely (by-ref to by-copy lifting requires knowing the monomorphic type, this optimization is present in v0, for types at most the size of a pointer) so the compiler on both sides of this must agree on the same final steps at least (ie. on the small-copy capture optimization) - this is true even if the compiler is the exact same build of lccc. The case in question is something like

fn mk_sized_array<T>(_: &T) -> [u8; core::mem::size_of::<T>()]{
    [0xFF; core::mem::size_of::<T>()]
}
pub fn foo<T: Copy + Display>(x: T) -> impl Any{
    let f = || println!("{}", {x});
    mk_sized_array(&f)
}

Two different calls to foo::<u32>(5) must be able to come up with the fact that typeof(f) has size 4, otherwise when the comdat groups are collapsed, one call site (presuming no inlining takes place) will have the incorrect size for the return value (the caller could transmute the array to u32, and the compiler could assume that it is fine to promote eax to rax, because the top bits of rax are all zeroes).

@Monadic-Cat
Copy link

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.

@chorman0773
Copy link
Contributor

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.

@zachs18
Copy link

zachs18 commented May 26, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants