From 5a9913c2c6ef2943e821bc935e95f1a575969120 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 12 Jan 2026 10:44:14 -0600 Subject: [PATCH 1/3] add test cases --- test/test_type_method_crash.py | 149 +++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 test/test_type_method_crash.py diff --git a/test/test_type_method_crash.py b/test/test_type_method_crash.py new file mode 100644 index 00000000..1e4f58b9 --- /dev/null +++ b/test/test_type_method_crash.py @@ -0,0 +1,149 @@ +""" +Test for method calls on custom types - reproduces crash in Universal ABI. + +This test creates a simple custom type with a NOARGS method and verifies +that the method can be called on an instance. +""" +from .support import HPyTest + + +class TestTypeMethodCrash(HPyTest): + """ + Reproducer for crash when calling methods on custom types in Universal ABI. + + The crash occurs when: + 1. A custom type is created via HPyType_FromSpec + 2. An instance is created via HPy_New + 3. A method (defined via HPyDef_METH) is called on the instance + + The crash happens before the method body is entered, suggesting an issue + in the method dispatch mechanism for Universal ABI. + """ + + def test_simple_noargs_method(self): + """ + Minimal reproducer: type with HPyType_HELPERS and a NOARGS method. + """ + mod = self.make_module(""" + typedef struct { + long value; + } MyObject; + + HPyType_HELPERS(MyObject) + + HPyDef_SLOT(My_new, HPy_tp_new) + static HPy My_new_impl(HPyContext *ctx, HPy cls, const HPy *args, + HPy_ssize_t nargs, HPy kw) + { + MyObject *obj; + HPy h = HPy_New(ctx, cls, &obj); + if (HPy_IsNull(h)) + return HPy_NULL; + obj->value = 42; + return h; + } + + HPyDef_METH(My_get_value, "get_value", HPyFunc_NOARGS) + static HPy My_get_value_impl(HPyContext *ctx, HPy self) + { + MyObject *obj = MyObject_AsStruct(ctx, self); + return HPyLong_FromLong(ctx, obj->value); + } + + static HPyDef *My_defines[] = { + &My_new, + &My_get_value, + NULL + }; + + static HPyType_Spec My_spec = { + .name = "mytest.MyType", + .basicsize = sizeof(MyObject), + .defines = My_defines, + }; + + @EXPORT_TYPE("MyType", My_spec) + @INIT + """) + obj = mod.MyType() + # This call crashes in Universal ABI before the method body is entered + result = obj.get_value() + assert result == 42 + + def test_simple_noargs_method_no_struct_access(self): + """ + Even simpler: method that doesn't access struct at all. + """ + mod = self.make_module(""" + typedef struct { + long value; + } MyObject; + + HPyType_HELPERS(MyObject) + + HPyDef_SLOT(My_new, HPy_tp_new) + static HPy My_new_impl(HPyContext *ctx, HPy cls, const HPy *args, + HPy_ssize_t nargs, HPy kw) + { + MyObject *obj; + HPy h = HPy_New(ctx, cls, &obj); + if (HPy_IsNull(h)) + return HPy_NULL; + obj->value = 42; + return h; + } + + HPyDef_METH(My_constant, "constant", HPyFunc_NOARGS) + static HPy My_constant_impl(HPyContext *ctx, HPy self) + { + // Doesn't access struct at all - just returns a constant + return HPyLong_FromLong(ctx, 123); + } + + static HPyDef *My_defines[] = { + &My_new, + &My_constant, + NULL + }; + + static HPyType_Spec My_spec = { + .name = "mytest.MyType", + .basicsize = sizeof(MyObject), + .defines = My_defines, + }; + + @EXPORT_TYPE("MyType", My_spec) + @INIT + """) + obj = mod.MyType() + # This also crashes - proving the issue is not in AsStruct + result = obj.constant() + assert result == 123 + + def test_type_without_basicsize(self): + """ + Test with basicsize=0 - no custom struct, just methods. + """ + mod = self.make_module(""" + HPyDef_METH(My_constant, "constant", HPyFunc_NOARGS) + static HPy My_constant_impl(HPyContext *ctx, HPy self) + { + return HPyLong_FromLong(ctx, 456); + } + + static HPyDef *My_defines[] = { + &My_constant, + NULL + }; + + static HPyType_Spec My_spec = { + .name = "mytest.MyType", + .defines = My_defines, + }; + + @EXPORT_TYPE("MyType", My_spec) + @INIT + """) + obj = mod.MyType() + result = obj.constant() + assert result == 456 From 8caa9532e77c20f5e05b490aabf35c1636d514d9 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Mon, 12 Jan 2026 14:59:26 -0600 Subject: [PATCH 2/3] Fix issue --- hpy/devel/src/runtime/ctx_type.c | 95 +++++++++++++++++++++++++++++++- test/test_type_method_crash.py | 34 ++++++------ 2 files changed, 111 insertions(+), 18 deletions(-) diff --git a/hpy/devel/src/runtime/ctx_type.c b/hpy/devel/src/runtime/ctx_type.c index ef89e259..d91c4df3 100644 --- a/hpy/devel/src/runtime/ctx_type.c +++ b/hpy/devel/src/runtime/ctx_type.c @@ -208,8 +208,39 @@ static int _decref_visitor(HPyField *pf, void *arg) return 0; } -static void hpytype_clear(PyObject *self) +/* Python 3.13+ requires proper handling of Py_TPFLAGS_MANAGED_DICT. + * Types that inherit from object get this flag, and must call + * PyObject_VisitManagedDict in tp_traverse and PyObject_ClearManagedDict + * in tp_clear/tp_dealloc. Without this, attribute lookup crashes. + */ +#if PY_VERSION_HEX >= 0x030D0000 +#define HPY_HAS_MANAGED_DICT 1 +#else +#define HPY_HAS_MANAGED_DICT 0 +#endif + +#if HPY_HAS_MANAGED_DICT +/* Generic tp_traverse for HPy types that don't define their own HPy_tp_traverse. + * This is needed on Python 3.13+ because we need to visit the managed dict. + * For types WITH a user-defined traverse, the trampoline calls + * call_traverseproc_from_trampoline which handles managed dict. */ +static int hpytype_traverse(PyObject *self, cpy_visitproc visit, void *arg) +{ + /* On Python 3.13+, we must visit the managed dict */ + PyObject_VisitManagedDict(self, visit, arg); + /* Visit the type object (required for heap types) */ + Py_VISIT(Py_TYPE(self)); + return 0; +} +#endif /* HPY_HAS_MANAGED_DICT */ + +static int hpytype_clear(PyObject *self) { +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, we must clear the managed dict */ + PyObject_ClearManagedDict(self); +#endif + // call tp_traverse on all the HPy types of the hierarchy PyTypeObject *tp = Py_TYPE(self); PyTypeObject *base = tp; @@ -223,6 +254,7 @@ static void hpytype_clear(PyObject *self) } base = base->tp_base; } + return 0; } /* this is a generic tp_dealloc which we use for all the user-defined HPy @@ -243,6 +275,13 @@ static void hpytype_dealloc(PyObject *self) // decref and clear all the HPyFields hpytype_clear(self); +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, ensure managed dict is cleared before dealloc. + * Note: hpytype_clear already calls PyObject_ClearManagedDict, but + * calling it again is safe and ensures proper cleanup. */ + PyObject_ClearManagedDict(self); +#endif + // call tp_destroy on all the HPy types of the hierarchy PyTypeObject *base = tp; while(base) { @@ -668,8 +707,18 @@ create_slot_defs(HPyType_Spec *hpyspec, HPyType_Extra_t *extra, hpyslot_count++; // Py_tp_getset if (needs_dealloc) hpyslot_count++; // Py_tp_dealloc +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, we always need tp_traverse and tp_clear to handle + * the managed dict properly, even for types without user-defined traverse. + * If user provides HPy_tp_traverse, their trampoline handles managed dict. + * If not, we add hpytype_traverse. Either way, we add hpytype_clear. */ + if (!has_tp_traverse(hpyspec)) + hpyslot_count++; // Py_tp_traverse (only if user didn't provide one) + hpyslot_count++; // Py_tp_clear (always needed on 3.13+) +#else if (has_tp_traverse(hpyspec)) hpyslot_count++; // Py_tp_clear +#endif // allocate the result PyType_Slot array HPy_ssize_t total_slot_count = hpyslot_count + legacy_slot_count; @@ -824,10 +873,26 @@ create_slot_defs(HPyType_Spec *hpyspec, HPyType_Extra_t *extra, result[dst_idx++] = (PyType_Slot){Py_tp_dealloc, (void*)hpytype_dealloc}; } +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, we always need tp_traverse and tp_clear to handle + * the managed dict properly. Py_TPFLAGS_MANAGED_DICT is inherited from + * object, and requires calling PyObject_VisitManagedDict/ClearManagedDict. + * If user provides HPy_tp_traverse, their trampoline handles managed dict + * via call_traverseproc_from_trampoline. Otherwise, add hpytype_traverse. + * + * IMPORTANT: Py_TPFLAGS_MANAGED_DICT requires Py_TPFLAGS_HAVE_GC to be set. + * Since managed dict is inherited from object, we must ensure HAVE_GC is set. */ + *flags |= Py_TPFLAGS_HAVE_GC; + if (!has_tp_traverse(hpyspec)) { + result[dst_idx++] = (PyType_Slot){Py_tp_traverse, (void*)hpytype_traverse}; + } + result[dst_idx++] = (PyType_Slot){Py_tp_clear, (void*)hpytype_clear}; +#else // add a tp_clear, if the user provided a tp_traverse if (has_tp_traverse(hpyspec)) { result[dst_idx++] = (PyType_Slot){Py_tp_clear, (void*)hpytype_clear}; } +#endif // add the NULL sentinel at the end result[dst_idx++] = (PyType_Slot){0, NULL}; @@ -956,6 +1021,13 @@ static bool has_tp_traverse(HPyType_Spec *hpyspec) static bool needs_hpytype_dealloc(HPyType_Spec *hpyspec) { +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, we always need hpytype_dealloc because we set + * Py_TPFLAGS_HAVE_GC for proper managed dict handling. The dealloc + * must call PyObject_GC_UnTrack and PyObject_ClearManagedDict. */ + (void)hpyspec; /* unused */ + return true; +#else if (hpyspec->defines != NULL) for (int i = 0; hpyspec->defines[i] != NULL; i++) { HPyDef *def = hpyspec->defines[i]; @@ -964,6 +1036,7 @@ static bool needs_hpytype_dealloc(HPyType_Spec *hpyspec) return true; } return false; +#endif } static int check_have_gc_and_tp_traverse(HPyContext *ctx, HPyType_Spec *hpyspec) @@ -1585,6 +1658,26 @@ _HPy_HIDDEN int call_traverseproc_from_trampoline(HPyFunc_traverseproc tp_traver cpy_visitproc cpy_visit, void *cpy_arg) { + /* Visit the type object (required for heap types on Python 3.9+) */ +#if PY_VERSION_HEX >= 0x03090000 + { + PyObject *_py_type = (PyObject*)Py_TYPE(self); + int res = cpy_visit(_py_type, cpy_arg); + if (res) + return res; + } +#endif + +#if HPY_HAS_MANAGED_DICT + /* On Python 3.13+, we must visit the managed dict */ + { + int res = PyObject_VisitManagedDict(self, cpy_visit, cpy_arg); + if (res) + return res; + } +#endif + + /* Now call the user's traverse implementation */ hpy2cpy_visit_args_t args = { cpy_visit, cpy_arg }; return tp_traverse(_pyobj_as_struct(self), hpy2cpy_visit, &args); } diff --git a/test/test_type_method_crash.py b/test/test_type_method_crash.py index 1e4f58b9..ae996146 100644 --- a/test/test_type_method_crash.py +++ b/test/test_type_method_crash.py @@ -10,12 +10,12 @@ class TestTypeMethodCrash(HPyTest): """ Reproducer for crash when calling methods on custom types in Universal ABI. - + The crash occurs when: 1. A custom type is created via HPyType_FromSpec 2. An instance is created via HPy_New 3. A method (defined via HPyDef_METH) is called on the instance - + The crash happens before the method body is entered, suggesting an issue in the method dispatch mechanism for Universal ABI. """ @@ -28,9 +28,9 @@ def test_simple_noargs_method(self): typedef struct { long value; } MyObject; - + HPyType_HELPERS(MyObject) - + HPyDef_SLOT(My_new, HPy_tp_new) static HPy My_new_impl(HPyContext *ctx, HPy cls, const HPy *args, HPy_ssize_t nargs, HPy kw) @@ -42,26 +42,26 @@ def test_simple_noargs_method(self): obj->value = 42; return h; } - + HPyDef_METH(My_get_value, "get_value", HPyFunc_NOARGS) static HPy My_get_value_impl(HPyContext *ctx, HPy self) { MyObject *obj = MyObject_AsStruct(ctx, self); return HPyLong_FromLong(ctx, obj->value); } - + static HPyDef *My_defines[] = { &My_new, &My_get_value, NULL }; - + static HPyType_Spec My_spec = { .name = "mytest.MyType", .basicsize = sizeof(MyObject), .defines = My_defines, }; - + @EXPORT_TYPE("MyType", My_spec) @INIT """) @@ -78,9 +78,9 @@ def test_simple_noargs_method_no_struct_access(self): typedef struct { long value; } MyObject; - + HPyType_HELPERS(MyObject) - + HPyDef_SLOT(My_new, HPy_tp_new) static HPy My_new_impl(HPyContext *ctx, HPy cls, const HPy *args, HPy_ssize_t nargs, HPy kw) @@ -92,26 +92,26 @@ def test_simple_noargs_method_no_struct_access(self): obj->value = 42; return h; } - + HPyDef_METH(My_constant, "constant", HPyFunc_NOARGS) static HPy My_constant_impl(HPyContext *ctx, HPy self) { // Doesn't access struct at all - just returns a constant return HPyLong_FromLong(ctx, 123); } - + static HPyDef *My_defines[] = { &My_new, &My_constant, NULL }; - + static HPyType_Spec My_spec = { .name = "mytest.MyType", .basicsize = sizeof(MyObject), .defines = My_defines, }; - + @EXPORT_TYPE("MyType", My_spec) @INIT """) @@ -130,17 +130,17 @@ def test_type_without_basicsize(self): { return HPyLong_FromLong(ctx, 456); } - + static HPyDef *My_defines[] = { &My_constant, NULL }; - + static HPyType_Spec My_spec = { .name = "mytest.MyType", .defines = My_defines, }; - + @EXPORT_TYPE("MyType", My_spec) @INIT """) From 88a244017288efc6b521f3637bddf52ff73212c1 Mon Sep 17 00:00:00 2001 From: Mohamed Koubaa Date: Tue, 13 Jan 2026 12:38:19 -0600 Subject: [PATCH 3/3] Fix Python 3.13 crash: add Py_TPFLAGS_MANAGED_DICT for HPy types Python 3.13 changed how type __dict__ is handled for heap types. When using PyType_FromSpec, the Py_TPFLAGS_MANAGED_DICT flag is NOT automatically inherited from object - it must be set explicitly. Without this flag, creating instances of HPy types would crash in PyObject_GC_New() because the GC system expects managed dict support. Changes: - ctx_type.c: Explicitly set Py_TPFLAGS_MANAGED_DICT | Py_TPFLAGS_HAVE_GC in create_slot_defs() on Python 3.13+ - autogen_hpyfunc_trampolines.h: Fix NOARGS trampoline to include required second parameter per CPython C API docs (METH_NOARGS requires 2 params) --- .../universal/autogen_hpyfunc_trampolines.h | 6 +++++- hpy/devel/src/runtime/ctx_type.c | 20 ++++++++++++------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hpy/devel/include/hpy/universal/autogen_hpyfunc_trampolines.h b/hpy/devel/include/hpy/universal/autogen_hpyfunc_trampolines.h index 568b5621..e300a4c7 100644 --- a/hpy/devel/include/hpy/universal/autogen_hpyfunc_trampolines.h +++ b/hpy/devel/include/hpy/universal/autogen_hpyfunc_trampolines.h @@ -15,8 +15,12 @@ typedef struct { cpy_PyObject * result; } _HPyFunc_args_NOARGS; +/* NOTE: METH_NOARGS requires 2 parameters per CPython C API docs: + "The function must have 2 parameters. Since the second parameter is unused, + Py_UNUSED can be used to prevent a compiler warning." + The second parameter is always NULL but must be present in the signature. */ #define _HPyFunc_TRAMPOLINE_HPyFunc_NOARGS(SYM, IMPL) \ - static cpy_PyObject *SYM(cpy_PyObject *self) \ + static cpy_PyObject *SYM(cpy_PyObject *self, cpy_PyObject *_HPy_UNUSED_ARG(ignored)) \ { \ _HPyFunc_args_NOARGS a = { self }; \ _HPy_CallRealFunctionFromTrampoline( \ diff --git a/hpy/devel/src/runtime/ctx_type.c b/hpy/devel/src/runtime/ctx_type.c index d91c4df3..80ec14f1 100644 --- a/hpy/devel/src/runtime/ctx_type.c +++ b/hpy/devel/src/runtime/ctx_type.c @@ -875,14 +875,17 @@ create_slot_defs(HPyType_Spec *hpyspec, HPyType_Extra_t *extra, #if HPY_HAS_MANAGED_DICT /* On Python 3.13+, we always need tp_traverse and tp_clear to handle - * the managed dict properly. Py_TPFLAGS_MANAGED_DICT is inherited from - * object, and requires calling PyObject_VisitManagedDict/ClearManagedDict. + * the managed dict properly. Unlike what the Python docs suggest, + * Py_TPFLAGS_MANAGED_DICT is NOT automatically inherited from object + * when using PyType_FromSpec - we must set it explicitly. + * + * Setting MANAGED_DICT requires calling PyObject_VisitManagedDict/ClearManagedDict + * in traverse/clear, which we do in hpytype_traverse and hpytype_clear. * If user provides HPy_tp_traverse, their trampoline handles managed dict * via call_traverseproc_from_trampoline. Otherwise, add hpytype_traverse. * - * IMPORTANT: Py_TPFLAGS_MANAGED_DICT requires Py_TPFLAGS_HAVE_GC to be set. - * Since managed dict is inherited from object, we must ensure HAVE_GC is set. */ - *flags |= Py_TPFLAGS_HAVE_GC; + * IMPORTANT: Py_TPFLAGS_MANAGED_DICT requires Py_TPFLAGS_HAVE_GC to be set. */ + *flags |= Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_MANAGED_DICT; if (!has_tp_traverse(hpyspec)) { result[dst_idx++] = (PyType_Slot){Py_tp_traverse, (void*)hpytype_traverse}; } @@ -1477,15 +1480,18 @@ ctx_New(HPyContext *ctx, HPy h_type, void **data) } PyObject *result; - if (PyType_IS_GC(tp)) + int is_gc = PyType_IS_GC(tp); + if (is_gc) { result = PyObject_GC_New(PyObject, tp); - else + } else { result = PyObject_New(PyObject, tp); + } // HPy_New guarantees that the memory is zeroed, but PyObject_{GC}_New // doesn't. But we need to make sure to NOT overwrite ob_refcnt and // ob_type. See test_HPy_New_initialize_to_zero const HPyType_BuiltinShape shape = _HPyType_Get_Shape(tp); + HPy_ssize_t payload_size; void *payload; if (shape != HPyType_BuiltinShape_Legacy) {