Skip to content

Commit 1ef4a37

Browse files
Fidget-SpinnerFidget-Spinner
committed
Make sure we don't reenter executors when guard exec ip fails
1 parent 897edf5 commit 1ef4a37

File tree

7 files changed

+65
-22
lines changed

7 files changed

+65
-22
lines changed

‎Include/internal/pycore_ceval.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,10 @@ _PyEval_SpecialMethodCanSuggest(PyObject *self, int oparg);
334334
#define _PY_EVAL_PLEASE_STOP_BIT (1U << 5)
335335
#define _PY_EVAL_EXPLICIT_MERGE_BIT (1U << 6)
336336
#define _PY_EVAL_JIT_INVALIDATE_COLD_BIT (1U << 7)
337+
#define _PY_EVAL_JIT_DO_NOT_REENTER (1U << 8)
337338

338339
/* Reserve a few bits for future use */
339-
#define _PY_EVAL_EVENTS_BITS 8
340+
#define _PY_EVAL_EVENTS_BITS 9
340341
#define _PY_EVAL_EVENTS_MASK ((1 << _PY_EVAL_EVENTS_BITS)-1)
341342

342343
static inline void

‎Python/bytecodes.c‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444

4545
#define USE_COMPUTED_GOTOS 0
4646
#include "ceval_macros.h"
47-
#include "../Include/internal/pycore_stackref.h"
4847

4948
/* Flow control macros */
5049

@@ -2988,6 +2987,9 @@ dummy_func(
29882987
if (succ) {
29892988
ENTER_TRACING();
29902989
}
2990+
else {
2991+
this_instr[1].counter = restart_backoff_counter(counter);
2992+
}
29912993
}
29922994
else {
29932995
ADVANCE_ADAPTIVE_COUNTER(this_instr[1].counter);
@@ -3047,7 +3049,8 @@ dummy_func(
30473049
/* If the eval breaker is set then stay in tier 1.
30483050
* This avoids any potentially infinite loops
30493051
* involving _RESUME_CHECK */
3050-
if (_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & _PY_EVAL_EVENTS_MASK) {
3052+
if (_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & (_PY_EVAL_EVENTS_MASK | _PY_EVAL_JIT_DO_NOT_REENTER)) {
3053+
_Py_unset_eval_breaker_bit(tstate, _PY_EVAL_JIT_DO_NOT_REENTER);
30513054
opcode = executor->vm_data.opcode;
30523055
oparg = (oparg & ~255) | executor->vm_data.oparg;
30533056
next_instr = this_instr;
@@ -5302,6 +5305,7 @@ dummy_func(
53025305
tstate->jit_exit = exit;
53035306
assert(exit->is_dynamic);
53045307
_PyExecutorObject *exec = exit->executor;
5308+
assert(exec->trace[0].opcode == _COLD_DYNAMIC_EXIT || exec->trace[0].opcode == _START_DYNAMIC_EXECUTOR);
53055309
TIER2_TO_TIER2(exec);
53065310
}
53075311

@@ -5412,6 +5416,7 @@ dummy_func(
54125416
#ifndef _Py_JIT
54135417
assert(current_executor == (_PyExecutorObject*)executor);
54145418
#endif
5419+
assert(tstate->jit_exit == NULL || tstate->jit_exit->executor == current_executor);
54155420
tstate->current_executor = (PyObject *)executor;
54165421
if (!current_executor->vm_data.valid) {
54175422
assert(tstate->jit_exit->executor == current_executor);
@@ -5470,6 +5475,11 @@ dummy_func(
54705475
PyCodeObject *code = _PyFrame_GetCode(frame);
54715476
executor = code->co_executors->executors[target->op.arg];
54725477
Py_INCREF(executor);
5478+
#if Py_DEBUG
5479+
if (executor->trace[2].opcode == _GUARD_EXECUTOR_IP) {
5480+
assert(executor->trace[2].operand0 == (uint64_t)target);
5481+
}
5482+
#endif
54735483
}
54745484
else {
54755485
if (frame->owner >= FRAME_OWNED_BY_INTERPRETER) {
@@ -5486,10 +5496,10 @@ dummy_func(
54865496
// The invariant in the optimizer is the deopt target always points back to the first EXTENDED_ARG.
54875497
// So setting it to anything else is wrong.
54885498
int succ = _PyJit_TryInitializeTracing(tstate, frame, target, target, target, STACK_LEVEL(), chain_depth, exit, previous_executor, target->op.arg, false);
5499+
exit->temperature = restart_backoff_counter(exit->temperature);
54895500
if (succ) {
54905501
GOTO_TIER_ONE_CONTINUE_TRACING(target);
54915502
}
5492-
exit->temperature = restart_backoff_counter(exit->temperature);
54935503
GOTO_TIER_ONE(target);
54945504
}
54955505
assert(tstate->jit_exit == exit);
@@ -5500,18 +5510,24 @@ dummy_func(
55005510
tier2 op(_COLD_DYNAMIC_EXIT, ( -- )) {
55015511
_PyExitData *exit = tstate->jit_exit;
55025512
assert(exit != NULL);
5513+
assert(exit->is_dynamic);
55035514
_Py_CODEUNIT *target = frame->instr_ptr;
55045515
_Py_BackoffCounter temperature = exit->temperature;
55055516
_PyExecutorObject *executor;
55065517
if (target->op.code == ENTER_EXECUTOR) {
55075518
PyCodeObject *code = _PyFrame_GetCode(frame);
55085519
executor = code->co_executors->executors[target->op.arg];
5509-
if (executor->trace[0].opcode == _START_DYNAMIC_EXECUTOR && executor->vm_data.valid) {
5520+
if (executor->vm_data.valid &&
5521+
executor->trace[0].opcode == _START_DYNAMIC_EXECUTOR) {
5522+
assert(executor->trace[2].operand0 == (uint64_t)target);
55105523
Py_INCREF(executor);
55115524
assert(tstate->jit_exit == exit);
55125525
exit->executor = executor;
55135526
TIER2_TO_TIER2(executor);
55145527
}
5528+
else {
5529+
GOTO_TIER_ONE(target);
5530+
}
55155531
}
55165532
if (frame->owner >= FRAME_OWNED_BY_INTERPRETER) {
55175533
GOTO_TIER_ONE(target);
@@ -5524,15 +5540,18 @@ dummy_func(
55245540
assert(tstate->current_executor == (PyObject *)previous_executor);
55255541
int chain_depth = previous_executor->vm_data.chain_depth + 1;
55265542
int succ = _PyJit_TryInitializeTracing(tstate, frame, target, target, target, STACK_LEVEL(), chain_depth, exit, previous_executor, target->op.arg, true);
5543+
exit->temperature = restart_backoff_counter(exit->temperature);
55275544
if (succ) {
55285545
GOTO_TIER_ONE_CONTINUE_TRACING(target);
55295546
}
5530-
exit->temperature = restart_backoff_counter(exit->temperature);
55315547
GOTO_TIER_ONE(target);
55325548
}
55335549

55345550
tier2 op(_GUARD_EXECUTOR_IP, (ip/4 --)) {
5535-
EXIT_IF(frame->instr_ptr != (_Py_CODEUNIT*)ip);
5551+
if (frame->instr_ptr != (_Py_CODEUNIT*)ip) {
5552+
_Py_set_eval_breaker_bit(tstate, _PY_EVAL_JIT_DO_NOT_REENTER);
5553+
EXIT_IF(true);
5554+
}
55365555
}
55375556

55385557
tier2 op(_GUARD_IP__PUSH_FRAME, (ip/4 --)) {

‎Python/ceval_macros.h‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,9 @@ do { \
401401
next_instr = frame->instr_ptr + 1; \
402402
JUMP_TO_LABEL(error); \
403403
} \
404-
/* No progress made */ \
405-
if (next_instr == this_instr) { \
406-
opcode = executor->vm_data.opcode; \
407-
oparg = (oparg & ~255) | executor->vm_data.oparg; \
408-
if (_PyOpcode_Caches[_PyOpcode_Deopt[opcode]]) { \
409-
PAUSE_ADAPTIVE_COUNTER(this_instr[1].counter); \
410-
} \
411-
DISPATCH_GOTO(); \
404+
/* Progress made */ \
405+
if (next_instr != this_instr) { \
406+
_Py_unset_eval_breaker_bit(tstate, _PY_EVAL_JIT_DO_NOT_REENTER); \
412407
} \
413408
if (keep_tracing_bit) { \
414409
assert(tstate->interp->jit_state.code_curr_size == 2 || tstate->interp->jit_state.code_curr_size == 3); \

‎Python/executor_cases.c.h‎

Lines changed: 21 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/generated_cases.c.h‎

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/optimizer.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -969,7 +969,9 @@ _PyJit_TryInitializeTracing(PyThreadState *tstate, _PyInterpreterFrame *frame, _
969969
#endif
970970

971971
if (is_dynamic_target) {
972-
add_to_trace(tstate->interp->jit_state.code_buffer, 0, _START_DYNAMIC_EXECUTOR, 0, (uintptr_t)insert_exec_instr, INSTR_IP(insert_exec_instr, code));
972+
assert(curr_instr == frame->instr_ptr);
973+
assert(curr_instr == insert_exec_instr);
974+
add_to_trace(tstate->interp->jit_state.code_buffer, 0, _START_DYNAMIC_EXECUTOR, 0, (uintptr_t)insert_exec_instr, 0);
973975
add_to_trace(tstate->interp->jit_state.code_buffer, 1, _MAKE_WARM, 0, 0, 0);
974976
add_to_trace(tstate->interp->jit_state.code_buffer, 2, _GUARD_EXECUTOR_IP, 0, (uintptr_t)curr_instr, 0);
975977
tstate->interp->jit_state.code_curr_size = 3;
@@ -1186,6 +1188,9 @@ sanity_check(_PyExecutorObject *executor)
11861188
executor->trace[0].opcode == _COLD_EXIT ||
11871189
executor->trace[0].opcode == _COLD_DYNAMIC_EXIT ||
11881190
executor->trace[0].opcode == _START_DYNAMIC_EXECUTOR);
1191+
if (executor->trace[0].opcode == _START_DYNAMIC_EXECUTOR) {
1192+
CHECK(executor->trace[2].opcode == _GUARD_EXECUTOR_IP);
1193+
}
11891194
for (; i < executor->code_size; i++) {
11901195
const _PyUOpInstruction *inst = &executor->trace[i];
11911196
uint16_t opcode = inst->opcode;
@@ -1579,6 +1584,7 @@ _PyExecutor_GetColdDynamicExecutor(void)
15791584
{
15801585
PyInterpreterState *interp = _PyInterpreterState_GET();
15811586
if (interp->cold_dynamic_executor != NULL) {
1587+
assert(interp->cold_dynamic_executor->trace[0].opcode == _COLD_DYNAMIC_EXIT);
15821588
return interp->cold_dynamic_executor;
15831589
}
15841590
_PyExecutorObject *cold = allocate_executor(0, 1);

‎Tools/cases_generator/analyzer.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,8 @@ def has_error_without_pop(op: parser.CodeDef) -> bool:
700700
"_PyLong_CheckExactAndCompact",
701701
"_PyExecutor_FromExit",
702702
"_PyJit_TryInitializeTracing",
703+
"_Py_unset_eval_breaker_bit",
704+
"_Py_set_eval_breaker_bit",
703705
)
704706

705707

0 commit comments

Comments
 (0)