stamp_it triggers -Winvalid-memory-model when compiled with gcc12

[captaincrutches]

When using stamp_it as the reclamation policy in a vyukov_hash_map or a harris_michael_list_based_set, this warning pops up when compiling with gcc-12:

In member function ‘bool std::atomic<_Tp>::compare_exchange_weak(_Tp&, _Tp, std::memory_order, std::memory_order) [with _Tp = xenium::marked_ptr<xenium::reclamation::stamp_it::thread_control_block, 18>]’,
    inlined from ‘static bool xenium::reclamation::stamp_it::thread_order_queue::mark_next(marked_ptr, size_t)’ at /home/captaincrutches/Projects/newt/thirdparty/xenium/xenium/reclamation/impl/stamp_it.hpp:513:44,
    inlined from ‘static bool xenium::reclamation::stamp_it::thread_order_queue::remove_from_prev_list(marked_ptr&, marked_ptr, marked_ptr&)’ at /home/captaincrutches/Projects/newt/thirdparty/xenium/xenium/reclamation/impl/stamp_it.hpp:281:23,
    inlined from ‘bool xenium::reclamation::stamp_it::thread_order_queue::remove(marked_ptr)’ at /home/captaincrutches/Projects/newt/thirdparty/xenium/xenium/reclamation/impl/stamp_it.hpp:148:47,
    inlined from ‘void xenium::reclamation::stamp_it::thread_data::leave_region()’ at /home/captaincrutches/Projects/newt/thirdparty/xenium/xenium/reclamation/impl/stamp_it.hpp:563:34:
/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.1/include/g++-v12/atomic:325:41: warning: failure memory model ‘memory_order_acquire’ cannot be stronger than success memory model ‘memory_order_relaxed’ for ‘bool __atomic_compare_exchange_8(volatile void*, void*, long unsigned int, bool, int, int)’ [-Winvalid-memory-model]
  325 |         return __atomic_compare_exchange(std::__addressof(_M_i),
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
  326 |                                          std::__addressof(__e),
      |                                          ~~~~~~~~~~~~~~~~~~~~~~
  327 |                                          std::__addressof(__i),
      |                                          ~~~~~~~~~~~~~~~~~~~~~~
  328 |                                          true, int(__s), int(__f));
      |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.1/include/g++-v12/atomic:325:41: note: valid models are 'memory_order_relaxed'

This, along with some other warnings, has also been reported recently as a Debian bug.

I'm able to reproduce these -Werror failures building the gtest target on master using gcc-12.

Sounds like the current CI config might need to be updated from gcc-9 and clang-10...

[mpoeter]

Which language standard do you use? Just below the line you indicated there is a comment:

xenium/xenium/reclamation/impl/stamp_it.hpp

Lines 513 to 520 in e1f4c66

	block->next.compare_exchange_weak(
	link, marked_ptr(link.get(), mark | DeleteMark), std::memory_order_relaxed, std::memory_order_acquire)) {
	// Note: the C++11 standard states that "the failure argument shall be no stronger than
	// the success argument". However, this has been relaxed in C++17.
	// (see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0418r1.html)
	// Some implementations (e.g., the Microsoft STL) perform runtime checks to enforce these
	// requirements. So in case the above CAS operation causes runtime errors, one has to use
	// release instead of relaxed order.

Apparently gcc performs this check during compile time instead of runtime. Either way, this requirement has been removed with C++14. The current version uses some C++17 features, so building with an older language version will probably fail for other reasons as well. If you are building with C++17 or newer it might be worth creating a gcc bug report.

As a quick fix you can just use memory_order_release instead of memory_order_relaxed.

Regarding the warnings you linked, these are because TSan does not support fences. I am aware of that and have according counter measures in place to avoid false positives, but previously the compiler simply ignored the fences instead of producing warnings. I will update the CI and create a fix for that. Thank you for pointing this out!

[captaincrutches]

I am using C++20. I knew the standard had changed it, but hadn't looked below that line for that comment 🤦‍♂️

Looks like there is already a GCC bug logged for this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104200

The other warnings I was less sure of, the one I listed was the only one appearing in my own project until I tried building xenium itself to reproduce. In the meantime, yes, I can easily make a local change or disable that warning for now.

Thanks for looking, and for being so responsive!