Skip to content
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

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Optimize bytecode for common case.
  • Loading branch information
serhiy-storchaka committed Aug 18, 2019
commit 686bd900a5595137494250f21c6219852b58e57e
84 changes: 41 additions & 43 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,51 +977,49 @@ def jumpy():
Instruction(opname='LOAD_CONST', opcode=100, arg=6, argval='Who let lolcatz into this test suite?', argrepr="'Who let lolcatz into this test suite?'", offset=96, starts_line=None, is_jump_target=False),
Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=98, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=100, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=102, starts_line=20, is_jump_target=True),
Instruction(opname='SETUP_FINALLY', opcode=122, arg=70, argval=176, argrepr='to 176', offset=104, starts_line=None, is_jump_target=False),
Instruction(opname='SETUP_FINALLY', opcode=122, arg=12, argval=120, argrepr='to 120', offset=106, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_CONST', opcode=100, arg=5, argval=1, argrepr='1', offset=108, starts_line=21, is_jump_target=False),
Instruction(opname='LOAD_CONST', opcode=100, arg=7, argval=0, argrepr='0', offset=110, starts_line=None, is_jump_target=False),
Instruction(opname='BINARY_TRUE_DIVIDE', opcode=27, arg=None, argval=None, argrepr='', offset=112, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=114, starts_line=None, is_jump_target=False),
Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=116, starts_line=None, is_jump_target=False),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=28, argval=148, argrepr='to 148', offset=118, starts_line=None, is_jump_target=False),
Instruction(opname='DUP_TOP', opcode=4, arg=None, argval=None, argrepr='', offset=120, starts_line=22, is_jump_target=True),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=2, argval='ZeroDivisionError', argrepr='ZeroDivisionError', offset=122, starts_line=None, is_jump_target=False),
Instruction(opname='COMPARE_OP', opcode=107, arg=10, argval='exception match', argrepr='exception match', offset=124, starts_line=None, is_jump_target=False),
Instruction(opname='POP_JUMP_IF_FALSE', opcode=114, arg=146, argval=146, argrepr='', offset=126, starts_line=None, is_jump_target=False),
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=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),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=28, argval=146, argrepr='to 146', offset=116, starts_line=None, is_jump_target=False),
Instruction(opname='DUP_TOP', opcode=4, arg=None, argval=None, argrepr='', offset=118, starts_line=22, is_jump_target=True),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=2, argval='ZeroDivisionError', argrepr='ZeroDivisionError', offset=120, starts_line=None, is_jump_target=False),
Instruction(opname='COMPARE_OP', opcode=107, arg=10, argval='exception match', argrepr='exception match', offset=122, starts_line=None, is_jump_target=False),
Instruction(opname='POP_JUMP_IF_FALSE', opcode=114, arg=144, argval=144, argrepr='', offset=124, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=126, starts_line=None, is_jump_target=False),
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='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=132, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=134, 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=136, starts_line=None, is_jump_target=False),
Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=138, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=140, starts_line=None, is_jump_target=False),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=142, starts_line=None, is_jump_target=False),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=26, argval=172, argrepr='to 172', offset=144, starts_line=None, is_jump_target=False),
Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=146, starts_line=None, is_jump_target=True),
Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='i', argrepr='i', offset=148, starts_line=25, is_jump_target=True),
Instruction(opname='SETUP_WITH', opcode=143, arg=14, argval=166, argrepr='to 166', offset=150, starts_line=None, is_jump_target=False),
Instruction(opname='STORE_FAST', opcode=125, arg=1, argval='dodgy', argrepr='dodgy', offset=152, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=154, starts_line=26, is_jump_target=False),
Instruction(opname='LOAD_CONST', opcode=100, arg=9, argval='Never reach this', argrepr="'Never reach this'", offset=156, starts_line=None, is_jump_target=False),
Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=158, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=160, starts_line=None, is_jump_target=False),
Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=162, starts_line=None, is_jump_target=False),
Instruction(opname='BEGIN_FINALLY', opcode=53, arg=None, argval=None, argrepr='', offset=164, starts_line=None, is_jump_target=False),
Instruction(opname='WITH_CLEANUP_START', opcode=81, arg=None, argval=None, argrepr='', offset=166, starts_line=None, is_jump_target=True),
Instruction(opname='WITH_CLEANUP_FINISH', opcode=82, arg=None, argval=None, argrepr='', offset=168, starts_line=None, is_jump_target=False),
Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=170, starts_line=None, is_jump_target=False),
Instruction(opname='POP_BLOCK', opcode=87, arg=None, argval=None, argrepr='', offset=172, starts_line=None, is_jump_target=True),
Instruction(opname='BEGIN_FINALLY', opcode=53, arg=None, argval=None, argrepr='', offset=174, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='print', argrepr='print', offset=176, 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=178, starts_line=None, is_jump_target=False),
Instruction(opname='CALL_FUNCTION', opcode=131, arg=1, argval=1, argrepr='', offset=180, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=182, starts_line=None, is_jump_target=False),
Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=184, starts_line=None, is_jump_target=False),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=186, starts_line=None, is_jump_target=False),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=188, starts_line=None, is_jump_target=False),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=190, 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=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),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=26, argval=170, argrepr='to 170', offset=142, starts_line=None, is_jump_target=False),
Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=144, starts_line=None, is_jump_target=True),
Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='i', argrepr='i', offset=146, starts_line=25, is_jump_target=True),
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=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),
Instruction(opname='BEGIN_FINALLY', opcode=53, arg=None, argval=None, argrepr='', offset=162, starts_line=None, is_jump_target=False),
Instruction(opname='WITH_CLEANUP_START', opcode=81, arg=None, argval=None, argrepr='', offset=164, starts_line=None, is_jump_target=True),
Instruction(opname='WITH_CLEANUP_FINISH', opcode=82, arg=None, argval=None, argrepr='', offset=166, starts_line=None, is_jump_target=False),
Instruction(opname='END_FINALLY', opcode=88, arg=None, argval=None, argrepr='', offset=168, starts_line=None, is_jump_target=False),
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=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),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=184, starts_line=None, is_jump_target=False),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=186, starts_line=None, is_jump_target=False),
]

