bpo-37830: Fix compilation of break and continue in finally.

Lib/importlib/_bootstrap_external.py

@@ -270,6 +270,7 @@ def _write_atomic(path, data, mode=0o666):
 #                         comprehensions #35224)
 #     Python 3.8b2  3412 (Swap the position of positional args and positional
 #                         only args in ast.arguments #37593)
+#     Python 3.8b4  3413 (Fix "break" and "continue" in "finally" #37830)
 #
 # MAGIC must change whenever the bytecode emitted by the compiler may no
 # longer be understood by older implementations of the eval loop (usually
@@ -278,7 +279,7 @@ def _write_atomic(path, data, mode=0o666):
 # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
 # in PC/launcher.c must also be updated.
 
-MAGIC_NUMBER = (3412).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3413).to_bytes(2, 'little') + b'\r\n'
 _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little')  # For import.c
 
 _PYCACHE = '__pycache__'

Lib/test/test_dis.py

@@ -980,7 +980,7 @@ def jumpy():
   Instruction(opname='SETUP_FINALLY', opcode=122, arg=70, argval=174, argrepr='to 174', offset=102, starts_line=20, is_jump_target=True),
   Instruction(opname='SETUP_FINALLY', opcode=122, arg=12, argval=118, argrepr='to 118', offset=104, starts_line=None, is_jump_target=False),
   Instruction(opname='LOAD_CONST', opcode=100, arg=5, argval=1, argrepr='1', offset=106, starts_line=21, is_jump_target=False),
-  Instruction(opname='LOAD_CONST', opcode=100, arg=7, argval=0, argrepr='0', offset=108, starts_line=None, is_jump_target=False),
+  Instruction(opname='LOAD_CONST', opcode=100, arg=8, argval=0, argrepr='0', offset=108, starts_line=None, is_jump_target=False),
   Instruction(opname='BINARY_TRUE_DIVIDE', opcode=27, arg=None, argval=None, argrepr='', offset=110, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=112, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=114, starts_line=None, is_jump_target=False),
@@ -993,7 +993,7 @@ def jumpy():
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=128, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=130, starts_line=None, is_jump_target=False),
   Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=132, starts_line=23, is_jump_target=False),
-  Instruction(opname='LOAD_CONST', opcode=100, arg=8, argval='Here we go, here we go, here we go...', argrepr="'Here we go, here we go, here we go...'", offset=134, starts_line=None, is_jump_target=False),
+  Instruction(opname='LOAD_CONST', opcode=100, arg=9, argval='Here we go, here we go, here we go...', argrepr="'Here we go, here we go, here we go...'", offset=134, starts_line=None, is_jump_target=False),
   Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=136, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=138, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=140, starts_line=None, is_jump_target=False),
@@ -1003,7 +1003,7 @@ def jumpy():
   Instruction(opname='SETUP_WITH', opcode=143, arg=14, argval=164, argrepr='to 164', offset=148, starts_line=None, is_jump_target=False),
   Instruction(opname='STORE_FAST', opcode=125, arg=1, argval='dodgy', argrepr='dodgy', offset=150, starts_line=None, is_jump_target=False),
   Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=152, starts_line=26, is_jump_target=False),
-  Instruction(opname='LOAD_CONST', opcode=100, arg=9, argval='Never reach this', argrepr="'Never reach this'", offset=154, starts_line=None, is_jump_target=False),
+  Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval='Never reach this', argrepr="'Never reach this'", offset=154, starts_line=None, is_jump_target=False),
   Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=156, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=158, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=160, starts_line=None, is_jump_target=False),
@@ -1014,7 +1014,7 @@ def jumpy():
   Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=170, starts_line=None, is_jump_target=True),
   Instruction(opname='BEGIN_FINALLY', opcode=53, arg=None, argval=None, argrepr='', offset=172, starts_line=None, is_jump_target=False),
   Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=174, starts_line=28, is_jump_target=True),
-  Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=176, starts_line=None, is_jump_target=False),
+  Instruction(opname='LOAD_CONST', opcode=100, arg=7, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=176, starts_line=None, is_jump_target=False),
   Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=178, starts_line=None, is_jump_target=False),
   Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=180, starts_line=None, is_jump_target=False),
   Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=182, starts_line=None, is_jump_target=False),

Lib/test/test_grammar.py

@@ -991,6 +991,60 @@ def g3():
                 return 4
         self.assertEqual(g3(), 4)
 
