[all-commits] [llvm/llvm-project] f668a8: [scudo][standalone] Remove unused atomic_compare_e...

Kostya Kortchinsky via All-commits all-commits at lists.llvm.org
Mon Sep 28 16:25:54 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: f668a84b58dcd816656236d7ba7e820dce52e7f8
      https://github.com/llvm/llvm-project/commit/f668a84b58dcd816656236d7ba7e820dce52e7f8
  Author: Kostya Kortchinsky <kostyak at google.com>
  Date:   2020-09-28 (Mon, 28 Sep 2020)

  Changed paths:
    M compiler-rt/lib/scudo/standalone/atomic_helpers.h
    M compiler-rt/lib/scudo/standalone/tests/atomic_test.cpp

  Log Message:
  -----------
  [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




More information about the All-commits mailing list