Skip to content

Commit 848a1a1

Browse files
committed
Use atomic-maybe-uninit's atomic memcpy API
1 parent b3fea26 commit 848a1a1

File tree

6 files changed

+71
-21
lines changed

6 files changed

+71
-21
lines changed

crossbeam-deque/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ std = ["crossbeam-epoch/std", "crossbeam-utils/std"]
2727
[dependencies]
2828
crossbeam-epoch = { version = "0.9.17", path = "../crossbeam-epoch", default-features = false }
2929
crossbeam-utils = { version = "0.8.18", path = "../crossbeam-utils", default-features = false }
30+
atomic-maybe-uninit = { version = "0.3.15", git = "https://github.com/taiki-e/atomic-maybe-uninit", branch = "memcpy" }
3031

3132
[dev-dependencies]
3233
fastrand = "2"

crossbeam-deque/build.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// The rustc-cfg emitted by the build script are *not* public API.
2+
3+
use std::env;
4+
5+
fn main() {
6+
println!("cargo:rerun-if-changed=build.rs");
7+
println!("cargo:rustc-check-cfg=cfg(crossbeam_sanitize_any)");
8+
9+
// `cfg(sanitize = "..")` is not stabilized.
10+
if let Ok(sanitize) = env::var("CARGO_CFG_SANITIZE") {
11+
if !sanitize.is_empty() {
12+
println!("cargo:rustc-cfg=crossbeam_sanitize_any");
13+
}
14+
}
15+
}

crossbeam-deque/src/deque.rs

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::ptr;
99
use std::sync::atomic::{self, AtomicIsize, AtomicPtr, AtomicUsize, Ordering};
1010
use std::sync::Arc;
1111

12+
use atomic_maybe_uninit::PerByteAtomicMaybeUninit;
1213
use crossbeam_epoch::{self as epoch, Atomic, Owned};
1314
use crossbeam_utils::{Backoff, CachePadded};
1415

@@ -41,7 +42,7 @@ impl<T> Buffer<T> {
4142

4243
let ptr = Box::into_raw(
4344
(0..cap)
44-
.map(|_| MaybeUninit::<T>::uninit())
45+
.map(|_| PerByteAtomicMaybeUninit::new(MaybeUninit::<T>::uninit()))
4546
.collect::<Box<[_]>>(),
4647
)
4748
.cast::<T>();
@@ -68,22 +69,37 @@ impl<T> Buffer<T> {
6869
}
6970

7071
/// Writes `task` into the specified `index`.
71-
///
72-
/// This method might be concurrently called with another `read` at the same index, which is
73-
/// technically speaking a data race and therefore UB. We should use an atomic store here, but
74-
/// that would be more expensive and difficult to implement generically for all types `T`.
75-
/// Hence, as a hack, we use a volatile write instead.
7672
unsafe fn write(&self, index: isize, task: MaybeUninit<T>) {
73+
atomic_maybe_uninit::cfg_has_atomic_memcpy! {
74+
if cfg!(not(any(miri, crossbeam_sanitize_any))) {
75+
unsafe {
76+
(*self.at(index).cast::<PerByteAtomicMaybeUninit<T>>()).store(task, Ordering::Relaxed);
77+
}
78+
return;
79+
}
80+
}
81+
// This method might be concurrently called with another `read` at the same index, which is
82+
// technically speaking a data race and therefore UB. We should use an atomic store here
83+
// like the code above, but inline assembly is not stable on some tier 2/3 targets and
84+
// unsupported in Miri/Sanitizer. Hence, as a hack, we use a volatile write instead.
85+
// See also https://github.com/rust-lang/rfcs/pull/3301.
7786
unsafe { ptr::write_volatile(self.at(index).cast::<MaybeUninit<T>>(), task) }
7887
}
7988

8089
/// Reads a task from the specified `index`.
81-
///
82-
/// This method might be concurrently called with another `write` at the same index, which is
83-
/// technically speaking a data race and therefore UB. We should use an atomic load here, but
84-
/// that would be more expensive and difficult to implement generically for all types `T`.
85-
/// Hence, as a hack, we use a volatile load instead.
8690
unsafe fn read(&self, index: isize) -> MaybeUninit<T> {
91+
atomic_maybe_uninit::cfg_has_atomic_memcpy! {
92+
if cfg!(not(any(miri, crossbeam_sanitize_any))) {
93+
unsafe {
94+
return (*self.at(index).cast::<PerByteAtomicMaybeUninit<T>>()).load(Ordering::Relaxed);
95+
}
96+
}
97+
}
98+
// This method might be concurrently called with another `write` at the same index, which is
99+
// technically speaking a data race and therefore UB. We should use an atomic load here
100+
// like the code above, but inline assembly is not stable on some tier 2/3 targets and
101+
// unsupported in Miri/Sanitizer. Hence, as a hack, we use a volatile write instead.
102+
// See also https://github.com/rust-lang/rfcs/pull/3301.
87103
unsafe { ptr::read_volatile(self.at(index).cast::<MaybeUninit<T>>()) }
88104
}
89105
}

