Skip to content

Commit 3502613

Browse files
taiki-etaiki-e
committed
Use atomic-maybe-uninit's atomic memcpy API
1 parent a404b7b commit 3502613

File tree

12 files changed

+175
-91
lines changed

12 files changed

+175
-91
lines changed

‎ci/san.sh‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,5 @@ RUSTFLAGS="${RUSTFLAGS:-} -Z sanitizer=memory" \
3434
# TODO: Use x86_64-unknown-linux-gnutsan once https://github.com/rust-lang/rust/pull/152757 merged
3535
# Run thread sanitizer
3636
cargo clean
37-
TSAN_OPTIONS="${TSAN_OPTIONS:-} suppressions=$(pwd)/ci/tsan" \
3837
RUSTFLAGS="${RUSTFLAGS:-} -Z sanitizer=thread" \
3938
cargo test -Z build-std --all --all-features --release --target x86_64-unknown-linux-gnu --tests --exclude benchmarks -- --test-threads=1

‎ci/tsan‎

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎crossbeam-deque/Cargo.toml‎

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

3940
[dev-dependencies]
4041
fastrand = "2"

‎crossbeam-deque/build.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ use std::env;
44

55
fn main() {
66
println!("cargo:rerun-if-changed=build.rs");
7-
println!("cargo:rustc-check-cfg=cfg(crossbeam_sanitize_thread)");
7+
println!("cargo:rustc-check-cfg=cfg(crossbeam_sanitize_thread,crossbeam_sanitize_any)");
88

99
// `cfg(sanitize = "..")` is not stabilized.
1010
let sanitize = env::var("CARGO_CFG_SANITIZE").unwrap_or_default();
11-
if sanitize.contains("thread") {
12-
println!("cargo:rustc-cfg=crossbeam_sanitize_thread");
11+
if !sanitize.is_empty() {
12+
println!("cargo:rustc-cfg=crossbeam_sanitize_any");
13+
if sanitize.contains("thread") {
14+
println!("cargo:rustc-cfg=crossbeam_sanitize_thread");
15+
}
1316
}
1417
}

‎crossbeam-deque/src/atomic_memcpy.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../crossbeam-utils/src/atomic/atomic_memcpy.rs

‎crossbeam-deque/src/deque.rs‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ use core::{
99
sync::atomic::{self, AtomicIsize, AtomicPtr, AtomicUsize, Ordering},
1010
};
1111

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

15-
use crate::alloc_helper::Global;
16+
use crate::{alloc_helper::Global, atomic_memcpy};
1617

1718
// Minimum buffer capacity.
1819
const MIN_CAP: usize = 64;
@@ -43,7 +44,7 @@ impl<T> Buffer<T> {
4344

4445
let ptr = Box::into_raw(
4546
(0..cap)
46-
.map(|_| MaybeUninit::<T>::uninit())
47+
.map(|_| PerByteAtomicMaybeUninit::new(MaybeUninit::<T>::uninit()))
4748
.collect::<Box<[_]>>(),
4849
)
4950
.cast::<T>();
@@ -70,23 +71,13 @@ impl<T> Buffer<T> {
7071
}
7172

7273
/// Writes `task` into the specified `index`.
73-
///
74-
/// This method might be concurrently called with another `read` at the same index, which is
75-
/// technically speaking a data race and therefore UB. We should use an atomic store here, but
76-
/// that would be more expensive and difficult to implement generically for all types `T`.
77-
/// Hence, as a hack, we use a volatile write instead.
7874
unsafe fn write(&self, index: isize, task: MaybeUninit<T>) {
79-
unsafe { ptr::write_volatile(self.at(index).cast::<MaybeUninit<T>>(), task) }
75+
unsafe { atomic_memcpy::store(self.at(index), task) }
8076
}
8177

8278
/// Reads a task from the specified `index`.
83-
///
84-
/// This method might be concurrently called with another `write` at the same index, which is
85-
/// technically speaking a data race and therefore UB. We should use an atomic load here, but
86-
/// that would be more expensive and difficult to implement generically for all types `T`.
87-
/// Hence, as a hack, we use a volatile load instead.
8879
unsafe fn read(&self, index: isize) -> MaybeUninit<T> {
89-
unsafe { ptr::read_volatile(self.at(index).cast::<MaybeUninit<T>>()) }
80+
unsafe { atomic_memcpy::load(self.at(index)) }
9081
}
9182
}
9283

‎crossbeam-deque/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ extern crate std;
102102

103103
#[cfg(feature = "std")]
104104
mod alloc_helper;
105+
#[cfg(feature = "std")]
106+
mod atomic_memcpy;
105107

106108
#[cfg(feature = "std")]
107109
mod deque;

‎crossbeam-utils/Cargo.toml‎

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

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

4141
# Enable the use of loom for concurrency testing.
4242
#

‎crossbeam-utils/build.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ include!("build-common.rs");
1818
fn main() {
1919
println!("cargo:rerun-if-changed=no_atomic.rs");
2020
println!(
21-
"cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread,crossbeam_atomic_cell_force_fallback)"
21+
"cargo:rustc-check-cfg=cfg(crossbeam_no_atomic,crossbeam_sanitize_thread,crossbeam_sanitize_any,crossbeam_atomic_cell_force_fallback)"
2222
);
2323

2424
let target = match env::var("TARGET") {
@@ -41,10 +41,12 @@ fn main() {
4141
}
4242

4343
// `cfg(sanitize = "..")` is not stabilized.
44-
if let Ok(sanitize) = env::var("CARGO_CFG_SANITIZE") {
44+
let sanitize = env::var("CARGO_CFG_SANITIZE").unwrap_or_default();
45+
if !sanitize.is_empty() {
46+
println!("cargo:rustc-cfg=crossbeam_sanitize_any");
47+
println!("cargo:rustc-cfg=crossbeam_atomic_cell_force_fallback");
4548
if sanitize.contains("thread") {
4649
println!("cargo:rustc-cfg=crossbeam_sanitize_thread");
4750
}
48-
println!("cargo:rustc-cfg=crossbeam_atomic_cell_force_fallback");
4951
}
5052
}

‎crossbeam-utils/src/atomic/atomic_cell.rs‎

Lines changed: 58 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ use core::{
99
ptr,
1010
};
1111

12-
use super::seq_lock::SeqLock;
12+
use super::{
13+
atomic_memcpy::{self, store as write},
14+
seq_lock::SeqLock,
15+
};
1316
use crate::{
1417
CachePadded,
1518
primitive::sync::atomic::{self, Ordering},
@@ -407,10 +410,10 @@ macro_rules! impl_arithmetic {
407410
}
408411
},
409412
{
410-
let _guard = lock(self.as_ptr() as usize).write();
411-
let value = unsafe { &mut *(self.as_ptr()) };
412-
let old = *value;
413-
*value = value.wrapping_add(val);
413+
let dst = self.as_ptr();
414+
let _guard = lock(dst as usize).write();
415+
let old = unsafe { ptr::read(dst) };
416+
unsafe { write(dst, MaybeUninit::new(old.wrapping_add(val))) }
414417
old
415418
}
416419
}
@@ -446,10 +449,10 @@ macro_rules! impl_arithmetic {
446449
}
447450
},
448451
{
449-
let _guard = lock(self.as_ptr() as usize).write();
450-
let value = unsafe { &mut *(self.as_ptr()) };
451-
let old = *value;
452-
*value = value.wrapping_sub(val);
452+
let dst = self.as_ptr();
453+
let _guard = lock(dst as usize).write();
454+
let old = unsafe { ptr::read(dst) };
455+
unsafe { write(dst, MaybeUninit::new(old.wrapping_sub(val))) }
453456
old
454457
}
455458
}
@@ -483,10 +486,10 @@ macro_rules! impl_arithmetic {
483486
}
484487
},
485488
{
486-
let _guard = lock(self.as_ptr() as usize).write();
487-
let value = unsafe { &mut *(self.as_ptr()) };
488-
let old = *value;
489-
*value &= val;
489+
let dst = self.as_ptr();
490+
let _guard = lock(dst as usize).write();
491+
let old = unsafe { ptr::read(dst) };
492+
unsafe { write(dst, MaybeUninit::new(old & val)) }
490493
old
491494
}
492495
}
@@ -520,10 +523,10 @@ macro_rules! impl_arithmetic {
520523
}
521524
},
522525
{
523-
let _guard = lock(self.as_ptr() as usize).write();
524-
let value = unsafe { &mut *(self.as_ptr()) };
525-
let old = *value;
526-
*value = !(old & val);
526+
let dst = self.as_ptr();
527+
let _guard = lock(dst as usize).write();
528+
let old = unsafe { ptr::read(dst) };
529+
unsafe { write(dst, MaybeUninit::new(!(old & val))) }
527530
old
528531
}
529532
}
@@ -557,10 +560,10 @@ macro_rules! impl_arithmetic {
557560
}
558561
},
559562
{
560-
let _guard = lock(self.as_ptr() as usize).write();
561-
let value = unsafe { &mut *(self.as_ptr()) };
562-
let old = *value;
563-
*value |= val;
563+
let dst = self.as_ptr();
564+
let _guard = lock(dst as usize).write();
565+
let old = unsafe { ptr::read(dst) };
566+
unsafe { write(dst, MaybeUninit::new(old | val)) }
564567
old
565568
}
566569
}
@@ -594,10 +597,10 @@ macro_rules! impl_arithmetic {
594597
}
595598
},
596599
{
597-
let _guard = lock(self.as_ptr() as usize).write();
598-
let value = unsafe { &mut *(self.as_ptr()) };
599-
let old = *value;
600-
*value ^= val;
600+
let dst = self.as_ptr();
601+
let _guard = lock(dst as usize).write();
602+
let old = unsafe { ptr::read(dst) };
603+
unsafe { write(dst, MaybeUninit::new(old ^ val)) }
601604
old
602605
}
603606
}
@@ -632,10 +635,10 @@ macro_rules! impl_arithmetic {
632635
}
633636
},
634637
{
635-
let _guard = lock(self.as_ptr() as usize).write();
636-
let value = unsafe { &mut *(self.as_ptr()) };
637-
let old = *value;
638-
*value = cmp::max(old, val);
638+
let dst = self.as_ptr();
639+
let _guard = lock(dst as usize).write();
640+
let old = unsafe { ptr::read(dst) };
641+
unsafe { write(dst, MaybeUninit::new(cmp::max(old, val))) }
639642
old
640643
}
641644
}
@@ -670,10 +673,10 @@ macro_rules! impl_arithmetic {
670673
}
671674
},
672675
{
673-
let _guard = lock(self.as_ptr() as usize).write();
674-
let value = unsafe { &mut *(self.as_ptr()) };
675-
let old = *value;
676-
*value = cmp::min(old, val);
676+
let dst = self.as_ptr();
677+
let _guard = lock(dst as usize).write();
678+
let old = unsafe { ptr::read(dst) };
679+
unsafe { write(dst, MaybeUninit::new(cmp::min(old, val))) }
677680
old
678681
}
679682
}
@@ -792,10 +795,10 @@ impl AtomicCell<bool> {
792795
}
793796
},
794797
{
795-
let _guard = lock(self.as_ptr() as usize).write();
796-
let value = unsafe { &mut *(self.as_ptr()) };
797-
let old = *value;
798-
*value &= val;
798+
let dst = self.as_ptr();
799+
let _guard = lock(dst as usize).write();
800+
let old = unsafe { ptr::read(dst) };
801+
unsafe { write(dst, MaybeUninit::new(old & val)) }
799802
old
800803
}
801804
}
@@ -835,10 +838,10 @@ impl AtomicCell<bool> {
835838
}
836839
},
837840
{
838-
let _guard = lock(self.as_ptr() as usize).write();
839-
let value = unsafe { &mut *(self.as_ptr()) };
840-
let old = *value;
841-
*value = !(old & val);
841+
let dst = self.as_ptr();
842+
let _guard = lock(dst as usize).write();
843+
let old = unsafe { ptr::read(dst) };
844+
unsafe { write(dst, MaybeUninit::new(!(old & val))) }
842845
old
843846
}
844847
}
@@ -875,10 +878,10 @@ impl AtomicCell<bool> {
875878
}
876879
},
877880
{
878-
let _guard = lock(self.as_ptr() as usize).write();
879-
let value = unsafe { &mut *(self.as_ptr()) };
880-
let old = *value;
881-
*value |= val;
881+
let dst = self.as_ptr();
882+
let _guard = lock(dst as usize).write();
883+
let old = unsafe { ptr::read(dst) };
884+
unsafe { write(dst, MaybeUninit::new(old | val)) }
882885
old
883886
}
884887
}
@@ -915,10 +918,10 @@ impl AtomicCell<bool> {
915918
}
916919
},
917920
{
918-
let _guard = lock(self.as_ptr() as usize).write();
919-
let value = unsafe { &mut *(self.as_ptr()) };
920-
let old = *value;
921-
*value ^= val;
921+
let dst = self.as_ptr();
922+
let _guard = lock(dst as usize).write();
923+
let old = unsafe { ptr::read(dst) };
924+
unsafe { write(dst, MaybeUninit::new(old ^ val)) }
922925
old
923926
}
924927
}
@@ -1045,13 +1048,7 @@ where
10451048

