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

@@ -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 */

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
 }

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))
 

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.

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

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,

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

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))