-
-
Notifications
You must be signed in to change notification settings - Fork 34.8k
gh-152192: Fix JUMP_BACKWARD passing a truncated oparg to the jit tracer #152382
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import contextlib | ||
| import dis | ||
| import itertools | ||
| import sys | ||
| import textwrap | ||
|
|
@@ -247,6 +248,28 @@ def many_vars(): | |
| 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): | ||
| # gh-152192: a JUMP_BACKWARD that needs an EXTENDED_ARG must record its | ||
| # deopt target at the EXTENDED_ARG, not the JUMP_BACKWARD. | ||
| ns = {} | ||
| src = ("def f(n):\n" | ||
| " i = 0\n" | ||
| " while i < n:\n" | ||
| " i += 1\n" | ||
| + "".join(f" a = {j}\n" for j in range(140))) | ||
| exec(src, ns) | ||
| f = ns["f"] | ||
|
|
||
| instrs = list(dis.get_instructions(f)) | ||
| ext, jb = next((p, i) for p, i in zip(instrs, instrs[1:]) | ||
| if i.opname == "JUMP_BACKWARD" and p.opname == "EXTENDED_ARG") | ||
|
|
||
| f(TIER2_THRESHOLD + 1) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to clarify a little bit about this test. When the jit counter is enabled we enter into while (oparg > 255) {
oparg >>= 8
insert_exec_at--;
}
In tracer->prev_state.instr_oparg = oparg;And call In next calls code uses // 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 P.S: I may not be fully correct in the internals but the test catches the shift on the buggy build |
||
| ex = _opcode.get_executor(f.__code__, ext.offset) | ||
| set_ips = {t for op, _, t, _ in ex if op == "_SET_IP"} | ||
| self.assertIn(ext.offset // 2, set_ips) | ||
| self.assertNotIn(jb.offset // 2, set_ips) | ||
|
|
||
| def test_unspecialized_unpack(self): | ||
| # An example of an unspecialized opcode | ||
| def testfunc(x): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Fix a truncated ``oparg`` being passed to JIT trace initialization for a | ||
| ``JUMP_BACKWARD`` with an ``EXTENDED_ARG``. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment that this is testing for #152192
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! Thank you for the review. Just pushed the comment note on test