GeezerVison is showing github.com — Python macros don’t shield arguments · Issue #87347 · python/cpython
Python macros don’t shield arguments #87347Description
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-02-15.16:21:14.723>
created_at = <Date 2021-02-09.22:05:50.575>
labels = ['expert-C-API', 'build', '3.10']
title = 'Python macros don\xe2\x80\x99t shield arguments'
updated_at = <Date 2021-03-22.16:47:15.483>
user = 'https://github.com/numberZero' bugs.python.org fields:
activity = <Date 2021-03-22.16:47:15.483>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-02-15.16:21:14.723>
closer = 'vstinner'
components = ['C API']
creation = <Date 2021-02-09.22:05:50.575>
creator = 'numberZero'
dependencies = []
files = []
hgrepos = []
issue_num = 43181
keywords = ['patch', '3.9regression']
message_count = 17.0
messages = ['386746', '386754', '386760', '386822', '386827', '386998', '387001', '387007', '387023', '387024', '387360', '388509', '388714', '388727', '388730', '388733', '389331']
nosy_count = 5.0
nosy_names = ['gregory.p.smith', 'vstinner', 'WildCard65', 'erlendaasland', 'numberZero']
pr_nums = ['24533']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue43181'
versions = ['Python 3.10'] ActivitynumberZero commented on Feb 9, 2021 MannequinAuthor More actions There is a lot of macros like:
#define PyObject_TypeCheck(ob, tp) \
(Py_IS_TYPE(ob, tp) || PyType_IsSubtype(Py_TYPE(ob), (tp)))These work fine until an argument happen to contain a comma. That’s possible as a result of other macro’s expansion. E.g. if U(x) is defined as x,
PyObject_TypeCheck(ob, U(f<a,b>(c))) expands to
(Py_IS_TYPE(ob, f<a,b>(c)) || ...) but < and > aren’t treated as brackets by the preprocessor so Py_IS_TYPE is now invoked with 3 arguments instead of just 2, breaking module compilation.
As arguments are expected to be values, surrounding each with parentheses solves the problem. But there are many such macros so that’s not an one-line fix.
Note: the example is from PyGLM (simplified), it doesn’t compile on 3.9 due to this issue. WildCard65 commented on Feb 10, 2021 This feels like it's more of an issue with the C++ compiler you're using. (I can tell it's C++ because of template syntax) gpshead commented on Feb 10, 2021 This bug is valid, that macro and likely others aren't up to best practices.
C macros are a PITA to get right. vstinner commented on Feb 11, 2021 I'm replacing macros with static inline functions to avoid issues like this one (ex: bpo-35059). Are you interested to convert the PyObject_TypeCheck() macro to a static inline function?
"""
There is a lot of macros like:
#define PyObject_TypeCheck(ob, tp) \
(Py_IS_TYPE(ob, tp) || PyType_IsSubtype(Py_TYPE(ob), (tp)))
These work fine until an argument happen to contain a comma.
"""This macro also has a common bug of macros: if one argument has a side effect, it is executed twice! For example, if an argument is a function call, the function is called twice.
See:
erlend-aasland commented on Feb 11, 2021
Are you interested to convert the PyObject_TypeCheck() macro to a static inline function?
I'd like to give it a shot if it's ok for you all. erlend-aasland commented on Feb 15, 2021 Silence implies consent? I'll throw up a PR, then :) vstinner commented on Feb 15, 2021 I don't think that PR 24533 should be backported to Python 3.8 and Python 3.9. I prefer to avoid any risk of regression, and so only change Python 3.10. vstinner commented on Mar 11, 2021
Yet... the first argument is still unshielded, passed to a macro that expects one single macro argument.
Are you talking about the current code in the master branch or the 3.9 branch? If you are talking about the _PyObject_CAST(ob) call, would you mind to elaborate how it is an issue? The code in master LGTM.
That’s not a regression, it wasn’t shielded in 3.8 either, but why not just parenthesise each macro argument that denotes an expression (as opposed to e.g. name)?
In the master branch, Include/ and its subdirectories contains 158 .h files for a total of 20K lines of C code. It's not easy to check all files. If you spotted bugs, would you mind to give examples, or a way to automatically detect bugs?
As I wrote previously, I dislike macros. If someone is changed, I would prefer to convert the function into a static inline function which doesn't have macros pitfalls. erlend-aasland commented on Mar 15, 2021
As I wrote previously, I dislike macros. If someone is changed, I would prefer to convert the function into a static inline function which doesn't have macros pitfalls.
Should we create a meta issue for this? Most macros are trivial, and can safely be converted, given that they're not used as l-values. Converting one header file at the time would make it easy to review, and it would be possible to make good progress in a few months time. vstinner commented on Mar 15, 2021 If possible, I would prefer to only replace macros with static inline functions if the changes avoids clear macro pitfalls. It's not because macros have pitfalls that we must replace them all blindly.
Also, this issue is closed. At this point, I'm not convinced that it's worth it to fix PyObject_TypeCheck() in Python 3.8 and 3.9, so I leave the issue closed. erlend-aasland commented on Mar 15, 2021
If possible, I would prefer to only replace macros with static inline functions if the changes avoids clear macro pitfalls.
Yes, of course. Should I create a meta issue for this, or do you prefer raising one issue pr. fix? vstinner commented on Mar 15, 2021 You can create a single issue, and later we will see if it's needed to split it into sub-issues. vstinner commented on Mar 22, 2021 Me:
I don't think that PR 24533 should be backported to Python 3.8 and Python 3.9. I prefer to avoid any risk of regression, and so only change Python 3.10.
Sometimes. I'm wise :-D This change broken 2 extension modules which rely on the fact that the macro magically add parenthesis:
python-cvxopt: "if Matrix_Check((PyObject *)val)" with: "#define Matrix_Check(self) PyObject_TypeCheck(self, &matrix_tp)"
https://bugzilla.redhat.com/show_bug.cgi?id=1941557
PR proposed: cvxopt/cvxopt#190
python-Bottleneck: "if PyArray_Check(a_obj) {" with: "#define PyArray_Check(op) PyObject_TypeCheck(op, &PyArray_Type)"
https://bugzilla.redhat.com/show_bug.cgi?id=1941559
"if func(...) {" is not valid C code, but the old macro added magically parenthesis!
It's hard to say if the Python change is a backward incompatible or not... I prefer to keep it and fix the few broken C extensions. vstinner commented on Jun 20, 2022
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|