10461049
// Try doing an optimistic read first.
10471050
if let Some(stamp) = lock.optimistic_read() {
1048-
// We need a volatile read here because other threads might concurrently modify the
1049-
// value. In theory, data races are *always* UB, even if we use volatile reads and
1050-
// discard the data when a data race is detected. The proper solution would be to
1051-
// do atomic reads and atomic writes, but we can't atomically read and write all
1052-
// kinds of data since `AtomicU8` is not available on stable Rust yet.
1053-
// Load as `MaybeUninit` because we may load a value that is not valid as `T`.
1054-
let val = unsafe { ptr::read_volatile(src.cast::<MaybeUninit<T>>()) };
1051+
let val = unsafe { atomic_memcpy::load(src) };
10551052

10561053
if lock.validate_read(stamp) {
10571054
return unsafe { val.assume_init() };
@@ -1082,7 +1079,7 @@ unsafe fn atomic_store<T>(dst: *mut T, val: T) {
10821079
},
10831080
{
10841081
let _guard = lock(dst as usize).write();
1085-
unsafe { ptr::write(dst, val) }
1082+
unsafe { write(dst, MaybeUninit::new(val)) }
10861083
}
10871084
}
10881085
}
@@ -1102,7 +1099,9 @@ unsafe fn atomic_swap<T>(dst: *mut T, val: T) -> T {
11021099
},
11031100
{
11041101
let _guard = lock(dst as usize).write();
1105-
unsafe { ptr::replace(dst, val) }
1102+
let old = unsafe { ptr::read(dst.cast::<MaybeUninit<T>>()) };
1103+
unsafe { write(dst, MaybeUninit::new(val)) }
1104+
unsafe { old.assume_init() }
11061105
}
11071106
}
11081107
}
@@ -1156,7 +1155,7 @@ where
11561155

11571156
let old = unsafe { ptr::read(dst) };
11581157
if T::eq(&old, &current) {
1159-
unsafe { ptr::write(dst, new) }
1158+
unsafe { write(dst, MaybeUninit::new(new)) }
11601159
Ok(old)
11611160
} else {
11621161
// The value hasn't been changed. Drop the guard without incrementing the stamp.

0 commit comments

Comments
 (0)