Clarify provenance of {Arc, Rc}::as_ptr pointer

[y21]

Fixes #87862.
This spawned off of a discussion in the Rust discord server last year about whether the following code is valid:

use std::rc::Rc;

let foo = Rc::new(1u8);

unsafe {
  // *const u8 -> &mut u8
  let foo_ref = &mut *(Rc::as_ptr(&foo) as *mut u8);

  *foo_ref += 1;
};

assert_eq!(Rc::try_unwrap(foo), Ok(2));

That is, can you mutate the returned *const T from Rc::as_ptr (if the Rc is unique).

Looking at the function signature, one might think that mutating the *const T is illegal because it comes from &Rc<T> (effectively a &T -> &mut T transmute), however Rc does not directly store the T; instead it is stored in RcBox that the Rc always has a *mut to. This has always been an implementation detail up until now and one would rely on implementation details when writing to the returned pointer.

It is even noted in the function body of Rc::as_ptr that the provenance must be retained such that methods like {Rc, Arc}::get_mut could create a &mut T to the contained item using this function (if I understand correctly).

rust/library/alloc/src/rc.rs

Lines 860 to 866 in cd12888

	pub fn as_ptr(this: &Self) -> *const T {
	let ptr: *mut RcBox<T> = NonNull::as_ptr(this.ptr);
	// SAFETY: This cannot go through Deref::deref or Rc::inner because
	// this is required to retain raw/mut provenance such that e.g. `get_mut` can
	// write through the pointer after the Rc is recovered through `from_raw`.
	unsafe { ptr::addr_of_mut!((*ptr).value) }

This PR documents/guarantees the ability to write to the pointer

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[Mark-Simulacrum]

r? libs-api

I think we likely want to run this past UCG -- in particular we don't really have much normative documentation about validity for reads/writes on raw pointers like this yet (at least to my awareness). This seems broadly correct but may not be something we want to add yet.

[m-ou-se]

I agree that we should be careful with how we document this. I think this new documentation leaves edge cases unanswered. E.g. what about a situation with a non-zero weak counter? (Using some other mechanism to make sure the weak pointers aren't upgraded while writing through the *const T?)

[m-ou-se]

The Arc type currently has no way to check that condition atomically. There is no public is_unique. Only weak_count and strong_count, which leave space for a race condition when called separately.

(Well it has one way: Arc::get_mut(). But if you call that then you can just use the resulting &mut T instead of the *const T.)

[y21]

Right, so this would act more like Arc::get_mut_unchecked in that it can only be used for code sections where it's statically known that the Arc is unique. Anyway, I don't think I had a use case for Arc in mind when I created this PR, but I thought it would make sense to have the read/write validity guarantees of Rc::as_ptr match with Arc::as_ptr for symmetry (and I couldn't find a reason for why we shouldn't guarantee this for Arc also).

Well it has one way: Arc::get_mut(). But if you call that then you can just use the resulting &mut T instead of the *const T.

Now thinking about it, I'm having a bit of a hard time coming up with cases where Arc::as_ptr is needed for mutation and Arc::get_mut(_unchecked) wouldn't do. I tried to search for Arc::as_ptr on GitHub and most of the uses boil down to:

• Comparing/hashing the pointer (no mutation involved)
• Immediately turning it into a &mut T (which is technically not allowed without this guarantee, but this could be done with Arc::get_mut_unchecked, too)
• Storing it as a *mut T somewhere and mutating it later on when uniqueness is logically/statically guaranteed in some other way (in which case Arc::as_ptr is probably the right thing to use, because it doesn't go through an intermediate reference, unlike Arc::get_mut(_unchecked))

There are some cases (like this one) where a *mut T is passed to FFI and it's not so obvious what happens to it (and not obvious if get_mut(_unchecked) could work, because that one goes through a &mut). In these cases, it's probably good to have the guarantee that the pointer returned by Arc::as_ptr is valid for writes if unique.

[JohnCSimon]

@y21
Ping from triage - can you please address the reviewer's comments?

[y21]

Sorry! I've been a bit busy the past few weeks so I forgot about this PR.

I think this new documentation leaves edge cases unanswered. E.g. what about a situation with a non-zero weak counter? (Using some other mechanism to make sure the weak pointers aren't upgraded while writing through the *const T?)

I'm not sure I understand this concern. The PR guarantees that writes are valid if and only if the strong count is 1 and weak count is 0. So, for a non-zero weak counter, writes are not valid.

My concern is that this "conflicts" with the documentation of {Rc, Arc}::get_mut_unchecked.
The new documentation here is rather conservative (the strong count probably doesn't necessarily need to be 1), whereas get_mut_unchecked is documented to be valid even if the Rc/Arc is not unique (strong count = 1 and weak count = 0).

Safety
If any other Rc or Weak pointers to the same allocation exist, then they must be must not be dereferenced or have active borrows for the duration of the returned borrow, and their inner type must be exactly the same as the inner type of this Rc (including lifetimes). This is trivially the case if no such pointers exist, for example immediately after Rc::new.

Is it fine for the guarantees here to be different from get_mut_unchecked or should they match?

[JohnCSimon]

@y21

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive