Skip to content
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.

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

Improve build system integration #1

Closed
wants to merge 37 commits into from

Conversation

daniel-thompson
Copy link
Contributor

Hi Paul

I recently upstreamed a change for nRF52 that enabled FPU support (and it immediately broke the uPy build as a result due to incompatible float ABIs).

This is the patch series I wrote to fix it. Tested with qemu_x86 and qemu_cortex_m3 (although in the later case I did have to turn off mbedssl). Older version of this patchset was also tested on Nitrogen but now all the network code is turned on uPy doesn't fit in the nitrogen's tiny, tiny flash.

Daniel.

An interpreted language needs good size of stack to do anything useful
(minimum can be set to a lower value of course).
As it supports QEMU virtual networking.
For this, we first build Z part with libmicropython.a not existing. This
produces autoconf.h with all setiings, but eventually fails on linking
stage due to missing libmicropython.a. We then build MicroPython, which
now can access values in autoconf.h. Then we execute Z build again, which
now succeeds.
Zephyr actually filters these out, and requires patching to let them thru.
And underlying uIP uses them to e.g. communicate changes in connection
state (like peer closed connection). Without this change (and associated
Zephyr patch), socket read requests may hang if peer close happened after
read request was issued.
Unlike standard keyfile= and certfile=, these accept byte buffer objects
(to not depend on FS implementation).
In CPython, module-level .wrap_socket() function actually doesn't accept
(or document) this param, only SSLContext.wrap_socket() has.
This is required to use mbedTLS versions from various sources, e.g.
mainline vs embedded into Zephyr RTOS.
Using Zephyr's builtin mbedtls.
TODO: Think about connection reset in other places too.
@pfalcon pfalcon force-pushed the zephyr branch 5 times, most recently from 81fc205 to 8fa11c2 Compare September 22, 2016 15:09
@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

Previous comment was wrong (removed).

Cherry-picked "zephyr: Use ?= for BOARD", will probably squash it if you don't mind.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

A bit concerned about "zephyr: Support extra make targets", main makefile should provide MicroPython's, not Zephyr's targets (at least so I think now). But there doesn't seem to be conflict so far, merging.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

That "initconfig" target is neat. I initially wanted to "reverse engineer" Z makefiles to see if I can find suitable partial targets. But then I thought: "If I find some internal target, there's no guarantee it won't change in the future. So, maybe not so pretty, but more robust approach is to make Z build as much possible, but eventually fail. Then build libmicropython.a, and restart Z build". Well, getting rid of need to set ARCH is a very good thing, so you patch is cool improvement. But autoconf.h is still needed, so my "build, fail, build completely" approach stays too.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

Except I rename DOTCONFIG to Z_DOTCONFIG for consistency with Z_AUTOCONF_H.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

Unfortunately, your change for automatic ARCH broke my cute hack for automatic target CFLAGS. I figure, make doesn't honor dependencies on include files (i.e. you can't tell it - before including that file, make sure its dependencies are satisfied). I'll see if I can workaround that with recursive make...

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

make doesn't honor dependencies on include files

I worked that around by including non-existent proxy file, which forces make to honor dependencies ;-).

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

Ok, I pushed my part. Please rebase (sorry for conflicts) and test how it works for you. If you look, there're still some hardcoded hacks, but that's very close to supporting any board supported by Zephyr.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

And we should add ability to have multiple prf*.conf files...

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

If you look, there're still some hardcoded hacks

Ok, now even these are gone.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

And nope, it either broke, or include order by make is non-deterministic.

@pfalcon
Copy link
Owner

pfalcon commented Sep 22, 2016

Reworked again, seems to work.

@pfalcon pfalcon force-pushed the zephyr branch 2 times, most recently from 15d8c66 to ef0bf69 Compare September 23, 2016 11:31
@daniel-thompson
Copy link
Contributor Author

Regarding the "build, fail, build again" approach, personally I think it better to see tools integrate better with the Zephyr build system. The third-party-build is an obvious use case (and one they acknowledge) so exploring good ways to integrate with the Zephyr build system is part of nudging a very young project in the right direction.

If you find that at all convincing then you might want the "outputmakefile" target for the build-that-currently-fails!

@pfalcon pfalcon force-pushed the zephyr branch 5 times, most recently from 84d422b to 7b562a2 Compare September 30, 2016 00:35
@pfalcon pfalcon force-pushed the zephyr branch 4 times, most recently from c036655 to 508c539 Compare October 9, 2016 23:54
pfalcon pushed a commit that referenced this pull request Dec 2, 2021
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants