Skip to content

Commit

Permalink
Rollup merge of #65097 - tmiasko:arc, r=Amanieu
Browse files Browse the repository at this point in the history
Make std::sync::Arc compatible with ThreadSanitizer

The memory fences used previously in Arc implementation are not properly
understood by thread sanitizer 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 to address the issue.

Fixes #39608.
  • Loading branch information
Centril authored Mar 21, 2020
2 parents 1057dc9 + fd0e15b commit 4b91729
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 4b91729

Please sign in to comment.