bpo-45773: Stop "optimizing" certain jump patterns (GH-29505)

Lib/test/test_peepholer.py

@@ -600,5 +600,12 @@ def test_bpo_42057(self):
             except Exception or Exception:
                 pass
 
+    def test_bpo_45773_pop_jump_if_true(self):
+        compile("while True or spam: pass", "<test>", "exec")
+
+    def test_bpo_45773_pop_jump_if_false(self):
+        compile("while True or not spam: pass", "<test>", "exec")
+
+
 if __name__ == "__main__":
     unittest.main()

Misc/NEWS.d/next/Core and Builtins/2021-11-09-13-01-35.bpo-45773.POU8A4.rst

@@ -0,0 +1 @@
+Fix a compiler hang when attempting to optimize certain jump patterns.

Python/compile.c

@@ -8052,29 +8052,24 @@ fold_rotations(struct instr *inst, int n)
     }
 }
 
-
+// Attempt to eliminate jumps to jumps by updating inst to jump to
-static int
+// target->i_target using the provided opcode. Return whether or not the
-eliminate_jump_to_jump(basicblock *bb, int opcode) {
+// optimization was successful.
-    assert (bb->b_iused > 0);
+static bool
-    struct instr *inst = &bb->b_instr[bb->b_iused-1];
+jump_thread(struct instr *inst, struct instr *target, int opcode)
+{
     assert(is_jump(inst));
-    assert (inst->i_target->b_iused > 0);
+    assert(is_jump(target));
-    struct instr *target = &inst->i_target->b_instr[0];
+    // bpo-45773: If inst->i_target == target->i_target, then nothing actually
-    if (inst->i_target == target->i_target) {
+    // changes (and we fall into an infinite loop):
-        /* Nothing to do */
+    if (inst->i_lineno == target->i_lineno &&
-        return 0;
+        inst->i_target != target->i_target)
+    {
+        inst->i_target = target->i_target;
+        inst->i_opcode = opcode;
+        return true;
     }
-    int lineno = target->i_lineno;
+    return false;
-    int end_lineno = target->i_end_lineno;
-    int col_offset = target->i_col_offset;
-    int end_col_offset = target->i_end_col_offset;
-    if (add_jump_to_block(bb, opcode, lineno, end_lineno, col_offset,
-                          end_col_offset, target->i_target) == 0) {
-        return -1;
-    }
-    assert (bb->b_iused >= 2);
-    bb->b_instr[bb->b_iused-2].i_opcode = NOP;
-    return 0;
 }
 
 /* Maximum size of basic block that should be copied in optimizer */
@@ -8201,23 +8196,19 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts)
             case JUMP_IF_FALSE_OR_POP:
                 switch (target->i_opcode) {
                     case POP_JUMP_IF_FALSE:
-                        if (inst->i_lineno == target->i_lineno) {
+                        i -= jump_thread(inst, target, POP_JUMP_IF_FALSE);
-                            *inst = *target;
-                            i--;
-                        }
                         break;
                     case JUMP_ABSOLUTE:
                     case JUMP_FORWARD:
                     case JUMP_IF_FALSE_OR_POP:
-                        if (inst->i_lineno == target->i_lineno &&
+                        i -= jump_thread(inst, target, JUMP_IF_FALSE_OR_POP);
-                            inst->i_target != target->i_target) {
-                            inst->i_target = target->i_target;
-                            i--;
-                        }
                         break;
                     case JUMP_IF_TRUE_OR_POP:
-                        assert (inst->i_target->b_iused == 1);
+                    case POP_JUMP_IF_TRUE:
                         if (inst->i_lineno == target->i_lineno) {
+                            // We don't need to bother checking for loops here,
+                            // since a block's b_next cannot point to itself:
+                            assert(inst->i_target != inst->i_target->b_next);
                             inst->i_opcode = POP_JUMP_IF_FALSE;
                             inst->i_target = inst->i_target->b_next;
                             --i;
@@ -8225,27 +8216,22 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts)
                         break;
                 }
                 break;
-
             case JUMP_IF_TRUE_OR_POP:
                 switch (target->i_opcode) {
                     case POP_JUMP_IF_TRUE:
-                        if (inst->i_lineno == target->i_lineno) {
+                        i -= jump_thread(inst, target, POP_JUMP_IF_TRUE);
-                            *inst = *target;
-                            i--;
-                        }
                         break;
                     case JUMP_ABSOLUTE:
                     case JUMP_FORWARD:
                     case JUMP_IF_TRUE_OR_POP:
-                        if (inst->i_lineno == target->i_lineno &&
+                        i -= jump_thread(inst, target, JUMP_IF_TRUE_OR_POP);
-                            inst->i_target != target->i_target) {
-                            inst->i_target = target->i_target;
-                            i--;
-                        }
                         break;
                     case JUMP_IF_FALSE_OR_POP:
-                        assert (inst->i_target->b_iused == 1);
+                    case POP_JUMP_IF_FALSE:
                         if (inst->i_lineno == target->i_lineno) {
+                            // We don't need to bother checking for loops here,
+                            // since a block's b_next cannot point to itself:
+                            assert(inst->i_target != inst->i_target->b_next);
                             inst->i_opcode = POP_JUMP_IF_TRUE;
                             inst->i_target = inst->i_target->b_next;
                             --i;
@@ -8253,54 +8239,33 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts)
                         break;
                 }
                 break;
-
             case POP_JUMP_IF_FALSE:
                 switch (target->i_opcode) {
                     case JUMP_ABSOLUTE:
                     case JUMP_FORWARD:
-                        if (inst->i_lineno == target->i_lineno) {
+                    case JUMP_IF_FALSE_OR_POP:
-                            inst->i_target = target->i_target;
+                        i -= jump_thread(inst, target, POP_JUMP_IF_FALSE);
-                            i--;
                 }
                 break;
-                }
-                break;
-
             case POP_JUMP_IF_TRUE:
                 switch (target->i_opcode) {
                     case JUMP_ABSOLUTE:
                     case JUMP_FORWARD:
-                        if (inst->i_lineno == target->i_lineno) {
+                    case JUMP_IF_TRUE_OR_POP:
-                            inst->i_target = target->i_target;
+                        i -= jump_thread(inst, target, POP_JUMP_IF_TRUE);
-                            i--;
                 }
                 break;
-                }
-                break;
-
             case JUMP_ABSOLUTE:
             case JUMP_FORWARD:
-                assert (i == bb->b_iused-1);
                 switch (target->i_opcode) {
-                    case JUMP_FORWARD:
-                        if (eliminate_jump_to_jump(bb, inst->i_opcode)) {
-                            goto error;
-                        }
-                        break;
-
                     case JUMP_ABSOLUTE:
-                        if (eliminate_jump_to_jump(bb, JUMP_ABSOLUTE)) {
+                    case JUMP_FORWARD:
-                            goto error;
+                        i -= jump_thread(inst, target, JUMP_ABSOLUTE);
-                        }
-                        break;
                 }
                 break;
             case FOR_ITER:
-                assert (i == bb->b_iused-1);
                 if (target->i_opcode == JUMP_FORWARD) {
-                    if (eliminate_jump_to_jump(bb, inst->i_opcode)) {
+                    i -= jump_thread(inst, target, FOR_ITER);
-                        goto error;
-                    }
                 }
                 break;
             case ROT_N: