GeezerVison is showing github.com — [C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type · Issue #83754 · python/cpython
[C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type #83754Description
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 2021-09-08.16:32:15.871>
created_at = <Date 2020-02-06.23:07:12.131>
labels = ['expert-C-API', '3.11']
title = '[C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type'
updated_at = <Date 2022-01-20.00:24:00.180>
user = 'https://github.com/vstinner' bugs.python.org fields:
activity = <Date 2022-01-20.00:24:00.180>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-09-08.16:32:15.871>
closer = 'vstinner'
components = ['C API']
creation = <Date 2020-02-06.23:07:12.131>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 39573
keywords = ['patch']
message_count = 96.0
messages = ['361513', '361514', '361515', '361516', '361517', '361518', '361519', '361522', '361523', '361526', '361527', '361529', '361531', '361540', '361549', '361555', '361557', '361590', '361593', '361607', '361611', '361626', '361631', '361639', '361904', '361960', '361961', '361963', '361964', '361965', '361971', '361977', '361987', '361988', '362033', '362034', '362133', '362134', '362166', '362212', '362216', '362445', '363345', '363494', '363564', '365690', '366473', '366493', '368047', '369896', '369898', '370074', '370303', '370638', '370663', '370665', '370666', '370671', '370729', '370902', '370932', '372308', '373460', '379675', '379679', '379680', '379757', '379759', '381337', '381345', '381365', '381374', '381403', '381404', '382260', '382534', '382539', '382780', '382781', '382783', '394954', '394971', '395018', '395205', '395206', '395287', '395323', '395536', '401365', '401370', '401378', '401395', '401396', '401399', '403252', '410995']
nosy_count = 1.0
nosy_names = ['vstinner']
pr_nums = ['18388', '18389', '18390', '18391', '18392', '18393', '18394', '18398', '18400', '18402', '18411', '18419', '18488', '18496', '18507', '18508', '18521', '18601', '18789', '18798', '18799', '18804', '18809', '19882', '19975', '20290', '20391', '20429', '20610', '21262', '21433', '23366', '23375', '26493', '26550', '26596', '28128']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39573'
versions = ['Python 3.11'] Linked PRs
Activityvstinner commented on Feb 6, 2020 Today, CPython is leaking too many implementation through its public C API. We cannot easily change the "default" C API, but we can enhance the "limited" C API (when Py_LIMITED_API macro is defined). Example of leaking implementation details: memory allocator, garbage collector, structure layouts, etc.
Making PyObject an opaque structure would allow in the long term of modify structures to implement more efficient types (ex: list specialized for small integers), and it can prepare CPython to experiment tagged pointers.
Longer rationale:
I propose to incremental evolve the existing limited C API towards opaque PyObject, by trying to reduce the risk of breakage.
We may test changes on PyQt which uses the limited C API.
Another idea would be to convert some C extensions of the standard library to the limited C API. It would ensure that the limited C API contains enough functions to be useful, but would also notify us directly if the API is broken. vstinner commented on Feb 6, 2020
Another idea would be to convert some C extensions of the standard library to the limited C API. It would ensure that the limited C API contains enough functions to be useful, but would also notify us directly if the API is broken.
First issues that I met when I tried that:
- C code generated by Argument Clinic is incompatible the limited C API: METH_FASTCALL, _PyArg_CheckPositional(), static _PyArg_Parser, etc. are excluded from the limited C API.
- PyTypeObject is opaque and so it's not possible to implement a deallocator function (tp_dealloc) which calls tp_free like:
Py_TYPE(self)->tp_free((PyObject*)self);
- _Py_IDENTIFIER() is not part of the limited C API
vstinner commented on Feb 6, 2020 vstinner commented on Feb 7, 2020 vstinner commented on Feb 7, 2020 In the limited C API, Py_REFCNT() should be converted to:
static inline Py_ssize_t _Py_REFCNT(const PyObject *ob)
{ return ob->ob_refcnt; }
#define Py_REFCNT(ob) _Py_REFCNT(_PyObject_CAST(ob))It would enforce the usage of newly added Py_SET_REFCNT() (PR 18389) and advertise that the object is not modified (const).
That would only be the first step towards a really opaque Py_REFCNT() function. vstinner commented on Feb 7, 2020 TODO: Add Py_IS_TYPE() macro:
#define Py_IS_TYPE(ob, tp) (Py_TYPE(ob) == (tp)) For example, replace:
#define PyBool_Check(x) (Py_TYPE(x) == &PyBool_Type) with:
#define PyBool_Check(x) Py_IS_TYPE(x, &PyBool_Type) IMHO it makes the code more readable.
nascheme@c156300 vstinner commented on Feb 7, 2020 vstinner commented on Feb 7, 2020 vstinner commented on Feb 7, 2020 Py_TYPE() is commonly used to render the type name in an error message. Example:
PyErr_Format(PyExc_TypeError,
"cannot convert '%.200s' object to bytearray",
Py_TYPE(arg)->tp_name);This code has multiple issues:
- It truncates type name to 200 characters: there is no Python exception, not even a marker to indicate that the string has been truncated
- It's only the short name: the qualified name (tp_qualname) would be more helpful. The best would be to generate the fully qualified name: module + qualname.
- Py_TYPE() returns a borrowed reference which is causing multiple issues: https://pythoncapi.readthedocs.io/bad_api.html#borrowed-references
In September 2018, I created bpo-34595: "PyUnicode_FromFormat(): add %T format for an object type name". But there was disagreement, so I rejected my change.
I started "bpo-34595: How to format a type name?" thread on python-dev:
I didn't continue this work (until now), since it wasn't my priority. vstinner commented on Feb 7, 2020 vstinner commented on Feb 7, 2020 Make PyObject an opaque structure is also a first step towards the more ambitious project "HPy" project which is fully opaque:
https://github.com/pyhandle/hpy
This API is written from scratch and currently implemented on top on the existing C API.
The following article is a nice introduction to the overall idea:
https://morepypy.blogspot.com/2019/12/hpy-kick-off-sprint-report.html
From my point of view, the long term goal would be to get better performance on PyPy and having a single API for C extension which would be efficient on all Python implementations (not only CPython).
Currently, the C API is not only a performance issue to run C extensions on PyPy. It's also an issue in CPython. Because the C API leaks too many implementation details, we cannot experiment optimizations.
See also: https://pythoncapi.readthedocs.io/rationale.html vstinner commented on Feb 7, 2020 vstinner commented on Feb 7, 2020 vstinner commented on Sep 8, 2021 Oh and obviously, it's not possible possible to define structures which *include* PyObject or PyVarObject if PyObject and PyVarObject become opaque. Example:
typedef struct {
PyObject ob_base;
Py_ssize_t ob_size; /* Number of items in variable part */
} PyVarObject;This C code requires the PyObject structure to be fully defined (not being opaque).
A new C API and ABI where structures *don't* include PyObject or PyVarObject should be designed to allocate their members "before" the PyObject* pointer value. Something like the current PyGC_Head structure which is excluded from PyObject and stored *before* the "PyObject*" pointer.
Simplified code which allocates memory for an object implementin the GC protocol:
static PyObject *
_PyObject_GC_Malloc(size_t basicsize)
{
...
size_t size = sizeof(PyGC_Head) + basicsize;
...
PyGC_Head *g = (PyGC_Head *)PyObject_Malloc(size);
...
PyObject *op = (PyObject *)(g + 1);
return op;
}vstinner commented on Sep 8, 2021 I changed the issue title to restrict its scope: "[C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type".
Making PyObject and PyVarObject structures opaque is a broader topic which should be splited into sub-issues.
"Py_TYPE(obj)=type;" is now disallowed. I consider that the work of this issue is now completed and I close the issue.
Thanks everyone who help to fix these tedious issues!
You can continue to use this issue if you need my help to adapt your C extensions to Py_SET_TYPE()/Py_SET_SIZE().
See also the upgrade_pythoncapi.py script of the pythoncapi_compat project which helps to port your C extensions without losing support for old Python versions:
https://github.com/pythoncapi/pythoncapi_compat
See also the Py_TYPE() change announcement on the capi-sig list:
https://mail.python.org/archives/list/[email protected]/thread/WGRLTHTHC32DQTACPPX36TPR2GLJAFRB/ changed the title [-][C API] Make PyObject an opaque structure in the limited C API[/-] [+][C API] Avoid accessing PyObject and PyVarObject members directly: add Py_SET_TYPE() and Py_IS_TYPE(), disallow Py_TYPE(obj)=type[/+] on Sep 8, 2021vstinner commented on Oct 5, 2021 vstinner commented on Jan 20, 2022
@victor, git bisect tells me the change f3fa63e caused test_exceptions.ExceptionTests.test_recursion_in_except_handler to stack overflow only on windows debug builds.
FYI this regression was handled last year in bpo-44348 "test_exceptions.ExceptionTests.test_recursion_in_except_handler stack overflow on Windows debug builds" and fixed at 2021-09-07 by using the trashcan mecanism in the BaseException deallocator function:
New changeset fb30509 by Victor Stinner in branch 'main':
bpo-44348: BaseException deallocator uses trashcan (GH-28190)
fb30509
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|