[RFC] AtomicPerByte (aka "atomic memcpy") - #3301
Conversation
m-ou-se
added
the
T-libs-api
|
cc @ojeda |
|
This could mention the |
|
With some way for the language to be able to express "this type is valid for any bit pattern", which project safe transmute presumably will provide (and that exists in the ecosystem as This would also require removing the safe That's extra complexity, but means that with some help from the ecosystem/future stdlib work, this can be used in 100% safe code, if the data is fine with being torn. |
|
The "uninit" part of not without the fabled and legendary Freeze Intrinsic anyway. |
|
On the other hand, |
|
note that LLVM already implements this operation: |
|
The trouble with that intrinsic is that |
| - In order for this to be efficient, we need an additional intrinsic hooking into | ||
| special support in LLVM. (Which LLVM needs to have anyway for C++.) |
There was a problem hiding this comment.
How do you plan to implement this until LLVM implements this?
I don't think it is necessary to explain the implementation details in the RFC, but if we provide an unsound implementation until the as yet unmerged C++ proposal is implemented in LLVM in the future, that seems to be a problem.
(Also, if the language provides the functionality necessary to implement this soundly in Rust, the ecosystem can implement this soundly as well without inline assembly.)
There was a problem hiding this comment.
I haven't looked into the details yet of what's possible today with LLVM. There's a few possible outcomes:
- We wait until LLVM supports this. (Or contribute it to LLVM.) This feature is delayed until some point in the future when we can rely on an LLVM version that includes it.
- Until LLVM supports it, we use a theoretically unsound but known-to-work-today hack like
ptr::{read_volatile, write_volatile}combined with a fence. In the standard library we can more easily rely on implementation details of today's compiler. - We use the existing
llvm.memcpy.element.unordered.atomic, after figuring out the consequences of theunorderedproperty. - Until LLVM supports appears, we implement it in the library using a loop of
AtomicUsize::load()/store()s and a fence, possibly using an efficient inline assembly alternative for some popular architectures.
I'm not fully sure yet which of these are feasible.
There was a problem hiding this comment.
IMO, having the efficient assembly version for popular architectures is going to be important for adoption. If the cost of soundness is a 25% performance drop for bulk copies, people might well keep using an unsound version.
I'm very familiar with the standard Rust and C++ memory orderings, but I don't know much about llvm's (It seems |
| but it's easy to accidentally cause undefined behavior by using `load` | ||
| to make an extra copy of data that shouldn't be copied. | ||
|
|
||
| - Naming: `AtomicPerByte`? `TearableAtomic`? `NoDataRace`? `NotQuiteAtomic`? |
There was a problem hiding this comment.
Given these options and considering what the C++ paper chose, AtomicPerByte sounds OK and has the advantage of having Atomic as a prefix.
There was a problem hiding this comment.
AtomicPerByteMaybeUninit or AtomicPerByteManuallyDrop to also resolve the other concern around dropping? Those are terrible names though...
|
Unordered is not monotonic (as in, it has no total order across all accesses), so LLVM is free to reorder loads/stores in ways it would not be allowed to with Relaxed (it behaves a lot more like a non-atomic variable in this sense) In practical terms, in single-thread scenarios it behaves as expected, but when you load an atomic variable with unordered where the previous writer was another thread, you basically have to be prepared for it to hand you back any value previously written by that thread, due to the reordering allowed. Concretely, I don't know how we'd implement relaxed ordering by fencing without having that fence have a cost on weakly ordered machines (e.g. without implementing it as an overly-strong acquire/release fence). That said, I think we could add an intrinsic to LLVM that does what we want here. I just don't think it already exists. (FWIW, another part of the issue is that this stuff is not that well specified, but it's likely described by the "plain" accesses explained in https://www.cs.tau.ac.il/~orilahav/papers/popl17.pdf) |
|
CC @RalfJung who has stronger opinions on Unordered (and is the one who provided that link in the past). I think we can easily implement this with relaxed in compiler-builtins though, but it should get a new intrinsic, since many platforms can implement it more efficiently. |
|
We already have unordered atomic memcpy intrinsics in compiler-builtins. For 1, 2, 4 and 8 byte access sizes. |
|
I'm not sure we'd want unordered, as mentioned above... |
|
To clarify on the difference between relaxed and unordered (in terms of loads and stores), if you have static ATOM: AtomicU8 = AtomicU8::new(0);
const O: Ordering = ???;
fn thread1() {
ATOM.store(1, O);
ATOM.store(2, O);
}
fn thread2() {
let a = ATOM.load(O);
let b = ATOM.load(O);
assert!(a <= b);
}
In other words, for unordered, it would be legal for 2 to be stored before 1, or for |
|
something that could work but not be technically correct is: those fences are no-ops at runtime, but prevent the compiler from reordering the unordered atomics -- assuming your on any modern cpu (except Alpha iirc) it will behave like relaxed atomics because that's what standard load/store instructions do. |
|
Those fences aren't always no-ops at runtime, they actually emit code on several platforms (rust-lang/rust#62256). It's also unclear what can and can't be reordered across compiler fences (rust-lang/unsafe-code-guidelines#347), certainly plain stores can in some cases (this is easy to show happening in godbolt). Either way, my point has not been that we can't implement this. We absolutely can and it's probably even straightforward. My point is just that I don't really think those existing intrinsics help us do that. |
|
I like |
| loop { | ||
| let s1 = self.seq.load(Acquire); | ||
| let data = read_data(&self.data, Acquire); | ||
| let s2 = self.seq.load(Relaxed); |
There was a problem hiding this comment.
There's something very subtle here that I had not appreciated until a few weeks ago: we have to ensure that the load here cannot return an outdated value that would prevent us from noticing a seqnum bump.
The reason this is the case is that if there is a concurrent write, and if any
part of data reads from that write, then we have a release-acquire pair, so then we are guaranteed to see at least the first fetch_add from write, and thus we will definitely see a version conflict. OTOH if the s1 reads-from some second fetch_add in write, then that forms a release-acquire pair, and we will definitely see the full data.
So, all the release/acquire are necessary here. (I know this is not a seqlock tutorial, and @m-ou-se is certainly aware of this, but it still seemed worth pointing out -- many people reading this will not be aware of this.)
(This is related to this comment by @cbeuw.)
There was a problem hiding this comment.
Yeah exactly. This is why people are sometimes asking for a "release-load" operation. This second load operation needs to happen "after" the read_data() part, but the usual (incorrect) read_data implementation doesn't involve atomic operations or a memory ordering, so they attempt to solve this issue with a memory ordering on that final load, which isn't possible. The right solution is a memory ordering on the read_data() operation.
There was a problem hiding this comment.
Under a reordering based atomic model (as CPUs use), a release load makes sense and works. Release loads don't really work unless they are also RMWs (fetch_add(0)) under the C11 model.
There was a problem hiding this comment.
Yeah, the famous seqlock paper discusses "read dont-modify write" operations.
| while the second one is basically a memory fence followed by series of `AtomicU8::store`s. | ||
| Except the implementation can be much more efficient. | ||
| The implementation is allowed to load/store the bytes in any order, | ||
| and doesn't have to operate on individual bytes. |
There was a problem hiding this comment.
The "load/store bytes in any order" part is quite tricky, and I think means that the specification needs to be more complicated to allow for that.
I was originally thinking this would be specified as a series of AtomicU8 load/store with the respective order, no fence involved. That would still allow merging adjacent writes (I think), but it would not allow reordering bytes. I wonder if we could get away with that, or if implementations actually need the ability to reorder.
There was a problem hiding this comment.
For a memcpy (meaning the two regions are exclusive) you generally want to copy using increasing address order ("forward") on all hardware I've ever heard of. Even if a forward copy isn't faster (which it often is), it's still the same speed as a reverse copy.
I suspect the "any order is allowed" is just left in as wiggle room for potentially strange situations where somehow a reverse order copy would improve performance.
There was a problem hiding this comment.
The "load/store bytes in any order" part is quite tricky, and I think means that the specification needs to be more complicated to allow for that.
A loop of relaxed load/store operations followed/preceded by an acquire/release fence already effectively allows for the relaxed operations to happen in any order, right?
I was originally thinking this would be specified as a series of AtomicU8 load/store with the respective order, no fence involved.
In the C++ paper they are basically as:
for (size_t i = 0; i < count; ++i) { reinterpret_cast<char*>(dest)[i] = atomic_ref<char>(reinterpret_cast<char*>(source)[i]).load(memory_order::relaxed); } atomic_thread_fence(order);
and
atomic_thread_fence(order); for (size_t i = 0; i < count; ++i) { atomic_ref<char>(reinterpret_cast<char*>(dest)[i]).store( reinterpret_cast<char*>(source)[i], memory_order::relaxed); }
There was a problem hiding this comment.
A loop of relaxed load/store operations followed/preceded by an acquire/release fence already effectively allows for the relaxed operations to happen in any order, right?
Yes, relaxed loads/stores to different locations can be reordered, so specifying their order is moot under the as-if rule.
In the C++ paper they are basically as:
Hm... but usually fences and accesses are far from equivalent. If we specify them like this, calling code can rely on the presence of these fences. For example changing a 4-byte atomic acquire memcpy to an AtomicU32 acquire load would not be correct (even if we know everything is initialized and aligned etc).
Fence make all preceding/following relaxed accesses potentially induce synchronization, whereas release/acquire accesses only do that for that particular access.
Yeah, I don't think we should expose Unordered to users in any way until we are ready and willing to have our own concurrency memory model separate from that of C++ (or until C++ has something like unordered, and it's been shown to also make sense formally). There are some formal memory models with "plain" memory accesses, which are similar to unordered (no total mo order but race conditions allowed), but I have no idea if those are an accurate model of LLVM's unordered accesses. Both serve the same goal though, so there's a high chance they are at least related: both aim to model Java's regular memory accesses.
Well I sure hope we're not using them in any way that actually becomes observable in program behavior, as that would be unsound. |
|
turns out mixed <= 64-bit atomics does work on x86 if you use a new enough cpu: rust-lang/unsafe-code-guidelines#345 (comment) |
|
The debate about I don't think it makes sense for general-use primitives to have semantics that involve the concrete machine, which is why I think it makes sense for that to be a separate API. |
|
I don't understand what you mean by this. This operation is fully defined by saying that it behaves like a series of |
If there are Of course, you can argue that an untrusted program is not able to put |
|
Ah, I guess it's more of a series of Untrusted code is linked in at the assembly level, so it cannot write
I don't think such a guarantee makes sense. You are basically suggesting |
Assuming we have an AM way of doing an |
|
I don't think it is a useful enough to guarantee to justify the enormous amounts of work it'd take to make this reasonably precise. The compiler is and should be allowed to entirely omit operations that boil down to "store If you want assembly-level guarantees, write assembly code. |
|
In that case I think it would be right to recommend that people use an asm memcpy crate instead of calling |
|
Hi folks! I have wanted to implement a seqlock that safely works for any |
| // Incomplete example | ||
|
|
||
| pub struct SeqLock<T> { | ||
| seq: AtomicUsize, | ||
| data: UnsafeCell<T>, | ||
| } | ||
|
|
||
| unsafe impl Sync<T: Copy + Send> for SeqLock<T> {} | ||
|
|
||
| impl<T: Copy> SeqLock<T> { | ||
| /// Safety: Only call from one thread. | ||
| pub unsafe fn write(&self, value: T) { | ||
| self.seq.fetch_add(1, Relaxed); | ||
| write_data(&mut self.data, value, Release); | ||
| self.seq.fetch_add(1, Release); | ||
| } | ||
|
|
||
| pub fn read(&self) -> T { | ||
| loop { | ||
| let s1 = self.seq.load(Acquire); | ||
| let data = read_data(&self.data, Acquire); | ||
| let s2 = self.seq.load(Relaxed); | ||
| if s1 & 1 == 0 && s1 == s2 { | ||
| return unsafe { assume_valid(data) }; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Is the SeqLock implementation here sound? Couldn’t seq overflow such that s1 == s2 succeeds even though what truly happened is that the counter wrapped around the whole usize space once?
[Edit: I think I meant to quote the other SeqLock code block, but the point applies either way.]
There was a problem hiding this comment.
That's a known issue with seqlocks. The soundness relies on the overflow never happening in practice since it would require one thread to be suspended long enough for another thread to perform usize::MAX atomic increments.
There was a problem hiding this comment.
This should either be explained in a comment, or there should be an abort on overflow.
DemiMarie
left a comment
DemiMarie
left a comment
There was a problem hiding this comment.
It’s hard for me to tell if this API is sufficient without a lot of examples:
- When can one call
.assume_init()or cast to a ordinary Rust slice? - How does one get an
AtomicPerBytefrom a raw pointer?
Some examples would be extremely useful here. Ones I would like to see:
- Loading a
Copytype with all bit patterns valid and no padding (such asu32) from a pointer. - Storing a
Copytype with all bit patterns valid and no padding (such asu32) to a pointer. - Copying to a slice of a type with all bit patterns valid and no padding.
- Copying from a slice of a type with all bit patterns valid and no padding.
- Copying from an ordinary raw pointer.
- Copying to an ordinary raw pointer.
- Copying between
AtomicPerBytetypes, such as source and destination buffers that are both shared with untrusted code. - Copying to/from an SIMD vector type.
All of these should be sound even if the pointer is concurrently modified by untrusted code.
| - In order for this to be efficient, we need an additional intrinsic hooking into | ||
| special support in LLVM. (Which LLVM needs to have anyway for C++.) |
There was a problem hiding this comment.
IMO, having the efficient assembly version for popular architectures is going to be important for adoption. If the cost of soundness is a 25% performance drop for bulk copies, people might well keep using an unsound version.
| The `AtomicPerByte<T>` type can be thought of as | ||
| the `Sync` (data race free) equivalent of `MaybeUninit<T>`. | ||
| It can contain a `T`, but it might be invalid in various ways | ||
| due to concurrent store operations. |
There was a problem hiding this comment.
| due to concurrent store operations. | |
| due to concurrent store operations. However, it will never be invalid | |
| if all bit patterns are valid for the type. |
Otherwise, what would be the correct way to use this with memory shared with untrusted code?
| impl<T> AtomicPerByte<T> { | ||
| pub const fn new(value: T) -> Self; | ||
| pub const fn uninit() -> Self; | ||
|
|
||
| pub fn store(&self, value: T, ordering: Ordering); | ||
| pub fn load(&self, ordering: Ordering) -> MaybeUninit<T>; | ||
|
|
||
| pub fn store_from(&self, src: &MaybeUninit<T>, ordering: Ordering); | ||
| pub fn load_to(&self, dest: &mut MaybeUninit<T>, ordering: Ordering); | ||
|
|
||
| pub fn store_from_slice(this: &[Self], src: &[MaybeUninit<T>], ordering: Ordering); | ||
| pub fn load_to_slice(this: &[Self], dest: &mut [MaybeUninit<T>], ordering: Ordering); | ||
|
|
||
| pub const fn into_inner(self) -> MaybeUninit<T>; | ||
|
|
||
| pub const fn as_ptr(&self) -> *const T; | ||
| pub const fn as_mut_ptr(&self) -> *mut T; | ||
|
|
||
| pub const fn get_mut(&mut self) -> &mut MaybeUninit<T>; | ||
| pub const fn get_mut_slice(this: &mut [Self]) -> &mut [MaybeUninit<T>]; | ||
|
|
||
| pub const fn from_mut(value: &mut MaybeUninit<T>) -> &mut Self; | ||
| pub const fn from_mut_slice(slice: &mut [MaybeUninit<T>]) -> &mut [Self]; | ||
| } |
There was a problem hiding this comment.
I think the API is sufficient, but there are some areas where the documentation could use improvement:
- When can one call
.assume_init()or cast to a ordinary Rust slice? - How does one get an
AtomicPerBytefrom a raw pointer?
Some examples would be extremely useful here. Ones I would like to see:
- Loading a
Copytype with all bit patterns valid and no padding (such asu32) from a pointer. - Storing a
Copytype with all bit patterns valid and no padding (such asu32) to a pointer. - Copying to a slice of a type with all bit patterns valid and no padding.
- Copying from a slice of a type with all bit patterns valid and no padding.
- Copying from an ordinary raw pointer.
- Copying to an ordinary raw pointer.
- Copying between
AtomicPerBytetypes, such as source and destination buffers that are both shared with untrusted code. - Copying to/from an SIMD vector type.
| The `MaybeUninit` type is used to represent the potentially invalid state | ||
| the data might be in, since it might be the result of tearing during a race. | ||
|
|
||
| Only after confirming that there was no race and the data is valid | ||
| can one safely use `MaybeUninit::assume_init` to get the actual `T` out. |
There was a problem hiding this comment.
This isn’t sufficient for interoperating with untrusted code. The untrusted code can cause tearing to happen at will, so one can never guarantee that there was no race. However, one can choose to only use types for which all bit patterns are valid.
Alternatively, one could define that POD types (no padding, all bit patterns are valid) never tear.
There was a problem hiding this comment.
Alternatively, one could define that POD types (no padding, all bit patterns are valid) never tear.
arbitrarily large types are POD, e.g. [u8; 1000000], never tearing is impossible for all POD types on any reasonable multi-core CPU design. (technically you could have a CPU that blocks all other cores for the duration of an atomic memcpy, but I don't think that's reasonable for multi-core CPUs).
imo the best fix is to not make torn reads UB when all bit patterns are valid. That may need to be combined with some freeze primitive to convert stores of undef to some initialized bit pattern.
There was a problem hiding this comment.
“never tear” in the sense that this is not UB. Obviously they can tear in hardware.
|
A while back (2008!) I came up with a nifty algorithm for a seqlock with (for some use cases, anyway) dramatically better performance than the standard one: https://link.springer.com/chapter/10.1007/978-3-540-92221-6_40 and I was contemplating implementing it in Rust. And I found this thread while wondering how people handle ordinary seqlocks in Rust. After trying to catch up on it, I'm wondering how any of the proposals here handle provenance. It seems to me that the ultimate goal is to have a safely-callable API to stick any As far as I know, this is problem even in a single-threaded world: if I decompose some I can imagine this being solved be careful language in the spec, but I don't see any language about this at all in the RFC. Or would it actually need to be visible at the API level with some actual object, zero-sized or otherwise, that carries the provenance through the seqlock container? At least the But it's one thing to say that I'm suspicious that this construction does not work -- it's basically the same as the classic A possibly silly solution would be to introduce a (I'm not sure that "phantom" is the right word. I'm also not sure how this would fit in with architectures and runtimes that have real provenance, CHERI or Fil-C-style.) (I should see if I'm allowed to post a preprint or something of this paper somewhere that isn't paywalled, although the paper has nothing whatsoever to say about implementing any of this in a real programming language.) |
Indeed your question seems to have nothing to do with concurrency, so this isn't the right place to discuss it. Please have a look at https://doc.rust-lang.org/nightly/std/ptr/index.html#provenance and https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#validity and if there are still questions remaining, you can find us on Zulip. But note that a "zero-sized type that holds provenance" can not exist in Rust. Provenance is always attached to bytes. |
|
I've just written a post on IRLO about racy reads, and it actually gave me an idea about how to solve the Let me introduce struct AtomicCell<T>(UnsafeCell<T>);
unsafe impl<T: Send> Sync for AtomicCell<T> {}
impl<T> AtomicCell<T> {
pub const fn new(value: T) -> Self { Self(UnsafeCell::new(value)) }
/// Safety
///
/// Calls to `store` must be serialized,
/// i.e. two thread shall not call `store` concurrently.
pub unsafe fn store(&self, value: T, ordering: Ordering) { /* .. */ }
pub fn load(&self, ordering: Ordering) -> MaybeUninit<T> { /* .. */ }
pub const fn get_mut(&mut self) -> &mut T { self.0.get_mut() }
pub const fn as_ptr(&self) -> *mut T { self.0.get() }
}The key difference with
|
|
I’m much more interested in the raw low-level copy APIs. The reason is that some operations, such as copying from one process or VM to another, need this flexibility. This is because both reads and writes can race and the size of the operation is not known at compile-time. |
In the general case, unless your type is a POD with no invariant, racing stores will result into unusable garbage content. And even in the case of a POD with no invariant, the result would be so unpredictable that I wouldn't know how to use it. Could you elaborate a concrete use case where racing stores would be a desired feature? Regarding arbitrary size, I didn't include |
|
I’m one of the maintainers of iceoryx2 and here we use lock-free data structures in shared memory and need the equivalent of an “atomic memcpy” for some of them. Our data structures must be crash-resilient: if one process crashes in the middle of a modification, the data structure must remain valid and usable by other processes. Consider a queue where a process is terminated during a Since we are also working toward safety certification, undefined behavior caused by using @FerdinandSpitzschnueffler recently published an article about the implementation journey, including the challenges posed by padding and uninitialized bytes: https://ekxide.io/blog/byte-wise-atomic-wrapper-to-prevent-ub |
Writing to memory shared with untrusted code needs this. The untrusted code should not write the memory (if it does, it’s buggy), but if it does write, this must not cause a security hole. (The contents of the memory after the conflicting writes are irrelevant.) |
|
Sharing memory with untrusted code is an interesting use case but IMO is out-of-scope in this RFC as it is a way harder problem than what this RFC is trying to solve. |
Why is it harder? Genuine question. The straightforward desugaring to inline assembly works fine, so long as the memory is never visible to the AM (which, in my use-cases, it never will be). |
|
Keeping the memory entirely outside the AM is an option, but then you have to do everything with inline asm or volatile accesses (and this RFC is irrelevant for you). But I am not aware of any work exploring concurrency semantics where data races are not UB, and as you said that is a requirement for sharing memory with untrusted code where that memory actually becomes AM memory. See rust-lang/unsafe-code-guidelines#607 for details. |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Please keep this discussion on-topic for atomic memcpy. There are many things I want that are not atomic memcpy but that's irrelevant here. |
That's a very neat way of doing seqlocks. :) Shows that with some restrictions on |
20 participants
View all comments
Rendered