py: Fix VM crash with unwinding jump out of a finally block.

This patch fixes a bug in the VM when breaking within a try-finally.  The
bug has to do with executing a break within the finally block of a
try-finally statement.  For example:

    def f():
        for x in (1,):
            print('a', x)
            try:
                raise Exception
            finally:
                print(1)
                break
            print('b', x)
    f()

Currently in uPy the above code will print:

    a 1
    1
    1
    segmentation fault (core dumped)  micropython

Not only is there a seg fault, but the "1" in the finally block is printed
twice.  This is because when the VM executes a finally block it doesn't
really know if that block was executed due to a fall-through of the try (no
exception raised), or because an exception is active.  In particular, for
nested finallys the VM has no idea which of the nested ones have active
exceptions and which are just fall-throughs.  So when a break (or continue)
is executed it tries to unwind all of the finallys, when in fact only some
may be active.

It's questionable whether break (or return or continue) should be allowed
within a finally block, because they implicitly swallow any active
exception, but nevertheless it's allowed by CPython (although almost never
used in the standard library).  And uPy should at least not crash in such a
case.

The solution here relies on the fact that exception and finally handlers
always appear in the bytecode after the try body.

Note: there was a similar bug with a return in a finally block, but that
was previously fixed in b735208403
This commit is contained in:
Damien George 2019-01-02 17:48:43 +11:00
parent b5f33ac2cb
commit e1fb03f3e2
6 changed files with 114 additions and 12 deletions

View File

