GH-104584: Fix ENTER_EXECUTOR (GH-106141)

* Check eval-breaker in ENTER_EXECUTOR.

* Make sure that frame->prev_instr is set before entering executor.

Author: markshannon

Include/internal/pycore_ceval.h

@@ -152,6 +152,8 @@ extern struct _PyInterpreterFrame* _PyEval_GetFrame(void);
 
 extern PyObject* _Py_MakeCoro(PyFunctionObject *func);
 
+/* Handle signals, pending calls, GIL drop request
+   and asynchronous exception */
 extern int _Py_HandlePending(PyThreadState *tstate);
 
 

Python/bytecodes.c

@@ -141,8 +141,8 @@ dummy_func(
                 ERROR_IF(err, error);
                 next_instr--;
             }
-            else if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker) && oparg < 2) {
-                goto handle_eval_breaker;
+            else if (oparg < 2) {
+                CHECK_EVAL_BREAKER();
             }
         }
 
@@ -158,6 +158,9 @@ dummy_func(
                 next_instr--;
             }
             else {
+                if (oparg < 2) {
+                    CHECK_EVAL_BREAKER();
+                }
                 _PyFrame_SetStackPointer(frame, stack_pointer);
                 int err = _Py_call_instrumentation(
                         tstate, oparg > 0, frame, next_instr-1);
@@ -168,9 +171,6 @@ dummy_func(
                     next_instr = frame->prev_instr;
                     DISPATCH();
                 }
-                if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker) && oparg < 2) {
-                    goto handle_eval_breaker;
-                }
             }
         }
 
@@ -2223,6 +2223,7 @@ dummy_func(
         }
 
         inst(JUMP_BACKWARD, (--)) {
+            CHECK_EVAL_BREAKER();
             _Py_CODEUNIT *here = next_instr - 1;
             assert(oparg <= INSTR_OFFSET());
             JUMPBY(1-oparg);
@@ -2240,7 +2241,6 @@ dummy_func(
                 goto resume_frame;
             }
             #endif  /* ENABLE_SPECIALIZATION */
-            CHECK_EVAL_BREAKER();
         }
 
         pseudo(JUMP) = {
@@ -2254,8 +2254,13 @@ dummy_func(
         };
 
         inst(ENTER_EXECUTOR, (--)) {
+            CHECK_EVAL_BREAKER();
+
             PyCodeObject *code = _PyFrame_GetCode(frame);
             _PyExecutorObject *executor = (_PyExecutorObject *)code->co_executors->executors[oparg&255];
+            int original_oparg = executor->vm_data.oparg | (oparg & 0xfffff00);
+            JUMPBY(1-original_oparg);
+            frame->prev_instr = next_instr - 1;
             Py_INCREF(executor);
             frame = executor->execute(executor, frame, stack_pointer);
             if (frame == NULL) {
@@ -3570,8 +3575,8 @@ dummy_func(
         }
 
         inst(INSTRUMENTED_JUMP_BACKWARD, ( -- )) {
-            INSTRUMENTED_JUMP(next_instr-1, next_instr+1-oparg, PY_MONITORING_EVENT_JUMP);
             CHECK_EVAL_BREAKER();
+            INSTRUMENTED_JUMP(next_instr-1, next_instr+1-oparg, PY_MONITORING_EVENT_JUMP);
         }
 
         inst(INSTRUMENTED_POP_JUMP_IF_TRUE, ( -- )) {

Python/ceval.c

@@ -763,68 +763,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 
     DISPATCH();
 
-handle_eval_breaker:
-
-    /* Do periodic things, like check for signals and async I/0.
-     * We need to do reasonably frequently, but not too frequently.
-     * All loops should include a check of the eval breaker.
-     * We also check on return from any builtin function.
-     *
-     * ## More Details ###
-     *
-     * The eval loop (this function) normally executes the instructions
-     * of a code object sequentially.  However, the runtime supports a
-     * number of out-of-band execution scenarios that may pause that
-     * sequential execution long enough to do that out-of-band work
-     * in the current thread using the current PyThreadState.
-     *
-     * The scenarios include:
-     *
-     *  - cyclic garbage collection
-     *  - GIL drop requests
-     *  - "async" exceptions
-     *  - "pending calls"  (some only in the main thread)
-     *  - signal handling (only in the main thread)
-     *
-     * When the need for one of the above is detected, the eval loop
-     * pauses long enough to handle the detected case.  Then, if doing
-     * so didn't trigger an exception, the eval loop resumes executing
-     * the sequential instructions.
-     *
-     * To make this work, the eval loop periodically checks if any
-     * of the above needs to happen.  The individual checks can be
-     * expensive if computed each time, so a while back we switched
-     * to using pre-computed, per-interpreter variables for the checks,
-     * and later consolidated that to a single "eval breaker" variable
-     * (now a PyInterpreterState field).
-     *
-     * For the longest time, the eval breaker check would happen
-     * frequently, every 5 or so times through the loop, regardless
-     * of what instruction ran last or what would run next.  Then, in
-     * early 2021 (gh-18334, commit 4958f5d), we switched to checking
-     * the eval breaker less frequently, by hard-coding the check to
-     * specific places in the eval loop (e.g. certain instructions).
-     * The intent then was to check after returning from calls
-     * and on the back edges of loops.
-     *
-     * In addition to being more efficient, that approach keeps
-     * the eval loop from running arbitrary code between instructions
-     * that don't handle that well.  (See gh-74174.)
-     *
-     * Currently, the eval breaker check happens here at the
-     * "handle_eval_breaker" label.  Some instructions come here
-     * explicitly (goto) and some indirectly.  Notably, the check
-     * happens on back edges in the control flow graph, which
-     * pretty much applies to all loops and most calls.
-     * (See bytecodes.c for exact information.)
-     *
-     * One consequence of this approach is that it might not be obvious
-     * how to force any specific thread to pick up the eval breaker,
-     * or for any specific thread to not pick it up.  Mostly this
-     * involves judicious uses of locks and careful ordering of code,
-     * while avoiding code that might trigger the eval breaker
-     * until so desired.
-     */
     if (_Py_HandlePending(tstate) != 0) {
         goto error;
     }
@@ -2796,13 +2734,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
     PyThreadState *tstate = _PyThreadState_GET();
     _PyUOpExecutorObject *self = (_PyUOpExecutorObject *)executor;
 
-    // Equivalent to CHECK_EVAL_BREAKER()
-    _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY();
-    if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker)) {
-        if (_Py_HandlePending(tstate) != 0) {
-            goto error;
-        }
-    }
+    CHECK_EVAL_BREAKER();
 
     OBJECT_STAT_INC(optimization_traces_executed);
     _Py_CODEUNIT *ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;

Python/ceval_gil.c

@@ -1052,8 +1052,65 @@ _PyEval_FiniState(struct _ceval_state *ceval)
     }
 }
 
-/* Handle signals, pending calls, GIL drop request
-   and asynchronous exception */
+
+/* Do periodic things, like check for signals and async I/0.
+* We need to do reasonably frequently, but not too frequently.
+* All loops should include a check of the eval breaker.
+* We also check on return from any builtin function.
+*
+* ## More Details ###
+*
+* The eval loop (this function) normally executes the instructions
+* of a code object sequentially.  However, the runtime supports a
+* number of out-of-band execution scenarios that may pause that
+* sequential execution long enough to do that out-of-band work
+* in the current thread using the current PyThreadState.
+*
+* The scenarios include:
+*
+*  - cyclic garbage collection
+*  - GIL drop requests
+*  - "async" exceptions
+*  - "pending calls"  (some only in the main thread)
+*  - signal handling (only in the main thread)
+*
+* When the need for one of the above is detected, the eval loop
+* pauses long enough to handle the detected case.  Then, if doing
+* so didn't trigger an exception, the eval loop resumes executing
+* the sequential instructions.
+*
+* To make this work, the eval loop periodically checks if any
+* of the above needs to happen.  The individual checks can be
+* expensive if computed each time, so a while back we switched
+* to using pre-computed, per-interpreter variables for the checks,
+* and later consolidated that to a single "eval breaker" variable
+* (now a PyInterpreterState field).
+*
+* For the longest time, the eval breaker check would happen
+* frequently, every 5 or so times through the loop, regardless
+* of what instruction ran last or what would run next.  Then, in
+* early 2021 (gh-18334, commit 4958f5d), we switched to checking
+* the eval breaker less frequently, by hard-coding the check to
+* specific places in the eval loop (e.g. certain instructions).
+* The intent then was to check after returning from calls
+* and on the back edges of loops.
+*
+* In addition to being more efficient, that approach keeps
+* the eval loop from running arbitrary code between instructions
+* that don't handle that well.  (See gh-74174.)
+*
+* Currently, the eval breaker check happens on back edges in
+* the control flow graph, which pretty much applies to all loops,
+* and most calls.
+* (See bytecodes.c for exact information.)
+*
+* One consequence of this approach is that it might not be obvious
+* how to force any specific thread to pick up the eval breaker,
+* or for any specific thread to not pick it up.  Mostly this
+* involves judicious uses of locks and careful ordering of code,
+* while avoiding code that might trigger the eval breaker
+* until so desired.
+*/
 int
 _Py_HandlePending(PyThreadState *tstate)
 {

Python/ceval_macros.h

@@ -117,7 +117,9 @@
 #define CHECK_EVAL_BREAKER() \
     _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY(); \
     if (_Py_atomic_load_relaxed_int32(&tstate->interp->ceval.eval_breaker)) { \
-        goto handle_eval_breaker; \
+        if (_Py_HandlePending(tstate) != 0) { \
+            goto error; \
+        } \
     }