gh-152192: Fix JUMP_BACKWARD passing a truncated oparg to the jit tracer - #152382
Conversation
There was a problem hiding this comment.
I want to clarify a little bit about this test.
When the jit counter is enabled we enter into _JIT op with original oparg = 299
Then oparg truncates with buggy loop
while (oparg > 255) {
oparg >>= 8
insert_exec_at--;
}oparg becomes 1, and we pass it to _PyJit_TryInitializeTracing
In _PyJit_TryInitializeTracing we store this broken oparg
tracer->prev_state.instr_oparg = oparg;And call ENTER_TRACING()
In next calls code uses TRACE_RECORD which in turn calls _PyJit_translate_single_bytecode_to_trace. This is the point where shift exists
// Rewind EXTENDED_ARG so that we see the whole thing.
// We must point to the first EXTENDED_ARG when deopting.
int oparg = tracer->prev_state.instr_oparg;
int opcode = this_instr->op.code;
int rewind_oparg = oparg;
while (rewind_oparg > 255) {
rewind_oparg >>= 8;
target--;
}We get the broken oparg from tracer->prev_state.instr_oparg and the loop does not executes because 1 < 255. target does not decrements and the instruction is not correct (should be EXTENDED_ARG but actually JUMP_BACKWARD)
P.S: I may not be fully correct in the internals but the test catches the shift on the buggy build
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
This looks good. Thanks for fixing this.
Can you link to the issue in the test, then I'll merge.
| self.assertTrue(any((opcode, oparg, operand) == ("_LOAD_FAST_BORROW", 259, 0) | ||
| for opcode, oparg, _, operand in list(ex))) | ||
|
|
||
| def test_jump_backward_extended_arg(self): |
There was a problem hiding this comment.
Can you add a comment that this is testing for #152192
There was a problem hiding this comment.
Hello! Thank you for the review. Just pushed the comment note on test
* main: (266 commits) pythongh-151626: Fix tests that fail when PYTHONPYCACHEPREFIX is set (pythonGH-151952) pythongh-152728: IDLE - move 3 toplevel fix_xyz functions to idlelb.util (python#152729) pythongh-152711: Add pythoninfo-build command to Platforms/Android (python#152713) pythongh-152715: Add pythoninfo-build command to Platforms/Apple (python#152716) pythongh-152433: Windows: enable mmapmodule for UWP (python#152473) pythongh-152433: Windows: use GetFileSizeEx instead of GetFileSize for memory mapped files (python#152383) pythonGH-81881: Raise `SpecialFileError` for sockets and devices in `shutil.copyfile` (python#142693) pythongh-152502: Detect the curses mouse interface and is_* methods portably (pythonGH-152705) pythongh-145857: Replace `DELETE_GLOBAL` with `PUSH_NULL; STORE_GLOBAL` (pythonGH-146314) pythongh-145854: Replace `DELETE_NAME` with `PUSH_NULL; STORE_NAME` (pythonGH-146006) pythongh-152680: Detect container/VM in test.pythoninfo (python#152668) pythongh-152682: Fix NULL dereference on OOM in `symtable_visit_type_param_bound_or_default` (python#152684) pythongh-151881: Skip tk_inactive negativity check on Windows (pythonGH-152683) pythongh-152546: Refactor `mappingproxy.__new__` to use `PyDictProxy_New` (python#152547) pythongh-151126: Fix a possible crash during the startup with no memory under `Py_STACKREF_DEBUG` (python#152478) pythongh-152635: Raise MemoryError when the lock allocation fails in `_interpchannels.create()` (python#152642) pythongh-151029: Fix `test_remote_exec_deleted_static_executable` on static installed builds (pythonGH-152653) pythongh-121249: Deprecate using F/D type codes in the struct module (python#152309) pythongh-152192: Fix JUMP_BACKWARD passing a truncated oparg to the jit tracer (pythonGH-152382) Don't require the `_test{internal}capi` modules in `test_monitoring.py` (python#152311) ...
2 participants
The
_JITop used oparg as the counter in thewhileloop, truncating it before reaching_PyJit_TryInitializeTracing.Fixed it by using a temporary value.
For more details see gh-152192