colesbury · GitHub

Bug report

Found by Mohammed Zuhaib in https://discuss.python.org/t/code-explanation-for-absence-of-stdatomic-h-for-python-build/101623/30.

static inline uint16_t
_Py_atomic_load_uint16(const uint16_t *obj)
{
_Py_USING_STD;
return atomic_load((const _Atomic(uint32_t)*)obj);
}

We cast to a uint32_t instead of uint16_t. I suspect this probably doesn't have noticeable effects on little-endian systems, but would load the wrong value on big-endian systems. We also mostly use pyatomic_gcc.h or pyatomic_msvc.h, which is probably why we didn't catch this in big-endian buildbots.

I asked Claude to review these files. In addition to the above bug:

  2. pyatomic_msc.h:974-978 — _Py_atomic_store_uint_release broken on ARM64
  static inline void
  _Py_atomic_store_uint_release(unsigned int *obj, unsigned int value)
  {
      *(volatile unsigned int *)obj = value;
  }
  This is placed among the relaxed stores and uses only a volatile store. On x86/x64, volatile stores have release semantics, so this happens to work. But on ARM64, volatile accesses are only relaxed
  (as stated in the file's header comment). Every other _release function in this file (e.g., _Py_atomic_store_int_release at line 1057, _Py_atomic_store_uint32_release at line 1108) properly uses
  __stlr32 on ARM64. This function is missing the #if/#elif ARM64 branch.
  3. pyatomic_gcc.h — Missing _Py_atomic_store_uint_release
  Both pyatomic_std.h (line 1035) and pyatomic_msc.h (line 975) define _Py_atomic_store_uint_release, but pyatomic_gcc.h does not. If any code calls it when compiled with GCC/Clang, it would fail to
  link/compile.
  4. pyatomic.h:75-76 — Incorrect pseudo-code comment
  def _Py_atomic_store_ptr_release(obj):
      return obj  # release
  This is documented as a one-arg function that returns a value, but it's actually a two-arg store (like the real declaration at line 518). Should be:
  def _Py_atomic_store_ptr_release(obj, value):
      obj = value  # release

Linked PRs

Read the original on github.com ↗