Repository navigation
JIT error stubs don't account for peeks #126222
Description
Crash report
When deciding how many stack items to pop in an _ERROR_POP_N stub, we use _PyUop_num_popped. However, for opcodes with "peeked" items that never get popped, this is incorrect.
For example, consider SET_ADD(2). It has three inputs and two outputs. However, both of the bottom inputs are the same as the outputs, and are never really popped. When compiling an error stub for the JIT, using _PyUop_num_popped will create _ERROR_POP_N(3), which leaks both references and shrinks the stack too far:
# 17 Nones to warm up, then a list to raise TypeError:
items = 17 * [None] + [[]]
{item for item in items}
# Assertion failed: (STACK_LEVEL() >= level), function _PyEval_EvalFrameDefault, file ceval.c, line 966.The compiled trace for the comprehension is:
0 OPTIMIZED: _START_EXECUTOR (0, target=20, operand=0x101644de0)
1 OPTIMIZED: _MAKE_WARM (0, target=0, operand=0)
2 OPTIMIZED: _SET_IP (0, target=20, operand=0x1017b85f8)
3 OPTIMIZED: _CHECK_PERIODIC (0, jump_target=0, operand=0, error_target=13)
4 OPTIMIZED: _CHECK_VALIDITY (0, jump_target=14, operand=0x1017b85f0)
5 OPTIMIZED: _ITER_CHECK_LIST (4, jump_target=15, operand=0)
6 OPTIMIZED: _GUARD_NOT_EXHAUSTED_LIST (4, jump_target=16, operand=0)
7 OPTIMIZED: _ITER_NEXT_LIST (4, target=16, operand=0)
8 OPTIMIZED: _STORE_FAST_0 (0, target=18, operand=0)
9 OPTIMIZED: _LOAD_FAST_0 (0, target=18, operand=0)
10 OPTIMIZED: _SET_IP (0, target=19, operand=0x1017b85f6)
11 OPTIMIZED: _SET_ADD (2, jump_target=0, operand=0, error_target=17)
12 OPTIMIZED: _JUMP_TO_TOP (0, jump_target=1, operand=0)
13 OPTIMIZED: _ERROR_POP_N (0, target=0, operand=0x14)
14 OPTIMIZED: _DEOPT (0, target=16, operand=0)
15 OPTIMIZED: _EXIT_TRACE (0, target=16, operand=0x101644e60)
16 OPTIMIZED: _EXIT_TRACE (0, target=23, operand=0x101644e70)
17 OPTIMIZED: _ERROR_POP_N (3, target=0, operand=0x13)
@markshannon, does this make sense to generate new tables for in the cases generator? Not sure what the best option is.
Activity
markshannon commentedon Nov 6, 2024
I suspect that prior to #124392
_PyUop_num_poppeddid not include the "peeks", but now it does. It should be simple enough to go back to the old behavior.Looking at the code_PyUop_num_popped/_PyUop_num_pushedare used in two places.One to compute the net stack effect, and to for computing
_ERROR_POP_Nopargs.Since the stack effect is
popped - pushed, adding any value to both will have no effect there.The
_PyUop_num_poppedis only used for computing_ERROR_POP_Nopargs.Stack effects are computed using
_PyOpcode_num_poppedand_PyOpcode_num_pushedGH-126222: Fix `_PyUop_num_popped` (GH-126507)
pythongh-125038: generated file is updated, see python#126222
pythonGH-126222: Fix `_PyUop_num_popped` (pythonGH-126507)
pythonGH-126222: Fix `_PyUop_num_popped` (pythonGH-126507)