-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Enable exceptions, update to optimized newlib, migrate to new toolchain #5376
Conversation
A Newlib (libc) patch is in progress to move the _P functions from inside Arduino into first-class citizens in libc. This Arduino patch cleans up code that's been migrated there. Binaries for the new libs are included because it seems they're part of the Arduino git tree, and should be replaced with @igrr built ones when/if the Newlib changes are accepted. Notable changes/additions for Arduino: Allow for use of PROGMEM based format and parameter strings in all *printf functions. No need for copying PSTR()s into RAM before printing them out (transparently saves heap space when using _P functions) and makes it easier to print out constant strings for applications. Add "%S" (capital-S) format that I've been told, but cannot verify, is used in Arduino to specify a PROGMEM string parameter in printfs, as an alias for "%s" since plain "%s" can now handle PROGMEM. Optimized the memcpy_P, strnlen_P, and strncpy_P functions to use 32-bit direct reads whenver possible (source and dest alignment mediated), but there is still room for improvement in others. Finally, move several constant arrays from RODATA into PROGMEM and update their accessors. Among these are the ctype array, ~260 bytes, mprec* arrays, ~300 bytes, and strings/daycounts in the time formatting functions, ~200 bytes. All told, sketches will see from 300 to 800 additional RAM heap free on startup (depending on their use of these routines).
Host tests now use the sys/pgmspace.h for compiles instead of the ESP8266-specific version.
Rebuild the binaries using a git clone of https://github.com/igrr/newlib-xtensa Build commands for posterity: ```` rm -rf ./xtensa-lx106-elf/ ./configure --prefix=<DIR>/esp8266/tools/sdk/libc --with-newlib \ --enable-multilib --disable-newlib-io-c99-formats \ --disable-newlib-supplied-syscalls \ --enable-newlib-nano-formatted-io --enable-newlib-reent-small \ --enable-target-optspace \ --program-transform-name="s&^&xtensa-lx106-elf-&" \ --disable-option-checking --with-target-subdir=xtensa-lx106-elf \ --target=xtensa-lx106-elf rm -f etc/config.cache CROSS_CFLAGS="-fno-omit-frame-pointer -DSIGNAL_PROVIDED -DABORT_PROVIDED"\ " -DMALLOC_PROVIDED" \ PATH=<DIR>/esp8266/tools/xtensa-lx106-elf/bin/:$PATH \ make all install ````
Include fix from newlib-xtensa/fix-strlen branch cleaning up misaligned access on a non-aligned source string.
Ran the included test suite on ESP8266 tstring.c with the following defines: #define MAX_1 50 #define memcmp memcmp_P #define memcpy memcpy_P #define memmem memmem_P #define memchr memchr_P #define strcat strcat_P #define strncat strncat_P #define strcpy strcpy_P #define strlen strlen_P #define strnlen strnlen_P #define strcmp strcmp_P #define strncmp strncmp_P Uncovered edge case and return value problems in the optimized versions of the strnlen_P and strncpy_P functions. Corrected.
memcpy-1.c test suite showed error in return value of memcpy_P. Correct it.
Random crashes, often on String constructors using a PSTR, would occur due to the accelerated strnlen_P going past the end of the string. Would make debug builds fail, too (ESP.getVersionString() failure). Fix to fall through to normal copy on a word that's got a 0 byte anywhere in it.
Add test suite used to debug libc optimized _P functions to the device tests.
Rebuild .a from igrr's repo at 347260af117b4177389e69fd4d04169b11d87a97
Need to look at the quick-toolchain options, there seems to be a definition for atexit defined there (libgcc?) that needs to be excised. For now, remove our local do-nothing copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally, LGTM!
My one comment is about reducing the oom emergency heap to only 1 or 2 objects, which would reduce the heap price. I suggest reducing to 1, then test a throw-catch-rethrow sequence during oom. I think that if that works we can leave it at 1. If not, test with 2.
I'm approving as-is based on our internal discussion and testing, which is good enough for me!
And congratulations to both @d-a-v and @earlephilhower for getting this to work!
Still got a way to go before it's done. Debug builds seem to overflow, number of emergency OOM spaces needs to be tested, the libc replacement That said it seems stable enough to be ready for people who need/want exceptions to give it a shakedown and help find hidden issues beyond the ones I know about. |
The esp-quick-toolchain generated libgcc.a needed to have the soft-FP routines that are in ROM removed from it. Remove them in the new esp-quick-toolchain and update.
I just downloaded Arduino for Windows, got this branch ran the get.py, and was able to build an exception-enabled sketch without any issues. Combining with PR #4160 which was approved but overlapped this since we had a few newlib changes required. Need to make sure any merge commit messages include that one's commit info as there were some new options in printf and big speedups in some operations. To test on your own, check out this PR and re-run tools/get.py so it grabs the new compiler, toolset, and headers. Restart Arduino to get the new flags, and off you go. |
When doing the panic on unhandled exceptions, try and grab the .what() pointer and dump it as part of the termination info. Makes it easy to see mem errors (std::bad_alloc) or std::runtime_error strings.
9ee3f77
to
f5e80fb
Compare
Just discovered a problem with the merge of newlib where updated header files aren't quite correct. IT works fine, but does not include all of the updates. Expect a fix on the 2nd. |
Makes sure proper libraries and includes are present by using a scripted installation from esp-quick-install instead of a manual one.
esp-quick-toolchain now does a full newlib install to the Arduion dirs and copies the required Testing looks good, so we're good to go AFAICT. |
Updates framework to build using GCC 9.2.0 toolchain for C++17. Toolchains are at https://github.com/earlephilhower/esp-quick-toolchain/releases. See `docs/source/arch/esp8266/getting-started/eqt.rst` for installation details. (Also at https://smingdev.readthedocs.io/en/dev-esp-quick-toolchain/arch/esp8266/getting-started/eqt.html) Extract the toolchain to a suitable location, e.g. `opt/esp-quick-toolchain` or `C:\tools\esp-quick-toolchain`, and set `ESP_HOME` accordingly. The main change for the new toolchain is that the core PROGMEM handling stuff is included in `sys/pgmspace.h`. See esp8266/Arduino#5376 for more details. Also includes de-duplication for NUL-terminated PSTR definitions.
This is a superset of #4694 that enables exceptions in user and library code by rebuilding the stdlibc++ to enable them, with patches (see esp-quick-toolchain for the patches to GCC building required).
It takes the work @d-a-v started, combines it with @igrr's suggestions in the original thread, fixes whatever problems I found, and makes exception handling "just work" on the 8266.
The current cost is ~1.2KB of RAM and ~19K of ROM (plus whatever add'l exception handling blocks your app would contain). Looking to reduce that <1KB by limiting the emergency exception area (at the cost of OOM exceptions possibly double-faulting, but probably better than wasting 1KB of RAM for all apps).
It works as well as I've tested it (which is minimal), but it's 99.9% libstdc++ and G++, so I hope there's little for me to have flubbed.
WIP to keep the good stuff that #4694 started going.