@ -1599,7 +1599,6 @@ STATIC void compile_try_except(compiler_t *comp, mp_parse_node_t pn_body, int n_
} }
compile_node(comp, pns_except->nodes[1]); // the <body> compile_node(comp, pns_except->nodes[1]); // the <body>
if (qstr_exception_local != 0) { if (qstr_exception_local != 0) {
EMIT(pop_block);
EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE);
EMIT_ARG(label_assign, l3); EMIT_ARG(label_assign, l3);
EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE);
@ -1635,7 +1634,6 @@ STATIC void compile_try_finally(compiler_t *comp, mp_parse_node_t pn_body, int n
} else { } else {
compile_try_except(comp, pn_body, n_except, pn_except, pn_else); compile_try_except(comp, pn_body, n_except, pn_except, pn_else);
} }
EMIT(pop_block);
EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE); EMIT_ARG(load_const_tok, MP_TOKEN_KW_NONE);
EMIT_ARG(label_assign, l_finally_block); EMIT_ARG(label_assign, l_finally_block);
compile_node(comp, pn_finally); compile_node(comp, pn_finally);
@ -1811,8 +1809,7 @@ STATIC void compile_async_with_stmt_helper(compiler_t *comp, int n, mp_parse_nod
compile_async_with_stmt_helper(comp, n - 1, nodes + 1, body); compile_async_with_stmt_helper(comp, n - 1, nodes + 1, body);
EMIT_ARG(adjust_stack_size, -3); EMIT_ARG(adjust_stack_size, -3);
// Finish the "try" block // We have now finished the "try" block and fall through to the "finally"
EMIT(pop_block);
// At this point, after the with body has executed, we have 3 cases: // At this point, after the with body has executed, we have 3 cases:
// 1. no exception, we just fall through to this point; stack: (..., ctx_mgr) // 1. no exception, we just fall through to this point; stack: (..., ctx_mgr)

View File

@ -738,7 +738,6 @@ void mp_emit_bc_setup_block(emit_t *emit, mp_uint_t label, int kind) {
} }
void mp_emit_bc_with_cleanup(emit_t *emit, mp_uint_t label) { void mp_emit_bc_with_cleanup(emit_t *emit, mp_uint_t label) {
mp_emit_bc_pop_block(emit);
mp_emit_bc_load_const_tok(emit, MP_TOKEN_KW_NONE); mp_emit_bc_load_const_tok(emit, MP_TOKEN_KW_NONE);
mp_emit_bc_label_assign(emit, label); mp_emit_bc_label_assign(emit, label);
emit_bc_pre(emit, 2); // ensure we have enough stack space to call the __exit__ method emit_bc_pre(emit, 2); // ensure we have enough stack space to call the __exit__ method

10
py/vm.c
View File

@ -633,8 +633,6 @@ dispatch_loop:
// replacing it with None, which signals END_FINALLY to just // replacing it with None, which signals END_FINALLY to just
// execute the finally handler normally. // execute the finally handler normally.
SET_TOP(mp_const_none); SET_TOP(mp_const_none);
assert(exc_sp >= exc_stack);
POP_EXC_BLOCK();
} else { } else {
// We need to re-raise the exception. We pop __exit__ handler // We need to re-raise the exception. We pop __exit__ handler
// by copying the exception instance down to the new top-of-stack. // by copying the exception instance down to the new top-of-stack.
@ -654,7 +652,7 @@ unwind_jump:;
while ((unum & 0x7f) > 0) { while ((unum & 0x7f) > 0) {
unum -= 1; unum -= 1;
assert(exc_sp >= exc_stack); assert(exc_sp >= exc_stack);
if (MP_TAGPTR_TAG1(exc_sp->val_sp)) { if (MP_TAGPTR_TAG1(exc_sp->val_sp) && exc_sp->handler > ip) {
// Getting here the stack looks like: // Getting here the stack looks like:
// (..., X, dest_ip) // (..., X, dest_ip)
// where X is pointed to by exc_sp->val_sp and in the case // where X is pointed to by exc_sp->val_sp and in the case
@ -698,6 +696,8 @@ unwind_jump:;
// if TOS is an integer, finishes coroutine and returns control to caller // if TOS is an integer, finishes coroutine and returns control to caller
// if TOS is an exception, reraises the exception // if TOS is an exception, reraises the exception
if (TOP() == mp_const_none) { if (TOP() == mp_const_none) {
assert(exc_sp >= exc_stack);
POP_EXC_BLOCK();
sp--; sp--;
} else if (mp_obj_is_small_int(TOP())) { } else if (mp_obj_is_small_int(TOP())) {
// We finished "finally" coroutine and now dispatch back // We finished "finally" coroutine and now dispatch back
@ -1063,7 +1063,7 @@ unwind_jump:;
unwind_return: unwind_return:
// Search for and execute finally handlers that aren't already active // Search for and execute finally handlers that aren't already active
while (exc_sp >= exc_stack) { while (exc_sp >= exc_stack) {
if (!currently_in_except_block && MP_TAGPTR_TAG1(exc_sp->val_sp)) { if (!currently_in_except_block && MP_TAGPTR_TAG1(exc_sp->val_sp) && exc_sp->handler > ip) {
// Found a finally handler that isn't active. // Found a finally handler that isn't active.
// Getting here the stack looks like: // Getting here the stack looks like:
// (..., X, [iter0, iter1, ...,] ret_val) // (..., X, [iter0, iter1, ...,] ret_val)
@ -1419,7 +1419,7 @@ unwind_loop:
mp_obj_exception_add_traceback(MP_OBJ_FROM_PTR(nlr.ret_val), source_file, source_line, block_name); mp_obj_exception_add_traceback(MP_OBJ_FROM_PTR(nlr.ret_val), source_file, source_line, block_name);
} }
while (currently_in_except_block) { while (currently_in_except_block || (exc_sp >= exc_stack && exc_sp->handler <= code_state->ip)) {
// nested exception // nested exception
assert(exc_sp >= exc_stack); assert(exc_sp >= exc_stack);

View File

@ -0,0 +1,99 @@
# test break within (nested) finally
# basic case with break in finally
def f():
for _ in range(2):
print(1)
try:
pass
finally:
print(2)
break
print(3)
print(4)
print(5)
f()
# where the finally swallows an exception
def f():
lst = [1, 2, 3]
for x in lst:
print('a', x)
try:
raise Exception
finally:
print(1)
break
print('b', x)
f()
# basic nested finally with break in inner finally
def f():
for i in range(2):
print('iter', i)
try:
raise TypeError
finally:
print(1)
try:
raise ValueError
finally:
break
print(f())
# similar to above but more nesting
def f():
for i in range(2):
try:
raise ValueError
finally:
print(1)
try:
raise TypeError
finally:
print(2)
try:
pass
finally:
break
print(f())
# lots of nesting
def f():
for i in range(2):
try:
raise ValueError
finally:
print(1)
try:
raise TypeError
finally:
print(2)
try:
raise Exception
finally:
break
print(f())
# basic case combined with try-else
def f(arg):
for _ in range(2):
print(1)
try:
if arg == 1:
raise ValueError
elif arg == 2:
raise TypeError
except ValueError:
print(2)
else:
print(3)
finally:
print(4)
break
print(5)
print(6)
print(7)
f(0) # no exception, else should execute
f(1) # exception caught, else should be skipped
f(2) # exception not caught, finally swallows exception, else should be skipped

View File

@ -1,5 +1,14 @@
# test use of return with try-except # test use of return with try-except
def f():
try:
print(1)
return
except:
print(2)
print(3)
f()
def f(l, i): def f(l, i):
try: try:
return l[i] return l[i]

View File

@ -265,7 +265,6 @@ Raw bytecode (code_info_size=\\d\+, bytecode_size=\\d\+):
\\d\+ POP_EXCEPT \\d\+ POP_EXCEPT
\\d\+ JUMP \\d\+ \\d\+ JUMP \\d\+
\\d\+ END_FINALLY \\d\+ END_FINALLY
\\d\+ POP_BLOCK
\\d\+ LOAD_CONST_NONE \\d\+ LOAD_CONST_NONE
\\d\+ LOAD_FAST 1 \\d\+ LOAD_FAST 1
\\d\+ POP_TOP \\d\+ POP_TOP
@ -286,7 +285,6 @@ Raw bytecode (code_info_size=\\d\+, bytecode_size=\\d\+):
\\d\+ POP_TOP \\d\+ POP_TOP
\\d\+ LOAD_DEREF 14 \\d\+ LOAD_DEREF 14
\\d\+ POP_TOP \\d\+ POP_TOP
\\d\+ POP_BLOCK
\\d\+ LOAD_CONST_NONE \\d\+ LOAD_CONST_NONE
\\d\+ WITH_CLEANUP \\d\+ WITH_CLEANUP
\\d\+ END_FINALLY \\d\+ END_FINALLY