Atomics must be mutable by RalfJung · Pull Request #2464 · rust-lang/miri · GitH...
source link: https://github.com/rust-lang/miri/pull/2464
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Conversation
Member
RalfJung commented 14 days ago
Outdated
let (alloc_id, _offset, _prov) = |
||
this.ptr_try_get_alloc_id(place.ptr).expect("there are no zero-sized atomic accesses"); |
||
if this.get_alloc_mutability(alloc_id)? == Mutability::Not { |
||
throw_ub_format!("atomic operations cannot be performed on read-only memory"); |
Is it worth noting that this is target/size-specific for atomic loads? Or maybe there's a way to phrase it less strictly?
The guidance I've always been given for dealing with cross-process shared memory is that this is that atomics must be used if another process may write to it and that raw pointers (since obviously references are out of the question) would be UB (even if volatile were used), as it would still be a data race.
I do sympathize with not writing to add the target-specific logic for when this is broken to miri (it seems fragile and gross), but my concern is that encouraging people to not use atomics for this would lead them to perform worse UB.
(Unless we want to consider it not a data race?)
AFAIK, the following are cases where atomic loads perform operations that could be problematic on read-only memory. 1
- aarch64: 128-bit loads (LL/SC by ld(a)xp/st(l)xp) (if FEAT_LSE2 which makes load pair/store pair single-copy atomic is not available)
- x86_64: 128-bit loads (CAS by cmpxchg16b)
- x86: 64-bit loads (CAS by cmpxchg8b) (if both SSE and X87 are not available)
- arm (pre-v6,
Linux only): any non-relaxed/non-unordered loads (CAS by__kuser_cmpxchg
via__sync_*
builtins)
EDIT: pre-v6 arm issue is not Linux only.
EDIT: see rust-lang/unsafe-code-guidelines#355 (comment) for the emulator's case.
Considering the above, I think it is ok to allow atomic load on read-only memory when all of the following are met. (assuming we don't want to do complex and fragile target-specific processing)
- size is pointer-width or smaller. 2
- ordering is relaxed (or unordered).
And the error message would probably be something like:
atomic load operations with non-relaxed ordering or grater than pointer-width cannot be performed on read-only memory
Also, even if Miri does not allow them, I agree that additional notes would be helpful.
Footnotes
-
armv6+'s 64-bit load is
ldrexd
, powerpc64 and s390x have 128-bit atomic load instructions, and RISC-V and MIPS don't have double-width atomics, so those platforms are fine. I don't know about 32-bit powerpc/sparc's 64-bit atomic load -- IIRC LLVM does not support the former, and the latter is a tier 3 target. -
However, on 64-bit architecture's ILP32 ABIs, such as x86_64's X32 ABI, 64-bit load is definitely okay, so if Miri or rustc is aware of the architecture's pointer width, we may want to allow those as well.
Member
Author
RalfJung 13 days ago
Considering the above, I think it is ok to allow atomic load on read-only memory when all of the following are met. (assuming we don't want to do complex and fragile target-specific processing)
That's hoping we don't find further exceptions to these rules later? :D
We can possibly relax this rule later if there is consensus that all targets supported by Rust will support relaxed atomic loads of pointer size and smaller on read-only memory, but how sure are we no targets will violate this in the future?
Note that Miri anyway does not support sharing memory with other processes, so read-only memory here refers to static
without interior mutability. Due to rust-lang/unsafe-code-guidelines#303, this PR does not change behavior on any program that Miri currently accepts.
Member
Author
RalfJung 13 days ago
Also let's discuss the Miri implementation here; the spec is being discussed at rust-lang/unsafe-code-guidelines#355.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK