py/bc: Fix calculation of opcode size for opcodes with map caching.

dpgeorge · dpgeorge · commit 6bf8ecfe3ab0 · 2018-12-13T01:26:55.000+11:00
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.
diff --git 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) {
