-
Notifications
You must be signed in to change notification settings - Fork 2k
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
cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 and newlib 4.x #18719
Conversation
Murdock results❌ FAILED 8066eba cpu/esp8266: fixes for compilation with GCC 12.2.0 Build failures (9)
ArtifactsThis only reflects a subset of all builds from https://ci-prod.riot-os.org. Please refer to https://ci.riot-os.org for a complete build for now. |
There are many more compilation issues that I want to add to this PR. I should have added a WIP label :/
9a205bd
to
ef43890
Compare
Wow, from where did you get this GCC version for ESP32? The official ESP32 GCC version is still 8.4.0. |
It is hand baked; I'm intending to provide the packages for Alpine once they work as I hope for. For Arch Linux 12.2.0 is already available via the AUR: https://aur.archlinux.org/packages/xtensa-esp32-elf-gcc |
In the past I also compiled the toolchain (GCC version 5.2.0) especially for RIOT using Therefore, the question is whether it wouldn't be better to use the same GCC version as the vendor uses for the SDK? It is not our task to fix compile problems with newer GCC version in the SDK.
If I interpret the comment correctly, the Arch Linux gcc 12.2 for xtensa esp32 is intended for bare metal implementations and not for use with ESP-IDF. |
Date: Mon, 10 Oct 2022 13:09:07 +0200 | ||
Subject: [PATCH 1/4] components: fix format specifier | ||
|
||
In various places `"%u"` or `"%x"` are used instead of `"%" PRIu32` and |
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.
Is this really an issue or just a cleanup?
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.
It is an issue: uint32_t
can also be unsinged long
on 32 bit systems - and apparently is here. This causes GCC to complain about mismatching type and format specifier.
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, I see. In former GCC versions using %u
and %x
was working since uint32_t
is unsigned int
on Xtensa-based ESP32x SoCs.
ef43890
to
4dc9b87
Compare
ab1527e
to
8066eba
Compare
This is too much WIP to be rushed into the next release anyway. So lets give other PRs the CI time instead. |
See the commit messages in the patches for details.
8066eba
to
efd279f
Compare
I have cherry-picked the commits for ESP8266 and tried to compile it with the latest toolchain version provided by Espressif. Unfortunatly it seems that they use POSIX threads so that the linking process fails because of unknown references to In the toolchain version I provided, I disabled the POSIX threads. But this version becomes pretty old now. So the question is how we should continue with the toolchain for ESP8266. The options are:
Option 1 would have the advantage that we can control the settings when building the toolchain. When thinking about option 2, I'm wondering whether we could compile the SDK code directly from SDK source in furture as we do for ESP32. But this would require a lot of work. Option 3 would only work on native Ubuntu systems or in riotdocker. |
Too bad, Ubuntu 22.04 doesn't provide a |
@gschorcht has already fixed theses issues, so this is no longer relevant |
Contribution description
Some trivial compilation fixes. These won't change generated binaries for older versions of GCC, but fixes compilation for newer versions that are a bit more pedantic in regard to types.
Testing procedure
Generated binaries should not change.
Issues/PRs references