(Suspected) regression in bytecode dump handling routines #4341

Activity

1. pfalcon commented on Dec 10, 2018

   

   pfalcon

   on Dec 10, 2018

   ContributorAuthor

   More actions

   According to emitbc.c and bc.c MP_BC_LOAD_NAME and related opcodes can
   have one extra byte when MICROPY_OPT_CACHE_MAP_LOOKUP_IN_BYTECODE is
   enabled.

   +    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
   +    );
        if (f == MP_OPCODE_QSTR) {
   -        ip += 3;
   +        ip += 3 + extra_byte;
        } 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
   -        );
   
2. dpgeorge commented on Dec 10, 2018

   

   dpgeorge

   on Dec 10, 2018

   Member

   More actions

   Thanks for bringing this up. But it's not technically a regression because caching in the bytecode was added first, then frozen bytecode, then support for frozen bytecode with caching.

   Just for the record, to understand what could be affected, the timeline is:

   • Jan 2015: bytecode caching 7ee91cf
   • Jan 2016: frozen .mpy files (not supporting caching) 0699c6b
   • Jan 2017: support for caching in frozen bytecode 98458a4

   It's unlikely that any embedded targets use bytecode caching and frozen bytecode because in such a case the frozen bytecode must be stored in RAM, which really limits its usefulness.
3. pfalcon commented on Dec 11, 2018

   

   pfalcon

   on Dec 11, 2018

   ContributorAuthor

   More actions

   But it's not technically a regression

   I stand corrected, and I used all those modals here because I indeed didn't try to investigate it myself (and don't remember all those details by now). All things bytecode are otherwise rather underused in MicroPython, some people show interest in knowing more about it, and having such issues could puzzle them a lot on their learning curve. Thanks for looking into this!
4. dpgeorge commented on Dec 12, 2018

   

   dpgeorge

   on Dec 12, 2018

   Member

   More actions

   Fixed by above-linked PR.
5. dpgeorge

   closed this as completedon Dec 12, 2018