document when atomic loads are guaranteed read-only #115577

RalfJung · 2023-09-05T17:54:03Z

The values for x86 and x86_64 are complete guesswork on my side, and I have no clue what the values might be for other architectures. I hope we can get the right people to chime in to gather the required information. :)

I'll update Miri to respect these rules once we have more data.

rustbot · 2023-09-05T17:54:11Z

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

cuviper · 2023-09-05T18:11:20Z

I'm going to put this in the API domain, since it's making guarantees.

@rustbot label -T-libs +t-libs-api
r? libs-api

Amanieu · 2023-09-05T18:21:15Z

Some additions to the table:
aarch64 8 bytes¹
arm: 4 bytes²
riscv64: 8 bytes
riscv32: 4 bytes

ARMv8.4 raises this to 16 bytes, but I don't think we need to document this here. We're only guaranteeing minimums. ↩
Similarly, this is raised to 8 bytes with the LPAE extension. ↩

RalfJung · 2023-09-05T18:41:40Z

Cc @rust-lang/opsem for help in filling out the table

bjorn3 · 2023-09-06T12:04:25Z

Can GCC guarantee this too? cc @antoyo

library/core/src/sync/atomic.rs

antoyo · 2023-09-07T12:07:13Z

Can GCC guarantee this too? cc @antoyo

I'm not sure I have enough context/understanding of the situation.
Does Rust support atomic loads of bigger size (e.g. more than 8 bytes on x86_64)?
Isn't __atomic_load always guaranteed read-only?
Or are we talking about cases where Rust atomic loads use __atomic_compare_exchange?

bjorn3 · 2023-09-07T12:14:24Z

On x86 LLVM emits cmpxchg8b for 64bit loads and on x86_64 cmpxchg16b for 128bit loads as far as I understand, which is considered a write (and thus causes SIGSEGV on read-only memory) even if it wouldn't actually change the value at the memory location in question. For values of at most the register width LLVM will use mov on x86/x86_64 which is guaranteed read-only. Does GCC behave the same?

antoyo · 2023-09-07T12:30:19Z

On x86 LLVM emits cmpxchg8b for 64bit loads and on x86_64 cmpxchg16b for 128bit loads as far as I understand, which is considered a write (and thus causes SIGSEGV on read-only memory) even if it wouldn't actually change the value at the memory location in question. For values of at most the register width LLVM will use mov on x86/x86_64 which is guaranteed read-only. Does GCC behave the same?

The doc mentions:

16-byte integral types are also allowed if ‘__int128’ (see 128-bit Integers) is supported by the architecture.

but trying to do so on my computer results in the following error:

