Skip to content

Commit a89bbde

Browse files
authored
[3.10] bpo-45773: Stop "optimizing" certain jump patterns (GH-29526)
1 parent c5bfb88 commit a89bbde

File tree

6 files changed

+60
-81
lines changed

6 files changed

+60
-81
lines changed

Lib/test/test_peepholer.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,5 +514,12 @@ def test_bpo_42057(self):
514514
except Exception or Exception:
515515
pass
516516

517+
def test_bpo_45773_pop_jump_if_true(self):
518+
compile("while True or spam: pass", "<test>", "exec")
519+
520+
def test_bpo_45773_pop_jump_if_false(self):
521+
compile("while True or not spam: pass", "<test>", "exec")
522+
523+
517524
if __name__ == "__main__":
518525
unittest.main()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a compiler hang when attempting to optimize certain jump patterns.

Python/compile.c

Lines changed: 43 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
* objects.
2222
*/
2323

24+
#include <stdbool.h>
25+
2426
#include "Python.h"
2527
#include "pycore_ast.h" // _PyAST_GetDocString()
2628
#include "pycore_compile.h" // _PyFuture_FromAST()
@@ -7264,25 +7266,24 @@ fold_rotations(struct instr *inst, int n)
72647266
}
72657267
}
72667268

7267-
7268-
static int
7269-
eliminate_jump_to_jump(basicblock *bb, int opcode) {
7270-
assert (bb->b_iused > 0);
7271-
struct instr *inst = &bb->b_instr[bb->b_iused-1];
7272-
assert (is_jump(inst));
7273-
assert (inst->i_target->b_iused > 0);
7274-
struct instr *target = &inst->i_target->b_instr[0];
7275-
if (inst->i_target == target->i_target) {
7276-
/* Nothing to do */
7277-
return 0;
7278-
}
7279-
int lineno = target->i_lineno;
7280-
if (add_jump_to_block(bb, opcode, lineno, target->i_target) == 0) {
7281-
return -1;
7269+
// Attempt to eliminate jumps to jumps by updating inst to jump to
7270+
// target->i_target using the provided opcode. Return whether or not the
7271+
// optimization was successful.
7272+
static bool
7273+
jump_thread(struct instr *inst, struct instr *target, int opcode)
7274+
{
7275+
assert(is_jump(inst));
7276+
assert(is_jump(target));
7277+
// bpo-45773: If inst->i_target == target->i_target, then nothing actually
7278+
// changes (and we fall into an infinite loop):
7279+
if (inst->i_lineno == target->i_lineno &&
7280+
inst->i_target != target->i_target)
7281+
{
7282+
inst->i_target = target->i_target;
7283+
inst->i_opcode = opcode;
7284+
return true;
72827285
}
7283-
assert (bb->b_iused >= 2);
7284-
bb->b_instr[bb->b_iused-2].i_opcode = NOP;
7285-
return 0;
7286+
return false;
72867287
}
72877288

72887289
/* Maximum size of basic block that should be copied in optimizer */
@@ -7399,108 +7400,78 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts)
73997400
where y+1 is the instruction following the second test.
74007401
*/
74017402
case JUMP_IF_FALSE_OR_POP:
7402-
switch(target->i_opcode) {
7403+
switch (target->i_opcode) {
74037404
case POP_JUMP_IF_FALSE:
7404-
if (inst->i_lineno == target->i_lineno) {
7405-
*inst = *target;
7406-
i--;
7407-
}
7405+
i -= jump_thread(inst, target, POP_JUMP_IF_FALSE);
74087406
break;
74097407
case JUMP_ABSOLUTE:
74107408
case JUMP_FORWARD:
74117409
case JUMP_IF_FALSE_OR_POP:
7412-
if (inst->i_lineno == target->i_lineno &&
7413-
inst->i_target != target->i_target) {
7414-
inst->i_target = target->i_target;
7415-
i--;
7416-
}
7410+
i -= jump_thread(inst, target, JUMP_IF_FALSE_OR_POP);
74177411
break;
74187412
case JUMP_IF_TRUE_OR_POP:
7419-
assert (inst->i_target->b_iused == 1);
7413+
case POP_JUMP_IF_TRUE:
74207414
if (inst->i_lineno == target->i_lineno) {
7415+
// We don't need to bother checking for loops here,
7416+
// since a block's b_next cannot point to itself:
7417+
assert(inst->i_target != inst->i_target->b_next);
74217418
inst->i_opcode = POP_JUMP_IF_FALSE;
74227419
inst->i_target = inst->i_target->b_next;
74237420
--i;
74247421
}
74257422
break;
74267423
}
74277424
break;
7428-
74297425
case JUMP_IF_TRUE_OR_POP:
7430-
switch(target->i_opcode) {
7426+
switch (target->i_opcode) {
74317427
case POP_JUMP_IF_TRUE:
7432-
if (inst->i_lineno == target->i_lineno) {
7433-
*inst = *target;
7434-
i--;
7435-
}
7428+
i -= jump_thread(inst, target, POP_JUMP_IF_TRUE);
74367429
break;
74377430
case JUMP_ABSOLUTE:
74387431
case JUMP_FORWARD:
74397432
case JUMP_IF_TRUE_OR_POP:
7440-
if (inst->i_lineno == target->i_lineno &&
7441-
inst->i_target != target->i_target) {
7442-
inst->i_target = target->i_target;
7443-
i--;
7444-
}
7433+
i -= jump_thread(inst, target, JUMP_IF_TRUE_OR_POP);
74457434
break;
74467435
case JUMP_IF_FALSE_OR_POP:
7447-
assert (inst->i_target->b_iused == 1);
7436+
case POP_JUMP_IF_FALSE:
74487437
if (inst->i_lineno == target->i_lineno) {
7438+
// We don't need to bother checking for loops here,
7439+
// since a block's b_next cannot point to itself:
7440+
assert(inst->i_target != inst->i_target->b_next);
74497441
inst->i_opcode = POP_JUMP_IF_TRUE;
74507442
inst->i_target = inst->i_target->b_next;
74517443
--i;
74527444
}
74537445
break;
74547446
}
74557447
break;
7456-
74577448
case POP_JUMP_IF_FALSE:
7458-
switch(target->i_opcode) {
7449+
switch (target->i_opcode) {
74597450
case JUMP_ABSOLUTE:
74607451
case JUMP_FORWARD:
7461-
if (inst->i_lineno == target->i_lineno) {
7462-
inst->i_target = target->i_target;
7463-
i--;
7464-
}
7465-
break;
7452+
case JUMP_IF_FALSE_OR_POP:
7453+
i -= jump_thread(inst, target, POP_JUMP_IF_FALSE);
74667454
}
74677455
break;
7468-
74697456
case POP_JUMP_IF_TRUE:
7470-
switch(target->i_opcode) {
7457+
switch (target->i_opcode) {
74717458
case JUMP_ABSOLUTE:
74727459
case JUMP_FORWARD:
7473-
if (inst->i_lineno == target->i_lineno) {
7474-
inst->i_target = target->i_target;
7475-
i--;
7476-
}
7477-
break;
7460+
case JUMP_IF_TRUE_OR_POP:
7461+
i -= jump_thread(inst, target, POP_JUMP_IF_TRUE);
74787462
}
74797463
break;
7480-
74817464
case JUMP_ABSOLUTE:
74827465
case JUMP_FORWARD:
7483-
assert (i == bb->b_iused-1);
7484-
switch(target->i_opcode) {
7485-
case JUMP_FORWARD:
7486-
if (eliminate_jump_to_jump(bb, inst->i_opcode)) {
7487-
goto error;
7488-
}
7489-
break;
7490-
7466+
switch (target->i_opcode) {
74917467
case JUMP_ABSOLUTE:
7492-
if (eliminate_jump_to_jump(bb, JUMP_ABSOLUTE)) {
7493-
goto error;
7494-
}
7495-
break;
7468+
case JUMP_FORWARD:
7469+
i -= jump_thread(inst, target, JUMP_ABSOLUTE);
74967470
}
74977471
break;
74987472
case FOR_ITER:
7499-
assert (i == bb->b_iused-1);
75007473
if (target->i_opcode == JUMP_FORWARD) {
7501-
if (eliminate_jump_to_jump(bb, inst->i_opcode)) {
7502-
goto error;
7503-
}
7474+
i -= jump_thread(inst, target, FOR_ITER);
75047475
}
75057476
break;
75067477
case ROT_N:

Python/importlib.h

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

Python/importlib_external.h

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

Python/importlib_zipimport.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,9 +1004,9 @@ const unsigned char _Py_M__zipimport[] = {
10041004
1,0,1,0,89,0,113,9,119,0,124,8,100,4,25,0,
10051005
125,9,116,8,124,0,106,4,124,8,131,2,125,10,100,0,
10061006
125,11,124,5,114,91,122,10,116,9,124,0,124,9,124,7,
1007-
124,1,124,10,131,5,125,11,87,0,110,25,4,0,116,10,
1007+
124,1,124,10,131,5,125,11,87,0,113,96,4,0,116,10,
10081008
121,90,1,0,125,12,1,0,122,8,124,12,125,3,87,0,
1009-
89,0,100,0,125,12,126,12,110,10,100,0,125,12,126,12,
1009+
89,0,100,0,125,12,126,12,113,96,100,0,125,12,126,12,
10101010
119,1,119,0,116,11,124,9,124,10,131,2,125,11,124,11,
10111011
100,0,117,0,114,101,113,9,124,8,100,4,25,0,125,9,
10121012
124,11,124,6,124,9,102,3,2,0,1,0,83,0,124,3,

0 commit comments

Comments
 (0)