GH-117442: Check eval-breaker at start (rather than end) of tier 2 loops (GH-118482)

5 files changed, 30 insertions, 61 deletions

diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 002b5a3..b1be40d 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2410,20 +2410,21 @@ dummy_func(
 
         tier1 inst(ENTER_EXECUTOR, (--)) {
             #ifdef _Py_TIER2
-            int prevoparg = oparg;
-            CHECK_EVAL_BREAKER();
-            if (this_instr->op.code != ENTER_EXECUTOR ||
-                this_instr->op.arg != prevoparg) {
-                next_instr = this_instr;
-                DISPATCH();
-            }
-
             PyCodeObject *code = _PyFrame_GetCode(frame);
             _PyExecutorObject *executor = code->co_executors->executors[oparg & 255];
             assert(executor->vm_data.index == INSTR_OFFSET() - 1);
             assert(executor->vm_data.code == code);
             assert(executor->vm_data.valid);
             assert(tstate->previous_executor == NULL);
+            /* If the eval breaker is set then stay in tier 1.
+             * This avoids any potentially infinite loops
+             * involving _RESUME_CHECK */
+            if (_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & _PY_EVAL_EVENTS_MASK) {
+                opcode = executor->vm_data.opcode;
+                oparg = (oparg & ~255) | executor->vm_data.oparg;
+                next_instr = this_instr;
+                DISPATCH_GOTO();
+            }
             tstate->previous_executor = Py_None;
             Py_INCREF(executor);
             GOTO_TIER_TWO(executor);
@@ -4111,7 +4112,6 @@ dummy_func(
 #ifndef _Py_JIT
             next_uop = &current_executor->trace[1];
 #endif
-            CHECK_EVAL_BREAKER();
         }
 
         tier2 op(_SET_IP, (instr_ptr/4 --)) {
@@ -4277,30 +4277,18 @@ dummy_func(
             GOTO_UNWIND();
         }
 
-
-        /* Special version of RESUME_CHECK that (when paired with _EVAL_BREAKER_EXIT)
-         * is safe for tier 2. Progress is guaranteed because _EVAL_BREAKER_EXIT calls
-         * _Py_HandlePending which clears the eval_breaker so that _TIER2_RESUME_CHECK
-         * will not exit if it is immediately executed again. */
+        /* Progress is guaranteed if we DEOPT on the eval breaker, because
+         * ENTER_EXECUTOR will not re-enter tier 2 with the eval breaker set. */
         tier2 op(_TIER2_RESUME_CHECK, (--)) {
 #if defined(__EMSCRIPTEN__)
-            EXIT_IF(_Py_emscripten_signal_clock == 0);
+            DEOPT_IF(_Py_emscripten_signal_clock == 0);
             _Py_emscripten_signal_clock -= Py_EMSCRIPTEN_SIGNAL_HANDLING;
 #endif
             uintptr_t eval_breaker = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker);
-            EXIT_IF(eval_breaker & _PY_EVAL_EVENTS_MASK);
+            DEOPT_IF(eval_breaker & _PY_EVAL_EVENTS_MASK);
             assert(eval_breaker == FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(_PyFrame_GetCode(frame)->_co_instrumentation_version));
         }
 
-        tier2 op(_EVAL_BREAKER_EXIT, (--)) {
-            _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY();
-            QSBR_QUIESCENT_STATE(tstate);
-            if (_Py_HandlePending(tstate) != 0) {
-                GOTO_UNWIND();
-            }
-            EXIT_TO_TRACE();
-        }
-
 // END BYTECODES //
 
     }
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index b17f376..f97f279 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -4094,7 +4094,6 @@
             #ifndef _Py_JIT
             next_uop = &current_executor->trace[1];
             #endif
-            CHECK_EVAL_BREAKER();
             break;
         }
 
@@ -4353,14 +4352,4 @@
             break;
         }
 
-        case _EVAL_BREAKER_EXIT: {
-            _Py_CHECK_EMSCRIPTEN_SIGNALS_PERIODICALLY();
-            QSBR_QUIESCENT_STATE(tstate);
-            if (_Py_HandlePending(tstate) != 0) {
-                GOTO_UNWIND();
-            }
-            EXIT_TO_TRACE();
-            break;
-        }
-
 #undef TIER_TWO
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 32b485e..2a0f268 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -2493,19 +2493,21 @@
             next_instr += 1;
             INSTRUCTION_STATS(ENTER_EXECUTOR);
             #ifdef _Py_TIER2