undefined reference to `__atomic_load_16'

and I read somewhere that 16-byte atomic operations always call a function in gcc, so not read-only.

I assume we would need to call __atomic_always_lock_free for every value in this table to make sure, but I'm not sure it would make sense that "always_lock_free" means "guaranteed read-only".

RalfJung · 2023-09-07T13:11:40Z

Rust atomics must always be lock-free, so if the GGC backend uses a lock implementation, that's already a bug -- but separate from the issue discussed here.

dtolnay · 2023-09-07T21:22:36Z

I'm going to put this in the API domain, since it's making guarantees.

@rust-lang/libs-api:

This is module-level documentation for core::sync::atomic. https://doc.rust-lang.org/1.72.0/core/sync/atomic/index.html

As I understand it, the new guarantee introduced here would make the following sound: we want a value that can be atomically loaded but the backing data might either be the read-write memory where interior mutable values ordinarily go, or could be readonly memory on certain architectures that have this new guarantee.

#[repr(transparent)]
pub struct AtomicallyLoadableU64(AtomicU64);

impl AtomicallyLoadableU64 {
    #[cfg(any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "riscv64"))]
    pub fn from_readonly(readonly: &u64) -> &Self {
        unsafe { ... }
    }

    pub fn from_interior_mutable(interior_mutable: &AtomicU64) -> &Self {
        unsafe { ... }
    }

    pub fn load(&self, ordering: Ordering) -> u64 {
        self.0.load(ordering)
    }
}

Perhaps we'd be happy to delegate this to a T-opsem FCP? I'm not sure that I would have any relevant API insight on this. It seems fine to guarantee, if this is a stable property these architectures and our compiler backends can guarantee.

@rfcbot poll libs-api -- opsem makes the call

rfcbot · 2023-09-07T21:22:38Z

Team member @dtolnay has asked teams: T-libs-api, for consensus on:

-- opsem makes the call

library/core/src/sync/atomic.rs

RalfJung · 2023-09-19T18:29:43Z

@BurntSushi @joshtriplett @m-ou-se there's an rfcbot poll waiting for your checkmark here. :)

RalfJung · 2023-09-22T11:48:10Z

The t-libs-api rfcbot poll has now reached the usual threshold for team consensus (a strict majority of approvals with at most two votes still open). So I assume we can hand this over to t-opsem.

@rfcbot merge

rfcbot · 2023-09-22T11:48:11Z

Team member @RalfJung has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

RalfJung · 2023-10-17T13:30:42Z

All right, so we have FCP passed and approval from our domain expert.
@Amanieu I think this is ready to land. :)

Amanieu · 2023-10-17T13:42:25Z

I'm still not very happy about not supporting load(Acquire), I honestly think it's a bug that this doesn't work on read-only memory and we should fix targets (like armv5) where this happens. But this can be done later.

@bors r+ rollup

bors · 2023-10-17T13:42:27Z

📌 Commit e494df4 has been approved by Amanieu

It is now in the queue for this repository.

bors · 2023-10-17T14:11:34Z

⌛ Testing commit e494df4 with merge 93e62a2...

bors · 2023-10-17T16:30:14Z

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 93e62a2 to master...

rust-timer · 2023-10-17T19:06:05Z

Finished benchmarking commit (93e62a2): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.7%	[1.7%, 1.7%]	1
Improvements ✅ (primary)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-1.9%	[-2.8%, -0.9%]	2
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.6%	[-5.6%, -5.6%]	1
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 626.338s -> 623.96s (-0.38%)
Artifact size: 305.62 MiB -> 305.57 MiB (-0.02%)

57: Pull upstream master 2023 10 18 r=pietroalbini a=Veykril * rust-lang/rust#116505 * rust-lang/rust#116840 * rust-lang/rust#116767 * rust-lang/rust#116855 * rust-lang/rust#116827 * rust-lang/rust#116787 * rust-lang/rust#116719 * rust-lang/rust#116717 * rust-lang/rust#111072 * rust-lang/rust#116844 * rust-lang/rust#115577 * rust-lang/rust#116756 * rust-lang/rust#116518 Co-authored-by: Urgau <urgau@numericable.fr> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Deadbeef <ent3rm4n@gmail.com> Co-authored-by: Ralf Jung <post@ralfj.de> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Celina G. Val <celinval@amazon.com> Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com> Co-authored-by: Arthur Lafrance <lafrancearthur@gmail.com> Co-authored-by: Nikolay Arhipov <n@arhipov.net> Co-authored-by: Nikita Popov <npopov@redhat.com> Co-authored-by: bors <bors@rust-lang.org>

taiki-e · 2023-10-28T05:01:05Z

@RalfJung

So, that sounds like we need an issue tracking that on that target, our atomics aren't lock-free as they should be? I haven't really been able to understand the details; @taiki-e I'd appreciate if you could file an issue. :)

Filed as #117305 and #117306.

accept some atomic loads from read-only memory matches rust-lang/rust#115577

…Jung accept some atomic loads from read-only memory matches rust-lang#115577

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 16, in accordance with the upstream changes. * Mark that on NetBSD we now need >= 9.0, so 8.x is no longer supported. * On NetBSD/sparc64 10.x, we now need GCC 12 to build the embedded LLVM, which is version 17; apparently GCC 10.4 or 10.5 mis-compiles it, resulting in an illegal instruction fault during the build. Ref. rust-lang/rust#117231 Upstream changes: Version 1.75.0 (2023-12-28) ========================== - [Stabilize `async fn` and return-position `impl Trait` in traits.] (rust-lang/rust#115822) - [Allow function pointer signatures containing `&mut T` in `const` contexts.] (rust-lang/rust#116015) - [Match `usize`/`isize` exhaustively with half-open ranges.] (rust-lang/rust#116692) - [Guarantee that `char` has the same size and alignment as `u32`.] (rust-lang/rust#116894) - [Document that the null pointer has the 0 address.] (rust-lang/rust#116988) - [Allow partially moved values in `match`.] (rust-lang/rust#103208) - [Add notes about non-compliant FP behavior on 32bit x86 targets.] (rust-lang/rust#113053) - [Stabilize ratified RISC-V target features.] (rust-lang/rust#116485) Compiler -------- - [Rework negative coherence to properly consider impls that only partly overlap.] (rust-lang/rust#112875) - [Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny, and warn in dependencies.] (rust-lang/rust#116493) - [Consider alias bounds when computing liveness in NLL.] (rust-lang/rust#116733) - [Add the V (vector) extension to the `riscv64-linux-android` target spec.] (rust-lang/rust#116618) - [Automatically enable cross-crate inlining for small functions] (rust-lang/rust#116505) - Add several new tier 3 targets: - [`csky-unknown-linux-gnuabiv2hf`] (rust-lang/rust#117049) - [`i586-unknown-netbsd`] (rust-lang/rust#117170) - [`mipsel-unknown-netbsd`] (rust-lang/rust#117356) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Override `Waker::clone_from` to avoid cloning `Waker`s unnecessarily.] (rust-lang/rust#96979) - [Implement `BufRead` for `VecDeque<u8>`.] (rust-lang/rust#110604) - [Implement `FusedIterator` for `DecodeUtf16` when the inner iterator does.] (rust-lang/rust#110729) - [Implement `Not, Bit{And,Or}{,Assign}` for IP addresses.] (rust-lang/rust#113747) - [Implement `Default` for `ExitCode`.] (rust-lang/rust#114589) - [Guarantee representation of None in NPO] (rust-lang/rust#115333) - [Document when atomic loads are guaranteed read-only.] (rust-lang/rust#115577) - [Broaden the consequences of recursive TLS initialization.] (rust-lang/rust#116172) - [Windows: Support sub-millisecond sleep.] (rust-lang/rust#116461) - [Fix generic bound of `str::SplitInclusive`'s `DoubleEndedIterator` impl] (rust-lang/rust#100806) - [Fix exit status / wait status on non-Unix `cfg(unix)` platforms.] (rust-lang/rust#115108) Stabilized APIs --------------- - [`Atomic*::from_ptr`] (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.from_ptr) - [`FileTimes`] (https://doc.rust-lang.org/stable/std/fs/struct.FileTimes.html) - [`FileTimesExt`] (https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTimesExt.html) - [`File::set_modified`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_modified) - [`File::set_times`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_times) - [`IpAddr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/enum.IpAddr.html#method.to_canonical) - [`Ipv6Addr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_canonical) - [`Option::as_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_slice) - [`Option::as_mut_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_mut_slice) - [`pointer::byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_add) - [`pointer::byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset) - [`pointer::byte_offset_from`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset_from) - [`pointer::byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_sub) - [`pointer::wrapping_byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_add) - [`pointer::wrapping_byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_offset) - [`pointer::wrapping_byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_sub) These APIs are now stable in const contexts: - [`Ipv6Addr::to_ipv4_mapped`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_ipv4_mapped) - [`MaybeUninit::assume_init_read`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.assume_init_read) - [`MaybeUninit::zeroed`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.zeroed) - [`mem::discriminant`] (https://doc.rust-lang.org/stable/core/mem/fn.discriminant.html) - [`mem::zeroed`] (https://doc.rust-lang.org/stable/core/mem/fn.zeroed.html) Cargo ----- - [Add new packages to `[workspace.members]` automatically.] (rust-lang/cargo#12779) - [Allow version-less `Cargo.toml` manifests.] (rust-lang/cargo#12786) - [Make browser links out of HTML file paths.] (rust-lang/cargo#12889) Rustdoc ------- - [Accept less invalid Rust in rustdoc.] (rust-lang/rust#117450) - [Document lack of object safety on affected traits.] (rust-lang/rust#113241) - [Hide `#[repr(transparent)]` if it isn't part of the public ABI.] (rust-lang/rust#115439) - [Show enum discriminant if it is a C-like variant.] (rust-lang/rust#116142) Compatibility Notes ------------------- - [FreeBSD targets now require at least version 12.] (rust-lang/rust#114521) - [Formally demote tier 2 MIPS targets to tier 3.] (rust-lang/rust#115238) - [Make misalignment a hard error in `const` contexts.] (rust-lang/rust#115524) - [Fix detecting references to packed unsized fields.] (rust-lang/rust#115583) - [Remove support for compiler plugins.] (rust-lang/rust#116412)

rustbot assigned cuviper Sep 5, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 5, 2023

RalfJung force-pushed the atomic-load branch from badf7d6 to f330560 Compare September 5, 2023 17:55

rustbot added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 5, 2023

rustbot assigned dtolnay and unassigned cuviper Sep 5, 2023

programmerjake reviewed Sep 7, 2023

View reviewed changes

library/core/src/sync/atomic.rs Show resolved Hide resolved

programmerjake reviewed Sep 8, 2023

View reviewed changes

library/core/src/sync/atomic.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the atomic-load branch from 7827676 to 7239980 Compare September 9, 2023 15:07

RalfJung commented Sep 9, 2023

View reviewed changes

library/core/src/sync/atomic.rs Outdated Show resolved Hide resolved

RalfJung added S-waiting-on-team DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ... T-opsem Relevant to the opsem team and removed T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 22, 2023

rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Sep 22, 2023

remove 128bit atomics, they are anyway not exposed on those targets

e494df4

taiki-e approved these changes Oct 17, 2023

View reviewed changes

RalfJung removed the S-waiting-on-team DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ... label Oct 17, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 17, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 17, 2023

bors merged commit 93e62a2 into rust-lang:master Oct 17, 2023

rustbot added this to the 1.75.0 milestone Oct 17, 2023

Veykril mentioned this pull request Oct 18, 2023

Pull upstream master 2023 10 18 ferrocene/ferrocene#57

Merged

apiraino removed the to-announce Announce this issue on triage meeting label Oct 19, 2023

RalfJung deleted the atomic-load branch October 20, 2023 17:04

taiki-e mentioned this pull request Oct 22, 2023

Detect non-perfectly-overlapping unordered atomic accesses as UB? rust-lang/miri#2303

Closed

RalfJung mentioned this pull request Oct 22, 2023

Stabilize atomic_from_ptr #115719

Merged

This was referenced Oct 28, 2023

ESP-IDF targets' Atomic*64 is not lock-free #117305

Closed

csky targets' atomic RMW is not lock-free #117306

Open

RalfJung mentioned this pull request Oct 28, 2023

accept some atomic loads from read-only memory rust-lang/miri#3149

Merged

bors added a commit to rust-lang/miri that referenced this pull request Oct 28, 2023

Auto merge of #3149 - RalfJung:atomic-readonly-loads, r=RalfJung

7577b35

accept some atomic loads from read-only memory matches rust-lang/rust#115577

RalfJung pushed a commit to RalfJung/rust that referenced this pull request Nov 4, 2023

Auto merge of rust-lang#3149 - RalfJung:atomic-readonly-loads, r=Ralf…

2c9baab

…Jung accept some atomic loads from read-only memory matches rust-lang#115577

taiki-e mentioned this pull request Jan 25, 2025

Mark all NuttX targets as tier 3 target and support the standard library #136037

Merged

taiki-e mentioned this pull request Sep 27, 2025

Support C-SKY (experimental) taiki-e/atomic-maybe-uninit#32

Merged

liamkinne mentioned this pull request Oct 24, 2025

systick-64bit on armv7 can deadlock rtic-rs/rtic#1117

Open

document when atomic loads are guaranteed read-only #115577

document when atomic loads are guaranteed read-only #115577

Uh oh!

Conversation

RalfJung commented Sep 5, 2023

Uh oh!

rustbot commented Sep 5, 2023

Uh oh!

cuviper commented Sep 5, 2023

Uh oh!

Amanieu commented Sep 5, 2023

Footnotes

Uh oh!

RalfJung commented Sep 5, 2023

Uh oh!

bjorn3 commented Sep 6, 2023

Uh oh!

Uh oh!

antoyo commented Sep 7, 2023

Uh oh!

bjorn3 commented Sep 7, 2023

Uh oh!

antoyo commented Sep 7, 2023

Uh oh!

RalfJung commented Sep 7, 2023

Uh oh!

dtolnay commented Sep 7, 2023

Uh oh!

rfcbot commented Sep 7, 2023 • edited by BurntSushi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RalfJung commented Sep 19, 2023

Uh oh!

RalfJung commented Sep 22, 2023

Uh oh!

rfcbot commented Sep 22, 2023 • edited by digama0 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RalfJung commented Oct 17, 2023

Uh oh!

Amanieu commented Oct 17, 2023

Uh oh!

bors commented Oct 17, 2023

Uh oh!

bors commented Oct 17, 2023

Uh oh!

bors commented Oct 17, 2023

Uh oh!

rust-timer commented Oct 17, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

taiki-e commented Oct 28, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

rfcbot commented Sep 7, 2023 •

edited by BurntSushi

Loading

rfcbot commented Sep 22, 2023 •

edited by digama0

Loading