Author: Armin Rigo <[email protected]>
Branch: cpyext-avoid-roundtrip
Changeset: r92716:0d92b4ee9763
Date: 2017-10-11 12:18 +0200
http://bitbucket.org/pypy/pypy/changeset/0d92b4ee9763/
Log: (antocuni, ronan, arigo)
Add hack_for_result_often_existing_obj(), used in a few functions
which officially return a new reference, but where the caller might
be bogus code that expects the new reference to stay alive even
after a Py_DecRef() (which is "often" the case in CPython)
diff --git a/pypy/module/cpyext/mapping.py b/pypy/module/cpyext/mapping.py
--- a/pypy/module/cpyext/mapping.py
+++ b/pypy/module/cpyext/mapping.py
@@ -1,7 +1,8 @@
from rpython.rtyper.lltypesystem import lltype, rffi
from pypy.module.cpyext.api import (
cpython_api, CANNOT_FAIL, CONST_STRING, Py_ssize_t)
-from pypy.module.cpyext.pyobject import PyObject
+from pypy.module.cpyext.pyobject import (
+ PyObject, hack_for_result_often_existing_obj)
@cpython_api([PyObject], rffi.INT_real, error=CANNOT_FAIL)
@@ -37,12 +38,13 @@
the Python expression o.items()."""
return space.call_method(w_obj, "items")
-@cpython_api([PyObject, CONST_STRING], PyObject)
+@cpython_api([PyObject, CONST_STRING], PyObject, result_is_ll=True)
def PyMapping_GetItemString(space, w_obj, key):
"""Return element of o corresponding to the object key or NULL on failure.
This is the equivalent of the Python expression o[key]."""
w_key = space.newtext(rffi.charp2str(key))
- return space.getitem(w_obj, w_key)
+ w_res = space.getitem(w_obj, w_key)
+ return hack_for_result_often_existing_obj(space, w_res)
@cpython_api([PyObject, CONST_STRING, PyObject], rffi.INT_real, error=-1)
def PyMapping_SetItemString(space, w_obj, key, w_value):
diff --git a/pypy/module/cpyext/object.py b/pypy/module/cpyext/object.py
--- a/pypy/module/cpyext/object.py
+++ b/pypy/module/cpyext/object.py
@@ -6,7 +6,7 @@
Py_GE, CONST_STRING, FILEP, fwrite, c_only)
from pypy.module.cpyext.pyobject import (
PyObject, PyObjectP, from_ref, incref, decref,
- get_typedescr)
+ get_typedescr, hack_for_result_often_existing_obj)
from pypy.module.cpyext.typeobject import PyTypeObjectPtr
from pypy.module.cpyext.pyerrors import PyErr_NoMemory, PyErr_BadInternalCall
from pypy.objspace.std.typeobject import W_TypeObject
@@ -69,20 +69,22 @@
def PyObject_Not(space, w_obj):
return not space.is_true(w_obj)
-@cpython_api([PyObject, PyObject], PyObject)
+@cpython_api([PyObject, PyObject], PyObject, result_is_ll=True)
def PyObject_GetAttr(space, w_obj, w_name):
"""Retrieve an attribute named attr_name from object o. Returns the
attribute
value on success, or NULL on failure. This is the equivalent of the Python
expression o.attr_name."""
- return space.getattr(w_obj, w_name)
+ w_res = space.getattr(w_obj, w_name)
+ return hack_for_result_often_existing_obj(space, w_res)
-@cpython_api([PyObject, CONST_STRING], PyObject)
+@cpython_api([PyObject, CONST_STRING], PyObject, result_is_ll=True)
def PyObject_GetAttrString(space, w_obj, name_ptr):
"""Retrieve an attribute named attr_name from object o. Returns the
attribute
value on success, or NULL on failure. This is the equivalent of the Python
expression o.attr_name."""
name = rffi.charp2str(name_ptr)
- return space.getattr(w_obj, space.newtext(name))
+ w_res = space.getattr(w_obj, space.newtext(name))
+ return hack_for_result_often_existing_obj(space, w_res)
@cpython_api([PyObject, PyObject], rffi.INT_real, error=CANNOT_FAIL)
def PyObject_HasAttr(space, w_obj, w_name):
@@ -141,11 +143,12 @@
and 0 otherwise. This function always succeeds."""
return int(space.is_true(space.callable(w_obj)))
-@cpython_api([PyObject, PyObject], PyObject)
+@cpython_api([PyObject, PyObject], PyObject, result_is_ll=True)
def PyObject_GetItem(space, w_obj, w_key):
"""Return element of o corresponding to the object key or NULL on failure.
This is the equivalent of the Python expression o[key]."""
- return space.getitem(w_obj, w_key)
+ w_res = space.getitem(w_obj, w_key)
+ return hack_for_result_often_existing_obj(space, w_res)
@cpython_api([PyObject, PyObject, PyObject], rffi.INT_real, error=-1)
def PyObject_SetItem(space, w_obj, w_key, w_value):
diff --git a/pypy/module/cpyext/pyobject.py b/pypy/module/cpyext/pyobject.py
--- a/pypy/module/cpyext/pyobject.py
+++ b/pypy/module/cpyext/pyobject.py
@@ -286,6 +286,13 @@
keepalive_until_here(w_obj)
return pyobj
+def hack_for_result_often_existing_obj(space, w_obj):
+ # Equivalent to get_pyobj_and_incref() and not to make_ref():
+ # it builds a PyObject from a W_Root, but ensures that the result
+ # gets attached to the original W_Root. This is needed to work around
+ # some obscure abuses: https://github.com/numpy/numpy/issues/9850
+ return get_pyobj_and_incref(space, w_obj)
+
def make_ref(space, w_obj, w_userdata=None, immortal=False):
"""Turn the W_Root into a corresponding PyObject. You should
decref the returned PyObject later. Note that it is often the
diff --git a/pypy/module/cpyext/test/test_mapping.py
b/pypy/module/cpyext/test/test_mapping.py
--- a/pypy/module/cpyext/test/test_mapping.py
+++ b/pypy/module/cpyext/test/test_mapping.py
@@ -1,5 +1,8 @@
from pypy.module.cpyext.test.test_api import BaseApiTest
+from pypy.module.cpyext.test.test_cpyext import AppTestCpythonExtensionBase
from rpython.rtyper.lltypesystem import lltype, rffi
+from pypy.module.cpyext.pyobject import get_w_obj_and_decref
+
class TestMapping(BaseApiTest):
def test_check(self, space, api):
@@ -25,8 +28,8 @@
w_d = space.newdict()
key = rffi.str2charp("key")
api.PyMapping_SetItemString(w_d, key, space.wrap(42))
- assert 42 == space.unwrap(
- api.PyMapping_GetItemString(w_d, key))
+ assert 42 == space.unwrap(get_w_obj_and_decref(space,
+ api.PyMapping_GetItemString(w_d, key)))
rffi.free_charp(key)
def test_haskey(self, space, api):
@@ -38,3 +41,23 @@
assert api.PyMapping_HasKey(w_d, w_d) == 0
# and no error is set
+
+
+class AppTestMapping(AppTestCpythonExtensionBase):
+
+ def test_getitemstring_returns_new_but_borrowed_ref(self):
+ module = self.import_extension('foo', [
+ ("test_mapping", "METH_O",
+ '''
+ PyObject *value = PyMapping_GetItemString(args, "a");
+ /* officially, "value" can have a refcount equal to one,
+ but some code out there assumes that it has a refcnt
+ of at least two --- which is bogus --- because it
+ is generally kept alive by the container. */
+ PyObject *refcnt = PyInt_FromLong(value->ob_refcnt);
+ Py_DECREF(value);
+ return refcnt;
+ '''),])
+ d = {"a": 42}
+ res = module.test_mapping(d)
+ assert res > 1
diff --git a/pypy/module/cpyext/test/test_object.py
b/pypy/module/cpyext/test/test_object.py
--- a/pypy/module/cpyext/test/test_object.py
+++ b/pypy/module/cpyext/test/test_object.py
@@ -3,6 +3,7 @@
from pypy.module.cpyext.test.test_api import BaseApiTest, raises_w
from pypy.module.cpyext.test.test_cpyext import AppTestCpythonExtensionBase
from rpython.rtyper.lltypesystem import rffi, lltype
+from pypy.module.cpyext.pyobject import get_w_obj_and_decref
from pypy.module.cpyext.api import (
Py_LT, Py_LE, Py_NE, Py_EQ, Py_GE, Py_GT)
from pypy.module.cpyext.object import (
@@ -70,7 +71,8 @@
def test_getattr(self, space):
charp1 = rffi.str2charp("__len__")
charp2 = rffi.str2charp("not_real")
- assert PyObject_GetAttrString(space, space.wrap(""), charp1)
+ assert get_w_obj_and_decref(space,
+ PyObject_GetAttrString(space, space.wrap(""), charp1))
with raises_w(space, AttributeError):
PyObject_GetAttrString(space, space.wrap(""), charp2)
@@ -79,17 +81,20 @@
rffi.free_charp(charp1)
rffi.free_charp(charp2)
- assert PyObject_GetAttr(space, space.wrap(""), space.wrap("__len__"))
+ assert get_w_obj_and_decref(space,
+ PyObject_GetAttr(space, space.wrap(""), space.wrap("__len__")))
with raises_w(space, AttributeError):
PyObject_DelAttr(space, space.wrap(""), space.wrap("__len__"))
def test_getitem(self, space, api):
w_t = space.wrap((1, 2, 3, 4, 5))
- assert space.unwrap(api.PyObject_GetItem(w_t, space.wrap(3))) == 4
+ assert space.unwrap(get_w_obj_and_decref(space,
+ api.PyObject_GetItem(w_t, space.wrap(3)))) == 4
w_d = space.newdict()
space.setitem(w_d, space.wrap("a key!"), space.wrap(72))
- assert space.unwrap(api.PyObject_GetItem(w_d, space.wrap("a key!")))
== 72
+ assert space.unwrap(get_w_obj_and_decref(space,
+ api.PyObject_GetItem(w_d, space.wrap("a key!")))) == 72
assert api.PyObject_SetItem(w_d, space.wrap("key"), space.w_None) == 0
assert space.getitem(w_d, space.wrap("key")) is space.w_None
_______________________________________________
pypy-commit mailing list
[email protected]
https://mail.python.org/mailman/listinfo/pypy-commit