crossbeam-utils/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ std = []
3030
atomic = ["atomic-maybe-uninit"]
3131

3232
[dependencies]
33-
atomic-maybe-uninit = { version = "0.3.4", optional = true }
33+
atomic-maybe-uninit = { version = "0.3.15", optional = true, git = "https://github.com/taiki-e/atomic-maybe-uninit", branch = "memcpy" }
3434

3535
# Enable the use of loom for concurrency testing.
3636
#

crossbeam-utils/build.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ include!("build-common.rs");
1717

1818
fn main() {
1919
println!("cargo:rerun-if-changed=no_atomic.rs");
20-
println!("cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread,crossbeam_atomic_cell_force_fallback)");
20+
println!("cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread,crossbeam_sanitize_any,crossbeam_atomic_cell_force_fallback)");
2121

2222
let target = match env::var("TARGET") {
2323
Ok(target) => convert_custom_linux_target(target),
@@ -43,6 +43,9 @@ fn main() {
4343
if sanitize.contains("thread") {
4444
println!("cargo:rustc-cfg=crossbeam_sanitize_thread");
4545
}
46-
println!("cargo:rustc-cfg=crossbeam_atomic_cell_force_fallback");
46+
if !sanitize.is_empty() {
47+
println!("cargo:rustc-cfg=crossbeam_sanitize_any");
48+
println!("cargo:rustc-cfg=crossbeam_atomic_cell_force_fallback");
49+
}
4750
}
4851
}

crossbeam-utils/src/atomic/atomic_cell.rs

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,13 +1121,28 @@ where
11211121

11221122
// Try doing an optimistic read first.
11231123
if let Some(stamp) = lock.optimistic_read() {
1124-
// We need a volatile read here because other threads might concurrently modify the
1125-
// value. In theory, data races are *always* UB, even if we use volatile reads and
1126-
// discard the data when a data race is detected. The proper solution would be to
1127-
// do atomic reads and atomic writes, but we can't atomically read and write all
1128-
// kinds of data since `AtomicU8` is not available on stable Rust yet.
1129-
// Load as `MaybeUninit` because we may load a value that is not valid as `T`.
1130-
let val = unsafe { ptr::read_volatile(src.cast::<MaybeUninit<T>>()) };
1124+
atomic_maybe_uninit::cfg_has_atomic_memcpy! {
1125+
use atomic_maybe_uninit::PerByteAtomicMaybeUninit;
1126+
let val = if cfg!(not(any(miri, crossbeam_sanitize_any))) {
1127+
unsafe {
1128+
(*src.cast::<PerByteAtomicMaybeUninit<T>>()).load(Ordering::Relaxed)
1129+
}
1130+
} else {
1131+
// See the comment in cfg_no_atomic_memcpy case.
1132+
unsafe { ptr::read_volatile(src.cast::<MaybeUninit<T>>()) }
1133+
};
1134+
};
1135+
atomic_maybe_uninit::cfg_no_atomic_memcpy! {
1136+
// We need a volatile read here because other threads might concurrently modify the
1137+
// value. In theory, data races are *always* UB, even if we use volatile reads and
1138+
// discard the data when a data race is detected. The proper solution would be to
1139+
// do atomic reads and atomic writes like the code above, but inline assembly is
1140+
// not stable on some tier 2/3 targets and unsupported in Miri/Sanitizer. Hence,
1141+
// as a hack, we use a volatile write instead.
1142+
// See also https://github.com/rust-lang/rfcs/pull/3301.
1143+
// Load as `MaybeUninit` because we may load a value that is not valid as `T`.
1144+
let val = unsafe { ptr::read_volatile(src.cast::<MaybeUninit<T>>()) };
1145+
}
11311146

11321147
if lock.validate_read(stamp) {
11331148
return unsafe { val.assume_init() };

0 commit comments

Comments
 (0)