Make sure that the CFG is consistent throughout optimiization. Extend…

… line-number propagation logic so that implicit returns after try-except or with have the correct line numbers.
markshannon · Jan 13, 2021 · Jan 12, 2021 · Jan 12, 2021 · Jan 12, 2021 · Jan 12, 2021
commit 26953b798b732a07e6969f31238ee436183def55
diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
@@ -916,6 +916,46 @@ def func():
              (7, 'line'),
              (7, 'return')])
 
+    def test_if_false_in_with(self):
+
+        class C:
+            def __enter__(self):
+                return self
+            def __exit__(*args):
+                pass
+
+        def func():
+            with C():
+                if False:
+                    pass
+
+        self.run_and_compare(func,
+            [(0, 'call'),
+             (1, 'line'),
+             (-5, 'call'),
+             (-4, 'line'),
+             (-4, 'return'),
+             (2, 'line'),
+             (-3, 'call'),
+             (-2, 'line'),
+             (-2, 'return'),
+             (2, 'return')])
+
+    def test_if_false_in_try_except(self):
+
+        def func():
+            try:
+                if False:
+                    pass
+            except Exception:
+                X
+
+        self.run_and_compare(func,
+            [(0, 'call'),
+             (1, 'line'),
+             (2, 'line'),
+             (2, 'return')])
+
 
 class SkipLineEventsTraceTestCase(TraceTestCase):
     """Repeat the trace tests, but with per-line events skipped"""

diff --git a/Python/compile.c b/Python/compile.c
@@ -95,7 +95,8 @@ typedef struct basicblock_ {
     struct basicblock_ *b_next;
     /* b_return is true if a RETURN_VALUE opcode is inserted. */
     unsigned b_return : 1;
-    unsigned b_reachable : 1;
+    /* Number of predecssors that a block has. */
+    int b_predecessors;
     /* Basic block has no fall through (it ends with a return, raise or jump) */
     unsigned b_nofallthrough : 1;
     /* Basic block exits scope (it ends with a return or raise) */
@@ -825,6 +826,7 @@ compiler_copy_block(struct compiler *c, basicblock *block)
         result->b_instr[n] = block->b_instr[i];
     }
     result->b_exit = block->b_exit;
+    result->b_nofallthrough = 1;
     return result;
 }
 
@@ -4941,6 +4943,9 @@ compiler_with(struct compiler *c, stmt_ty s, int pos)
     else if (!compiler_with(c, s, pos))
             return 0;
 
+
+    /* Mark all following code as artificial */
+    c->u->u_lineno = -1;
     ADDOP(c, POP_BLOCK);
     compiler_pop_fblock(c, WITH, block);
 
