Visitar URL original
JIT error stubs don't account for peeks · Issue #126222 · python/cpython · GitHub
Skip to content

JIT error stubs don't account for peeks #126222

Description

@brandtbucher

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.

Linked PRs

Activity

  1. added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    type-crashA hard crash of the interpreter, possibly with a core dump
    3.13only security fixes
    3.14bugs and security fixes
    on Oct 31, 2024
  2. markshannon commented on Nov 6, 2024

    @markshannon
    Member

    I suspect that prior to #124392 _PyUop_num_popped did 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_pushed are used in two places.
    One to compute the net stack effect, and to for computing _ERROR_POP_N opargs.
    Since the stack effect is popped - pushed, adding any value to both will have no effect there.

    The _PyUop_num_popped is only used for computing _ERROR_POP_N opargs.
    Stack effects are computed using _PyOpcode_num_popped and _PyOpcode_num_pushed

  3. added a commit that references this issue on Nov 7, 2024
  4. added a commit that references this issue on Nov 7, 2024
  5. added a commit that references this issue on Dec 8, 2024
  6. added a commit that references this issue on Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.13only security fixes3.14bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)topic-JITtype-crashA hard crash of the interpreter, possibly with a core dump

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions