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
Reactions are currently unavailable
Bug report
Found by Mohammed Zuhaib in https://discuss.python.org/t/code-explanation-for-absence-of-stdatomic-h-for-python-build/101623/30.
cpython/Include/cpython/pyatomic_std.h
Lines 458 to 463 in 6450b1d
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 # releaseLinked PRs