From 003ba71dcbe94f0d5cb1d0c56d7f1d5a6dae56f7 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Thu, 6 Jul 2023 11:39:53 -0700 Subject: gh-104584: Fix error handling from backedge optimization (#106484) 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 d7df54b139bcc47f5ea094bfaa9824f79bc45adc. 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. * Issue: gh-104584 --- Python/bytecodes.c | 2 +- Python/ceval.c | 4 ++-- Python/generated_cases.c.h | 2 +- Python/optimizer.c | 18 +++++++++++------- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f69ac2b..70b5239 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -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); diff --git a/Python/ceval.c b/Python/ceval.c index 6714229..0ee95bc 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -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(); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index eb24229..6000ab2 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -3193,7 +3193,7 @@ 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); diff --git a/Python/optimizer.c b/Python/optimizer.c index c3ab649..2c1be61 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -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; @@ -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; } @@ -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 @@ -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)); @@ -492,21 +496,21 @@ done: 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; -- cgit v0.12