Skip to content

gh-148219: Fix JIT+pystats crash: is_terminator fails for base opcodes#148220

Open
eendebakpt wants to merge 1 commit intopython:mainfrom
eendebakpt:fix-pystats-jit-terminator
Open

gh-148219: Fix JIT+pystats crash: is_terminator fails for base opcodes#148220
eendebakpt wants to merge 1 commit intopython:mainfrom
eendebakpt:fix-pystats-jit-terminator

Conversation

@eendebakpt
Copy link
Copy Markdown
Contributor

@eendebakpt eendebakpt commented Apr 7, 2026

Two bugs when building with --enable-experimental-jit --enable-pystats:

  1. is_terminator() uses _PyUop_Uncached[opcode] to map replicated variants back to base opcodes, but the array only contains entries for replicated variants (e.g. _JUMP_TO_TOP_r00). For base opcodes like _JUMP_TO_TOP itself, it returns 0, so the terminator is not recognized.

  2. jit.c references jit_got_size in OPT_STAT_ADD but the field is missing from OptimizationStats in pystats.h, causing a build failure.

Two bugs when building with --enable-experimental-jit --enable-pystats:

1. `is_terminator()` uses `_PyUop_Uncached[opcode]` to map replicated
   variants back to base opcodes, but the array only contains entries
   for replicated variants (e.g. `_JUMP_TO_TOP_r00`). For base opcodes
   like `_JUMP_TO_TOP` itself, it returns 0, so the terminator is not
   recognized. This causes `effective_trace_length()` to hit
   `Py_FatalError("No terminating instruction")`.

   Fix: fall back to the raw opcode when `_PyUop_Uncached` returns 0.

2. `jit.c` references `jit_got_size` in `OPT_STAT_ADD` but the field
   is missing from `OptimizationStats` in `pystats.h`, causing a build
   failure.

   Fix: add the missing `jit_got_size` field.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
uint64_t jit_code_size;
uint64_t jit_trampoline_size;
uint64_t jit_data_size;
uint64_t jit_got_size;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, thanks! :)

is_terminator(const _PyUOpInstruction *uop)
{
int opcode = _PyUop_Uncached[uop->opcode];
if (opcode == 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should remove the line int opcode = _PyUop_Uncached[uop->opcode]; as the only time we should see a TOS caching uop is in the sanity_check function.

Maybe move that line there, and just assert(opcode <= MAX_UOP_ID) here?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants