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.

<!-- gh-issue-number: gh-104584 -->
* Issue: gh-104584
<!-- /gh-issue-number -->

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index f69ac2beef4c209510a0cef08641633c925a0332..70b52391e6bdaafc8f714bf3b898c0598d34040d 100644 (file)
--- 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 6714229fd0784648e176e0e5efd847dd6a47d527..0ee95bc3a3a4bb838cbad20fd7a188202de45115 100644 (file)
--- 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 eb2422943984b1b1adda04a79b367bd02c4a877e..6000ab2da1133038f04a68e6438685b4387e8eff 100644 (file)
--- 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 c3ab649b51b0ebc4a86dfb8d886fe662b63827dc..2c1be61f8d614e86ef6595e1db51484f6575c4a5 100644 (file)
--- 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;