From ea62ce7f4fefc66bc0adba16bcd7666d5bbd5b44 Mon Sep 17 00:00:00 2001 From: Christian Tismer Date: Sat, 9 Jun 2018 20:32:25 +0200 Subject: [PATCH] bpo-33738: Fix macros which contradict PEP 384 (GH-7477) During development of the limited API support for PySide, we saw an error in a macro that accessed a type field. This patch fixes the 7 errors in the Python headers. Macros which were not written as capitals were implemented as function. To do the necessary analysis again, a script was included that parses all headers and looks for "->tp_" in serctions which can be reached with active limited API. It is easily possible to call this script as a test. Error listing: ../../Include/objimpl.h:243 #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))) Action: commented only ../../Include/objimpl.h:362 #define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0) Action: commented only ../../Include/objimpl.h:364 #define PyObject_GET_WEAKREFS_LISTPTR(o) \ ((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset)) Action: commented only ../../Include/pyerrors.h:143 #define PyExceptionClass_Name(x) \ ((char *)(((PyTypeObject*)(x))->tp_name)) Action: implemented function ../../Include/abstract.h:593 #define PyIter_Check(obj) \ ((obj)->ob_type->tp_iternext != NULL && \ (obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented) Action: implemented function ../../Include/abstract.h:713 #define PyIndex_Check(obj) \ ((obj)->ob_type->tp_as_number != NULL && \ (obj)->ob_type->tp_as_number->nb_index != NULL) Action: implemented function ../../Include/abstract.h:924 #define PySequence_ITEM(o, i)\ ( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) ) Action: commented only --- Include/abstract.h | 15 ++ Include/objimpl.h | 4 + Include/pyerrors.h | 4 + .../2018-06-07-18-34-19.bpo-33738.ODZS7a.rst | 3 + Objects/abstract.c | 15 ++ Objects/exceptions.c | 6 + PC/python3.def | 3 + Tools/scripts/pep384_macrocheck.py | 148 ++++++++++++++++++ 8 files changed, 198 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst create mode 100644 Tools/scripts/pep384_macrocheck.py diff --git a/Include/abstract.h b/Include/abstract.h index 4088f75ff3c..e7bc2d24bc2 100644 --- a/Include/abstract.h +++ b/Include/abstract.h @@ -590,9 +590,16 @@ PyAPI_FUNC(PyObject *) PyObject_Format(PyObject *obj, returns itself. */ PyAPI_FUNC(PyObject *) PyObject_GetIter(PyObject *); +/* Returns 1 if the object 'obj' provides iterator protocols, and 0 otherwise. + + This function always succeeds. */ +#ifndef Py_LIMITED_API #define PyIter_Check(obj) \ ((obj)->ob_type->tp_iternext != NULL && \ (obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented) +#else +PyAPI_FUNC(int) PyIter_Check(PyObject*); +#endif /* Takes an iterator object and calls its tp_iternext slot, returning the next value. @@ -710,9 +717,15 @@ PyAPI_FUNC(PyObject *) PyNumber_Xor(PyObject *o1, PyObject *o2); This is the equivalent of the Python expression: o1 | o2. */ PyAPI_FUNC(PyObject *) PyNumber_Or(PyObject *o1, PyObject *o2); +/* Returns 1 if obj is an index integer (has the nb_index slot of the + tp_as_number structure filled in), and 0 otherwise. */ +#ifndef Py_LIMITED_API #define PyIndex_Check(obj) \ ((obj)->ob_type->tp_as_number != NULL && \ (obj)->ob_type->tp_as_number->nb_index != NULL) +#else +PyAPI_FUNC(int) PyIndex_Check(PyObject *); +#endif /* Returns the object 'o' converted to a Python int, or NULL with an exception raised on failure. */ @@ -921,8 +934,10 @@ PyAPI_FUNC(PyObject *) PySequence_Fast(PyObject *o, const char* m); /* Assume tp_as_sequence and sq_item exist and that 'i' does not need to be corrected for a negative index. */ +#ifndef Py_LIMITED_API #define PySequence_ITEM(o, i)\ ( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) ) +#endif /* Return a pointer to the underlying item array for an object retured by PySequence_Fast */ diff --git a/Include/objimpl.h b/Include/objimpl.h index 057bb50cbda..a38906c7dc8 100644 --- a/Include/objimpl.h +++ b/Include/objimpl.h @@ -240,8 +240,10 @@ PyAPI_FUNC(Py_ssize_t) _PyGC_CollectIfEnabled(void); #define PyType_IS_GC(t) PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC) /* Test if an object has a GC head */ +#ifndef Py_LIMITED_API #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))) +#endif PyAPI_FUNC(PyVarObject *) _PyObject_GC_Resize(PyVarObject *, Py_ssize_t); #define PyObject_GC_Resize(type, op, n) \ @@ -359,10 +361,12 @@ PyAPI_FUNC(void) PyObject_GC_Del(void *); /* Test if a type supports weak references */ +#ifndef Py_LIMITED_API #define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0) #define PyObject_GET_WEAKREFS_LISTPTR(o) \ ((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset)) +#endif #ifdef __cplusplus } diff --git a/Include/pyerrors.h b/Include/pyerrors.h index f289471be20..a9929f5f19f 100644 --- a/Include/pyerrors.h +++ b/Include/pyerrors.h @@ -140,8 +140,12 @@ PyAPI_FUNC(void) _PyErr_ChainExceptions(PyObject *, PyObject *, PyObject *); #define PyExceptionInstance_Check(x) \ PyType_FastSubclass((x)->ob_type, Py_TPFLAGS_BASE_EXC_SUBCLASS) +#ifndef Py_LIMITED_API #define PyExceptionClass_Name(x) \ ((char *)(((PyTypeObject*)(x))->tp_name)) +#else + PyAPI_FUNC(char *) PyExceptionClass_Name(PyObject*); +#endif #define PyExceptionInstance_Class(x) ((PyObject*)((x)->ob_type)) diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst b/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst new file mode 100644 index 00000000000..0d66c4b1f19 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst @@ -0,0 +1,3 @@ +Seven macro incompatibilities with the Limited API were fixed, and the +macros PyIter_Check, PyIndex_Check and PyExceptionClass_Name were added as +functions. A script for automatic macro checks was added. diff --git a/Objects/abstract.c b/Objects/abstract.c index 93eda62a274..e2700e3f80d 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -1244,6 +1244,14 @@ PyNumber_Absolute(PyObject *o) return type_error("bad operand type for abs(): '%.200s'", o); } +#undef PyIndex_Check +int +PyIndex_Check(PyObject *obj) +{ + return obj->ob_type->tp_as_number != NULL && + obj->ob_type->tp_as_number->nb_index != NULL; +} + /* Return a Python int from the object item. Raise TypeError if the result is not an int or if the object cannot be interpreted as an index. @@ -2535,6 +2543,13 @@ PyObject_GetIter(PyObject *o) } } +#undef PyIter_Check +int PyIter_Check(PyObject *obj) +{ + return obj->ob_type->tp_iternext != NULL && + obj->ob_type->tp_iternext != &_PyObject_NextNotImplemented; +} + /* Return next item. * If an error occurs, return NULL. PyErr_Occurred() will be true. * If the iteration terminates normally, return NULL and clear the diff --git a/Objects/exceptions.c b/Objects/exceptions.c index bfc818f7b4c..bcb1fd5d07c 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -342,6 +342,12 @@ PyException_SetContext(PyObject *self, PyObject *context) Py_XSETREF(((PyBaseExceptionObject *)self)->context, context); } +#undef PyExceptionClass_Name +char * +PyExceptionClass_Name(PyObject *ob) +{ + return ((PyTypeObject*)ob)->tp_name; +} static struct PyMemberDef BaseException_members[] = { {"__suppress_context__", T_BOOL, diff --git a/PC/python3.def b/PC/python3.def index 48a2d07aab1..5d93c18af87 100644 --- a/PC/python3.def +++ b/PC/python3.def @@ -248,6 +248,7 @@ EXPORTS PyExc_Warning=python38.PyExc_Warning DATA PyExc_WindowsError=python38.PyExc_WindowsError DATA PyExc_ZeroDivisionError=python38.PyExc_ZeroDivisionError DATA + PyExceptionClass_Name=python38.PyExceptionClass_Name PyException_GetCause=python38.PyException_GetCause PyException_GetContext=python38.PyException_GetContext PyException_GetTraceback=python38.PyException_GetTraceback @@ -294,9 +295,11 @@ EXPORTS PyImport_ImportModuleLevelObject=python38.PyImport_ImportModuleLevelObject PyImport_ImportModuleNoBlock=python38.PyImport_ImportModuleNoBlock PyImport_ReloadModule=python38.PyImport_ReloadModule + PyIndex_Check=python38.PyIndex_Check PyInterpreterState_Clear=python38.PyInterpreterState_Clear PyInterpreterState_Delete=python38.PyInterpreterState_Delete PyInterpreterState_New=python38.PyInterpreterState_New + PyIter_Check=python38.PyIter_Check PyIter_Next=python38.PyIter_Next PyListIter_Type=python38.PyListIter_Type DATA PyListRevIter_Type=python38.PyListRevIter_Type DATA diff --git a/Tools/scripts/pep384_macrocheck.py b/Tools/scripts/pep384_macrocheck.py new file mode 100644 index 00000000000..142d248dd2f --- /dev/null +++ b/Tools/scripts/pep384_macrocheck.py @@ -0,0 +1,148 @@ +""" +pep384_macrocheck.py + +This programm tries to locate errors in the relevant Python header +files where macros access type fields when they are reachable from +the limided API. + +The idea is to search macros with the string "->tp_" in it. +When the macro name does not begin with an underscore, +then we have found a dormant error. + +Christian Tismer +2018-06-02 +""" + +import sys +import os +import re + + +DEBUG = False + +def dprint(*args, **kw): + if DEBUG: + print(*args, **kw) + +def parse_headerfiles(startpath): + """ + Scan all header files which are reachable fronm Python.h + """ + search = "Python.h" + name = os.path.join(startpath, search) + if not os.path.exists(name): + raise ValueError("file {} was not found in {}\n" + "Please give the path to Python's include directory." + .format(search, startpath)) + errors = 0 + with open(name) as python_h: + while True: + line = python_h.readline() + if not line: + break + found = re.match(r'^\s*#\s*include\s*"(\w+\.h)"', line) + if not found: + continue + include = found.group(1) + dprint("Scanning", include) + name = os.path.join(startpath, include) + if not os.path.exists(name): + name = os.path.join(startpath, "../PC", include) + errors += parse_file(name) + return errors + +def ifdef_level_gen(): + """ + Scan lines for #ifdef and track the level. + """ + level = 0 + ifdef_pattern = r"^\s*#\s*if" # covers ifdef and ifndef as well + endif_pattern = r"^\s*#\s*endif" + while True: + line = yield level + if re.match(ifdef_pattern, line): + level += 1 + elif re.match(endif_pattern, line): + level -= 1 + +def limited_gen(): + """ + Scan lines for Py_LIMITED_API yes(1) no(-1) or nothing (0) + """ + limited = [0] # nothing + unlimited_pattern = r"^\s*#\s*ifndef\s+Py_LIMITED_API" + limited_pattern = "|".join([ + r"^\s*#\s*ifdef\s+Py_LIMITED_API", + r"^\s*#\s*(el)?if\s+!\s*defined\s*\(\s*Py_LIMITED_API\s*\)\s*\|\|", + r"^\s*#\s*(el)?if\s+defined\s*\(\s*Py_LIMITED_API" + ]) + else_pattern = r"^\s*#\s*else" + ifdef_level = ifdef_level_gen() + status = next(ifdef_level) + wait_for = -1 + while True: + line = yield limited[-1] + new_status = ifdef_level.send(line) + dir = new_status - status + status = new_status + if dir == 1: + if re.match(unlimited_pattern, line): + limited.append(-1) + wait_for = status - 1 + elif re.match(limited_pattern, line): + limited.append(1) + wait_for = status - 1 + elif dir == -1: + # this must have been an endif + if status == wait_for: + limited.pop() + wait_for = -1 + else: + # it could be that we have an elif + if re.match(limited_pattern, line): + limited.append(1) + wait_for = status - 1 + elif re.match(else_pattern, line): + limited.append(-limited.pop()) # negate top + +def parse_file(fname): + errors = 0 + with open(fname) as f: + lines = f.readlines() + type_pattern = r"^.*?->\s*tp_" + define_pattern = r"^\s*#\s*define\s+(\w+)" + limited = limited_gen() + status = next(limited) + for nr, line in enumerate(lines): + status = limited.send(line) + line = line.rstrip() + dprint(fname, nr, status, line) + if status != -1: + if re.match(define_pattern, line): + name = re.match(define_pattern, line).group(1) + if not name.startswith("_"): + # found a candidate, check it! + macro = line + "\n" + idx = nr + while line.endswith("\\"): + idx += 1 + line = lines[idx].rstrip() + macro += line + "\n" + if re.match(type_pattern, macro, re.DOTALL): + # this type field can reach the limited API + report(fname, nr + 1, macro) + errors += 1 + return errors + +def report(fname, nr, macro): + f = sys.stderr + print(fname + ":" + str(nr), file=f) + print(macro, file=f) + +if __name__ == "__main__": + p = sys.argv[1] if sys.argv[1:] else "../../Include" + errors = parse_headerfiles(p) + if errors: + # somehow it makes sense to raise a TypeError :-) + raise TypeError("These {} locations contradict the limited API." + .format(errors))