Skip to content

Commit

Permalink
gh-104584: Fix error handling from backedge optimization (#106484)
Browse files Browse the repository at this point in the history
When `_PyOptimizer_BackEdge` returns `NULL`, we should restore `next_instr` (and `stack_pointer`). To accomplish this we should jump to `resume_with_error` instead of just `error`.

The problem this causes is subtle -- the only repro I have is in PR gh-106393, at commit d7df54b. But the fix is real (as shown later in that PR).

While we're at it, also improve the debug output: the offsets at which traces are identified are now measured in bytes, and always show the start offset. This makes it easier to correlate executor calls with optimizer calls, and either with `dis` output.

<!-- gh-issue-number: gh-104584 -->
* Issue: gh-104584
<!-- /gh-issue-number -->
  • Loading branch information
gvanrossum authored Jul 6, 2023
1 parent 56353b1 commit 003ba71
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 11 deletions.
2 changes: 1 addition & 1 deletion Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2234,7 +2234,7 @@ dummy_func(
frame = _PyOptimizer_BackEdge(frame, here, next_instr, stack_pointer);
if (frame == NULL) {
frame = cframe.current_frame;
goto error;
goto resume_with_error;
}
assert(frame == cframe.current_frame);
here[1].cache &= ((1 << OPTIMIZER_BITS_IN_COUNTER) -1);
Expand Down
4 changes: 2 additions & 2 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -2737,11 +2737,11 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
#endif

DPRINTF(3,
"Entering _PyUopExecute for %s (%s:%d) at offset %ld\n",
"Entering _PyUopExecute for %s (%s:%d) at byte offset %ld\n",
PyUnicode_AsUTF8(_PyFrame_GetCode(frame)->co_qualname),
PyUnicode_AsUTF8(_PyFrame_GetCode(frame)->co_filename),
_PyFrame_GetCode(frame)->co_firstlineno,
(long)(frame->prev_instr + 1 -
2 * (long)(frame->prev_instr + 1 -
(_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive));

PyThreadState *tstate = _PyThreadState_GET();
Expand Down
2 changes: 1 addition & 1 deletion Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 11 additions & 7 deletions Python/optimizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ _PyOptimizer_BackEdge(_PyInterpreterFrame *frame, _Py_CODEUNIT *src, _Py_CODEUNI
}
insert_executor(code, src, index, executor);
assert(frame->prev_instr == src);
frame->prev_instr = dest - 1;
return executor->execute(executor, frame, stack_pointer);
jump_to_destination:
frame->prev_instr = dest - 1;
Expand All @@ -201,7 +202,7 @@ PyUnstable_GetExecutor(PyCodeObject *code, int offset)
}
i += _PyInstruction_GetLength(code, i);
}
PyErr_SetString(PyExc_ValueError, "no executor at given offset");
PyErr_SetString(PyExc_ValueError, "no executor at given byte offset");
return NULL;
}

Expand Down Expand Up @@ -373,6 +374,9 @@ translate_bytecode_to_trace(
_PyUOpInstruction *trace,
int max_length)
{
#ifdef Py_DEBUG
_Py_CODEUNIT *initial_instr = instr;
#endif
int trace_length = 0;

#ifdef Py_DEBUG
Expand All @@ -398,11 +402,11 @@ translate_bytecode_to_trace(
trace_length++;

DPRINTF(4,
"Optimizing %s (%s:%d) at offset %ld\n",
"Optimizing %s (%s:%d) at byte offset %ld\n",
PyUnicode_AsUTF8(code->co_qualname),
PyUnicode_AsUTF8(code->co_filename),
code->co_firstlineno,
(long)(instr - (_Py_CODEUNIT *)code->co_code_adaptive));
2 * (long)(initial_instr - (_Py_CODEUNIT *)code->co_code_adaptive));

for (;;) {
ADD_TO_TRACE(SAVE_IP, (int)(instr - (_Py_CODEUNIT *)code->co_code_adaptive));
Expand Down Expand Up @@ -492,21 +496,21 @@ translate_bytecode_to_trace(
if (trace_length > 3) {
ADD_TO_TRACE(EXIT_TRACE, 0);
DPRINTF(1,
"Created a trace for %s (%s:%d) at offset %ld -- length %d\n",
"Created a trace for %s (%s:%d) at byte offset %ld -- length %d\n",
PyUnicode_AsUTF8(code->co_qualname),
PyUnicode_AsUTF8(code->co_filename),
code->co_firstlineno,
(long)(instr - (_Py_CODEUNIT *)code->co_code_adaptive),
2 * (long)(initial_instr - (_Py_CODEUNIT *)code->co_code_adaptive),
trace_length);
return trace_length;
}
else {
DPRINTF(4,
"No trace for %s (%s:%d) at offset %ld\n",
"No trace for %s (%s:%d) at byte offset %ld\n",
PyUnicode_AsUTF8(code->co_qualname),
PyUnicode_AsUTF8(code->co_filename),
code->co_firstlineno,
(long)(instr - (_Py_CODEUNIT *)code->co_code_adaptive));
2 * (long)(initial_instr - (_Py_CODEUNIT *)code->co_code_adaptive));
}
return 0;

Expand Down

0 comments on commit 003ba71

Please sign in to comment.