Stackless issue #168: make Stackless compatible with old Cython modules (#170)

Many extension modules were created by Cython versions before commit
037bcf0 and were compiled with regular C-Python. These modules call
PyEval_EvalFrameEx() with a broken frame object.
This commit add code to recover a broken frame in PyEval_EvalFrameEx().

(cherry picked from commit f9094d2)

Author: Anselm Kruis

Objects/frameobject.c

@@ -442,6 +442,13 @@ frame_dealloc(PyFrameObject *f)
 
     PyObject_GC_UnTrack(f);
     Py_TRASHCAN_SAFE_BEGIN(f)
+
+#if defined(STACKLESS) && PY_VERSION_HEX < 0x03080000
+    /* Clear the magic for the old Cython frame hack.
+     * See below in PyFrame_New() for a detailed explanation.
+     */
+    f->f_blockstack[0].b_type = 0;
+#endif
     /* Kill all local variables */
     valuestack = f->f_valuestack;
     for (p = f->f_localsplus; p < valuestack; p++)
@@ -754,6 +761,25 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code, PyObject *globals,
 
 #ifdef STACKLESS
     f->f_execute = NULL;
+#if PY_VERSION_HEX < 0x03080000
+    if (code->co_argcount > 0) {
+        /*
+         * A hack for binary compatibility with Cython extension modules, which
+         * were created with an older Cythoncompiled with regular C-Python. These
+         * modules create frames using PyFrame_New then write to frame->f_localsplus
+         * to set the arguments. But C-Python f_localsplus is Stackless f_code.
+         * Therefore we add a copy of f_code and a magic number in the
+         * uninitialized f_blockstack array.
+         * If the blockstack is used, the magic is overwritten.
+         * To make sure, the pointer is aligned correctly, we address it relative to
+         * f_code.
+         * See Stackless issue #168
+         */
+        (&(f->f_code))[-1] = code;
+        /* an arbitrary negative number which is not an opcode */
+        f->f_blockstack[0].b_type = -31683;
+    }
+#endif
 #endif
     _PyObject_GC_TRACK(f);
     return f;

Python/ceval.c

@@ -4160,16 +4160,51 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
     if (f == NULL)
         return NULL;
 
