Skip to content
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

Tracking Issue for arithmetic and certain bitwise ops on AtomicPtr #99108

Open
1 of 3 tasks
WaffleLapkin opened this issue Jul 10, 2022 · 7 comments
Open
1 of 3 tasks
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

Feature gate: #![feature(strict_provenance_atomic_ptr)]

This is a tracking issue for arithmetic and certain bitwise operations on AtomicPtr.
As part of the strict provenance experiment #95228.

This feature adds arithmetic (add, sub) and bitwise (or, end, xor) atomic operations for AtomicPtr in order to replace uses of AtomicUsize-but-actually-a-pointer to preserve provenance information for the compiler and remove usize->ptr casts from existing code.

Arithmetic ops behave as the their non-atomic wrapping versions. Bitwise ops behave as .map_addr(|x| x op val).

Public API

// core::sync::atomic

impl<T> AtomicPtr<T> {
    pub fn fetch_ptr_add(&self, val: usize, order: Ordering) -> *mut T;
    pub fn fetch_ptr_sub(&self, val: usize, order: Ordering) -> *mut T;

    pub fn fetch_byte_add(&self, val: usize, order: Ordering) -> *mut T;
    pub fn fetch_byte_sub(&self, val: usize, order: Ordering) -> *mut T;

    pub fn fetch_or(&self, val: usize, order: Ordering) -> *mut T;
    pub fn fetch_and(&self, val: usize, order: Ordering) -> *mut T;
    pub fn fetch_xor(&self, val: usize, order: Ordering) -> *mut T;
}

Steps / History

Unresolved Questions

  • Naming

Footnotes

  1. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html

@WaffleLapkin WaffleLapkin added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 10, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 10, 2022
fill new tracking issue for `feature(strict_provenance_atomic_ptr)`

New tracking issue: rust-lang#99108.

The generic strict provenance issue has a lot of discussions on its own, so I think it's meaningful to have a separate issue for atomic ptr methods.
@RalfJung
Copy link
Member

In terms of implementation details, I think we should also improve the types at which we call the atomic_add and other core atomic intrinsics. Right now we pass them two pointers and then the backend has to know that the left one is actually a pointer and we care about its provenance, and the right one isn't. Ideally we'd give a pointer as first argument and an integer as second argument.

@WaffleLapkin
Copy link
Member Author

"The problem" with making rhs usize is that all atomic intrinsics are currently defined as *mut T -> T -> T, for example:

pub fn atomic_xadd_relaxed<T: Copy>(dst: *mut T, src: T) -> T;

Making the second argument of a different type will probably require compiler changes.

@RalfJung
Copy link
Member

Indeed, and I think those compiler changes should be made. :)

bors bot added a commit to taiki-e/portable-atomic that referenced this issue Jul 31, 2022
23: Provide stable equivalent of #![feature(strict_provenance_atomic_ptr)] r=taiki-e a=taiki-e

This provides stable equivalent of [`#![feature(strict_provenance_atomic_ptr)]`](rust-lang/rust#99108).

- `AtomicPtr::fetch_ptr_{add,sub}`
- `AtomicPtr::fetch_byte_{add,sub}`
- `AtomicPtr::fetch_{or,and,xor}`

These APIs are compatible with strict-provenance on `cfg(miri)`.
Otherwise, they are compatible with permissive-provenance.

Once `#![feature(strict_provenance_atomic_ptr)]` is stabilized, these APIs will be strict-provenance compatible in all cases from the version in which it is stabilized.

(This is also a generalization of what [I did in crossbeam-epoch](crossbeam-rs/crossbeam#796).)

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@WaffleLapkin
Copy link
Member Author

I think ptr in method names (fetch_ptr_add and fetch_ptr_sub) is confusing, we are not adding/subtracting a pointer but instead just add/sub in units of T. IMO it's too similar to sub_ptr.

@ibraheemdev
Copy link
Member

I think the only open question about this API is whether there is a better name for fetch_byte_{add,sub}/fetch_ptr_{add,sub} (rust-lang/libs-team#126 (comment))? Can we move forward towards stabilization? If those methods are still up for debate, fetch_{or,and,xor} are pretty straightforward additions (and probably the more common operations) so can probably be stabilized.

@tgross35
Copy link
Contributor

tgross35 commented Feb 29, 2024

Why not just fetch_wrapping_{add, sub} rather than fetch_ptr_sub? ptr in the name seems redundant, and I would expect fetch_add to be the equivalent of <*const T>::add (not <*const T>::wrapping_add)

@WaffleLapkin
Copy link
Member Author

@tgross35 note that all atomic operations are wrapping, so wrapping is probably even more redundant than ptr 😅

I don't really like _ptr_ either, but it does disambiguate that it's in units of T, as opposed to fetch_byte_add.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants