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
Update toolchains to C++17 #1825
Conversation
cee146a
to
83a0e4c
Compare
54f318d
to
23cec4a
Compare
23cec4a
to
bfface8
Compare
…en use a link wrapper
Keeps `esp8266` Component focused on SDK Add `libc_replacements` definitions required by newlib
bfface8
to
997f2e4
Compare
OK, ready for review. |
.gitmodules
Outdated
@@ -71,7 +71,7 @@ | |||
|
|||
[submodule "Host.lwip"] | |||
path = Sming/Arch/Host/Components/lwip/lwip | |||
url = git://git.savannah.gnu.org/lwip.git | |||
url = https://git.savannah.nongnu.org/git/lwip.git |
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.
@mikee47 Why is that change here?
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.
#1988 refers. This same repo. is used as a submodule in esp8266 LWIP, using this URL. It didn't fix the problem so can pull this commit out and deal with that separately.
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.
so can pull this commit out and deal with that separately.
Yes, please.
diff --git a/lib/libc.a b/lib/libc_.a | ||
rename from lib/libc.a | ||
rename to lib/libc_.a | ||
diff --git a/lib/libgcc.a b/lib/libgcc_.a |
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.
Why is the underscore renaming here?
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.
Renaming them eliminates the risk of their being mistakely picked up instead of the newlib ones.
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.
Ok, can you rename them to lib(x).a -> lib(x).orig.a instead of underscore?
COMPONENT_VARS += UMM_POISON_CHECK | ||
ifeq ($(UMM_POISON_CHECK),1) | ||
GLOBAL_CFLAGS += -DUMM_POISON_CHECK | ||
endif | ||
|
||
COMPONENT_CFLAGS += -Wno-array-bounds |
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.
Why is that change needed?
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.
If you comment this out then try compiling with ENABLE_CUSTOM_HEAP=1
:
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c: In function 'umm_init':
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:73:32: error: array subscript 1 is outside array bounds of 'char[1]' [-Werror=array-bounds]
73 | #define UMM_BLOCK(b) (umm_heap[b])
| ~~~~~~~~~^~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:75:24: note: in expansion of macro 'UMM_BLOCK'
75 | #define UMM_NBLOCK(b) (UMM_BLOCK(b).header.used.next)
| ^~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:231:5: note: in expansion of macro 'UMM_NBLOCK'
231 | UMM_NBLOCK(block_1th) = block_last | UMM_FREELIST_MASK;
| ^~~~~~~~~~
In file included from s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:35:
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc_cfg.h:60:13: note: while referencing '_heap_start'
60 | extern char _heap_start;
| ^~~~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:73:32: error: array subscript 1 is outside array bounds of 'char[1]' [-Werror=array-bounds]
73 | #define UMM_BLOCK(b) (umm_heap[b])
| ~~~~~~~~~^~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:77:24: note: in expansion of macro 'UMM_BLOCK'
77 | #define UMM_NFREE(b) (UMM_BLOCK(b).body.free.next)
| ^~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:232:5: note: in expansion of macro 'UMM_NFREE'
232 | UMM_NFREE(block_1th) = 0;
| ^~~~~~~~~
In file included from s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:35:
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc_cfg.h:60:13: note: while referencing '_heap_start'
60 | extern char _heap_start;
| ^~~~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:73:32: error: array subscript 1 is outside array bounds of 'char[1]' [-Werror=array-bounds]
73 | #define UMM_BLOCK(b) (umm_heap[b])
| ~~~~~~~~~^~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:76:24: note: in expansion of macro 'UMM_BLOCK'
76 | #define UMM_PBLOCK(b) (UMM_BLOCK(b).header.used.prev)
| ^~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:233:5: note: in expansion of macro 'UMM_PBLOCK'
233 | UMM_PBLOCK(block_1th) = block_0th;
| ^~~~~~~~~~
In file included from s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:35:
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc_cfg.h:60:13: note: while referencing '_heap_start'
60 | extern char _heap_start;
| ^~~~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:73:32: error: array subscript 1 is outside array bounds of 'char[1]' [-Werror=array-bounds]
73 | #define UMM_BLOCK(b) (umm_heap[b])
| ~~~~~~~~~^~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:78:24: note: in expansion of macro 'UMM_BLOCK'
78 | #define UMM_PFREE(b) (UMM_BLOCK(b).body.free.prev)
| ^~~~~~~~~
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:234:5: note: in expansion of macro 'UMM_PFREE'
234 | UMM_PFREE(block_1th) = block_0th;
| ^~~~~~~~~
In file included from s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc.c:35:
s:/sandboxes/sming-dev/Sming/Arch/Esp8266/Components/heap/umm_malloc/src/umm_malloc_cfg.h:60:13: note: while referencing '_heap_start'
60 | extern char _heap_start;
| ^~~~~~~~~~~
cc1.exe: all warnings being treated as errors
Looks good to me. Newer language features for a small price. Here is the comparison that I've done for one of my applications (same laptop, same Sming version and same SDK version): Roughly 300 bytes less free RAM, 50 bytes less IRAM, and 4K bigger file size. Details below: OLD toolchainMemory Usage
File size524K for out/Esp8266/debug/firmware/rom0.bin New toolchainMemory Usage
File size528K for out/Esp8266/debug/firmware/rom0.bin |
Plus the compilers are now consistent for all supported platforms (I'm assuming MacOS uses the Linux toolchain?). I'll be especially happy to be rid of GCC 4.8.5 as it's problematic. |
Move library binaries into `libc` Component Fix linker map Rename conflicting libraries in SDK - not used
Sming/build.mk
Outdated
else | ||
CPP_STD := c++17 | ||
endif | ||
CXXFLAGS += -std=$(CPP_STD) |
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.
Note that I've elected to select C++17 for the current toolchains (except linux which doesn't support it). This has the potential to break some existing code, but AFAIK only insofar as it includes better checks and so not a bad thing IMHO. The alternative is to select C++17 for the new toolchain only. This could also be made user-selectable, but not sure that's necessarily a good move.
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.
.. although being able to select the standard would be useful for testing against C++20, etc.
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.
I've added support for this and included note in documentation.
Another thing to consider is where the toolchains live. At the moment they're pre-release, but perhaps before releasing Sming 4.0.1 we might consider adding them to our SmingTools repo. and pulling them from there. |
I'll also add some documentation updates covering this PR. |
COMPONENT_SRCFILES += pgmspace.c | ||
LIBDIRS += $(COMPONENT_PATH)/lib | ||
EXTRA_LIBS += microc microgcc setjmp | ||
endif |
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.
You may have spotted that stdc++
has been removed from the list of libraries. One reason is that it contains allocation operators (new, delete, etc.) which conflict with our own. We don't use STL streams or a bunch of other stuff that it might be required for so leaving it out simplifies things. We can always revisit in the future if it's required for any other reason.
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.
Hello! I think that stdc++ is worth to keep. If there is any chance to do so. I use std::vector in my code and plan to switch to std:array all plain array. STL itself is good thing if used properly. Also there are libs that depend on STL flatbuffers as example. So do please keep this in Sming if possible.
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.
The STL shouldn't need the library as it's all templated code. It was only added in #1734 because of an issue with placeholders when compiling without optimisations.
It's probable that at least some parts of libstdc++.a
will be required to support exception handling, RTTI, iostream, etc. but at present Sming doesn't use those.
Would you try some test compiles with your code to see if it builds OK without the library? Just remove the stdc++
entry from Arch/Esp8266/app.mk
. Post back if it turns out there is stuff you do need from there then we can address that.
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.
Note: Host code builds in GDB mode at -O0
but as we've since updated Windows from GCC 5 to 8.2 it is no longer an issue.
@mikee47 In addition it seems that you have done great job improving also the memory usage. It looks like the new toolchain not only provides new C++ standards but also decreases the ROM size and the RAM and IRAM usage giving us additional room to expand our Sming applications.
NEW
OLD
|
Remove ICACHE_FLASH_ATTR definition, conflicts with one in esp_attr.h. **Don't disable interrupts during heap operations.** Sming is single-threaded and heap allocations are prohibited from interrupt context, therefore disabling interrupts during heap allocation operations is not necessary and detrimental to performance.
…rogcc.a as well (old toolchain)
@mikee47 Would you have time to look at this issue or you would prefer to have this PR merged and address the issue in a separate PR? |
When I built Basic_Ssl as you indicated I got a conflict error with |
@mikee47 Thanks a lot! Basic_Ssl with Umm Malloc compiles now as expected. |
Have you tested out GDB with the new toolchain? It includes the un-patched version which sends more stuff over the wire, but should not make any difference to how its used. |
.travis.yml
Outdated
- SDK_VERSION=3.0.1 | ||
- ESP_HOME=$TRAVIS_BUILD_DIR/opt/esp-alt-sdk | ||
|
||
- stage: build c++17 |
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.
@mikee47 Can you try to use build
as a stage name also here? It would be better to have the different builds running in parallel and not wait for build
to finish in order for build c++17
to start. I am not sure if travis will merge them under one stage and run them in parallel but it is worth trying.
I've attempted to test the custom heap on hardware, but it appears to be broken... Investigating.... |
I've reverted to #1745 and still cannot get even Basic_Blink to work using custom heap. Very odd. |
I will try it too... |
Hm... I have compiled Basic_Blink and Basic_Serial with ENABLE_CUSTOM_HEAP=1 and both are just not starting for me. I can see that rBoot is up and running but the application itself does not get started at all. And to make things worse there is no error message. |
Same here. I have two thoughts:
|
What's really bugging me is that it the original commit (1745) which was supposed to fix the custom heap doesn't work any more, which might support (1) above. |
Ok, add the parallel build change. I will merge this PR and then you can open a new PR related to umm_malloc. |
New features ------------------ - No-WiFi build option SmingHub#2004 - get more resources if your application is not using WIFI. - Multiple SSL adapters based on axTLS and BearSSL. SmingHub#1999 - Added basic Crypto support library SmingHub#2014 - Updates framework to build using GCC 9.2.0 toolchain for C++17. SmingHub#1825 - Modbus master SmingHub#1992 - Implemented Small String Optimisation (SSO). SmingHub#1951 - Webcam stream and sample webcam web server. SmingHub#1981 - Allow HTTP connections to ignore rejected body content SmingHub#1928 Improvements ------------------- - Some improvements to multipart parser SmingHub#2007 - Update ArduinoJson to 6.13.0 SmingHub#1979 - Added precaching from Arduino for ESP8266. SmingHub#1965 - Add support for 'Expect: 100-continue' in HTTP server. SmingHub#1931 - Upgrade to FlashString Library SmingHub#1974, SmingHub#2013 Bug fixes ------------- - Updated mqtt-codec to allow publish messages without payload. SmingHub#1976 - HttpConnection freed twice. SmingHub#1938 - Hangs at startup when custom heap enabled. SmingHub#1996 - Fix issues reported by valgrind SmingHub#2017 Breaking changes and Migration ------------------------------------------- - See our [dedicated page](https://sming.readthedocs.io/en/latest/upgrading/4.0-4.1.html) for migration from 4.0.0 to 4.1.0. All PRs scheduled for this release can be seen from [here](https://github.com/SmingHub/Sming/milestone/23)
New features ------------------ - No-WiFi build option SmingHub#2004 - get more resources if your application is not using WIFI. - Multiple SSL adapters based on axTLS and BearSSL. SmingHub#1999 - Added basic Crypto support library SmingHub#2014 - Updates framework to build using GCC 9.2.0 toolchain for C++17. SmingHub#1825 - Modbus master SmingHub#1992 - Implemented Small String Optimisation (SSO). SmingHub#1951 - Webcam stream and sample webcam web server. SmingHub#1981 - Allow HTTP connections to ignore rejected body content SmingHub#1928 Improvements ------------------- - Some improvements to multipart parser SmingHub#2007 - Update ArduinoJson to 6.13.0 SmingHub#1979 - Added precaching from Arduino for ESP8266. SmingHub#1965 - Add support for 'Expect: 100-continue' in HTTP server. SmingHub#1931 - Upgrade to FlashString Library SmingHub#1974, SmingHub#2013 Bug fixes ------------- - Updated mqtt-codec to allow publish messages without payload. SmingHub#1976 - HttpConnection freed twice. SmingHub#1938 - Hangs at startup when custom heap enabled. SmingHub#1996 - Fix issues reported by valgrind SmingHub#2017 Breaking changes and Migration ------------------------------------------- - See our [dedicated page](https://sming.readthedocs.io/en/latest/upgrading/4.0-4.1.html) for migration from 4.0.0 to 4.1.0. All PRs scheduled for this release can be seen from [here](https://github.com/SmingHub/Sming/milestone/23)
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
orC:\tools\esp-quick-toolchain
, and setESP_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.