Stackless issue #269: Fix frame_setstate() error handling

A failure to unpickle a frame could cause a NULL pointer access when
deallocating the frame. This has been fixed.

Author: akruis

Stackless/changelog.txt

@@ -9,6 +9,10 @@ What's New in Stackless 3.X.X?
 
 *Release date: 20XX-XX-XX*
 
+- https://github.com/stackless-dev/stackless/issues/269
+  A failure to unpickle a frame could cause a NULL pointer access when
+  deallocating the frame. This has been fixed.
+
 - https://github.com/stackless-dev/stackless/issues/265
   Fix (invalid) function cast warnings with gcc 8 for method conventions
   different from METH_NOARGS, METH_O and METH_VARARGS excluding Argument Clinic

Stackless/pickling/prickelpit.c

@@ -934,8 +934,8 @@ frameobject_reduce(PyFrameObject *f, PyObject *unused)
     return res;
 }
 
-#define frametuplenewfmt "O!"
-#define frametuplesetstatefmt "O!iUO!iO!OiiO!O:frame_new"
+#define frametuplenewfmt "O!:frame.__new__"
+#define frametuplesetstatefmt "O!iUO!iO!OiiO!O:frame.__setstate__"
 
 static PyObject *
 frame_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
@@ -980,7 +980,6 @@ frame_setstate(PyFrameObject *f, PyObject *args)
 
     if (is_wrong_type(Py_TYPE(f))) return NULL;
 
-    Py_CLEAR(f->f_globals);
     Py_CLEAR(f->f_locals);
 
     if (!PyArg_ParseTuple (args, frametuplesetstatefmt,
@@ -1007,15 +1006,12 @@ frame_setstate(PyFrameObject *f, PyObject *args)
                            &bad_func))
         return NULL;
 
-    Py_CLEAR(f->f_locals);
-    Py_CLEAR(f->f_globals);
-
     if (have_locals) {
         Py_INCREF(f_locals);
         f->f_locals = f_locals;
     }
     Py_INCREF(f_globals);
-    f->f_globals = f_globals;
+    Py_SETREF(f->f_globals, f_globals);
 
     if (trace != Py_None) {
         if (!PyCallable_Check(trace)) {

Stackless/unittests/test_pickle.py

@@ -546,8 +546,7 @@ def func(current):
             self.assertIsInstance(cell, cell_type)
             self.assertIs(cell.cell_contents, result)
 
-    def testSetstateFailure(self):
-        # incomplete, just one of many failure modes of stackless._stackless._wrap.frame.__setstate__
+    def _test_setstate_prepare(self):
         foo = "foo"
 
         def f1(bar="bar"):
@@ -567,14 +566,43 @@ def f2():
 
         r = f2()
         self.assertEqual(len(r), 3)
+
+        # r is a tuple (frame_type, (f_code), (state))
+        # state is a tuple of the form
+        # ('f_code', 'valid', 'exec_name', 'f_globals', 'have_locals',
+        #  'f_locals', 'f_trace', 'f_lasti', 'f_lineno',
+        #  'blockstack_as_tuple', 'localsplus_as_tuple')
+        return r
+
+    def test_setstate_OK(self):
+        r = self._test_setstate_prepare()
         wrap_frame = r[0](*r[1])
         self.assertIsInstance(wrap_frame, stackless._stackless._wrap.frame)
-        invalid_state = r[2][:-2] + ((("Not a", "tuple of 3", "integers"),), r[2][-1])
-        self.assertRaisesRegex(TypeError, "an integer is required", wrap_frame.__setstate__, invalid_state)
         # must not raise an assertion
         wrap_frame.__setstate__(r[2])
         self.assertIs(type(wrap_frame), types.FrameType)
 
+    def test_setstate_invalid_blockstack(self):
+        r = self._test_setstate_prepare()
+        # incomplete, just one of many failure modes of stackless._stackless._wrap.frame.__setstate__
+        wrap_frame = r[0](*r[1])
+        invalid_state = r[2][:-2] + ((("Not a", "tuple of 3", "integers"),), r[2][-1])
+        self.assertRaisesRegex(TypeError, "an integer is required", wrap_frame.__setstate__, invalid_state)
+
+    def test_setstate_invalid_state(self):
+        r = self._test_setstate_prepare()
+        # incomplete, just one of many failure modes of stackless._stackless._wrap.frame.__setstate__
+        wrap_frame = r[0](*r[1])
+        invalid_state = ('completely', 'wrong')
+        self.assertRaisesRegex(TypeError, "takes exactly 11 arguments", wrap_frame.__setstate__, invalid_state)
+
+    def test_setstate_different_code(self):
+        r = self._test_setstate_prepare()
+        # incomplete, just one of many failure modes of stackless._stackless._wrap.frame.__setstate__
+        wrap_frame = r[0]((lambda:None).__code__)
+        invalid_state = r[2]
+        self.assertRaisesRegex(TypeError, "invalid code object for frame_setstate", wrap_frame.__setstate__, invalid_state)
+
 
 class TestDictViewPickling(StacklessPickleTestCase):