Skip to content

clarify that addr_of creates read-only pointers #129653

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

Merged
merged 2 commits into from
Sep 6, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions library/core/src/ptr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,10 @@ impl<F: FnPtr> fmt::Debug for F {
/// `addr_of!(expr)` is equivalent to `&raw const expr`. The macro is *soft-deprecated*;
/// use `&raw const` instead.
///
/// It is still an open question whether writing through an `addr_of!`-created pointer is permitted
/// or not. Until that is decided, the same rules as for shared references apply: it is UB to write
/// through a pointer created with this operation, except for bytes located inside an `UnsafeCell`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wording sounds like taking a raw pointer raw_ptr and transforming it using addr_of!((*raw_ptr).field) results in a pointer that only has read-only permissions, even if raw_ptr has read/write permissions. Is that intended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... that is currently not the case, but given that this is all undecided, maybe we should say that indeed this makes a read-only pointer? If there's a good reason to be more fine-grained here I am open to that, the only concern here is that it makes it much harder to say what is and isn't read-only.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh ... I think that would be problematic. I would think that many people, like me, go around and assume that as long as you stay in raw pointer land, you never give up permissions on your pointers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, addr_of! would be a bit of a pitfall, but given the wording I would have expected that.

Do we want an addr_of_const! instead, and tell people to use that unless you are very careful with addr_of!?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure a third macro makes sense. I think addr_of! should guarantee that if both the input and output types are raw pointers, then permissions are preserved and the behavior is identical to using ptr::offset. It's very useful for teaching if I can say that the only thing that matters for the permissions of raw pointers is where it originally comes from. Adding exceptions to that general rule is counterproductive.

In the same vein, the usual motivation for not roundtripping through &T is that doing so gives up permissions to write. The entire point of addr_of! is that it does not roundtrip through &T.

When doing addr_of!(MY_STATIC) you are creating the raw pointer, so it is consistent with the above rule that the resulting pointer is read-only. In this case, the addr_of! is where the raw pointer originally came from, so this decides its permissions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both the input and output types are raw pointers

The thing is, the input is a place, it's not raw or anything. So we have to distinguish on "what is the place based on". It's annoying. But more UB is also annoying so I'll give it a shot.

///
/// Creating a reference with `&`/`&mut` is only allowed if the pointer is properly aligned
/// and points to initialized data. For cases where those requirements do not hold,
/// raw pointers should be used instead. However, `&expr as *const _` creates a reference
Expand Down
Loading