[pypy-commit] pypy cpyext-avoid-roundtrip: _Py_NewReference did not initialize pypy_link: as such, reused tuples from the freelist could contain a wrong value. Test&fix

[antocuni]

Author: Antonio Cuni <anto.cuni at gmail.com>
Branch: cpyext-avoid-roundtrip
Changeset: r92775:0cd1a1fbbad9
Date: 2017-10-16 12:22 +0200
http://bitbucket.org/pypy/pypy/changeset/0cd1a1fbbad9/

Log:	_Py_NewReference did not initialize pypy_link: as such, reused
	tuples from the freelist could contain a wrong value. Test&fix

diff --git a/pypy/module/cpyext/include/object.h b/pypy/module/cpyext/include/object.h
--- a/pypy/module/cpyext/include/object.h
+++ b/pypy/module/cpyext/include/object.h
@@ -65,7 +65,10 @@
 #define Py_TYPE(ob)		(((PyObject*)(ob))->ob_type)
 #define Py_SIZE(ob)		(((PyVarObject*)(ob))->ob_size)
 
-#define _Py_NewReference(ob) (((PyObject *)(ob))->ob_refcnt = 1)
+#define _Py_NewReference(op)                                        \
+    ( ((PyObject *)(op))->ob_refcnt = 1,                            \
+      ((PyObject *)(op))->ob_pypy_link = 0 )
+
 #define _Py_ForgetReference(ob) /* nothing */
 
 #define Py_None (&_Py_NoneStruct)
diff --git a/pypy/module/cpyext/src/object.c b/pypy/module/cpyext/src/object.c
--- a/pypy/module/cpyext/src/object.c
+++ b/pypy/module/cpyext/src/object.c
@@ -17,7 +17,16 @@
     Py_XDECREF(o);
 }
 
-Py_ssize_t _pypy_rawrefcount_w_marker_deallocating;  /* set from pyobject.py */
+/* 
+ * The actual value of this variable will be the address of
+ * pyobject.w_marker_deallocating, and will be set by
+ * pyobject.write_w_marker_deallocating().
+ *
+ * The value set here is used only as a marker by tests (because during the
+ * tests we cannot call set_marker(), so we need to set a special value
+ * directly here)
+ */
+Py_ssize_t _pypy_rawrefcount_w_marker_deallocating = 0xDEADFFF;
 
 void
 _Py_Dealloc(PyObject *obj)
diff --git a/pypy/module/cpyext/test/test_tupleobject.py b/pypy/module/cpyext/test/test_tupleobject.py
--- a/pypy/module/cpyext/test/test_tupleobject.py
+++ b/pypy/module/cpyext/test/test_tupleobject.py
@@ -52,6 +52,31 @@
         py_b = as_pyobj(space, w_b)
         assert py_a != py_b
 
+    def test_PyTuple_New_initialize_pypy_link(self, space, api):
+        from rpython.rlib.rawrefcount import _collect
+        state = space.fromcache(State)
+        # see object.c:_pypy_rawrefcount_w_marker_deallocating
+        MARKER = 0xDEADFFF
+        #
+        # first: create a pytuple, attach a w_obj, decref the pytuple and let
+        # the GC to collect the w_obj: this way, c_ob_pypy_link is set to
+        # w_marker_deallocating
+        py_a = state.C.PyTuple_New(0)
+        assert py_a.c_ob_pypy_link == 0
+        w_a = from_ref(space, py_a)
+        assert py_a.c_ob_pypy_link != 0
+        decref(space, py_a)
+        w_a = None
+        _collect()
+        assert py_a.c_ob_pypy_link == MARKER
+        #
+        # second: create another tuple, which will reuse the same memory as
+        # before thanks to the freelist. Check that c_ob_pypy_link has been
+        # initialized to 0.
+        py_b = state.C.PyTuple_New(0)
+        assert py_b == py_a
+        assert py_b.c_ob_pypy_link == 0
+
     def test_tuple_resize(self, space, api):
         state = space.fromcache(State)
         w_42 = space.wrap(42)