Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

_Py_atomic_load_uint16 incorrect in pyatomic_std.h #146227

Copy link
Copy link
@colesbury

Description

@colesbury
Issue body actions

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.13bugs and security fixesbugs and security fixes3.14bugs and security fixesbugs and security fixes3.15pre-release feature fixes, bugs and security fixespre-release feature fixes, bugs and security fixestopic-free-threadingtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.