Skip to content

Commit

Permalink
Make std::sync::Arc compatible with ThreadSanitizer
Browse files Browse the repository at this point in the history
The memory fences used previously in Arc implementation are not properly
understood by ThreadSanitizer as synchronization primitives. This had
unfortunate effect where running any non-trivial program compiled with
`-Z sanitizer=thread` would result in numerous false positives.

Replace acquire fences with acquire loads when using ThreadSanitizer to
address the issue.
  • Loading branch information
tmiasko committed Mar 19, 2020
1 parent f4c675c commit fd0e15b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
#![feature(box_into_raw_non_null)]
#![feature(box_patterns)]
#![feature(box_syntax)]
#![feature(cfg_sanitize)]
#![feature(cfg_target_has_atomic)]
#![feature(coerce_unsized)]
#![feature(const_generic_impls_guard)]
Expand Down
25 changes: 21 additions & 4 deletions src/liballoc/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ mod tests;
/// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references.
const MAX_REFCOUNT: usize = (isize::MAX) as usize;

#[cfg(not(sanitize = "thread"))]
macro_rules! acquire {
($x:expr) => {
atomic::fence(Acquire)
};
}

// ThreadSanitizer does not support memory fences. To avoid false positive
// reports in Arc / Weak implementation use atomic loads for synchronization
// instead.
#[cfg(sanitize = "thread")]
macro_rules! acquire {
($x:expr) => {
$x.load(Acquire)
};
}

/// A thread-safe reference-counting pointer. 'Arc' stands for 'Atomically
/// Reference Counted'.
///
Expand Down Expand Up @@ -402,7 +419,7 @@ impl<T> Arc<T> {
return Err(this);
}

atomic::fence(Acquire);
acquire!(this.inner().strong);

unsafe {
let elem = ptr::read(&this.ptr.as_ref().data);
Expand Down Expand Up @@ -739,7 +756,7 @@ impl<T: ?Sized> Arc<T> {
ptr::drop_in_place(&mut self.ptr.as_mut().data);

if self.inner().weak.fetch_sub(1, Release) == 1 {
atomic::fence(Acquire);
acquire!(self.inner().weak);
Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref()))
}
}
Expand Down Expand Up @@ -1243,7 +1260,7 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Arc<T> {
//
// [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html)
// [2]: (https://github.com/rust-lang/rust/pull/41714)
atomic::fence(Acquire);
acquire!(self.inner().strong);

unsafe {
self.drop_slow();
Expand Down Expand Up @@ -1701,7 +1718,7 @@ impl<T: ?Sized> Drop for Weak<T> {
let inner = if let Some(inner) = self.inner() { inner } else { return };

if inner.weak.fetch_sub(1, Release) == 1 {
atomic::fence(Acquire);
acquire!(inner.weak);
unsafe { Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())) }
}
}
Expand Down

0 comments on commit fd0e15b

Please sign in to comment.