Skip to content

Commit e370c8d

Browse files
gh-143123: Protect against recursive tracer calls/finalization (GH-143126)
* Stronger check for recursive traces * Add a stop_tracing field * Stop early when tracing exceptions
1 parent 6db952e commit e370c8d

File tree

9 files changed

+100
-31
lines changed

9 files changed

+100
-31
lines changed

Include/internal/pycore_optimizer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ _PyJit_TryInitializeTracing(PyThreadState *tstate, _PyInterpreterFrame *frame,
233233
_Py_CODEUNIT *close_loop_instr, int curr_stackdepth, int chain_depth, _PyExitData *exit,
234234
int oparg, _PyExecutorObject *current_executor);
235235

236-
void _PyJit_FinalizeTracing(PyThreadState *tstate);
236+
void _PyJit_FinalizeTracing(PyThreadState *tstate, int err);
237237
void _PyJit_TracerFree(_PyThreadStateImpl *_tstate);
238238

239239
void _PyJit_Tracer_InvalidateDependency(PyThreadState *old_tstate, void *obj);

Include/internal/pycore_tstate.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ typedef struct _PyJitTracerTranslatorState {
5454
} _PyJitTracerTranslatorState;
5555

5656
typedef struct _PyJitTracerState {
57+
bool is_tracing;
5758
_PyJitTracerInitialState initial_state;
5859
_PyJitTracerPreviousState prev_state;
5960
_PyJitTracerTranslatorState translator_state;

Lib/test/test_capi/test_opt.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3747,6 +3747,51 @@ async def async_for_driver():
37473747
"""), PYTHON_JIT="1")
37483748
self.assertEqual(result[0].rc, 0, result)
37493749

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+
37503795
def global_identity(x):
37513796
return x
37523797

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Protect the JIT against recursive tracing.

Python/bytecodes.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5620,6 +5620,9 @@ dummy_func(
56205620
#else
56215621
assert(_PyErr_Occurred(tstate));
56225622
#endif
5623+
SAVE_STACK();
5624+
STOP_TRACING();
5625+
RELOAD_STACK();
56235626

56245627
/* Log traceback info. */
56255628
assert(frame->owner != FRAME_OWNED_BY_INTERPRETER);
@@ -5634,6 +5637,9 @@ dummy_func(
56345637
}
56355638

56365639
spilled label(exception_unwind) {
5640+
SAVE_STACK();
5641+
STOP_TRACING();
5642+
RELOAD_STACK();
56375643
/* We can't use frame->instr_ptr here, as RERAISE may have set it */
56385644
int offset = INSTR_OFFSET()-1;
56395645
int level, handler, lasti;

Python/ceval.c

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,32 +1460,7 @@ stop_tracing_and_jit(PyThreadState *tstate, _PyInterpreterFrame *frame)
14601460
if (!_PyErr_Occurred(tstate) && !_is_sys_tracing) {
14611461
err = _PyOptimizer_Optimize(frame, tstate);
14621462
}
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);
14891464
return err;
14901465
}
14911466
#endif

Python/ceval_macros.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,19 @@
156156
# define LEAVE_TRACING() tracing_mode = 0
157157
#endif
158158

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+
159172
/* PRE_DISPATCH_GOTO() does lltrace if enabled. Normally a no-op */
160173
#ifdef Py_DEBUG
161174
#define PRE_DISPATCH_GOTO() if (frame->lltrace >= 5) { \

Python/generated_cases.c.h

Lines changed: 4 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: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,11 +1030,11 @@ _PyJit_TryInitializeTracing(
10301030
// Don't error, just go to next instruction.
10311031
return 0;
10321032
}
1033+
_tstate->jit_tracer_state->is_tracing = false;
10331034
}
10341035
_PyJitTracerState *tracer = _tstate->jit_tracer_state;
10351036
// 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) {
10381038
return 0;
10391039
}
10401040
if (oparg > 0xFFFF) {
@@ -1086,20 +1086,45 @@ _PyJit_TryInitializeTracing(
10861086
close_loop_instr[1].counter = trigger_backoff_counter();
10871087
}
10881088
_Py_BloomFilter_Init(&tracer->prev_state.dependencies);
1089+
tracer->is_tracing = true;
10891090
return 1;
10901091
}
10911092

10921093
Py_NO_INLINE void
1093-
_PyJit_FinalizeTracing(PyThreadState *tstate)
1094+
_PyJit_FinalizeTracing(PyThreadState *tstate, int err)
10941095
{
10951096
_PyThreadStateImpl *_tstate = (_PyThreadStateImpl *)tstate;
10961097
_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+
}
10971121
Py_CLEAR(tracer->initial_state.code);
10981122
Py_CLEAR(tracer->initial_state.func);
10991123
Py_CLEAR(tracer->initial_state.executor);
11001124
Py_CLEAR(tracer->prev_state.instr_code);
11011125
tracer->prev_state.code_curr_size = CODE_SIZE_EMPTY;
11021126
tracer->prev_state.code_max_size = UOP_MAX_TRACE_LENGTH/2 - 1;
1127+
tracer->is_tracing = false;
11031128
}
11041129

11051130
void

0 commit comments

Comments
 (0)