+    def test_break_in_finally_after_return(self):
+        # See issue #37830
+        def g1(x):
+            for count in [0, 1]:
+                count2 = 0
+                while count2 < 20:
+                    count2 += 10
+                    try:
+                        return count + count2
+                    finally:
+                        if x:
+                            break
+            return 'end', count, count2
+        self.assertEqual(g1(False), 10)
+        self.assertEqual(g1(True), ('end', 1, 10))
+
+        def g2(x):
+            for count in [0, 1]:
+                for count2 in [10, 20]:
+                    try:
+                        return count + count2
+                    finally:
+                        if x:
+                            break
+            return 'end', count, count2
+        self.assertEqual(g2(False), 10)
+        self.assertEqual(g2(True), ('end', 1, 10))
+
+    def test_continue_in_finally_after_return(self):
+        # See issue #37830
+        def g1(x):
+            count = 0
+            while count < 100:
+                count += 1
+                try:
+                    return count
+                finally:
+                    if x:
+                        continue
+            return 'end', count
+        self.assertEqual(g1(False), 1)
+        self.assertEqual(g1(True), ('end', 100))
+
+        def g2(x):
+            for count in [0, 1]:
+                try:
+                    return count
+                finally:
+                    if x:
+                        continue
+            return 'end', count
+        self.assertEqual(g2(False), 0)
+        self.assertEqual(g2(True), ('end', 1))
+
     def test_yield(self):
         # Allowed as standalone statement
         def g(): yield 1

Lib/test/test_importlib/test_util.py

