[pypy-commit] pypy default: Tentatively detect cases of recreation of a PyPy object from the
arigo
pypy.commits at gmail.com
Fri Nov 25 08:55:46 EST 2016
Author: Armin Rigo <arigo at tunes.org>
Branch:
Changeset: r88669:4540fd0f69d1
Date: 2016-11-25 14:54 +0100
http://bitbucket.org/pypy/pypy/changeset/4540fd0f69d1/
Log: Tentatively detect cases of recreation of a PyPy object from the
PyObject during tp_dealloc.
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
@@ -15,6 +15,7 @@
from rpython.rlib.objectmodel import keepalive_until_here
from rpython.rtyper.annlowlevel import llhelper
from rpython.rlib import rawrefcount
+from rpython.rlib.debug import fatalerror
#________________________________________________________
@@ -192,6 +193,8 @@
rawrefcount.create_link_pypy(w_obj, py_obj)
+w_marker_deallocating = W_Root()
+
def from_ref(space, ref):
"""
Finds the interpreter object corresponding to the given reference. If the
@@ -202,7 +205,23 @@
return None
w_obj = rawrefcount.to_obj(W_Root, ref)
if w_obj is not None:
- return w_obj
+ if w_obj is not w_marker_deallocating:
+ return w_obj
+ fatalerror(
+ "*** Invalid usage of a dying CPython object ***\n"
+ "\n"
+ "cpyext, the emulation layer, detected that while it is calling\n"
+ "an object's tp_dealloc, the C code calls back a function that\n"
+ "tries to recreate the PyPy version of the object. Usually it\n"
+ "means that tp_dealloc calls some general PyXxx() API. It is\n"
+ "a dangerous and potentially buggy thing to do: even in CPython\n"
+ "the PyXxx() function could, in theory, cause a reference to the\n"
+ "object to be taken and stored somewhere, for an amount of time\n"
+ "exceeding tp_dealloc itself. Afterwards, the object will be\n"
+ "freed, making that reference point to garbage.\n"
+ ">>> PyPy could contain some workaround to still work if\n"
+ "you are lucky, but it is not done so far; better fix the bug in\n"
+ "the CPython extension.")
# This reference is not yet a real interpreter object.
# Realize it.
@@ -233,7 +252,8 @@
INTERPLEVEL_API['as_pyobj'] = as_pyobj
def pyobj_has_w_obj(pyobj):
- return rawrefcount.to_obj(W_Root, pyobj) is not None
+ w_obj = rawrefcount.to_obj(W_Root, pyobj)
+ return w_obj is not None and w_obj is not w_marker_deallocating
INTERPLEVEL_API['pyobj_has_w_obj'] = staticmethod(pyobj_has_w_obj)
@@ -335,6 +355,7 @@
pto = obj.c_ob_type
#print >>sys.stderr, "Calling dealloc slot", pto.c_tp_dealloc, "of", obj, \
# "'s type which is", rffi.charp2str(pto.c_tp_name)
+ rawrefcount.mark_deallocating(w_marker_deallocating, obj)
generic_cpy_call(space, pto.c_tp_dealloc, obj)
@cpython_api([rffi.VOIDP], lltype.Signed, error=CANNOT_FAIL)
diff --git a/rpython/memory/gc/incminimark.py b/rpython/memory/gc/incminimark.py
--- a/rpython/memory/gc/incminimark.py
+++ b/rpython/memory/gc/incminimark.py
@@ -2936,6 +2936,12 @@
self._pyobj(pyobject).ob_pypy_link = objint
# there is no rrc_o_dict
+ def rawrefcount_mark_deallocating(self, gcobj, pyobject):
+ ll_assert(self.rrc_enabled, "rawrefcount.init not called")
+ obj = llmemory.cast_ptr_to_adr(gcobj) # should be a prebuilt obj
+ objint = llmemory.cast_adr_to_int(obj, "symbolic")
+ self._pyobj(pyobject).ob_pypy_link = objint
+
def rawrefcount_from_obj(self, gcobj):
obj = llmemory.cast_ptr_to_adr(gcobj)
if self.is_in_nursery(obj):
diff --git a/rpython/memory/gctransform/framework.py b/rpython/memory/gctransform/framework.py
--- a/rpython/memory/gctransform/framework.py
+++ b/rpython/memory/gctransform/framework.py
@@ -476,6 +476,10 @@
GCClass.rawrefcount_create_link_pyobj,
[s_gc, s_gcref, SomeAddress()],
annmodel.s_None)
+ self.rawrefcount_mark_deallocating = getfn(
+ GCClass.rawrefcount_mark_deallocating,
+ [s_gc, s_gcref, SomeAddress()],
+ annmodel.s_None)
self.rawrefcount_from_obj_ptr = getfn(
GCClass.rawrefcount_from_obj, [s_gc, s_gcref], SomeAddress(),
inline = True)
@@ -1281,6 +1285,14 @@
[self.rawrefcount_create_link_pyobj_ptr, self.c_const_gc,
v_gcobj, v_pyobject])
+ def gct_gc_rawrefcount_mark_deallocating(self, hop):
+ [v_gcobj, v_pyobject] = hop.spaceop.args
+ assert v_gcobj.concretetype == llmemory.GCREF
+ assert v_pyobject.concretetype == llmemory.Address
+ hop.genop("direct_call",
+ [self.rawrefcount_mark_deallocating, self.c_const_gc,
+ v_gcobj, v_pyobject])
+
def gct_gc_rawrefcount_from_obj(self, hop):
[v_gcobj] = hop.spaceop.args
assert v_gcobj.concretetype == llmemory.GCREF
diff --git a/rpython/rlib/rawrefcount.py b/rpython/rlib/rawrefcount.py
--- a/rpython/rlib/rawrefcount.py
+++ b/rpython/rlib/rawrefcount.py
@@ -36,6 +36,7 @@
_pypy2ob = {}
_pypy2ob_rev = {}
_d_list = []
+ _d_marker = None
_dealloc_trigger_callback = dealloc_trigger_callback
@not_rpython
@@ -62,6 +63,14 @@
_o_list.append(ob)
@not_rpython
+def mark_deallocating(marker, ob):
+ """mark the PyObject as deallocating, by storing 'marker'
+ inside its ob_pypy_link field"""
+ assert ob._obj not in _pypy2ob_rev
+ assert not ob.c_ob_pypy_link
+ ob.c_ob_pypy_link = _build_pypy_link(marker)
+
+ at not_rpython
def from_obj(OB_PTR_TYPE, p):
ob = _pypy2ob.get(p)
if ob is None:
@@ -221,7 +230,7 @@
class Entry(ExtRegistryEntry):
- _about_ = (create_link_pypy, create_link_pyobj)
+ _about_ = (create_link_pypy, create_link_pyobj, mark_deallocating)
def compute_result_annotation(self, s_p, s_ob):
pass
@@ -231,6 +240,8 @@
name = 'gc_rawrefcount_create_link_pypy'
elif self.instance is create_link_pyobj:
name = 'gc_rawrefcount_create_link_pyobj'
+ elif self.instance is mark_deallocating:
+ name = 'gc_rawrefcount_mark_deallocating'
v_p, v_ob = hop.inputargs(*hop.args_r)
hop.exception_cannot_occur()
hop.genop(name, [_unspec_p(hop, v_p), _unspec_ob(hop, v_ob)])
diff --git a/rpython/rlib/test/test_rawrefcount.py b/rpython/rlib/test/test_rawrefcount.py
--- a/rpython/rlib/test/test_rawrefcount.py
+++ b/rpython/rlib/test/test_rawrefcount.py
@@ -212,6 +212,15 @@
assert rawrefcount.to_obj(W_Root, ob) == p
lltype.free(ob, flavor='raw')
+ def test_mark_deallocating(self):
+ ob = lltype.malloc(PyObjectS, flavor='raw', zero=True)
+ w_marker = W_Root(42)
+ rawrefcount.mark_deallocating(w_marker, ob)
+ assert rawrefcount.to_obj(W_Root, ob) is w_marker
+ rawrefcount._collect()
+ assert rawrefcount.to_obj(W_Root, ob) is w_marker
+ lltype.free(ob, flavor='raw')
+
class TestTranslated(StandaloneTests):
@@ -222,6 +231,7 @@
state.seen = []
def dealloc_trigger():
state.seen.append(1)
+ w_marker = W_Root(-1)
def make_p():
p = W_Root(42)
@@ -257,6 +267,13 @@
if rawrefcount.next_dead(PyObject) != lltype.nullptr(PyObjectS):
print "NEXT_DEAD second time != NULL"
return 1
+ if rawrefcount.to_obj(W_Root, ob) is not None:
+ print "to_obj(dead) is not None?"
+ return 1
+ rawrefcount.mark_deallocating(w_marker, ob)
+ if rawrefcount.to_obj(W_Root, ob) is not w_marker:
+ print "to_obj(marked-dead) is not w_marker"
+ return 1
print "OK!"
lltype.free(ob, flavor='raw')
return 0
diff --git a/rpython/rtyper/lltypesystem/lloperation.py b/rpython/rtyper/lltypesystem/lloperation.py
--- a/rpython/rtyper/lltypesystem/lloperation.py
+++ b/rpython/rtyper/lltypesystem/lloperation.py
@@ -490,6 +490,7 @@
'gc_rawrefcount_init': LLOp(),
'gc_rawrefcount_create_link_pypy': LLOp(),
'gc_rawrefcount_create_link_pyobj': LLOp(),
+ 'gc_rawrefcount_mark_deallocating': LLOp(),
'gc_rawrefcount_from_obj': LLOp(sideeffects=False),
'gc_rawrefcount_to_obj': LLOp(sideeffects=False),
More information about the pypy-commit
mailing list