Description
Most of our quickened instructions start by asserting that tracing isn't enabled, and end by calling NOTRACE_DISPATCH()
, which skips tracing checks. This is safe for many of our specializations (like those for BINARY_OP
and COMPARE_OP
, which operate on known safe types), but not all of them. The problem is that any time we decref an unknown object, arbitrary finalizers could run and enable tracing.
It seems that our current practice is predicated on the idea that it's okay for tracing to start "sometime in the future", since we make no guarantees about when finalizers run. There are two issues with this:
- Trace functions may temporarily trace some frames, but not others.
- As mentioned, the interpreter loop is littered with asserts that will fail if this happens.
Here's a sort-of-minimal reproducer of what this can look like on debug builds:
Python 3.11.0rc1+ (heads/3.11:bb0dab5c48, Sep 6 2022, 22:29:10) [GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class C:
... def __init__(self, x):
... self.x = x
... def __del__(self):
... if self.x:
... breakpoint()
...
>>> def f(x):
... C(x).x, C
...
>>> for _ in range(8):
... f(False)
...
>>> f(True)
--Return--
> <stdin>(6)__del__()->None
(Pdb) next
python: Python/ceval.c:3076: _PyEval_EvalFrameDefault: Assertion `cframe.use_tracing == 0' failed.
Aborted
In this case LOAD_ATTR_INSTANCE_VALUE
's NOTRACE_DISPATCH()
invalidates LOAD_GLOBAL_MODULE
's assert(cframe.use_tracing == 0)
.
One option is to just rip out the asserts, which solves the more serious issue of crashing debug builds. However, I think that the best solution may be to stop using NOTRACE_DISPATCH()
altogether. It's really hard to get right, and only saves us a single memory load and bitwise |
. I benchmarked a branch that uses DISPATCH()
for all instructions, and the result was "1.00x slower" than main
. @markshannon tried the same thing, and got "1.01x slower". So going this route isn't free, but also not a huge penalty for correctness and peace-of-mind.
Flagging as a release blocker, since 3.11 is affected.
Metadata
Metadata
Assignees
Labels
Projects
Status
Done