Skip to content

Commit ea62ce7

Browse files
ctismerned-deily
ctismer
authored and
ned-deily
committed
bpo-33738: Fix macros which contradict PEP 384 (pythonGH-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
1 parent 3f45f5d commit ea62ce7

File tree

8 files changed

+198
-0
lines changed

8 files changed

+198
-0
lines changed

‎Include/abstract.h‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,9 +590,16 @@ PyAPI_FUNC(PyObject *) PyObject_Format(PyObject *obj,
590590
returns itself. */
591591
PyAPI_FUNC(PyObject *) PyObject_GetIter(PyObject *);
592592

593+
/* Returns 1 if the object 'obj' provides iterator protocols, and 0 otherwise.
594+
595+
This function always succeeds. */
596+
#ifndef Py_LIMITED_API
593597
#define PyIter_Check(obj) \
594598
((obj)->ob_type->tp_iternext != NULL && \
595599
(obj)->ob_type->tp_iternext != &_PyObject_NextNotImplemented)
600+
#else
601+
PyAPI_FUNC(int) PyIter_Check(PyObject*);
602+
#endif
596603

597604
/* Takes an iterator object and calls its tp_iternext slot,
598605
returning the next value.
@@ -710,9 +717,15 @@ PyAPI_FUNC(PyObject *) PyNumber_Xor(PyObject *o1, PyObject *o2);
710717
This is the equivalent of the Python expression: o1 | o2. */
711718
PyAPI_FUNC(PyObject *) PyNumber_Or(PyObject *o1, PyObject *o2);
712719

720+
/* Returns 1 if obj is an index integer (has the nb_index slot of the
721+
tp_as_number structure filled in), and 0 otherwise. */
722+
#ifndef Py_LIMITED_API
713723
#define PyIndex_Check(obj) \
714724
((obj)->ob_type->tp_as_number != NULL && \
715725
(obj)->ob_type->tp_as_number->nb_index != NULL)
726+
#else
727+
PyAPI_FUNC(int) PyIndex_Check(PyObject *);
728+
#endif
716729

717730
/* Returns the object 'o' converted to a Python int, or NULL with an exception
718731
raised on failure. */
@@ -921,8 +934,10 @@ PyAPI_FUNC(PyObject *) PySequence_Fast(PyObject *o, const char* m);
921934

922935
/* Assume tp_as_sequence and sq_item exist and that 'i' does not
923936
need to be corrected for a negative index. */
937+
#ifndef Py_LIMITED_API
924938
#define PySequence_ITEM(o, i)\
925939
( Py_TYPE(o)->tp_as_sequence->sq_item(o, i) )
940+
#endif
926941

927942
/* Return a pointer to the underlying item array for
928943
an object retured by PySequence_Fast */

‎Include/objimpl.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,10 @@ PyAPI_FUNC(Py_ssize_t) _PyGC_CollectIfEnabled(void);
240240
#define PyType_IS_GC(t) PyType_HasFeature((t), Py_TPFLAGS_HAVE_GC)
241241

242242
/* Test if an object has a GC head */
243+
#ifndef Py_LIMITED_API
243244
#define PyObject_IS_GC(o) (PyType_IS_GC(Py_TYPE(o)) && \
244245
(Py_TYPE(o)->tp_is_gc == NULL || Py_TYPE(o)->tp_is_gc(o)))
246+
#endif
245247

246248
PyAPI_FUNC(PyVarObject *) _PyObject_GC_Resize(PyVarObject *, Py_ssize_t);
247249
#define PyObject_GC_Resize(type, op, n) \
@@ -359,10 +361,12 @@ PyAPI_FUNC(void) PyObject_GC_Del(void *);
359361

360362

361363
/* Test if a type supports weak references */
364+
#ifndef Py_LIMITED_API
362365
#define PyType_SUPPORTS_WEAKREFS(t) ((t)->tp_weaklistoffset > 0)
363366

364367
#define PyObject_GET_WEAKREFS_LISTPTR(o) \
365368
((PyObject **) (((char *) (o)) + Py_TYPE(o)->tp_weaklistoffset))
369+
#endif
366370

367371
#ifdef __cplusplus
368372
}

‎Include/pyerrors.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,12 @@ PyAPI_FUNC(void) _PyErr_ChainExceptions(PyObject *, PyObject *, PyObject *);
140140
#define PyExceptionInstance_Check(x) \
141141
PyType_FastSubclass((x)->ob_type, Py_TPFLAGS_BASE_EXC_SUBCLASS)
142142

143+
#ifndef Py_LIMITED_API
143144
#define PyExceptionClass_Name(x) \
144145
((char *)(((PyTypeObject*)(x))->tp_name))
146+
#else
147+
PyAPI_FUNC(char *) PyExceptionClass_Name(PyObject*);
148+
#endif
145149

146150
#define PyExceptionInstance_Class(x) ((PyObject*)((x)->ob_type))
147151

‎Misc/NEWS.d/next/Core and Builtins/2018-06-07-18-34-19.bpo-33738.ODZS7a.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Seven macro incompatibilities with the Limited API were fixed, and the
2+
macros PyIter_Check, PyIndex_Check and PyExceptionClass_Name were added as
3+
functions. A script for automatic macro checks was added.

‎Objects/abstract.c‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,14 @@ PyNumber_Absolute(PyObject *o)
12441244
return type_error("bad operand type for abs(): '%.200s'", o);
12451245
}
12461246

1247+
#undef PyIndex_Check
1248+
int
1249+
PyIndex_Check(PyObject *obj)
1250+
{
1251+
return obj->ob_type->tp_as_number != NULL &&
1252+
obj->ob_type->tp_as_number->nb_index != NULL;
1253+
}
1254+
12471255
/* Return a Python int from the object item.
12481256
Raise TypeError if the result is not an int
12491257
or if the object cannot be interpreted as an index.
@@ -2535,6 +2543,13 @@ PyObject_GetIter(PyObject *o)
25352543
}
25362544
}
25372545

2546+
#undef PyIter_Check
2547+
int PyIter_Check(PyObject *obj)
2548+
{
2549+
return obj->ob_type->tp_iternext != NULL &&
2550+
obj->ob_type->tp_iternext != &_PyObject_NextNotImplemented;
2551+
}
2552+
25382553
/* Return next item.
25392554
* If an error occurs, return NULL. PyErr_Occurred() will be true.
25402555
* If the iteration terminates normally, return NULL and clear the

‎Objects/exceptions.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,12 @@ PyException_SetContext(PyObject *self, PyObject *context)
342342
Py_XSETREF(((PyBaseExceptionObject *)self)->context, context);
343343
}
344344

345+
#undef PyExceptionClass_Name
346+
char *
347+
PyExceptionClass_Name(PyObject *ob)
348+
{
349+
return ((PyTypeObject*)ob)->tp_name;
350+
}
345351

346352
static struct PyMemberDef BaseException_members[] = {
347353
{"__suppress_context__", T_BOOL,

‎PC/python3.def‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ EXPORTS
248248
PyExc_Warning=python38.PyExc_Warning DATA
249249
PyExc_WindowsError=python38.PyExc_WindowsError DATA
250250
PyExc_ZeroDivisionError=python38.PyExc_ZeroDivisionError DATA
251+
PyExceptionClass_Name=python38.PyExceptionClass_Name
251252
PyException_GetCause=python38.PyException_GetCause
252253
PyException_GetContext=python38.PyException_GetContext
253254
PyException_GetTraceback=python38.PyException_GetTraceback
@@ -294,9 +295,11 @@ EXPORTS
294295
PyImport_ImportModuleLevelObject=python38.PyImport_ImportModuleLevelObject
295296
PyImport_ImportModuleNoBlock=python38.PyImport_ImportModuleNoBlock
296297
PyImport_ReloadModule=python38.PyImport_ReloadModule
298+
PyIndex_Check=python38.PyIndex_Check
297299
PyInterpreterState_Clear=python38.PyInterpreterState_Clear
298300
PyInterpreterState_Delete=python38.PyInterpreterState_Delete
299301
PyInterpreterState_New=python38.PyInterpreterState_New
302+
PyIter_Check=python38.PyIter_Check
300303
PyIter_Next=python38.PyIter_Next
301304
PyListIter_Type=python38.PyListIter_Type DATA
302305
PyListRevIter_Type=python38.PyListRevIter_Type DATA

‎Tools/scripts/pep384_macrocheck.py‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
"""
2+
pep384_macrocheck.py
3+
4+
This programm tries to locate errors in the relevant Python header
5+
files where macros access type fields when they are reachable from
6+
the limided API.
7+
8+
The idea is to search macros with the string "->tp_" in it.
9+
When the macro name does not begin with an underscore,
10+
then we have found a dormant error.
11+
12+
Christian Tismer
13+
2018-06-02
14+
"""
15+
16+
import sys
17+
import os
18+
import re
19+
20+
21+
DEBUG = False
22+
23+
def dprint(*args, **kw):
24+
if DEBUG:
25+
print(*args, **kw)
26+
27+
def parse_headerfiles(startpath):
28+
"""
29+
Scan all header files which are reachable fronm Python.h
30+
"""
31+
search = "Python.h"
32+
name = os.path.join(startpath, search)
33+
if not os.path.exists(name):
34+
raise ValueError("file {} was not found in {}\n"
35+
"Please give the path to Python's include directory."
36+
.format(search, startpath))
37+
errors = 0
38+
with open(name) as python_h:
39+
while True:
40+
line = python_h.readline()
41+
if not line:
42+
break
43+
found = re.match(r'^\s*#\s*include\s*"(\w+\.h)"', line)
44+
if not found:
45+
continue
46+
include = found.group(1)
47+
dprint("Scanning", include)
48+
name = os.path.join(startpath, include)
49+
if not os.path.exists(name):
50+
name = os.path.join(startpath, "../PC", include)
51+
errors += parse_file(name)
52+
return errors
53+
54+
def ifdef_level_gen():
55+
"""
56+
Scan lines for #ifdef and track the level.
57+
"""
58+
level = 0
59+
ifdef_pattern = r"^\s*#\s*if" # covers ifdef and ifndef as well
60+
endif_pattern = r"^\s*#\s*endif"
61+
while True:
62+
line = yield level
63+
if re.match(ifdef_pattern, line):
64+
level += 1
65+
elif re.match(endif_pattern, line):
66+
level -= 1
67+
68+
def limited_gen():
69+
"""
70+
Scan lines for Py_LIMITED_API yes(1) no(-1) or nothing (0)
71+
"""
72+
limited = [0] # nothing
73+
unlimited_pattern = r"^\s*#\s*ifndef\s+Py_LIMITED_API"
74+
limited_pattern = "|".join([
75+
r"^\s*#\s*ifdef\s+Py_LIMITED_API",
76+
r"^\s*#\s*(el)?if\s+!\s*defined\s*\(\s*Py_LIMITED_API\s*\)\s*\|\|",
77+
r"^\s*#\s*(el)?if\s+defined\s*\(\s*Py_LIMITED_API"
78+
])
79+
else_pattern = r"^\s*#\s*else"
80+
ifdef_level = ifdef_level_gen()
81+
status = next(ifdef_level)
82+
wait_for = -1
83+
while True:
84+
line = yield limited[-1]
85+
new_status = ifdef_level.send(line)
86+
dir = new_status - status
87+
status = new_status
88+
if dir == 1:
89+
if re.match(unlimited_pattern, line):
90+
limited.append(-1)
91+
wait_for = status - 1
92+
elif re.match(limited_pattern, line):
93+
limited.append(1)
94+
wait_for = status - 1
95+
elif dir == -1:
96+
# this must have been an endif
97+
if status == wait_for:
98+
limited.pop()
99+
wait_for = -1
100+
else:
101+
# it could be that we have an elif
102+
if re.match(limited_pattern, line):
103+
limited.append(1)
104+
wait_for = status - 1
105+
elif re.match(else_pattern, line):
106+
limited.append(-limited.pop()) # negate top
107+
108+
def parse_file(fname):
109+
errors = 0
110+
with open(fname) as f:
111+
lines = f.readlines()
112+
type_pattern = r"^.*?->\s*tp_"
113+
define_pattern = r"^\s*#\s*define\s+(\w+)"
114+
limited = limited_gen()
115+
status = next(limited)
116+
for nr, line in enumerate(lines):
117+
status = limited.send(line)
118+
line = line.rstrip()
119+
dprint(fname, nr, status, line)
120+
if status != -1:
121+
if re.match(define_pattern, line):
122+
name = re.match(define_pattern, line).group(1)
123+
if not name.startswith("_"):
124+
# found a candidate, check it!
125+
macro = line + "\n"
126+
idx = nr
127+
while line.endswith("\\"):
128+
idx += 1
129+
line = lines[idx].rstrip()
130+
macro += line + "\n"
131+
if re.match(type_pattern, macro, re.DOTALL):
132+
# this type field can reach the limited API
133+
report(fname, nr + 1, macro)
134+
errors += 1
135+
return errors
136+
137+
def report(fname, nr, macro):
138+
f = sys.stderr
139+
print(fname + ":" + str(nr), file=f)
140+
print(macro, file=f)
141+
142+
if __name__ == "__main__":
143+
p = sys.argv[1] if sys.argv[1:] else "../../Include"
144+
errors = parse_headerfiles(p)
145+
if errors:
146+
# somehow it makes sense to raise a TypeError :-)
147+
raise TypeError("These {} locations contradict the limited API."
148+
.format(errors))

0 commit comments

Comments
 (0)