GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — JIT error stubs don't account for peeks · Issue #126222 · python/cpython

JIT error stubs don't account for peeks #126222

New issue
Copy link
New issue
Copy link
Closed
Closed
JIT error stubs don't account for peeks#126222
Copy link
Assignees
markshannon
Labels
3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-JITtype-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump

Description

@brandtbucher
brandtbucher
opened on Oct 31, 2024
Issue body actions

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

  • GH-126222: Fix _PyUop_num_popped #126507

Activity

  1. brandtbucher
    added
    interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)
    type-crashA hard crash of the interpreter, possibly with a core dumpA hard crash of the interpreter, possibly with a core dump
    3.13only security fixesonly security fixes
    3.14bugs and security fixesbugs and security fixes
    topic-JIT
    on Oct 31, 2024
  2. brandtbucher
    assigned
    markshannon
    on Nov 4, 2024

  3. markshannon commented on Nov 6, 2024

    @markshannon
    markshannon
    on Nov 6, 2024
    Member
    More actions

    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

  4. markshannon
    added a commit that references this issue on Nov 7, 2024

    GH-126222: Fix `_PyUop_num_popped` (GH-126507)

    85036c8
  5. brandtbucher
    closed this as completedon Nov 7, 2024
  6. efimov-mikhail
    added a commit that references this issue on Nov 7, 2024

    pythongh-125038: generated file is updated, see python#126222

    6065d84
  7. picnixz
    added a commit that references this issue on Dec 8, 2024

    pythonGH-126222: Fix `_PyUop_num_popped` (pythonGH-126507)

    4b242ec
  8. ebonnal
    added a commit that references this issue on Jan 12, 2025

    pythonGH-126222: Fix `_PyUop_num_popped` (pythonGH-126507)

    29ad416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

  • @markshannon
    markshannon

Labels

3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-JITtype-crashA hard crash of the interpreter, possibly with a core dumpA 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
    You are visitor 01402503 — GeezerVison does not store the pages you look up.
    Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.