@@ -6412,36 +6417,68 @@ mark_reachable(struct assembler *a) {
     if (stack == NULL) {
         return -1;
     }
-    a->a_entry->b_reachable = 1;
+    a->a_entry->b_predecessors = 1;
     *sp++ = a->a_entry;
     while (sp > stack) {
         basicblock *b = *(--sp);
-        if (b->b_next && !b->b_nofallthrough && b->b_next->b_reachable == 0) {
-            b->b_next->b_reachable = 1;
-            *sp++ = b->b_next;
+        if (b->b_next && !b->b_nofallthrough) {
+            if (b->b_next->b_predecessors == 0) {
+                *sp++ = b->b_next;
+            }
+            b->b_next->b_predecessors++;
         }
         for (int i = 0; i < b->b_iused; i++) {
             basicblock *target;
             if (is_jump(&b->b_instr[i])) {
 >
-                if (target->b_reachable == 0) {
-                    target->b_reachable = 1;
+                if (target->b_predecessors == 0) {
                     *sp++ = target;
                 }
+                target->b_predecessors++;
             }
         }
     }
     PyObject_Free(stack);
     return 0;
 }
 
+static void
+eliminate_empty_basic_blocks(basicblock *entry) {
+    /* Eliminate empty blocks */
+    for (basicblock *b = entry; b != NULL; b = b->b_next) {
+        basicblock *next = b->b_next;
+        if (next) {
+            while (next->b_iused == 0 && next->b_next) {
+                next = next->b_next;
+            }
+            b->b_next = next;
+        }
+    }
+    for (basicblock *b = entry; b != NULL; b = b->b_next) {
+        if (b->b_iused == 0) {
+            continue;
+        }
+        if (is_jump(&b->b_instr[b->b_iused-1])) {
+            basicblock *target = b->b_instr[b->b_iused-1].i_target;
+            while (target->b_iused == 0) {
+>
+            }
+            b->b_instr[b->b_iused-1].i_target = target;
+        }
+    }
+}
+
+
 /* If an instruction has no line number, but it's predecessor in the BB does,
  * then copy the line number. This reduces the size of the line number table,
  * but has no impact on the generated line number events.
  */
 static void
 minimize_lineno_table(struct assembler *a) {
     for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
+        if (b->b_iused == 0) {
+            continue;
+        }
         int prev_lineno = -1;
         for (int i = 0; i < b->b_iused; i++) {
             if (b->b_instr[i].i_lineno < 0) {
@@ -6451,7 +6488,20 @@ minimize_lineno_table(struct assembler *a) {
                 prev_lineno = b->b_instr[i].i_lineno;
             }
         }
-
+        if (!b->b_nofallthrough && b->b_next->b_predecessors == 1) {
+            assert(b->b_next->b_iused);
+            if (b->b_next->b_instr[0].i_lineno < 0) {
+                b->b_next->b_instr[0].i_lineno = prev_lineno;
+            }
+        }
+        if (is_jump(&b->b_instr[b->b_iused-1])) {
+            basicblock *target = b->b_instr[b->b_iused-1].i_target;
+            if (target->b_predecessors == 1) {
+                if (target->b_instr[0].i_lineno < 0) {
+                    target->b_instr[0].i_lineno = prev_lineno;
+                }
+            }
+        }
     }
 }
 
@@ -6472,35 +6522,34 @@ optimize_cfg(struct assembler *a, PyObject *consts)
             return -1;
         }
         clean_basic_block(b);
-        assert(b->b_reachable == 0);
+        assert(b->b_predecessors == 0);
     }
     if (mark_reachable(a)) {
         return -1;
     }
     /* Delete unreachable instructions */
     for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
-       if (b->b_reachable == 0) {
+       if (b->b_predecessors == 0) {
             b->b_iused = 0;
             b->b_nofallthrough = 0;
        }
     }
+    eliminate_empty_basic_blocks(a->a_entry);
     /* Delete jump instructions made redundant by previous step. If a non-empty
        block ends with a jump instruction, check if the next non-empty block
        reached through normal flow control is the target of that jump. If it
        is, then the jump instruction is redundant and can be deleted.
     */
+    int maybe_empty_blocks = 0;
     for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
         if (b->b_iused > 0) {
             struct instr *b_last_instr = &b->b_instr[b->b_iused - 1];
             if (b_last_instr->i_opcode == POP_JUMP_IF_FALSE ||
                 b_last_instr->i_opcode == POP_JUMP_IF_TRUE ||
                 b_last_instr->i_opcode == JUMP_ABSOLUTE ||
                 b_last_instr->i_opcode == JUMP_FORWARD) {
-                basicblock *b_next_act = b->b_next;
-                while (b_next_act != NULL && b_next_act->b_iused == 0) {
-                    b_next_act = b_next_act->b_next;
-                }
-                if (b_last_instr->i_target == b_next_act) {
+                assert(b->b_next == NULL || b->b_next->b_iused);
+                if (b_last_instr->i_target == b->b_next) {
                     b->b_nofallthrough = 0;
                     switch(b_last_instr->i_opcode) {
                         case POP_JUMP_IF_FALSE:
@@ -6513,18 +6562,16 @@ optimize_cfg(struct assembler *a, PyObject *consts)
                         case JUMP_FORWARD:
                             b_last_instr->i_opcode = NOP;
                             clean_basic_block(b);
+                            maybe_empty_blocks = 1;
                             break;
                     }
-                    /* The blocks after this one are now reachable through it */
-                    b_next_act = b->b_next;
-                    while (b_next_act != NULL && b_next_act->b_iused == 0) {
-                        b_next_act->b_reachable = 1;
-                        b_next_act = b_next_act->b_next;
-                    }
                 }
             }
         }
     }
+    if (maybe_empty_blocks) {
+        eliminate_empty_basic_blocks(a->a_entry);
+    }
     minimize_lineno_table(a);
     return 0;
 }