@@ -861,7 +861,7 @@ def test_magic_number(self):
         in advance. Such exceptional releases will then require an
         adjustment to this test case.
         """
-        EXPECTED_MAGIC_NUMBER = 3410
+        EXPECTED_MAGIC_NUMBER = 3413
         actual = int.from_bytes(importlib.util.MAGIC_NUMBER[:2], 'little')
 
         msg = (

Misc/NEWS.d/next/Core and Builtins/2019-08-17-18-41-59.bpo-37830.fNfMbz.rst

@@ -0,0 +1,3 @@
+Fixed compilation of :keyword:`break` and :keyword:`continue` in the
+:keyword:`finally` block when the corresponding :keyword:`try` block
+contains :keyword:`return` with a non-constant value.

Objects/frameobject.c

@@ -233,6 +233,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
      * the 'finally' blocks. */
     memset(blockstack, '\0', sizeof(blockstack));
     blockstack_top = 0;
+    unsigned char prevop = NOP;
     for (addr = 0; addr < code_len; addr += sizeof(_Py_CODEUNIT)) {
         unsigned char op = code[addr];
         switch (op) {
@@ -259,17 +260,24 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
                                 "can't jump into the middle of a block");
                 return -1;
             }
+            int in_for_loop = op == FOR_ITER || code[target_addr] == END_ASYNC_FOR;
             if (first_in && !second_in) {
-                if (op != FOR_ITER && code[target_addr] != END_ASYNC_FOR) {
-                    delta_iblock++;
+                if (!delta_iblock) {
+                    if (in_for_loop) {
+                        /* Pop the iterators of any 'for' and 'async for' loop
+                         * we're jumping out of. */
+                        delta++;
+                    }
+                    else if (prevop == LOAD_CONST) {
+                        /* Pops None pushed before SETUP_FINALLY. */
+                        delta++;
+                    }
                 }
-                else if (!delta_iblock) {
-                    /* Pop the iterators of any 'for' and 'async for' loop
-                     * we're jumping out of. */
-                    delta++;
+                if (!in_for_loop) {
+                    delta_iblock++;
                 }
             }
-            if (op != FOR_ITER && code[target_addr] != END_ASYNC_FOR) {
+            if (!in_for_loop) {
                 blockstack[blockstack_top++] = target_addr;
             }
             break;
@@ -293,6 +301,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
             break;
         }
         }
+        prevop = op;
     }
 
     /* Verify that the blockstack tracking code didn't get lost. */

PC/launcher.c

@@ -1139,7 +1139,7 @@ static PYC_MAGIC magic_values[] = {
     { 3320, 3351, L"3.5" },
     { 3360, 3379, L"3.6" },
     { 3390, 3399, L"3.7" },
-    { 3400, 3410, L"3.8" },
+    { 3400, 3419, L"3.8" },
     { 0 }
 };
 

Python/compile.c

@@ -81,7 +81,7 @@ It's called a frame block to distinguish it from a basic block in the
 compiler IR.
 */
 
-enum fblocktype { WHILE_LOOP, FOR_LOOP, EXCEPT, FINALLY_TRY, FINALLY_END,
+enum fblocktype { WHILE_LOOP, FOR_LOOP, EXCEPT, FINALLY_TRY, FINALLY_TRY2, FINALLY_END,
                   WITH, ASYNC_WITH, HANDLER_CLEANUP };
 
 struct fblockinfo {
@@ -1664,7 +1664,12 @@ compiler_unwind_fblock(struct compiler *c, struct fblockinfo *info,
             return 1;
 
         case FINALLY_END:
+            info->fb_exit = NULL;
             ADDOP_I(c, POP_FINALLY, preserve_tos);
+            if (preserve_tos) {
+                ADDOP(c, ROT_TWO);
+            }
+            ADDOP(c, POP_TOP);
             return 1;
 
         case FOR_LOOP:
@@ -1684,6 +1689,19 @@ compiler_unwind_fblock(struct compiler *c, struct fblockinfo *info,
             ADDOP_JREL(c, CALL_FINALLY, info->fb_exit);
             return 1;
 
+        case FINALLY_TRY2:
+            ADDOP(c, POP_BLOCK);
+            if (preserve_tos) {
+                ADDOP(c, ROT_TWO);
+                ADDOP(c, POP_TOP);
+                ADDOP_JREL(c, CALL_FINALLY, info->fb_exit);
+            }
+            else {
+                ADDOP_JREL(c, CALL_FINALLY, info->fb_exit);
+                ADDOP(c, POP_TOP);
+            }
+            return 1;
+
         case WITH:
         case ASYNC_WITH:
             ADDOP(c, POP_BLOCK);
@@ -2869,17 +2887,47 @@ compiler_continue(struct compiler *c)
 static int
 compiler_try_finally(struct compiler *c, stmt_ty s)
 {
-    basicblock *body, *end;
+    basicblock *start, *newcurblock, *body, *end;
+    int break_finally = 1;
 
     body = compiler_new_block(c);
     end = compiler_new_block(c);
     if (body == NULL || end == NULL)
         return 0;
 
+    start = c->u->u_curblock;
+
+    /* `finally` block. Compile it first to determine if any of "break",
+       "continue" or "return" are used in it. */
+    compiler_use_next_block(c, end);
+    if (!compiler_push_fblock(c, FINALLY_END, end, end))
+        return 0;
+    VISIT_SEQ(c, stmt, s->v.Try.finalbody);
+    ADDOP(c, END_FINALLY);
+    break_finally = (c->u->u_fblock[c->u->u_nfblocks - 1].fb_exit == NULL);
+    if (break_finally) {
+        /* Pops a placeholder. See below */
+        ADDOP(c, POP_TOP);
+    }
+    compiler_pop_fblock(c, FINALLY_END, end);
+
+    newcurblock = c->u->u_curblock;
+    c->u->u_curblock = start;
+    start->b_next = NULL;
+
     /* `try` block */
+    c->u->u_lineno_set = 0;
+    c->u->u_lineno = s->lineno;
+    c->u->u_col_offset = s->col_offset;
+    if (break_finally) {
+        /* Pushes a placeholder for the value of "return" in the "try" block
+           to balance the stack for "break", "continue" and "return" in
+           the "finally" block. */
+        ADDOP_LOAD_CONST(c, Py_None);
+    }
     ADDOP_JREL(c, SETUP_FINALLY, end);
     compiler_use_next_block(c, body);
-    if (!compiler_push_fblock(c, FINALLY_TRY, body, end))
+    if (!compiler_push_fblock(c, break_finally ? FINALLY_TRY2 : FINALLY_TRY, body, end))
         return 0;
     if (s->v.Try.handlers && asdl_seq_LEN(s->v.Try.handlers)) {
         if (!compiler_try_except(c, s))
@@ -2890,15 +2938,11 @@ compiler_try_finally(struct compiler *c, stmt_ty s)
     }
     ADDOP(c, POP_BLOCK);
     ADDOP(c, BEGIN_FINALLY);
-    compiler_pop_fblock(c, FINALLY_TRY, body);
+    compiler_pop_fblock(c, break_finally ? FINALLY_TRY2 : FINALLY_TRY, body);
+
+    c->u->u_curblock->b_next = end;
+    c->u->u_curblock = newcurblock;
 
-    /* `finally` block */
-    compiler_use_next_block(c, end);
-    if (!compiler_push_fblock(c, FINALLY_END, end, NULL))
-        return 0;
-    VISIT_SEQ(c, stmt, s->v.Try.finalbody);
-    ADDOP(c, END_FINALLY);
-    compiler_pop_fblock(c, FINALLY_END, end);
     return 1;
 }