Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C API] Prepare PyTypeObject structure for a stable ABI: avoid accessing members in the public API #84351

Closed
vstinner opened this issue Apr 3, 2020 · 59 comments
Labels
3.10 only security fixes topic-C-API

Comments

@vstinner
Copy link
Member

vstinner commented Apr 3, 2020

BPO 40170
Nosy @rhettinger, @ronaldoussoren, @vstinner, @corona10, @miss-islington, @shihai1991, @erlend-aasland
PRs
  • bpo-40170: Remove PyIndex_Check() macro #19375
  • bpo-40170: Convert PyObject_CheckBuffer() macro to a function #19376
  • bpo-40170: PyObject_GET_WEAKREFS_LISTPTR() becomes a function #19377
  • bpo-40170: PyType_HasFeature() no longer acccess directly tp_flags #19378
  • bpo-40170: PyObject_NEW() becomes an alias to PyObject_New() #19379
  • bpo-40170: Add _PyIndex_Check() internal function #19426
  • bpo-40170: Remove PyIndex_Check() macro #19428
  • bpo-40170: Convert PyObject_IS_GC() macro to a function #19464
  • bpo-40170: Add _PyObject_CheckBuffer() internal function #19541
  • Revert "bpo-40170: PyType_HasFeature() now always calls PyType_GetFlags() (GH-19378)" #21390
  • [3.9] Revert "bpo-40170: PyType_HasFeature() now always calls PyType_GetFlags() (GH-19378)" (GH-21390) #21391
  • bpo-40170: Use inline _PyType_HasFeature() function #22375
  • bpo-40170: Add PyType_SetBase() function #23153
  • bpo-40170: Hide implementation detail of Py_TRASHCAN_BEGIN macro #23235
  • bpo-40170: Fix PyType_Ready() refleak on static type #23236
  • bpo-40170: Convert PyDescr_IsData macro to C function #24535
  • bpo-40170: Convert PyIter_Check macro to a function #24548
  • bpo-40170: Always define PyExceptionClass_Name as a function #24553
  • bpo-40170: Move 3 NEWS entries to the C API section #24555
  • bpo-40170: Update What's New in Python 3.9 #29470
  • [3.10] bpo-40170: Update What's New in Python 3.9 (GH-29470) #29471
  • [3.9] bpo-40170: Update What's New in Python 3.9 (GH-29470) #29472
  • bpo-40170: PyType_SUPPORTS_WEAKREFS() becomes a regular function #30938
  • bpo-40170: Remove _Py_GetAllocatedBlocks() function #30940
  • bpo-40170: Remove PyHeapType_GET_MEMBERS() macro #30942
  • bpo-40170: Move _Py_GetAllocatedBlocks() to pycore_pymem.h #30943
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-01-28.14:25:55.330>
    created_at = <Date 2020-04-03.11:57:03.054>
    labels = ['expert-C-API', '3.10']
    title = '[C API] Prepare PyTypeObject structure for a stable ABI: avoid accessing members in the public API'
    updated_at = <Date 2022-01-28.14:25:55.330>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2022-01-28.14:25:55.330>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-01-28.14:25:55.330>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2020-04-03.11:57:03.054>
    creator = 'vstinner'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 40170
    keywords = ['patch']
    message_count = 59.0
    messages = ['365689', '365795', '365848', '365850', '365852', '365862', '365863', '365873', '365956', '365960', '365961', '365962', '365964', '365966', '366152', '366155', '366417', '366474', '366494', '366521', '366522', '366529', '372050', '372054', '372056', '373293', '373295', '373298', '373338', '373346', '377372', '380757', '381775', '385176', '387057', '387059', '387092', '387113', '387115', '387129', '387131', '387133', '387140', '387142', '387146', '387177', '405956', '405964', '405966', '411816', '411817', '411818', '411819', '411822', '411824', '411919', '411939', '411998', '411999']
    nosy_count = 7.0
    nosy_names = ['rhettinger', 'ronaldoussoren', 'vstinner', 'corona10', 'miss-islington', 'shihai1991', 'erlendaasland']
    pr_nums = ['19375', '19376', '19377', '19378', '19379', '19426', '19428', '19464', '19541', '21390', '21391', '22375', '23153', '23235', '23236', '24535', '24548', '24553', '24555', '29470', '29471', '29472', '30938', '30940', '30942', '30943']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue40170'
    versions = ['Python 3.10']

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 3, 2020

    Leaking the PyTypeObject structure in the C API indirectly causes ABI issue (especially for statically allocated types), cause practical issues when old fields are removed and new fields are added (ex: tp_vectorcall addition and tp_print removal caused a lot of troubles with C code generated by Cython: see bpo-37250), prevents us to add feature and experiment optimization.

    I don't expect that we will be able to make PyTypeObject opaque soon. The purpose of this issue is to track the work done towards this goal.

    I propose to slowly prepare the Python code base, the C API and third party code (especially Cython) to make PyTypeObject structure opaque.

    We have to identify most common code patterns which access directly PyTypeObject fields, provide helper functions, and ease the migration to solutions which don't access directly PyTypeObject.

    See also bpo-39573: "Make PyObject an opaque structure in the limited C API" and bpo-39947 "Make the PyThreadState structure opaque (move it to the internal C API)".

    Longer rationale about making structures of the C API opaque:

    --

    Multiple practical issues are preventing us to make PyTypeObject opaque right now.

    (*) Many C extension modules are still using statically allocated types: there is an on-going effort in bpo-40077 to convert C extension modules one by one to PyType_FromSpec().

    (*) Py_TYPE(obj)->tp_name is commonly accessed to format an error message. Example:

            PyErr_Format(PyExc_TypeError, "exec() globals must be a dict, not %.100s",
                         Py_TYPE(globals)->tp_name);

    I worked on bpo-34595 and started a discussion on python-dev to propose to add a new %T formatter to PyUnicode_FromFormatV() and so indirectly to PyUnicode_FromFormat() and PyErr_Format():
    https://mail.python.org/archives/list/python-dev@python.org/thread/HKYUMTVHNBVB5LJNRMZ7TPUQKGKAERCJ/#3UAMHYG6UF4MPLXBZORHO4JVKUBRUZ53

    Sadly, we failed to reach a consensus and I gave up on this idea. We should reconsider this idea.

    We need to agree on how types should be formatted:

    • just the name without any dot "type_name",
    • qualified name "something.type_name",
    • fully qualified name "module.something.type_name"

    There is also the question of breaking applications which rely on the current exact error message. And the question of removing legacy "%.100s" which was used before Python was able to allocate a buffer large enough to arbitrary string length. When an error is formatted in pure Python, names are never truncated.

    (*) Call the function of the parent type when a method is overriden in a subclass. Example with PyTypeObject.tp_free called in a deallocator:

    static void
    abc_data_dealloc(_abc_data *self)
    {
        PyTypeObject *tp = Py_TYPE(self);
        ...
        tp->tp_free(self);
        Py_DECREF(tp);
    }

    () The PEP-384 provides the most generic PyType_GetSlot() but it's not convenient to use: need to handle error (NULL), need to cast the void into the expected type (error prone cast), etc.

    We should slowly add more and more helper functions for most common use cases. We can try to convert a few C extension modules of the Python stdlib to see which use cases are the most common.

    Hopefully, many use cases are already abstracted by widely used functions like PyNumber_Add(), PySequence_Size(), etc.

    (*) Likely other issues that I forgot.

    @vstinner vstinner added 3.9 only security fixes topic-C-API labels Apr 3, 2020
    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 4, 2020

    Macros and static inline functions of the public C API which access directly PyTypeObject fields. There may be more.

    #define _PyObject_SIZE(typeobj) ( (typeobj)->tp_basicsize )

    static inline vectorcallfunc
    PyVectorcall_Function(PyObject *callable)
    {
    ...
    tp = Py_TYPE(callable);
    offset = tp->tp_vectorcall_offset;
    ...
    }

    #define PyObject_CheckBuffer(obj) \
        ((Py_TYPE(obj)->tp_as_buffer != NULL) &&  \
         (Py_TYPE(obj)->tp_as_buffer->bf_getbuffer != NULL))
    
    #define PyIndex_Check(obj)                              \
        (Py_TYPE(obj)->tp_as_number != NULL &&            \
         Py_TYPE(obj)->tp_as_number->nb_index != NULL)
    
    #define PyObject_GET_WEAKREFS_LISTPTR(o) \
        ((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
    
    static inline int
    PyType_HasFeature(PyTypeObject *type, unsigned long feature) {
    #ifdef Py_LIMITED_API
        return ((PyType_GetFlags(type) & feature) != 0);
    #else
        return ((type->tp_flags & feature) != 0);
    #endif
    }
    
    #define _PyObject_SIZE(typeobj) ( (typeobj)->tp_basicsize )

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 6, 2020

    New changeset 38aefc5 by Victor Stinner in branch 'master':
    bpo-40170: PyObject_GET_WEAKREFS_LISTPTR() becomes a function (GH-19377)
    38aefc5

    @aixtools
    Copy link
    Contributor

    aixtools commented Apr 6, 2020

    Just manually verified that PR19377, when compiled against xlc - crashes during make:

    rm -f libpython3.9d.a
    ar rcs libpython3.9d.a [Modules/getbuildinfo.o](https://github.com/python/cpython/blob/main/Modules/getbuildinfo.o)  [Parser/acceler.o](https://github.com/python/cpython/blob/main/Parser/acceler.o)  [Parser/grammar1.o](https://github.com/python/cpython/blob/main/Parser/grammar1.o)  [Parser/listnode.o](https://github.com/python/cpython/blob/main/Parser/listnode.o)  [Parser/node.o](https://github.com/python/cpython/blob/main/Parser/node.o)  [Parser/parser.o](https://github.com/python/cpython/blob/main/Parser/parser.o)  [Parser/token.o](https://github.com/python/cpython/blob/main/Parser/token.o)   [Parser/myreadline.o](https://github.com/python/cpython/blob/main/Parser/myreadline.o) [Parser/parsetok.o](https://github.com/python/cpython/blob/main/Parser/parsetok.o) [Parser/tokenizer.o](https://github.com/python/cpython/blob/main/Parser/tokenizer.o)  [Objects/abstract.o](https://github.com/python/cpython/blob/main/Objects/abstract.o)  [Objects/accu.o](https://github.com/python/cpython/blob/main/Objects/accu.o)  [Objects/boolobject.o](https://github.com/python/cpython/blob/main/Objects/boolobject.o)  [Objects/bytes_methods.o](https://github.com/python/cpython/blob/main/Objects/bytes_methods.o)  [Objects/bytearrayobject.o](https://github.com/python/cpython/blob/main/Objects/bytearrayobject.o)  [Objects/bytesobject.o](https://github.com/python/cpython/blob/main/Objects/bytesobject.o)  [Objects/call.o](https://github.com/python/cpython/blob/main/Objects/call.o)  [Objects/capsule.o](https://github.com/python/cpython/blob/main/Objects/capsule.o)  [Objects/cellobject.o](https://github.com/python/cpython/blob/main/Objects/cellobject.o)  [Objects/classobject.o](https://github.com/python/cpython/blob/main/Objects/classobject.o)  [Objects/codeobject.o](https://github.com/python/cpython/blob/main/Objects/codeobject.o)  [Objects/complexobject.o](https://github.com/python/cpython/blob/main/Objects/complexobject.o)  [Objects/descrobject.o](https://github.com/python/cpython/blob/main/Objects/descrobject.o)  [Objects/enumobject.o](https://github.com/python/cpython/blob/main/Objects/enumobject.o)  [Objects/exceptions.o](https://github.com/python/cpython/blob/main/Objects/exceptions.o)  [Objects/genobject.o](https://github.com/python/cpython/blob/main/Objects/genobject.o)  [Objects/fileobject.o](https://github.com/python/cpython/blob/main/Objects/fileobject.o)  [Objects/floatobject.o](https://github.com/python/cpython/blob/main/Objects/floatobject.o)  [Objects/frameobject.o](https://github.com/python/cpython/blob/main/Objects/frameobject.o)  [Objects/funcobject.o](https://github.com/python/cpython/blob/main/Objects/funcobject.o)  [Objects/interpreteridobject.o](https://github.com/python/cpython/blob/main/Objects/interpreteridobject.o)  [Objects/iterobject.o](https://github.com/python/cpython/blob/main/Objects/iterobject.o)  [Objects/listobject.o](https://github.com/python/cpython/blob/main/Objects/listobject.o)  [Objects/longobject.o](https://github.com/python/cpython/blob/main/Objects/longobject.o)  [Objects/dictobject.o](https://github.com/python/cpython/blob/main/Objects/dictobject.o)  [Objects/odictobject.o](https://github.com/python/cpython/blob/main/Objects/odictobject.o)  [Objects/memoryobject.o](https://github.com/python/cpython/blob/main/Objects/memoryobject.o)  [Objects/methodobject.o](https://github.com/python/cpython/blob/main/Objects/methodobject.o)  [Objects/moduleobject.o](https://github.com/python/cpython/blob/main/Objects/moduleobject.o)  [Objects/namespaceobject.o](https://github.com/python/cpython/blob/main/Objects/namespaceobject.o)  [Objects/object.o](https://github.com/python/cpython/blob/main/Objects/object.o)  [Objects/obmalloc.o](https://github.com/python/cpython/blob/main/Objects/obmalloc.o)  [Objects/picklebufobject.o](https://github.com/python/cpython/blob/main/Objects/picklebufobject.o)  [Objects/rangeobject.o](https://github.com/python/cpython/blob/main/Objects/rangeobject.o)  [Objects/setobject.o](https://github.com/python/cpython/blob/main/Objects/setobject.o)  [Objects/sliceobject.o](https://github.com/python/cpython/blob/main/Objects/sliceobject.o)  [Objects/structseq.o](https://github.com/python/cpython/blob/main/Objects/structseq.o)  [Objects/tupleobject.o](https://github.com/python/cpython/blob/main/Objects/tupleobject.o)  [Objects/typeobject.o](https://github.com/python/cpython/blob/main/Objects/typeobject.o)  [Objects/unicodeobject.o](https://github.com/python/cpython/blob/main/Objects/unicodeobject.o)  [Objects/unicodectype.o](https://github.com/python/cpython/blob/main/Objects/unicodectype.o)  [Objects/weakrefobject.o](https://github.com/python/cpython/blob/main/Objects/weakrefobject.o)  [Python/_warnings.o](https://github.com/python/cpython/blob/main/Python/_warnings.o)  [Python/Python-ast.o](https://github.com/python/cpython/blob/main/Python/Python-ast.o)  [Python/asdl.o](https://github.com/python/cpython/blob/main/Python/asdl.o)  [Python/ast.o](https://github.com/python/cpython/blob/main/Python/ast.o)  [Python/ast_opt.o](https://github.com/python/cpython/blob/main/Python/ast_opt.o)  [Python/ast_unparse.o](https://github.com/python/cpython/blob/main/Python/ast_unparse.o)  [Python/bltinmodule.o](https://github.com/python/cpython/blob/main/Python/bltinmodule.o)  [Python/ceval.o](https://github.com/python/cpython/blob/main/Python/ceval.o)  [Python/codecs.o](https://github.com/python/cpython/blob/main/Python/codecs.o)  [Python/compile.o](https://github.com/python/cpython/blob/main/Python/compile.o)  [Python/context.o](https://github.com/python/cpython/blob/main/Python/context.o)  [Python/dynamic_annotations.o](https://github.com/python/cpython/blob/main/Python/dynamic_annotations.o)  [Python/errors.o](https://github.com/python/cpython/blob/main/Python/errors.o)  [Python/frozenmain.o](https://github.com/python/cpython/blob/main/Python/frozenmain.o)  [Python/future.o](https://github.com/python/cpython/blob/main/Python/future.o)  [Python/getargs.o](https://github.com/python/cpython/blob/main/Python/getargs.o)  [Python/getcompiler.o](https://github.com/python/cpython/blob/main/Python/getcompiler.o)  [Python/getcopyright.o](https://github.com/python/cpython/blob/main/Python/getcopyright.o)  [Python/getplatform.o](https://github.com/python/cpython/blob/main/Python/getplatform.o)  [Python/getversion.o](https://github.com/python/cpython/blob/main/Python/getversion.o)  [Python/graminit.o](https://github.com/python/cpython/blob/main/Python/graminit.o)  [Python/hamt.o](https://github.com/python/cpython/blob/main/Python/hamt.o)  [Python/import.o](https://github.com/python/cpython/blob/main/Python/import.o)  [Python/importdl.o](https://github.com/python/cpython/blob/main/Python/importdl.o)  [Python/initconfig.o](https://github.com/python/cpython/blob/main/Python/initconfig.o)  [Python/marshal.o](https://github.com/python/cpython/blob/main/Python/marshal.o)  [Python/modsupport.o](https://github.com/python/cpython/blob/main/Python/modsupport.o)  [Python/mysnprintf.o](https://github.com/python/cpython/blob/main/Python/mysnprintf.o)  [Python/mystrtoul.o](https://github.com/python/cpython/blob/main/Python/mystrtoul.o)  [Python/pathconfig.o](https://github.com/python/cpython/blob/main/Python/pathconfig.o)  [Python/peephole.o](https://github.com/python/cpython/blob/main/Python/peephole.o)  [Python/preconfig.o](https://github.com/python/cpython/blob/main/Python/preconfig.o)  [Python/pyarena.o](https://github.com/python/cpython/blob/main/Python/pyarena.o)  [Python/pyctype.o](https://github.com/python/cpython/blob/main/Python/pyctype.o)  [Python/pyfpe.o](https://github.com/python/cpython/blob/main/Python/pyfpe.o)  [Python/pyhash.o](https://github.com/python/cpython/blob/main/Python/pyhash.o)  [Python/pylifecycle.o](https://github.com/python/cpython/blob/main/Python/pylifecycle.o)  [Python/pymath.o](https://github.com/python/cpython/blob/main/Python/pymath.o)  [Python/pystate.o](https://github.com/python/cpython/blob/main/Python/pystate.o)  [Python/pythonrun.o](https://github.com/python/cpython/blob/main/Python/pythonrun.o)  [Python/pytime.o](https://github.com/python/cpython/blob/main/Python/pytime.o)  [Python/bootstrap_hash.o](https://github.com/python/cpython/blob/main/Python/bootstrap_hash.o)  [Python/structmember.o](https://github.com/python/cpython/blob/main/Python/structmember.o)  [Python/symtable.o](https://github.com/python/cpython/blob/main/Python/symtable.o)  [Python/sysmodule.o](https://github.com/python/cpython/blob/main/Python/sysmodule.o)  [Python/thread.o](https://github.com/python/cpython/blob/main/Python/thread.o)  [Python/traceback.o](https://github.com/python/cpython/blob/main/Python/traceback.o)  [Python/getopt.o](https://github.com/python/cpython/blob/main/Python/getopt.o)  [Python/pystrcmp.o](https://github.com/python/cpython/blob/main/Python/pystrcmp.o)  [Python/pystrtod.o](https://github.com/python/cpython/blob/main/Python/pystrtod.o)  [Python/pystrhex.o](https://github.com/python/cpython/blob/main/Python/pystrhex.o)  [Python/dtoa.o](https://github.com/python/cpython/blob/main/Python/dtoa.o)  [Python/formatter_unicode.o](https://github.com/python/cpython/blob/main/Python/formatter_unicode.o)  [Python/fileutils.o](https://github.com/python/cpython/blob/main/Python/fileutils.o)  [Python/dynload_shlib.o](https://github.com/python/cpython/blob/main/Python/dynload_shlib.o)        [Modules/config.o](https://github.com/python/cpython/blob/main/Modules/config.o)  [Modules/getpath.o](https://github.com/python/cpython/blob/main/Modules/getpath.o)  [Modules/main.o](https://github.com/python/cpython/blob/main/Modules/main.o)  [Modules/gcmodule.o](https://github.com/python/cpython/blob/main/Modules/gcmodule.o)  [Modules/posixmodule.o](https://github.com/python/cpython/blob/main/Modules/posixmodule.o)  [Modules/errnomodule.o](https://github.com/python/cpython/blob/main/Modules/errnomodule.o)  [Modules/pwdmodule.o](https://github.com/python/cpython/blob/main/Modules/pwdmodule.o)  [Modules/_sre.o](https://github.com/python/cpython/blob/main/Modules/_sre.o)  [Modules/_codecsmodule.o](https://github.com/python/cpython/blob/main/Modules/_codecsmodule.o)  [Modules/_weakref.o](https://github.com/python/cpython/blob/main/Modules/_weakref.o)  [Modules/_functoolsmodule.o](https://github.com/python/cpython/blob/main/Modules/_functoolsmodule.o)  [Modules/_operator.o](https://github.com/python/cpython/blob/main/Modules/_operator.o)  [Modules/_collectionsmodule.o](https://github.com/python/cpython/blob/main/Modules/_collectionsmodule.o)  [Modules/_abc.o](https://github.com/python/cpython/blob/main/Modules/_abc.o)  [Modules/itertoolsmodule.o](https://github.com/python/cpython/blob/main/Modules/itertoolsmodule.o)  [Modules/atexitmodule.o](https://github.com/python/cpython/blob/main/Modules/atexitmodule.o)  [Modules/signalmodule.o](https://github.com/python/cpython/blob/main/Modules/signalmodule.o)  [Modules/_stat.o](https://github.com/python/cpython/blob/main/Modules/_stat.o)  [Modules/timemodule.o](https://github.com/python/cpython/blob/main/Modules/timemodule.o)  [Modules/_threadmodule.o](https://github.com/python/cpython/blob/main/Modules/_threadmodule.o)  [Modules/_localemodule.o](https://github.com/python/cpython/blob/main/Modules/_localemodule.o)  [Modules/_iomodule.o](https://github.com/python/cpython/blob/main/Modules/_iomodule.o) [Modules/iobase.o](https://github.com/python/cpython/blob/main/Modules/iobase.o) [Modules/fileio.o](https://github.com/python/cpython/blob/main/Modules/fileio.o) [Modules/bytesio.o](https://github.com/python/cpython/blob/main/Modules/bytesio.o) [Modules/bufferedio.o](https://github.com/python/cpython/blob/main/Modules/bufferedio.o) [Modules/textio.o](https://github.com/python/cpython/blob/main/Modules/textio.o) [Modules/stringio.o](https://github.com/python/cpython/blob/main/Modules/stringio.o)  [Modules/faulthandler.o](https://github.com/python/cpython/blob/main/Modules/faulthandler.o)  [Modules/_tracemalloc.o](https://github.com/python/cpython/blob/main/Modules/_tracemalloc.o) [Modules/hashtable.o](https://github.com/python/cpython/blob/main/Modules/hashtable.o)  [Modules/symtablemodule.o](https://github.com/python/cpython/blob/main/Modules/symtablemodule.o)  [Modules/xxsubtype.o](https://github.com/python/cpython/blob/main/Modules/xxsubtype.o)  [Python/frozen.o](https://github.com/python/cpython/blob/main/Python/frozen.o)
    ./Modules/makexp_aix [Modules/python.exp](https://github.com/python/cpython/blob/main/Modules/python.exp) . libpython3.9d.a;  xlc_r     -Wl,-bE:Modules/python.exp -lld -o python [Programs/python.o](https://github.com/python/cpython/blob/main/Programs/python.o) libpython3.9d.a -lintl -ldl  -lm   -lm 
     ./python -E -S -m sysconfig --generate-posix-vars ; if test $? -ne 0 ; then  echo "generate-posix-vars failed" ;  rm -f ./pybuilddir.txt ;  exit 1 ;  fi
    

    Objects/genobject.c:127: _PyObject_GC_TRACK: Assertion "!(((PyGC_Head *)(op)-1)->_gc_next != 0)" failed: object already tracked by the garbage collector
    Enable tracemalloc to get the memory block allocation traceback
    object address : 30084150
    object refcount : 0
    object type : 20013aa8
    object type name: generator
    object repr : <refcnt 0 at 30084150>
    Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
    Python runtime state: core initialized
    Current thread 0x00000001 (most recent call first):
    File "<frozen importlib._bootstrap_external>", line 1593 in _setup
    File "<frozen importlib._bootstrap_external>", line 1634 in _install
    File "<frozen importlib._bootstrap>", line 1189 in _install_external_importers
    /bin/sh: 24117648 IOT/Abort trap(coredump)
    make: 1254-004 The error code from the last command is 134.
    Stop.

    FYI: about two hours ago I verified that xlc and 08050e9 : bpo-40147: Fix a compiler warning on Windows in Python/compile.c (GH-19389)

    all was green.

    @aixtools
    Copy link
    Contributor

    aixtools commented Apr 6, 2020

    Just checked - seems to be SPECIFIC to xlc-v16 as neither xlv-v11 nor xlc-v13 have any issues building.

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 6, 2020

    Py_TRASHCAN_BEGIN() access directly PyTypeObject.tp_dealloc:

    #define Py_TRASHCAN_BEGIN(op, dealloc) \
        Py_TRASHCAN_BEGIN_CONDITION(op, \
            Py_TYPE(op)->tp_dealloc == (destructor)(dealloc))

    It should use PyType_GetSlot() or a new getter function (to read PyTypeObject.tp_dealloc) should be added.

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 6, 2020

    It should use PyType_GetSlot()

    Oh. It seems like currently, PyType_GetSlot() can only be used on a heap allocated types :-( The function starts with:

        if (!PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE) || slot < 0) {
            PyErr_BadInternalCall();
            return NULL;
        }

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 6, 2020

    Just checked - seems to be SPECIFIC to xlc-v16 as neither xlv-v11 nor xlc-v13 have any issues building.

    That sounds like an AIX specific issue. Please open a separated issue.

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 7, 2020

    New changeset 9205520 by Victor Stinner in branch 'master':
    bpo-40170: PyObject_NEW() becomes an alias to PyObject_New() (GH-19379)
    9205520

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 7, 2020

    New changeset ef5c615 by Victor Stinner in branch 'master':
    bpo-40170: Convert PyObject_CheckBuffer() macro to a function (GH-19376)
    ef5c615

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 7, 2020

    New changeset 45ec5b9 by Victor Stinner in branch 'master':
    bpo-40170: PyType_HasFeature() now always calls PyType_GetFlags() (GH-19378)
    45ec5b9

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 8, 2020

    New changeset a15e260 by Victor Stinner in branch 'master':
    bpo-40170: Add _PyIndex_Check() internal function (GH-19426)
    a15e260

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 8, 2020

    Hum, once most changes will land, maybe it would be worth it to document them at:
    https://docs.python.org/dev/whatsnew/3.9.html#build-and-c-api-changes

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 8, 2020

    New changeset 307b9d0 by Victor Stinner in branch 'master':
    bpo-40170: Remove PyIndex_Check() macro (GH-19428)
    307b9d0

    @vstinner
    Copy link
    Member Author

    /* Test if an object has a GC head */
    #define PyObject_IS_GC(o) \
        (PyType_IS_GC(Py_TYPE(o)) \
         && (Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))

    This macro should be converted to an opaque function.

    @shihai1991
    Copy link
    Member

    This macro should be converted to an opaque function
    Can I try it?

    @vstinner
    Copy link
    Member Author

    New changeset 675d9a3 by Hai Shi in branch 'master':
    bpo-40170: Convert PyObject_IS_GC() macro to a function (GH-19464)
    675d9a3

    @vstinner
    Copy link
    Member Author

    On python-dev, Ronald Oussoren asked to add support for the buffer protocol in PyType_FromSpec() and PyTypeSpec API:

    Ronald:

    BTW. This will require growing the PyTypeSpec ABI a little, there are features you cannot implement using that API for example the buffer protocol.

    https://mail.python.org/archives/list/python-dev@python.org/message/PGKRW7S2IUOWVRX6F7RT6VAWD3ZPUDYS/

    See also PyType_FromSpec() issue with opaque PyObject:
    https://bugs.python.org/issue39573#msg366473

    @ronaldoussoren
    Copy link
    Contributor

    Something else that probably needs attention with the TypeSpec API is subclassing type in an extension when that subclass adds fields to the type object.

    I use this in PyObjC to (dynamically) create types that have some additional data. I could probably work around this issue by adding a level of indirection (basically storing the extra data in a WeakKeyDictionary), but haven't looked into this yet.

    @shihai1991
    Copy link
    Member

    Py_TRASHCAN_BEGIN() access directly PyTypeObject.tp_dealloc

    Looks like this macro not recorded in docs. Do we need using function to replace this macro?

    @vstinner
    Copy link
    Member Author

    Looks like this macro not recorded in docs.

    It never prevented anyone to use a function of the C API :-)

    @shihai1991
    Copy link
    Member

    It never prevented anyone to use a function of the C API :-)
    Got it. If possible someone uses it, I will try to add a function to repalce it(MAYBE udpate this docs too;) )

    @vstinner
    Copy link
    Member Author

    Py_TRASHCAN_BEGIN() access directly PyTypeObject.tp_dealloc (...) currently, PyType_GetSlot() can only be used on a heap allocated types

    I created bpo-41073: [C API] PyType_GetSlot() should accept static types.

    @vstinner vstinner added 3.10 only security fixes and removed 3.9 only security fixes labels Jun 22, 2020
    @vstinner
    Copy link
    Member Author

    Should we strive to fix the cases in Include/internal as well?

    No. The internal C API access directly to structure members on purpose, for best performances.

    @vstinner
    Copy link
    Member Author

    New changeset 871eb42 by Erlend Egeberg Aasland in branch 'master':
    bpo-40170: Convert PyDescr_IsData() to static inline function (GH-24535)
    871eb42

    @vstinner
    Copy link
    Member Author

    New changeset cc54001 by Erlend Egeberg Aasland in branch 'master':
    bpo-40170: Always define PyIter_Check() as a function (GH-24548)
    cc54001

    @vstinner
    Copy link
    Member Author

    New changeset cc54001 by Erlend Egeberg Aasland in branch 'master':
    bpo-40170: Always define PyIter_Check() as a function (GH-24548)

    For macOS which doesn't use LTO compiler optimization, we added private static inline functions of some "Check" functions. But I don't think that it's worth it here (I don't think that the function is commonly called in "hot code").

    @erlend-aasland
    Copy link
    Contributor

    For PyExceptionClass_Name: Is it ok to just remove the macro version (like with #68736)?

    @vstinner
    Copy link
    Member Author

    For PyExceptionClass_Name: Is it ok to just remove the macro version (like with #68736)?

    Yes, I think so.

    @erlend-aasland
    Copy link
    Contributor

    Thanks, Victor.

    For PySequence_ITEM, I guess adding a private C version (for example _PySequence_Item) and redirecting the macro to the C version would be acceptable.

    Ditto for PyHeapType_GET_MEMBERS and PyType_SUPPORTS_WEAKREFS.

    @vstinner
    Copy link
    Member Author

    New changeset cd80f43 by Erlend Egeberg Aasland in branch 'master':
    bpo-40170: Always define PyExceptionClass_Name() as a function (GH-24553)
    cd80f43

    @vstinner
    Copy link
    Member Author

    For PySequence_ITEM, I guess adding a private C version (for example _PySequence_Item) and redirecting the macro to the C version would be acceptable.

    This can introduce a performance slowdown and so should wait until the PEP-620 is accepted.

    @erlend-aasland
    Copy link
    Contributor

    This can introduce a performance slowdown and so should wait until the PEP-620 is accepted.

    Noted.

    @vstinner
    Copy link
    Member Author

    New changeset 630264a by Erlend Egeberg Aasland in branch 'master':
    bpo-40170: Move 3 NEWS entries to the C API section (GH-24555)
    630264a

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 8, 2021

    New changeset 99c7e98 by Victor Stinner in branch 'main':
    bpo-40170: Update What's New in Python 3.9 (GH-29470)
    99c7e98

    @miss-islington
    Copy link
    Contributor

    New changeset 69b3de6 by Miss Islington (bot) in branch '3.10':
    bpo-40170: Update What's New in Python 3.9 (GH-29470)
    69b3de6

    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 8, 2021

    New changeset 80580f5 by Miss Islington (bot) in branch '3.9':
    bpo-40170: Update What's New in Python 3.9 (GH-29470) (GH-29472)
    80580f5

    @vstinner
    Copy link
    Member Author

    TODO:

    Other TODO tasks which can be addressed in follow-up issues:

    • Add buffer protocol to PyType_FromSpec()
    • Stdlib C extensions still define static types: see bpo-40077
    • 3rd party C extensions still define static types: PEP-630 and others propose heap types

    @vstinner
    Copy link
    Member Author

    I searched for "_PyGC_FINALIZED" in top 5000 PyPI projects. It seems like only Cython is impacted.

    ddtrace and guppy3 use directly the internal C API.

    == Cython 0.29.26 ==

    • Cython/Compiler/ModuleNode.py: finalised_check = '!_PyGC_FINALIZED(o)'
    • Cython/Compiler/ModuleNode.py: '(!PyType_IS_GC(Py_TYPE(o)) || !_PyGC_FINALIZED(o))')

    == ddtrace 0.57.3 ==

    In ddtrace/profiling/collector/stack.pyx:

        IF PY_MINOR_VERSION >= 9:
            # Needed for accessing _PyGC_FINALIZED when we build with -DPy_BUILD_CORE
            cdef extern from "<internal/pycore_gc.h>":
                pass
    

    == guppy3-3.1.2 ==

    In src/heapy/hv.c:

    #if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 9
    # define Py_BUILD_CORE
    /* PyGC_Head */
    #  undef _PyGC_FINALIZED
    #  include <internal/pycore_gc.h>
    # undef Py_BUILD_CORE
    #endif

    @vstinner
    Copy link
    Member Author

    I searched for "_PyObject_DebugMallocStats" in top 5000 PyPI projects. There is a single project using it: guppy3.

    Extract of guppy3 src/heapy/xmemstats.c:

    ...
        dlptr__PyObject_DebugMallocStats = addr_of_symbol("_PyObject_DebugMallocStats");
    ...
    static PyObject *
    hp_xmemstats(PyObject *self, PyObject *args)
    {
        if (dlptr__PyObject_DebugMallocStats) {
            fprintf(stderr, "======================================================================\n");
            fprintf(stderr, "Output from _PyObject_DebugMallocStats()\n\n");
            dlptr__PyObject_DebugMallocStats(stderr);
        }
        ...
    }

    addr_of_symbol() is implemented with dlsym() or GetModuleHandle(NULL)+GetProcAddress(): it searchs for the symbol in the current process.

    @vstinner
    Copy link
    Member Author

    In the top 5000 PyPI projects, the _PyObject_SIZE() and _PyObject_VAR_SIZE() functions are used by 7 projects:

    • Cython-0.29.26
    • frozendict-2.2.0: implement "sizeof" function, found in copies of Objects/dictobject.c file
    • JPype1-1.3.0
    • numpy-1.22.1: gentype_alloc() in numpy/core/src/multiarray/scalartypes.c.src, used as type tp_alloc functions
    • pickle5-0.0.12
    • pyobjc-core-8.2
    • recordclass-0.17.1

    @vstinner
    Copy link
    Member Author

    New changeset af32b3e by Victor Stinner in branch 'main':
    bpo-40170: PyType_SUPPORTS_WEAKREFS() becomes a regular function (GH-30938)
    af32b3e

    @vstinner
    Copy link
    Member Author

    New changeset 6b491b9 by Victor Stinner in branch 'main':
    bpo-40170: Remove _Py_GetAllocatedBlocks() function (GH-30940)
    6b491b9

    @vstinner
    Copy link
    Member Author

    New changeset 0575551 by Victor Stinner in branch 'main':
    bpo-40170: Move _Py_GetAllocatedBlocks() to pycore_pymem.h (GH-30943)
    0575551

    @vstinner
    Copy link
    Member Author

    New changeset 18ea973 by Victor Stinner in branch 'main':
    bpo-40170: Remove PyHeapType_GET_MEMBERS() macro (GH-30942)
    18ea973

    @vstinner
    Copy link
    Member Author

    Changes already done:

    • Macros converted to regular functions:

      • PyObject_GET_WEAKREFS_LISTPTR(); add internal inline _PyObject_GET_WEAKREFS_LISTPTR()
      • PyType_SUPPORTS_WEAKREFS(); add internal inline _PyType_SUPPORTS_WEAKREFS()
      • PyObject_CheckBuffer(); no fast internal API
      • PyObject_IS_GC(); no fast internal API
      • PyDescr_IsData(); no fast internal API
    • Always implemented as a function, remove macro optimization in the non-limited API:

      • PyIter_Check(); no fast internal API
      • PyIndex_Check(); add internal inline _PyIndex_Check()
      • PyExceptionClass_Name(); no fast internal API
    • Py_TRASHCAN_BEGIN() macro now calls _PyTrash_cond() function: no longer read directly tp_dealloc member

    • PyObject_NEW() macro becomes an alias to PyObject_New()

    • Remove PyHeapType_GET_MEMBERS() (move it to the internal C API)

    PyType_HasFeature() is left unchanged since the change caused performance issue on macOS, whereas Python is not built with LTO.

    @vstinner vstinner changed the title [C API] Make PyTypeObject structure an opaque structure in the public C API [C API] Prepare PyTypeObject structure for a stable ABI: avoid accessing members in the public API Jan 28, 2022
    @vstinner vstinner changed the title [C API] Make PyTypeObject structure an opaque structure in the public C API [C API] Prepare PyTypeObject structure for a stable ABI: avoid accessing members in the public API Jan 28, 2022
    @vstinner
    Copy link
    Member Author

    I close the issue. While this issue is not fully fixed, it's a milestone of my "stable ABI" goal. I prefer to address remaining issues in new separted issues.

    This issues is not fully fixed, there are are still a 4 macros which access directly PyTypeObject members:

    • PySequence_ITEM()
    • _PyObject_SIZE()
    • _PyObject_VAR_SIZE()
    • PyType_HasFeature() (if Py_LIMITED_API is not defined)

    PySequence_ITEM() and PyType_HasFeature() are important for performance, I prefer to have a PEP before changing these two functions.

    _PyObject_SIZE() and _PyObject_VAR_SIZE() should be made public with a better name like PyObject_SizeOf() and PyVarObject_SizeOf(). But I prefer to do that in a separated issue.

    IMO it would be interesting to merge the PyHeapTypeObject structure into the PyTypeObject structure:
    https://bugs.python.org/issue46433#msg411167

    And make the PyTypeObject opaque: deprecate static types and promote the usage of heap types in C extensions. That's a big project which may be splitted into multiple issues and the final change may need its own PEP.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-C-API
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants