py/vm: Change comparison for finally handler search from > to >=.

The search in these cases should include all finally handlers that are
after the current ip.  If a handler starts at exactly ip then it is
considered "after" the ip.  This can happen when END_FINALLY is followed
immediately by a finally handler (from a different finally).

Consider the function:

    def f():
        try:
            return 0
        finally:
            print(1)

The current bytecode emitter generates the following code:

    00 SETUP_FINALLY 5
    02 LOAD_CONST_SMALL_INT 0
    03 RETURN_VALUE
    04 LOAD_CONST_NONE              ****
    05 LOAD_GLOBAL print
    07 LOAD_CONST_SMALL_INT 1
    08 CALL_FUNCTION n=1 nkw=0
    10 POP_TOP
    11 END_FINALLY
    12 LOAD_CONST_NONE
    13 RETURN_VALUE

The LOAD_CONST_NONE marked with **** is dead code because it follows a
RETURN_VALUE, and nothing jumps to this LOAD_CONST_NONE.  If the emitter
could remove this this dead code it would produce:

    00 SETUP_FINALLY 4
    02 LOAD_CONST_SMALL_INT 0
    03 RETURN_VALUE
    04 LOAD_GLOBAL print
    06 LOAD_CONST_SMALL_INT 1
    07 CALL_FUNCTION n=1 nkw=0
    09 POP_TOP
    10 END_FINALLY
    11 LOAD_CONST_NONE
    12 RETURN_VALUE

In this case the finally block (which starts at offset 4) immediately
follows the RETURN_VALUE.  When RETURN_VALUE executes ip will point to
offset 4 in the bytecode (because the dispatch of the opcode does *ip++)
and so the finally handler will only be found if a >= comparison is used.

It's a similar story for break/continue:

    while True:
        try:
            break
        finally:
            print(1)

Although technically in this case the > comparison still works because the
extra byte from the UNWIND_JUMP (encoding the number of exception handlers
to unwind) doesn't have a *ip++ (just a *ip) so ip remains pointing within
the UNWIND_JUMP opcode, and not at the start of the following finally
handler.  Nevertheless, the change is made to use >= for consistency with
the RETURN_VALUE change.

Signed-off-by: Damien George <damien@micropython.org>
This commit is contained in:
Damien George 2022-06-17 23:01:55 +10:00
parent 794773cdf2
commit 0db046b67b

View File

@ -658,7 +658,7 @@ unwind_jump:;
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)) {
if (exc_sp->handler > ip) { if (exc_sp->handler >= ip) {
// Found a finally handler that isn't active; run it. // Found a finally handler that isn't active; run it.
// Getting here the stack looks like: // Getting here the stack looks like:
// (..., X, dest_ip) // (..., X, dest_ip)
@ -1079,7 +1079,7 @@ 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 (MP_TAGPTR_TAG1(exc_sp->val_sp)) { if (MP_TAGPTR_TAG1(exc_sp->val_sp)) {
if (exc_sp->handler > ip) { if (exc_sp->handler >= ip) {
// Found a finally handler that isn't active; run it. // Found a finally handler that isn't active; run it.
// Getting here the stack looks like: // Getting here the stack looks like:
// (..., X, [iter0, iter1, ...,] ret_val) // (..., X, [iter0, iter1, ...,] ret_val)