From 6bf8ecfe3ab08c702c4e002dc150fceed31176e6 Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 11 Dec 2018 00:46:38 +1100 Subject: [PATCH] py/bc: Fix calculation of opcode size for opcodes with map caching. All 4 opcodes that can have caching bytes also have qstrs, so the test for them must go in the qstr part of the code. The reason this incorrect calculation of the opcode size did not lead to a bug is because the caching byte is at the end of the opcode (byte, qstr, qstr, cache) and is always 0x00 when saving/loading, so was just treated as a single byte no-op opcode. Hence these opcodes were being saved/loaded/decoded correctly. Thanks to @malinah for finding the problem and providing the initial patch. --- py/bc.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/py/bc.c b/py/bc.c index 89d8b74f91..1d6e4322b2 100644 --- a/py/bc.c +++ b/py/bc.c @@ -292,7 +292,7 @@ continue2:; // MP_BC_MAKE_CLOSURE_DEFARGS // MP_BC_RAISE_VARARGS // There are 4 special opcodes that have an extra byte only when -// MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE is enabled: +// MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE is enabled (and they take a qstr): // MP_BC_LOAD_NAME // MP_BC_LOAD_GLOBAL // MP_BC_LOAD_ATTR @@ -386,18 +386,20 @@ uint mp_opcode_format(const byte *ip, size_t *opcode_size) { uint f = (opcode_format_table[*ip >> 2] >> (2 * (*ip & 3))) & 3; const byte *ip_start = ip; if (f == MP_OPCODE_QSTR) { + if (MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE_DYNAMIC) { + if (*ip == MP_BC_LOAD_NAME + || *ip == MP_BC_LOAD_GLOBAL + || *ip == MP_BC_LOAD_ATTR + || *ip == MP_BC_STORE_ATTR) { + ip += 1; + } + } ip += 3; } else { int extra_byte = ( *ip == MP_BC_RAISE_VARARGS || *ip == MP_BC_MAKE_CLOSURE || *ip == MP_BC_MAKE_CLOSURE_DEFARGS - #if MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE - || *ip == MP_BC_LOAD_NAME - || *ip == MP_BC_LOAD_GLOBAL - || *ip == MP_BC_LOAD_ATTR - || *ip == MP_BC_STORE_ATTR - #endif ); ip += 1; if (f == MP_OPCODE_VAR_UINT) {