# One last piece of inspect fodder to check the default line number handling
Expand Down
7 changes: 3 additions & 4 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -267,10 +268,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
* we're jumping out of. */
delta++;
}
else if (op == SETUP_FINALLY &&
code[target_addr] != POP_TOP &&
code[target_addr] != DUP_TOP)
{
else if (prevop == LOAD_CONST) {
/* Pops None pushed before SETUP_FINALLY. */
delta++;
}
Expand Down Expand Up @@ -303,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. */
Expand Down
62 changes: 43 additions & 19 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -1664,6 +1664,7 @@ 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);
Expand All @@ -1684,6 +1685,11 @@ compiler_unwind_fblock(struct compiler *c, struct fblockinfo *info,
return 1;

case FINALLY_TRY:
ADDOP(c, POP_BLOCK);
ADDOP_JREL(c, CALL_FINALLY, info->fb_exit);
return 1;

case FINALLY_TRY2:
ADDOP(c, POP_BLOCK);
if (preserve_tos) {
ADDOP(c, ROT_TWO);
Expand Down Expand Up @@ -2847,23 +2853,19 @@ compiler_continue(struct compiler *c)

/* Code generated for "try: <body> finally: <finalbody>" is as follows:

LOAD_CONST None
SETUP_FINALLY L
<code for body>
POP_BLOCK
BEGIN_FINALLY
L:
<code for finalbody>
END_FINALLY
POP_TOP

The special instructions use the block stack. Each block
stack entry contains the instruction that created it (here
SETUP_FINALLY), the level of the value stack at the time the
block stack entry was created, and a label (here L).

LOAD_CONST None:
Pushes a placeholder for the value of "return".
SETUP_FINALLY:
Pushes the current value stack level and the label
onto the block stack.
Expand All @@ -2874,8 +2876,6 @@ compiler_continue(struct compiler *c)
END_FINALLY:
Pops 1 (NULL or int) or 6 entries from the *value* stack and restore
the raised and the caught exceptions they specify.
POP_TOP:
Pops a None pushed before SETUP_FINALLY.

The block stack is unwound when an exception is raised:
when a SETUP_FINALLY entry is found, the raised and the caught
Expand All @@ -2887,18 +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 */
ADDOP_LOAD_CONST(c, Py_None);
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))
Expand All @@ -2909,16 +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);
ADDOP(c, POP_TOP);
compiler_pop_fblock(c, FINALLY_END, end);
return 1;
}

Expand Down
Loading