| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
I saw a SIGBUS crash with the following backtrace immediately on startup when running a free-threaded build of https://github.com/jax-ml/jax on Python 3.13t:
#0 PyMember_SetOne (addr=0x20006fa48bc "", addr@entry=0x20006fa4890 "`\367N~", <incomplete sequence \346>, l=<optimized out>,
v=v@entry='Context manager for `jax2tf_associative_scan_reductions` config option.\n\nJAX has two separate lowering rules for the cumulative reduction primitives (cumsum, cumprod, cummax, cummin). On CPUs and GPUs it uses a lax.associative_scan, while for TPUs it uses the HLO ReduceWindow. The latter has a slow implementation on CPUs and GPUs. By default, jax2tf uses the TPU lowering. Set this flag to True to use the associative scan lowering usage, and only if it makes a difference for your application. See the jax2tf README.md for more details.') at Python/structmember.c:308
#1 0x0000b1fea968e908 in member_set (self=<member_descriptor at remote 0x20006384210>, obj=<State at remote 0x20006fa4890>,
value='Context manager for `jax2tf_associative_scan_reductions` config option.\n\nJAX has two separate lowering rules for the cumulative reduction primitives (cumsum, cumprod, cummax, cummin). On CPUs and GPUs it uses a lax.associative_scan, while for TPUs it uses the HLO ReduceWindow. The latter has a slow implementation on CPUs and GPUs. By default, jax2tf uses the TPU lowering. Set this flag to True to use the associative scan lowering usage, and only if it makes a difference for your application. See the jax2tf README.md for more details.') at Objects/descrobject.c:238
#2 0x0000b1fea96e9928 in _PyObject_GenericSetAttrWithDict (obj=<State at remote 0x20006fa4890>, name='__doc__',
value='Context manager for `jax2tf_associative_scan_reductions` config option.\n\nJAX has two separate lowering rules for the cumulative reduction primitives (cumsum, cumprod, cummax, cummin). On CPUs and GPUs it uses a lax.associative_scan, while for TPUs it uses the HLO ReduceWindow. The latter has a slow implementation on CPUs and GPUs. By default, jax2tf uses the TPU lowering. Set this flag to True to use the associative scan lowering usage, and only if it makes a difference for your application. See the jax2tf README.md for more details.', dict=dict@entry=0x0) at Objects/object.c:1778
What we're doing here is running this code:
obj.__doc__ = "..."
and crashing.
We're at this line of code:
Line 308 in 4f02615
| FT_ATOMIC_STORE_PTR_RELEASE(*(PyObject **)addr, Py_XNewRef(v)); |
Digging a little deeper, the relevant disassembly is:
(gdb) display /10i $pc-20 10: x/10i $pc-20 0xb1fea9860094 <PyMember_SetOne+1596>: bl 0xb1fea985f690 <_PyCriticalSection_BeginMutex> 0xb1fea9860098 <PyMember_SetOne+1600>: ldr x19, [x19, x21] 0xb1fea986009c <PyMember_SetOne+1604>: cbz x20, 0xb1fea98600a8 <PyMember_SetOne+1616> 0xb1fea98600a0 <PyMember_SetOne+1608>: mov x0, x20 0xb1fea98600a4 <PyMember_SetOne+1612>: bl 0xb1fea985f42c <Py_INCREF> => 0xb1fea98600a8 <PyMember_SetOne+1616>: stlr x20, [x24] 0xb1fea98600ac <PyMember_SetOne+1620>: add x0, sp, #0x8 0xb1fea98600b0 <PyMember_SetOne+1624>: bl 0xb1fea985f774 <_PyCriticalSection_End> 0xb1fea98600b4 <PyMember_SetOne+1628>: mov x0, x19 0xb1fea98600b8 <PyMember_SetOne+1632>: bl 0xb1fea985f664 <Py_XDECREF> (gdb) info registers ... x20 0x200064f9610 2199129134608 x21 0x2c 44 x22 0x10 16 x23 0x0 0 x24 0x20006fa48bc 2199140321468 x25 0x0 0 ...
What's going on here is that stlr's target address must be 8-byte aligned on aarch64: https://developer.arm.com/documentation/102336/0100/Load-Acquire-and-Store-Release-instructions
but the __doc__ field of this object is only 4-byte aligned, with offset = 44.
(gdb) up
(gdb) print *descr->d_member
$38 = {name = 0xb1fea9bdc950 <_PyRuntime+63952> "__doc__", type = 16, offset = 44, flags = 0, doc = 0x0}
We've placed an object field at an unaligned address, and used it in an atomic access that requires alignment, which is an error.
How should we fix this?
This seems like it's a CPython bug: CPython shouldn't choose underaligned slot offsets for object fields.
However, in this particular case it comes from a Python subclass of a C extension base class that has tp_basicsize=44; I'm also not aware of any rule that says tp_basicsize has to be a multiple of the word size. Perhaps CPython should either enforce that or round up the size of base classes to ensure alignment.
Or we can argue that CPython shouldn't be using an aligned atomic in this case.
What do you think?
3.13
Linux
| Back | FazBrowse Home | New Git URL |
Activity
hawkinsp commented on Feb 5, 2025
It occurs to me that this problem may also be present but in a sense worse on x86. I believe on x86, the "atomic" access is silently not atomic if it's unaligned. Perhaps there's code to check this somewhere in the CPython implementation, though, otherwise this means ostensibly atomic accesses are actually racy.
colesbury commented on Feb 5, 2025
Can you provide some more information about how you end up with an offset and tp_basicsize of 44?
hawkinsp commented on Feb 5, 2025
Here's a trivial repro, using nanobind:
repro.cc:
#include <Python.h> #include "nanobind/nanobind.h" namespace nb = nanobind; struct AClass { int x; }; NB_MODULE(repro, m) { nb::class_<AClass> c(m, "AClass"); c.def(nb::init<>()); }r.py
import repro class Child(repro.AClass): __slots__ = ("x",) c = Child() c.x = "hello"CMakeLists.txt:
cmake_minimum_required(VERSION 3.16) project(repro_project) # https://nanobind.readthedocs.io/en/latest/building.html if (CMAKE_VERSION VERSION_LESS 3.18) set(DEV_MODULE Development) else() set(DEV_MODULE Development.Module) endif() find_package(Python 3.8 COMPONENTS Interpreter ${DEV_MODULE} REQUIRED) if (NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build." FORCE) set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo") endif() function(append value) foreach(variable ${ARGN}) set(${variable} "${${variable}} ${value}" PARENT_SCOPE) endforeach(variable) endfunction() # Detect the installed nanobind package and import it into CMake execute_process( COMMAND "${Python_EXECUTABLE}" -m nanobind --cmake_dir OUTPUT_STRIP_TRAILING_WHITESPACE OUTPUT_VARIABLE nanobind_ROOT) find_package(nanobind CONFIG REQUIRED) # Abseil requires C++14 set(CMAKE_CXX_STANDARD 14) nanobind_add_module( repro FREE_THREADED repro.cc ) set_target_properties(repro PROPERTIES POSITION_INDEPENDENT_CODE ON)colesbury commented on Feb 5, 2025
Thanks for the repro.
I think this is a bug in nanobind, although the Python documentation and validation could be improved.
We can also add some additional validation of tp_basicsize in PyType_Ready, but I don't think we should adjust positive values of tp_basicsize.
colesbury commented on Feb 5, 2025
cc @encukou since this is related to PEP 697
Fix underalignment of nanobind-bound classes.
Fix underalignment of nanobind-bound classes. (#913)
hawkinsp commented on Feb 6, 2025
This is now fixed in nanobind, but I think there's still a couple of action items here for CPython to document and enforce this.
markshannon commented on Feb 7, 2025
I think might be a bug in CPython.
Take this struct:
and the corresponding PyTypeObject:
For all major compilers, sizeof(struct object44) == 48 is true, but I don't think the C spec requires it.
I think a size of 44 is legal, but I might be wrong.
colesbury commented on Feb 7, 2025
Not if pointers have 8 byte alignment (and if PyObject contains pointers). You need trailing padding so that the size of object44 is a multiple of its alignment. Otherwise C arrays would be broken. For example, if you declare a fixed size array in C:
Or allocate it using malloc:
Every element must be satisfy the alignment of struct object44.
Some compilers support __attribute__((packed)) or similar as an extension, but that's outside of the standard and doesn't work with arrays. (Some things are just silently broken with __attribute__((packed))!)
encukou commented on Feb 8, 2025
See #129850 for the docs part. (If we add a warning, we shouldn't backport it, so it should be a separate PR.)
Huh, right, that's curious. I can't find it spelled out explicitly!
But, the spec does give sizeof array / sizeof array[0] as an example for getting an array size, and more than just Python would break if malloc(sizeof(t) * n) didn't give you a t[n] array.
encukou commented on Feb 24, 2025
I'd appreciate a review on the docs PR, especially from someone who knows* what tp_basicsize/tp_itemsize should mean -- everyone has a slightly different idea :)
I'll merge it next week if there are no objections.
gh-129675: Update documentation for tp_basicsize & tp_itemsize (#129850)
pythongh-129675: Update documentation for tp_basicsize & tp_itemsize (p…
pythongh-129675: Update documentation for tp_basicsize & tp_itemsize (p…
[3.12] gh-129675: Update documentation for tp_basicsize & tp_itemsize (…
[3.13] gh-129675: Update documentation for tp_basicsize & tp_itemsize (…
colesbury commented on Nov 21, 2025
Closing as this is fixed in nanobind and the Python docs are updated