Skip to content

Commit f668a84

Browse files
author
Kostya Kortchinsky
committed
[scudo][standalone] Remove unused atomic_compare_exchange_weak
`atomic_compare_exchange_weak` is unused in Scudo, and its associated test is actually wrong since the weak variant is allowed to fail spuriously (thanks Roland). This lead to flakes such as: ``` [ RUN ] ScudoAtomicTest.AtomicCompareExchangeTest ../../zircon/third_party/scudo/src/tests/atomic_test.cpp:98: Failure: Expected atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) is true. Expected: true Which is: 01 Actual : atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) Which is: 00 ../../zircon/third_party/scudo/src/tests/atomic_test.cpp:100: Failure: Expected atomic_compare_exchange_weak( reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) is false. Expected: false Which is: 00 Actual : atomic_compare_exchange_weak( reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed) Which is: 01 ../../zircon/third_party/scudo/src/tests/atomic_test.cpp:101: Failure: Expected OldVal == NewVal. Expected: NewVal Which is: 24 Actual : OldVal Which is: 42 [ FAILED ] ScudoAtomicTest.AtomicCompareExchangeTest (0 ms) [----------] 2 tests from ScudoAtomicTest (1 ms total) ``` So I am removing this, if someone ever needs the weak variant, feel free to add it back with a test that is not as terrible. This test was initially ported from sanitizer_common, but their weak version calls the strong version, so it works for them. Differential Revision: https://reviews.llvm.org/D88443
1 parent 1e86d63 commit f668a84

File tree

2 files changed

+7
-27
lines changed

2 files changed

+7
-27
lines changed

compiler-rt/lib/scudo/standalone/atomic_helpers.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,6 @@ inline bool atomic_compare_exchange_strong(volatile T *A, typename T::Type *Cmp,
106106
__ATOMIC_RELAXED);
107107
}
108108

109-
template <typename T>
110-
inline bool atomic_compare_exchange_weak(volatile T *A, typename T::Type *Cmp,
111-
typename T::Type Xchg,
112-
memory_order MO) {
113-
return __atomic_compare_exchange(&A->ValDoNotUse, Cmp, &Xchg, true, MO,
114-
__ATOMIC_RELAXED);
115-
}
116-
117109
// Clutter-reducing helpers.
118110

119111
template <typename T>

compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -80,26 +80,14 @@ TEST(ScudoAtomicTest, AtomicStoreLoad) {
8080

8181
template <typename T> void checkAtomicCompareExchange() {
8282
typedef typename T::Type Type;
83-
{
84-
Type OldVal = 42;
85-
Type NewVal = 24;
86-
Type V = OldVal;
87-
EXPECT_TRUE(atomic_compare_exchange_strong(
88-
reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
89-
EXPECT_FALSE(atomic_compare_exchange_strong(
90-
reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
91-
EXPECT_EQ(NewVal, OldVal);
92-
}
93-
{
94-
Type OldVal = 42;
95-
Type NewVal = 24;
96-
Type V = OldVal;
97-
EXPECT_TRUE(atomic_compare_exchange_weak(reinterpret_cast<T *>(&V), &OldVal,
83+
Type OldVal = 42;
84+
Type NewVal = 24;
85+
Type V = OldVal;
86+
EXPECT_TRUE(atomic_compare_exchange_strong(reinterpret_cast<T *>(&V), &OldVal,
9887
NewVal, memory_order_relaxed));
99-
EXPECT_FALSE(atomic_compare_exchange_weak(
100-
reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
101-
EXPECT_EQ(NewVal, OldVal);
102-
}
88+
EXPECT_FALSE(atomic_compare_exchange_strong(
89+
reinterpret_cast<T *>(&V), &OldVal, NewVal, memory_order_relaxed));
90+
EXPECT_EQ(NewVal, OldVal);
10391
}
10492

10593
TEST(ScudoAtomicTest, AtomicCompareExchangeTest) {

0 commit comments

Comments
 (0)