Repository navigation
When are references allowed to be deallocated while a function they were passed to runs? #433
Description
Activity
A fourth option that's probably worth listing:
- Somehow ignore padding when determining if an object may be deallocated. Only a meaningful variant if partial deallocation of a referenced object isn't permitted. (cc Can allocations have gaps? Can they be partially deallocated? Can they grow "in-place"? #430)
- I don't see how that could be done - deallocation is not a typed operation so we can't even know which bytes are padding at that moment. There could be multiple shared refs of different type pointing to this memory. This has to be distinguished at retag time. Or did you mean that at retag time we should treat padding as always being in an UnsafeCell? That would be possible, but I am not sure if it would be desirable.
- Or maybe you meant that at retag time, padding outside UnsafeCell should be made read-only but not protected against deallocation? Then we can only add 'dereferenceable' on shared refs if there is a non-padding byte that is not in an UnsafeCell (and this interacts with whether partial deallocation is a thing). I should also note that for all these variants, I am mainly talking about struct padding. Unions and enums get more tricky yet.
It's only a fully distinct option from option 3 with respect to partial deallocation. Without partial deallocation, "can the reference be deallocated" can be answered as "all bytes are unsafecell" (option 1), "any byte is unsafecell" (option 21), or "all bytes are unsafecell or padding" (option 3 or 4). With partial deallocation, the answer to "can this byte of the reference be deallocated" can be answered as "this byte is unsafecell" (option 1), "any byte is unsafecell" (option 21), "this byte is unsafecell or padding next to unsafecell" (option 3), "all bytes are unsafecell or padding" (option 4), or some other combinations.
So yeah it's basically just option 3 but without considering partial deallocation. The opsem difference would be that the "shan't be deallocated" protector always gets applied to all or none of the retagged bytes, and the decision tree for whether to apply it is:
- Any non-padding, non-unsafecell bytes? If so, protect. Otherwise:
- Any non-padding unsafecell bytes? If so, don't protect. Otherwise:
- All bytes are padding. Protect.
If I'm grasping at straws in the face of partial deallocation I could say option 4 retags padding as unsafecell iff another unsafecell byte is present, whereas option 3 uses a more clever adjacency rule.
But yes, the option is likely less different than I initially thought.
As mentioned over in #430 and probably the wrong place, I do not know why my initial reaction is that it's fine for
&noaliasto be "no fields are unsafecell and (recursive)" but that&dereferencableshould be "all fields are unsafecell or (recursive)" — I legitimately can't identify why I apparently feel a difference in polarity there.Footnotes
I do not know why my initial reaction is that it's fine for &noalias to be "no fields are unsafecell and (recursive)" but that &dereferencable should be "all fields are unsafecell or (recursive)" — I legitimately can't identify why I apparently feel a difference in polarity there.
Yeah, that does seem inconsistent to me.
I think my own option 1 is to declare all bytes mutable and dealloacteable when
T: !Freeze. That's what Tree Borrows does. If we do decide for more preciseUnsafeCelltracking, IMO we should add protectors if and only if we retag as read-only. But we should also not add LLVMdereferenceablefor theStype from the OP. So at least the padding bytes adjacent to anUnsafeCellshould be considered to basically be part of thatUnsafeCell(and hence mutable and deallocateable).Just for clarity, I'm in full agreement that the OP type should be allowed to be entirely deallocated. (I very deliberately used "fields (recursively)" rather than "bytes", to make padding irrelevant.)
How about mutex? Is it guaranteed that every byte of
std::sync::Mutex<()>is part of anUnsafeCell?- No. This thread is about the op.sem rules for all types, not about the internal guarantees of any libs type.
Generally, this is UB:
This lets us use the
dereferenceableattribute of LLVM, which means "dereferenceable for the entire duration this function runs". That is quite useful for e.g. hoisting memory accesses out of a loop without having to figure out if some other operation deallocatedx.However, we have an exception to that rule: if
xis a reference toUnsafeCell<T>(or a newtype around that), then the memory is allowed to be deallocated while the function runs. (The reference must still be dereferenceable when the function starts.) This was done to resolve rust-lang/rust#55005; the PR that manifested this change is rust-lang/rust#98017.IMO this was absolutely required; without a change like this,
Atomic*::compare_exchangecannot be used as a signal to another thread "you may now free this memory". We would need to provide raw pointer alternatives to even implement something likeArc. However @JakobDegen indicated they disagree so this is part 1 of the issue tracked here. (Though I will note that this will be hard to take back since it has been FCP'd and documented.)Part 2 is the problem that there are still footguns: with a type like
if we have a method on
&Sthat sets a flag to indicate "this can be deallocated", we still have UB. This is because there is padding, and the rules say only memory inside theUnsafeCellis allowed to be deallocated.There are a bunch of options here. Just to list a few:
UnsafeCelllives ever (Cc Stacked Borrows: How precise should UnsafeCell be tracked? #236). Even if we trackUnsafeCellprecisely for mutation, we could say that deallocation is allowed the momentT: !Freeze.UnsafeCellas also being inside theUnsafeCell.