Clarify provenance of {Arc, Rc}::as_ptr pointer by y21 · Pull Request #104337 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation9 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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).
| 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
Status: Awaiting review from the assignee but also interested parties.
Relevant to the library team, which will review and decide on the PR/issue.
labels
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
Relevant to the library API team, which will review and decide on the PR/issue.
label
Relevant to the library team, which will review and decide on the PR/issue.
label
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.
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?)
Comment on lines +855 to +857
| /// Note that even though the returned pointer is a `*const T`, it is also valid for writes |
|---|
| /// so long as this `Arc` remains unique (i.e. strong count is 1 and weak count is 0). |
| /// |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 Tinstead 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 withArc::get_mut_unchecked, too) - Storing it as a
*mut Tsomewhere and mutating it later on when uniqueness is logically/statically guaranteed in some other way (in which caseArc::as_ptris probably the right thing to use, because it doesn't go through an intermediate reference, unlikeArc::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.
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
@y21
Ping from triage - can you please address the reviewer's comments?
y21 commented
• Loading
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?
Status: This is awaiting some action (such as code changes or more information) from the author.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
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
Status: Inactive and waiting on the author. This is often applied to closed PRs.
label
This was referenced
Aug 5, 2025
Labels
Status: Inactive and waiting on the author. This is often applied to closed PRs.
Status: This is awaiting some action (such as code changes or more information) from the author.
Relevant to the library API team, which will review and decide on the PR/issue.