_Py_Dealloc is an escaping call #124483
Description
Crash report
We like to pretend that _Py_Dealloc (and, by extension, Py_DECREF, Py_XDECREF, Py_CLEAR, PyStackRef_CLOSE, PyStackRef_CLEAR, etc...) isn't an escaping call in tier two. However, it's perfectly capable of invalidating the world. For example, the following code crashes when run under the JIT:
import sys
class Crashy:
def __del__(self):
ns = sys._getframe(1).f_locals
if ns["i"] == 999:
ns["i"] = None
def crashy():
for i in range(1000):
n = Crashy()
i + i # Remove guards for i.
n = None # Change i.
i + i # Crash!
crashy()Longer term, we're hoping that removing refcounting operations in tier two will help us out here. Fortunately, the _SET_IP and _CHECK_VALIDITY instructions necessary to fix this issue aren't expensive enough to show up on benchmarks when added back (for now).
Activity
brandtbucher commentedon Nov 9, 2024
Doing this correctly is actually pretty tricky:
PyStackRef_CLOSEshould be an escaping call, but that means that refs need to be closed in the order they occur on the stack, and all input refs must be closed before output refs are created. That's actually quite awkward in practice, since it's common to create outputs before inputs are destroyed._PyTuple_FromStackRefSteal, become escaping only on error. That makes it basically impossible to adjust the stack pointer correctly.After talking it through with @markshannon, one possible approach to making this work could be:
PyStackRef_NULL. That way it doesn't matter what order things are consumed in, and the stack level doesn't need to be adjusted more than once per instruction._PyTuple_FromStackRefSteal(and functions like it) should only steal references on success. That way, they're non-escaping.That should give us a good path forward here. Fewer stack adjustments is also very nice for top-of-stack caching.
markshannon commentedon Jan 9, 2025
Note: the above example doesn't crash anymore as we've increased the thresholds. Increasing 999 to 9999 produces a crash.
markshannon commentedon Jan 9, 2025
Fixing the crash is actually quite straightforward.
All the stuff around keeping the stack consistent is potentially important for the GC, but we don't rely on it at the moment.
GH-124483: Mark `Py_DECREF`, etc. as escaping for the JIT (GH-128678)
brandtbucher commentedon Jan 14, 2025
Leaving this open because of #128678 (comment).