+    /* The layout of PyFrameObject differs between Stackless and C-Python.
+     * Stackless f->f_execute is C-Python f->f_code. Stackless f->f_code is at
+     * the end, just before f_localsplus.
+     */
     if (PyFrame_Check(f) && f->f_execute == NULL) {
         /* A new frame returned from PyFrame_New() has f->f_execute == NULL.
+         * Set the usual execution function.
          */
         f->f_execute = PyEval_EvalFrameEx_slp;
+
+#if PY_VERSION_HEX < 0x03080000
+        /* Older versions of Cython used to create frames using C-Python layout
+         * of PyFrameObject. As a consequence f_code is overwritten by the first
+         * item of f_localsplus[]. To be able to fix it, we have a copy of
+         * f_code and a signature at the end of the block-stack.
+         * The Py_BUILD_ASSERT_EXPR checks,that our assumptions about the layout
+         * of PyFrameObject are true.
+         * See Stackless issue #168
+         */
+        (void) Py_BUILD_ASSERT_EXPR(offsetof(PyFrameObject, f_code) ==
+               offsetof(PyFrameObject, f_localsplus) - Py_MEMBER_SIZE(PyFrameObject, f_localsplus[0]));
+
+        /* Check for an old Cython frame */
+        if (f->f_iblock == 0 && f->f_lasti == -1 && /* blockstack is empty */
+            f->f_blockstack[0].b_type == -31683 && /* magic is present */
+            /* and f_code has been overwritten */
+            f->f_code != (&(f->f_code))[-1] &&
+            /* and (&(f->f_code))[-1] looks like a valid code object */
+            (&(f->f_code))[-1] && PyCode_Check((&(f->f_code))[-1]) &&
+            /* and there are arguments */
+            (&(f->f_code))[-1]->co_argcount > 0 &&
+            /* the last argument is NULL */
+            f->f_localsplus[(&(f->f_code))[-1]->co_argcount - 1] == NULL)
+        {
+            PyCodeObject * code = (&(f->f_code))[-1];
+            memmove(f->f_localsplus, f->f_localsplus-1, code->co_argcount * sizeof(f->f_localsplus[0]));
+            f->f_code = code;
+        } else
+#endif
+        if (!(f->f_code != NULL && PyCode_Check(f->f_code))) {
+            PyErr_BadInternalCall();
+            return NULL;
+        }
     } else {
-        /* The layout of PyFrameObject differs between Stackless and C-Python.
-         * Stackless f->f_execute is C-Python f->f_code. Stackless f->f_code is at
-         * the end, just before f_localsplus.
-         *
-         * In order to detect a C-Python frame, we must compare f->f_execute
+        /* In order to detect a broken C-Python frame, we must compare f->f_execute
          * with every valid frame function. Hard to implement completely.
          * Therefore I'll check only for relevant functions.
          * Amend the list as needed.

Stackless/changelog.txt

@@ -13,6 +13,10 @@ What's New in Stackless 3.X.X?
   Fix C-API functions PyEval_EvalFrameEx() and PyEval_EvalFrame().
   They are now compatible with C-Python.
 
+- https://github.com/stackless-dev/stackless/issues/168
+  Make Stackless compatible with old Cython extension modules compiled
+  for regular C-Python.
+
 - https://github.com/stackless-dev/stackless/issues/167
   Replace 'printf(...)' calls by PySys_WriteStderr(...). They are used to emit
   an error message, if there is a pending error while entering Stackless

Stackless/module/stacklessmodule.c

@@ -1134,20 +1134,22 @@ by Stackless Python.\n\
 The function creates a frame from code, globals and args and executes the frame.");
 
 static PyObject* test_PyEval_EvalFrameEx(PyObject *self, PyObject *args, PyObject *kwds) {
-    static char *kwlist[] = {"code", "globals", "args", "alloca", "throw", NULL};
+    static char *kwlist[] = {"code", "globals", "args", "alloca", "throw", "oldcython",
+                             "code2", NULL};
     PyThreadState *tstate = PyThreadState_GET();
-    PyCodeObject *co;
+    PyCodeObject *co, *code2 = NULL;
     PyObject *globals, *co_args = NULL;
     Py_ssize_t alloca_size = 0;
     PyObject *exc = NULL;
+    PyObject *oldcython = NULL;
     PyFrameObject *f;
     PyObject *result = NULL;
     void *p;
     Py_ssize_t na;
 
-    if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!O!|O!nO:test_PyEval_EvalFrameEx", kwlist,
+    if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!O!|O!nOO!O!:test_PyEval_EvalFrameEx", kwlist,
             &PyCode_Type, &co, &PyDict_Type, &globals, &PyTuple_Type, &co_args, &alloca_size,
-            &exc))
+            &exc, &PyBool_Type, &oldcython, &PyCode_Type, &code2))
         return NULL;
     if (exc && !PyExceptionInstance_Check(exc)) {
         PyErr_SetString(PyExc_TypeError, "exc must be an exception instance");
@@ -1178,6 +1180,13 @@ static PyObject* test_PyEval_EvalFrameEx(PyObject *self, PyObject *args, PyObjec
             goto exit;
         }
         fastlocals = f->f_localsplus;
+        if (oldcython == Py_True) {
+            /* Use the f_localsplus offset from regular C-Python. Old versions of cython used to
+             * access f_localplus directly. Current versions compute the field offset for
+             * f_localsplus at run-time.
+             */
+            fastlocals--;
+        }
         for (i = 0; i < na; i++) {
             PyObject *arg = PyTuple_GetItem(co_args, i);
             if (arg == NULL) {
@@ -1193,6 +1202,10 @@ static PyObject* test_PyEval_EvalFrameEx(PyObject *self, PyObject *args, PyObjec
     if (exc) {
         PyErr_SetObject(PyExceptionInstance_Class(exc), exc);
     }
+    if (code2) {
+        Py_INCREF(code2);
+        Py_SETREF(f->f_code, code2);
+    }
     result = PyEval_EvalFrameEx(f, exc != NULL);
     /* result = Py_None; Py_INCREF(Py_None); */
 exit:

Stackless/unittests/test_capi.py

@@ -31,7 +31,8 @@
 import unittest
 
 from support import test_main  # @UnusedImport
-from support import StacklessTestCase, withThreads, require_one_thread
+from support import (StacklessTestCase, withThreads, require_one_thread,
+                     testcase_leaks_references)
 
 if withThreads:
     try:
@@ -161,6 +162,44 @@ def f():
         self.assertIn(str(exc), msg)
         #print(msg)
 
+    def test_oldcython_frame(self):
+        # A test for Stackless issue #168
+        self.assertEqual(self.call_PyEval_EvalFrameEx(47110816, oldcython=True), 47110816)
+
+    def test_oldcython_frame_code_is_1st_arg_good(self):
+        # A pathological test for Stackless issue #168
+        def f(code):
+            return code
+
+        def f2(code):
+            return code
+
+        self.assertIs(stackless.test_PyEval_EvalFrameEx(f.__code__, f.__globals__, (f.__code__,), oldcython=False), f.__code__)
+        self.assertIs(stackless.test_PyEval_EvalFrameEx(f.__code__, f.__globals__, (f2.__code__,), oldcython=True), f2.__code__)
+
+    @testcase_leaks_references("f->f_code get overwritten without Py_DECREF")
+    def test_oldcython_frame_code_is_1st_arg_bad(self):
+        # A pathological test for Stackless issue #168
+        def f(code):
+            return code
+
+        # we can't fix this particular case:
+        #  - running code object is its 1st arg and
+        #  - oldcython=True,
+        # because a fix would result in a segmentation fault, if the number of
+        # arguments is to low (test case test_0_args)
+        self.assertRaises(UnboundLocalError, stackless.test_PyEval_EvalFrameEx, f.__code__, f.__globals__, (f.__code__,), oldcython=True)
+
+    def test_other_code_object(self):
+        # A pathological test for Stackless issue #168
+        def f(arg):
+            return arg
+
+        def f2(arg):
+            return arg
+
+        self.assertIs(stackless.test_PyEval_EvalFrameEx(f.__code__, f.__globals__, (f2,), code2=f2.__code__), f2)
+
 
 if __name__ == "__main__":
     if not sys.argv[1:]: