Commit db4d4f29 authored by Aaryaman Sagar's avatar Aaryaman Sagar Committed by Facebook Github Bot

Fix RMW bug in DeterministicSchedule

Summary:
There were some read-modify-write operations that were behaving as read and
then modify.  This is incorrect when you use DeterministicAtomic without a
backing schedule.

This is okay if the test is operating in the presence of a
DeterministicSchedule schedule, but is probably worth fixing anyway.  If using
DeterministicAtomic without a backing schedule is considered a bug, we can add
an assertion in beforeSharedAccess() and afterSharedAccess() so users know not
to try that

Reviewed By: djwatson, nbronson

Differential Revision: D10190682

fbshipit-source-id: 6a3173d904a40bbc5c96b934a76aff90a48c8608
parent 80b2ad02
...@@ -292,7 +292,7 @@ struct DeterministicAtomic { ...@@ -292,7 +292,7 @@ struct DeterministicAtomic {
/* implicit */ operator T() const noexcept { /* implicit */ operator T() const noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.operator T();
FOLLY_TEST_DSCHED_VLOG(this << "() -> " << std::hex << rv); FOLLY_TEST_DSCHED_VLOG(this << "() -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
return rv; return rv;
...@@ -362,12 +362,9 @@ struct DeterministicAtomic { ...@@ -362,12 +362,9 @@ struct DeterministicAtomic {
return rv; return rv;
} }
T fetch_add( T fetch_add(T v, std::memory_order mo = std::memory_order_seq_cst) noexcept {
T v,
std::memory_order /* mo */ = std::memory_order_seq_cst) noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.fetch_add(v, mo);
data += v;
FOLLY_TEST_DSCHED_VLOG( FOLLY_TEST_DSCHED_VLOG(
this << ".fetch_add(" << std::hex << v << ") -> " << std::hex << rv); this << ".fetch_add(" << std::hex << v << ") -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
...@@ -383,12 +380,9 @@ struct DeterministicAtomic { ...@@ -383,12 +380,9 @@ struct DeterministicAtomic {
return rv; return rv;
} }
T fetch_sub( T fetch_sub(T v, std::memory_order mo = std::memory_order_seq_cst) noexcept {
T v,
std::memory_order /* mo */ = std::memory_order_seq_cst) noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.fetch_sub(v, mo);
data -= v;
FOLLY_TEST_DSCHED_VLOG( FOLLY_TEST_DSCHED_VLOG(
this << ".fetch_sub(" << std::hex << v << ") -> " << std::hex << rv); this << ".fetch_sub(" << std::hex << v << ") -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
...@@ -404,12 +398,9 @@ struct DeterministicAtomic { ...@@ -404,12 +398,9 @@ struct DeterministicAtomic {
return rv; return rv;
} }
T fetch_and( T fetch_and(T v, std::memory_order mo = std::memory_order_seq_cst) noexcept {
T v,
std::memory_order /* mo */ = std::memory_order_seq_cst) noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.fetch_and(v, mo);
data &= v;
FOLLY_TEST_DSCHED_VLOG( FOLLY_TEST_DSCHED_VLOG(
this << ".fetch_and(" << std::hex << v << ") -> " << std::hex << rv); this << ".fetch_and(" << std::hex << v << ") -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
...@@ -425,12 +416,9 @@ struct DeterministicAtomic { ...@@ -425,12 +416,9 @@ struct DeterministicAtomic {
return rv; return rv;
} }
T fetch_or( T fetch_or(T v, std::memory_order mo = std::memory_order_seq_cst) noexcept {
T v,
std::memory_order /* mo */ = std::memory_order_seq_cst) noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.fetch_or(v, mo);
data |= v;
FOLLY_TEST_DSCHED_VLOG( FOLLY_TEST_DSCHED_VLOG(
this << ".fetch_or(" << std::hex << v << ") -> " << std::hex << rv); this << ".fetch_or(" << std::hex << v << ") -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
...@@ -446,12 +434,9 @@ struct DeterministicAtomic { ...@@ -446,12 +434,9 @@ struct DeterministicAtomic {
return rv; return rv;
} }
T fetch_xor( T fetch_xor(T v, std::memory_order mo = std::memory_order_seq_cst) noexcept {
T v,
std::memory_order /* mo */ = std::memory_order_seq_cst) noexcept {
DeterministicSchedule::beforeSharedAccess(); DeterministicSchedule::beforeSharedAccess();
T rv = data; T rv = data.fetch_xor(v, mo);
data ^= v;
FOLLY_TEST_DSCHED_VLOG( FOLLY_TEST_DSCHED_VLOG(
this << ".fetch_xor(" << std::hex << v << ") -> " << std::hex << rv); this << ".fetch_xor(" << std::hex << v << ") -> " << std::hex << rv);
DeterministicSchedule::afterSharedAccess(true); DeterministicSchedule::afterSharedAccess(true);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment