GeezerVison is showing github.com — gh-143123: Protect against recursive tracer calls/finalization (GH-14… · python/cpython@e370c8d
File treeExpand file treeCollapse file tree Open diff view settings Misc/NEWS.d/next/Core_and_Builtins
Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: Include/internal/pycore_optimizer.h +1-1Lines changed: 1 addition & 1 deletion | Original file line number | Diff line number | Diff line change |
|---|
@@ -233,7 +233,7 @@ _PyJit_TryInitializeTracing(PyThreadState *tstate, _PyInterpreterFrame *frame, | 233 | 233 | _Py_CODEUNIT *close_loop_instr, int curr_stackdepth, int chain_depth, _PyExitData *exit, | 234 | 234 | int oparg, _PyExecutorObject *current_executor); | 235 | 235 |
| 236 | | -void _PyJit_FinalizeTracing(PyThreadState *tstate); | | 236 | +void _PyJit_FinalizeTracing(PyThreadState *tstate, int err); | 237 | 237 | void _PyJit_TracerFree(_PyThreadStateImpl *_tstate); | 238 | 238 |
| 239 | 239 | void _PyJit_Tracer_InvalidateDependency(PyThreadState *old_tstate, void *obj); | |
Collapse file Copy file name to clipboardExpand all lines: Include/internal/pycore_tstate.h +1Lines changed: 1 addition & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -54,6 +54,7 @@ typedef struct _PyJitTracerTranslatorState { | 54 | 54 | } _PyJitTracerTranslatorState; | 55 | 55 |
| 56 | 56 | typedef struct _PyJitTracerState { | | 57 | + bool is_tracing; | 57 | 58 | _PyJitTracerInitialState initial_state; | 58 | 59 | _PyJitTracerPreviousState prev_state; | 59 | 60 | _PyJitTracerTranslatorState translator_state; | |
Collapse file Copy file name to clipboardExpand all lines: Lib/test/test_capi/test_opt.py +45Lines changed: 45 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -3747,6 +3747,51 @@ async def async_for_driver(): | 3747 | 3747 | """), PYTHON_JIT="1") | 3748 | 3748 | self.assertEqual(result[0].rc, 0, result) | 3749 | 3749 |
| | 3750 | + def test_143358(self): | | 3751 | + # https://github.com/python/cpython/issues/143358 | | 3752 | + | | 3753 | + result = script_helper.run_python_until_end('-c', textwrap.dedent(f""" | | 3754 | + def f1(): | | 3755 | +
| | 3756 | + class EvilIterator: | | 3757 | +
| | 3758 | + def __init__(self): | | 3759 | + self._items = [1, 2] | | 3760 | + self._index = 1 | | 3761 | +
| | 3762 | + def __iter__(self): | | 3763 | + return self | | 3764 | +
| | 3765 | + def __next__(self): | | 3766 | + if not len(self._items) % 13: | | 3767 | + self._items.clear() | | 3768 | +
| | 3769 | + for i_loop_9279 in range(10): | | 3770 | + self._items.extend([1, "", None]) | | 3771 | +
| | 3772 | + if not len(self._items) % 11: | | 3773 | + return 'unexpected_type_from_iterator' | | 3774 | +
| | 3775 | + if self._index >= len(self._items): | | 3776 | + raise StopIteration | | 3777 | +
| | 3778 | + item = self._items[self._index] | | 3779 | + self._index += 1 | | 3780 | + return item | | 3781 | +
| | 3782 | + evil_iter = EvilIterator() | | 3783 | +
| | 3784 | + large_num = 2**31 | | 3785 | + for _ in range(400): | | 3786 | + try: | | 3787 | + _ = [x + y for x in evil_iter for y in evil_iter if evil_iter._items.append(x) or large_num] | | 3788 | + except TypeError: | | 3789 | + pass | | 3790 | +
| | 3791 | + f1() | | 3792 | + """), PYTHON_JIT="1", PYTHON_JIT_JUMP_BACKWARD_INITIAL_VALUE="64") | | 3793 | + self.assertEqual(result[0].rc, 0, result) | | 3794 | + | 3750 | 3795 | def global_identity(x): | 3751 | 3796 | return x | 3752 | 3797 |
| |
Collapse file Copy file name to clipboard +1Lines changed: 1 addition & 0 deletions - Display the source diff
- Display the rich diff
| Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +Protect the JIT against recursive tracing. |
Collapse file Copy file name to clipboardExpand all lines: Python/bytecodes.c +6Lines changed: 6 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -5620,6 +5620,9 @@ dummy_func( | 5620 | 5620 | #else | 5621 | 5621 | assert(_PyErr_Occurred(tstate)); | 5622 | 5622 | #endif | | 5623 | + SAVE_STACK(); | | 5624 | + STOP_TRACING(); | | 5625 | + RELOAD_STACK(); | 5623 | 5626 |
| 5624 | 5627 | /* Log traceback info. */ | 5625 | 5628 | assert(frame->owner != FRAME_OWNED_BY_INTERPRETER); | @@ -5634,6 +5637,9 @@ dummy_func( | 5634 | 5637 | } | 5635 | 5638 |
| 5636 | 5639 | spilled label(exception_unwind) { | | 5640 | + SAVE_STACK(); | | 5641 | + STOP_TRACING(); | | 5642 | + RELOAD_STACK(); | 5637 | 5643 | /* We can't use frame->instr_ptr here, as RERAISE may have set it */ | 5638 | 5644 | int offset = INSTR_OFFSET()-1; | 5639 | 5645 | int level, handler, lasti; | |
Collapse file Copy file name to clipboardExpand all lines: Python/ceval.c +1-26Lines changed: 1 addition & 26 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1460,32 +1460,7 @@ stop_tracing_and_jit(PyThreadState *tstate, _PyInterpreterFrame *frame) | 1460 | 1460 | if (!_PyErr_Occurred(tstate) && !_is_sys_tracing) { | 1461 | 1461 | err = _PyOptimizer_Optimize(frame, tstate); | 1462 | 1462 | } | 1463 | | - _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate; | 1464 | | - // Deal with backoffs | 1465 | | - _PyJitTracerState *tracer = _tstate->jit_tracer_state; | 1466 | | - assert(tracer != NULL); | 1467 | | - _PyExitData *exit = tracer->initial_state.exit; | 1468 | | - if (exit == NULL) { | 1469 | | - // We hold a strong reference to the code object, so the instruction won't be freed. | 1470 | | - if (err <= 0) { | 1471 | | - _Py_BackoffCounter counter = tracer->initial_state.jump_backward_instr[1].counter; | 1472 | | - tracer->initial_state.jump_backward_instr[1].counter = restart_backoff_counter(counter); | 1473 | | - } | 1474 | | - else { | 1475 | | - tracer->initial_state.jump_backward_instr[1].counter = initial_jump_backoff_counter(&_tstate->policy); | 1476 | | - } | 1477 | | - } | 1478 | | - else if (tracer->initial_state.executor->vm_data.valid) { | 1479 | | - // Likewise, we hold a strong reference to the executor containing this exit, so the exit is guaranteed | 1480 | | - // to be valid to access. | 1481 | | - if (err <= 0) { | 1482 | | - exit->temperature = restart_backoff_counter(exit->temperature); | 1483 | | - } | 1484 | | - else { | 1485 | | - exit->temperature = initial_temperature_backoff_counter(&_tstate->policy); | 1486 | | - } | 1487 | | - } | 1488 | | - _PyJit_FinalizeTracing(tstate); | | 1463 | + _PyJit_FinalizeTracing(tstate, err); | 1489 | 1464 | return err; | 1490 | 1465 | } | 1491 | 1466 | #endif | |
Collapse file Copy file name to clipboardExpand all lines: Python/ceval_macros.h +13Lines changed: 13 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 156 | 156 | # define LEAVE_TRACING() tracing_mode = 0 | 157 | 157 | #endif | 158 | 158 |
| | 159 | +#if _Py_TIER2 | | 160 | +#define STOP_TRACING() \ | | 161 | + do { \ | | 162 | + if (IS_JIT_TRACING()) { \ | | 163 | + LEAVE_TRACING(); \ | | 164 | + _PyJit_FinalizeTracing(tstate, 0); \ | | 165 | + } \ | | 166 | + } while (0); | | 167 | +#else | | 168 | +#define STOP_TRACING() ((void)(0)); | | 169 | +#endif | | 170 | + | | 171 | + | 159 | 172 | /* PRE_DISPATCH_GOTO() does lltrace if enabled. Normally a no-op */ | 160 | 173 | #ifdef Py_DEBUG | 161 | 174 | #define PRE_DISPATCH_GOTO() if (frame->lltrace >= 5) { \ | |
Collapse file Copy file name to clipboardExpand all lines: Python/generated_cases.c.h +4-1Lines changed: 4 additions & 1 deletion Collapse file Copy file name to clipboardExpand all lines: Python/optimizer.c +28-3Lines changed: 28 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -1030,11 +1030,11 @@ _PyJit_TryInitializeTracing( | 1030 | 1030 | // Don't error, just go to next instruction. | 1031 | 1031 | return 0; | 1032 | 1032 | } | | 1033 | + _tstate->jit_tracer_state->is_tracing = false; | 1033 | 1034 | } | 1034 | 1035 | _PyJitTracerState *tracer = _tstate->jit_tracer_state; | 1035 | 1036 | // A recursive trace. | 1036 | | - // Don't trace into the inner call because it will stomp on the previous trace, causing endless retraces. | 1037 | | - if (tracer->prev_state.code_curr_size > CODE_SIZE_EMPTY) { | | 1037 | + if (tracer->is_tracing) { | 1038 | 1038 | return 0; | 1039 | 1039 | } | 1040 | 1040 | if (oparg > 0xFFFF) { | @@ -1086,20 +1086,45 @@ _PyJit_TryInitializeTracing( | 1086 | 1086 | close_loop_instr[1].counter = trigger_backoff_counter(); | 1087 | 1087 | } | 1088 | 1088 | _Py_BloomFilter_Init(&tracer->prev_state.dependencies); | | 1089 | + tracer->is_tracing = true; | 1089 | 1090 | return 1; | 1090 | 1091 | } | 1091 | 1092 |
| 1092 | 1093 | Py_NO_INLINE void | 1093 | | -_PyJit_FinalizeTracing(PyThreadState *tstate) | | 1094 | +_PyJit_FinalizeTracing(PyThreadState *tstate, int err) | 1094 | 1095 | { | 1095 | 1096 | _PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate; | 1096 | 1097 | _PyJitTracerState *tracer = _tstate->jit_tracer_state; | | 1098 | + // Deal with backoffs | | 1099 | + assert(tracer != NULL); | | 1100 | + _PyExitData *exit = tracer->initial_state.exit; | | 1101 | + if (exit == NULL) { | | 1102 | + // We hold a strong reference to the code object, so the instruction won't be freed. | | 1103 | + if (err <= 0) { | | 1104 | + _Py_BackoffCounter counter = tracer->initial_state.jump_backward_instr[1].counter; | | 1105 | + tracer->initial_state.jump_backward_instr[1].counter = restart_backoff_counter(counter); | | 1106 | + } | | 1107 | + else { | | 1108 | + tracer->initial_state.jump_backward_instr[1].counter = initial_jump_backoff_counter(&_tstate->policy); | | 1109 | + } | | 1110 | + } | | 1111 | + else if (tracer->initial_state.executor->vm_data.valid) { | | 1112 | + // Likewise, we hold a strong reference to the executor containing this exit, so the exit is guaranteed | | 1113 | + // to be valid to access. | | 1114 | + if (err <= 0) { | | 1115 | + exit->temperature = restart_backoff_counter(exit->temperature); | | 1116 | + } | | 1117 | + else { | | 1118 | + exit->temperature = initial_temperature_backoff_counter(&_tstate->policy); | | 1119 | + } | | 1120 | + } | 1097 | 1121 | Py_CLEAR(tracer->initial_state.code); | 1098 | 1122 | Py_CLEAR(tracer->initial_state.func); | 1099 | 1123 | Py_CLEAR(tracer->initial_state.executor); | 1100 | 1124 | Py_CLEAR(tracer->prev_state.instr_code); | 1101 | 1125 | tracer->prev_state.code_curr_size = CODE_SIZE_EMPTY; | 1102 | 1126 | tracer->prev_state.code_max_size = UOP_MAX_TRACE_LENGTH/2 - 1; | | 1127 | + tracer->is_tracing = false; | 1103 | 1128 | } | 1104 | 1129 |
| 1105 | 1130 | void | |
|