-            int prevoparg = oparg;
-            CHECK_EVAL_BREAKER();
-            if (this_instr->op.code != ENTER_EXECUTOR ||
-                this_instr->op.arg != prevoparg) {
-                next_instr = this_instr;
-                DISPATCH();
-            }
             PyCodeObject *code = _PyFrame_GetCode(frame);
             _PyExecutorObject *executor = code->co_executors->executors[oparg & 255];
             assert(executor->vm_data.index == INSTR_OFFSET() - 1);
             assert(executor->vm_data.code == code);
             assert(executor->vm_data.valid);
             assert(tstate->previous_executor == NULL);
+            /* If the eval breaker is set then stay in tier 1.
+             * This avoids any potentially infinite loops
+             * involving _RESUME_CHECK */
+            if (_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & _PY_EVAL_EVENTS_MASK) {
+                opcode = executor->vm_data.opcode;
+                oparg = (oparg & ~255) | executor->vm_data.oparg;
+                next_instr = this_instr;
+                DISPATCH_GOTO();
+            }
             tstate->previous_executor = Py_None;
             Py_INCREF(executor);
             GOTO_TIER_TWO(executor);
diff --git a/Python/optimizer.c b/Python/optimizer.c
index 2389338..0509473 100644
--- a/Python/optimizer.c
+++ b/Python/optimizer.c
@@ -627,6 +627,9 @@ top:  // Jump here after _PUSH_FRAME or likely branches
             if (opcode == JUMP_BACKWARD || opcode == JUMP_BACKWARD_NO_INTERRUPT) {
                 instr += 1 + _PyOpcode_Caches[opcode] - (int32_t)oparg;
                 initial_instr = instr;
+                if (opcode == JUMP_BACKWARD) {
+                    ADD_TO_TRACE(_TIER2_RESUME_CHECK, 0, 0, target);
+                }
                 continue;
             }
             else {
@@ -976,6 +979,7 @@ prepare_for_execution(_PyUOpInstruction *buffer, int length)
     int32_t current_error = -1;
     int32_t current_error_target = -1;
     int32_t current_popped = -1;
+    int32_t current_exit_op = -1;
     /* Leaving in NOPs slows down the interpreter and messes up the stats */
     _PyUOpInstruction *copy_to = &buffer[0];
     for (int i = 0; i < length; i++) {
@@ -994,20 +998,11 @@ prepare_for_execution(_PyUOpInstruction *buffer, int length)
         int opcode = inst->opcode;
         int32_t target = (int32_t)uop_get_target(inst);
         if (_PyUop_Flags[opcode] & (HAS_EXIT_FLAG | HAS_DEOPT_FLAG)) {
-            if (target != current_jump_target) {
-                uint16_t exit_op;
-                if (_PyUop_Flags[opcode] & HAS_EXIT_FLAG) {
-                    if (opcode == _TIER2_RESUME_CHECK) {
-                        exit_op = _EVAL_BREAKER_EXIT;
-                    }
-                    else {
-                        exit_op = _SIDE_EXIT;
-                    }
-                }
-                else {
-                    exit_op = _DEOPT;
-                }
+            uint16_t exit_op = (_PyUop_Flags[opcode] & HAS_EXIT_FLAG) ?
+                _SIDE_EXIT : _DEOPT;
+            if (target != current_jump_target || current_exit_op != exit_op) {
                 make_exit(&buffer[next_spare], exit_op, target);
+                current_exit_op = exit_op;
                 current_jump_target = target;
                 current_jump = next_spare;
                 next_spare++;
@@ -1114,7 +1109,6 @@ sanity_check(_PyExecutorObject *executor)
         CHECK(
             opcode == _DEOPT ||
             opcode == _SIDE_EXIT ||
-            opcode == _EVAL_BREAKER_EXIT ||
             opcode == _ERROR_POP_N);
         if (opcode == _SIDE_EXIT) {
             CHECK(inst->format == UOP_FORMAT_EXIT);
diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h
index 0b69692..e680d76 100644
--- a/Python/optimizer_cases.c.h
+++ b/Python/optimizer_cases.c.h
@@ -2153,7 +2153,3 @@
             break;
         }
 
-        case _EVAL_BREAKER_EXIT: {
-            break;
-        }
-