8000 py/persistentcode: Use a .mpy format that's backwards compatible by aykevl · Pull Request #1 · dpgeorge/micropython · GitHub
[go: up one dir, main page]
Skip to content

py/persistentcode: Use a .mpy format that's backwards compatible #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

py/persistentcode: Use a .mpy format that's backwards compatible #1

wants to merge 1 commit into from

Conversation

aykevl
Copy link
@aykevl aykevl commented Sep 20, 2017

This is a slightly different file format, compatible with current .mpy files (previously this branch would break compatibility). Also fixed build errors related to the file format, so it actually builds now.

Tested with the modx example, it works.

@aykevl aykevl force-pushed the dynamic-native-modules-v2 branch from f781df3 to d24f540 Compare September 20, 2017 21:21
@dpgeorge
Copy link
Owner

This is a slightly different file format, compatible with current .mpy files (previously this branch would break compatibility)

The idea with the way I did it was so there could be both bytecode and native code in the one .mpy file. That's for two reasons:

  1. Use of @micropython.native/viper/asm_thumb decorators in a given script can mean that that script will have a mix of bytecode and native code.
  2. Opens the possibility to create a .mpy file that has speed-critical code in native machine code and the rest of the module written in Python, ie bytecode.

Also fixed build errors related to the file format, so it actually builds now.

I'm pretty sure I tested it before pushing... what were the errors exactly?

@aykevl
Copy link
Author
aykevl commented Sep 21, 2017

Oh, asm etc isn't possible right now? I kind of assumed it was. Then it makes sense to include them as well. It would be nice (I think) to keep compatibility, though. I can take a look to see whether it's possible. Being able to include both native code and bytecode would also be nice, hadn't thought of that before.
I do think the version should be incremented when creating incompatible files, though.

IIRC the errors were in mpy-tool.py. I can take a look when I get home.

@dpgeorge
Copy link
Owner

Oh, asm etc isn't possible right now? I kind of assumed it was.

If they were possible then the issue of putting native code into mpy files would be solved, and we could already put compiled C code in them :)

It would be nice (I think) to keep compatibility, though. I can take a look to see whether it's possible.

One way is to have a new flag in the existing 4-byte header that indicates if the mpy is pure bytecode or a mix, and if it's a mix then each function that is loaded must have a type associated with it. But this complicates the mpy loader.

I do think the version should be incremented when creating incompatible files, though.

Yes, the version number would increase when native code capability is added, so it's OK to modify the mpy format.

@aykevl
Copy link
Author
aykevl commented Sep 25, 2017 •
edited
Loading
8000

Steps to reproduce:

  1. make clean in mpy-cross
  2. make clean in ports/unix
  3. make axtls
  4. make

result (maybe different from what I first got as error):

[snip]
CC ../py/../lib/utils/printf.c
CC main.c
CC gccollect.c
LINK mpy-cross
   text	   data	    bss	    dec	    hex	filename
 141387	  14472	    872	 156731	  2643b	mpy-cross
make[1]: Leaving directory '/home/ayke/src/micropython/mpy-cross'
MPY modules/upip.py
MPY modules/upip_utarfile.py
Creating build/frozen_mpy.c
Traceback (most recent call last):
  File "../../tools/mpy-tool.py", line 596, in <module>
    main()
  File "../../tools/mpy-tool.py", line 584, in main
    raw_codes = [read_mpy(file) for file in args.files]
  File "../../tools/mpy-tool.py", line 456, in read_mpy
    return read_raw_code(f)
  File "../../tools/mpy-tool.py", line 431, in read_raw_code
    raise Exception('unsupported code type: %u' % code_type)
TypeError: %u format: a number is required, not str
../../py/mkrules.mk:121: recipe for target 'build/frozen_mpy.c' failed
make: *** [build/frozen_mpy.c] Error 1
make: *** Deleting file 'build/frozen_mpy.c'

(closed the pull request as this isn't about the pull request)

@aykevl aykevl closed this Sep 25, 2017
@dpgeorge
Copy link
Owner

@aykevl the build error was due to Python 2 being used instead of Python 3 which is why I didn't see it initially. Fixed in b19c51c

dpgeorge pushed a commit that referenced this pull request May 30, 2021
@jepler @dpgeorge
py/repl: Don't read past the end of import_str.
d67f411 
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        #1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        #2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        #3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        #4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        #5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        #6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        #7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        #8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <jepler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aykevl @dpgeorge
0