Optional semantics for Unique

[Vanille-N]

Use with -Zmiri-unique-is-unique, makes core::ptr::Unique get a reborrow with its own permissions.

[oli-obk]

did some discussions related to this happen somewhere? I'm not against this change, just curious since it came out of nowhere from my perspective :D

[RalfJung]

No, this is an experiment Neven and me discussed.

Unique having semantics came up in rust-lang/unsafe-code-guidelines#384, rust-lang/unsafe-code-guidelines#326.

[RalfJung]

Very nice, just some nits :)

[RalfJung]

Nothing here actually uses unique, or does it?

[Vanille-N]

There are Vecs and Boxes in the test.

[RalfJung]

Fair.

[saethlin]

Will we get any test breakage if libs decides to remove ptr::Unique from Vec? Or would that be just okay?

[RalfJung]

We will get test breakage but that seems fine for me, we can then just adjust the tests.

[Vanille-N]

Rebased at README: -Zmiri-unique-is-unique

Tests will fail.

[RalfJung]

If you rebase over latest Miri master (and run ./miri toolchain) you should get a rustc with your retag PR.

[Vanille-N]

The problem here is that Unique::as_ptr reborrows and thus does not hand out the actual pointer inside the Unique.

Problem

The following diff illustrates the issue:

--- tests/pass/tree_borrows/unique.uniq.stderr
 | Act|      └─┬──<TAG=base>
 | Act|        └─┬──<TAG=raw>
-| Act|          └────<TAG=uniq, uniq>
+|?Dis|          ├────<TAG=uniq>
+|?Dis|          └────<TAG=uniq>

uniq was named through _.as_ptr(), and independent invocations of as_ptr hand out cousin pointers.

This part of the diff shows that the wrong pointer is being named, since it gets invalidated when the pointer inside Unique should not be.

-| Act|
+|?Dis|

This part of the diff shows that pointers obtained from as_ptr are cousins instead of being identical.

- └────<TAG=uniq, uniq>
+ ├────<TAG=uniq>
+ └────<TAG=uniq>

Solution

In the long term, if we find out that Unique cannot be used for Vec or that there is too much breakage from -Zmiri-unique-is-unique, this is likely to be a cause.
We will have to decide (1) whether that is indented behavior, and (2) if it is then is it the fault of TB or the fault of the implementation of as_ptr ?

Right now, this illustrates that miri_pointer_name is not capable of naming the tags that we want to name without breaking upon -Zmiri-unique-is-unique, see #2939 for an idea of a solution.

[RalfJung]

Isn't this a bigger problem than just the ui tests? This means we cannot do the equivalent of addr_of_mut!(*mutable_ref), i.e. create a raw pointer with the tag of the noalias pointer (or an equivalent tag).

I guess if the tests still pass we can keep going with the experiment, but eventually we might want a new borrow state for this, one where alias requirements are only enforced while the tag is protected.

[Vanille-N]

Yes this would make Unique more strict than &mut which is unexpected. I'm looking for a test that shows that uses of &mut can alternate with uses of addr_of_mut (which apparently wouldn't be possible here with Unique), and if we don't have one yet it's a good occasion to add it.

[RalfJung]

marked this pull request as ready for review

FWIW github doesn't send notification for this so it's pretty much useless. Thankfully we have a bot that helps: @rustbot ready. :)

[RalfJung]

We will get test breakage but that seems fine for me, we can then just adjust the tests.

[RalfJung]

Some tweaks for the comments -- please apply the same changes to all copies of these comments.

Then please squash the commits and we can land this. :)

[RalfJung]

@bors r+

[bors]

📌 Commit 71f0db4 has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 71f0db4 with merge a646140...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing a646140 to master...