-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
Closed
Labels
3.13bugs and security fixesbugs and security fixes3.14bugs and security fixesbugs and security fixes3.15new features, bugs and security fixesnew features, bugs and security fixestopic-free-threadingtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
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
| 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
Metadata
Metadata
Assignees
Labels
3.13bugs and security fixesbugs and security fixes3.14bugs and security fixesbugs and security fixes3.15new features, bugs and security fixesnew features, bugs and security fixestopic-free-threadingtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error