Skip to content

cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 and newlib 4.x#18719

Closed
maribu wants to merge 7 commits intoRIOT-OS:masterfrom
maribu:cpu/esp32/GCC-12.2.0-fixes
Closed

cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 and newlib 4.x#18719
maribu wants to merge 7 commits intoRIOT-OS:masterfrom
maribu:cpu/esp32/GCC-12.2.0-fixes

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Oct 10, 2022 •
edited
Loading

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

@maribu maribu requested a review from gschorcht as a code owner October 10, 2022 11:18
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: pkg Area: External package ports labels Oct 10, 2022
@maribu maribu changed the title cpu/esp32: fix compilation with GCC 12.2.0 cpu/esp32,pkg/esp32_sdk: fix compilation with GCC 12.2.0 Oct 10, 2022
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 10, 2022
benpicco
benpicco previously approved these changes Oct 10, 2022
View reviewed changes
@riot-ci
Copy link
Copy Markdown

riot-ci commented Oct 10, 2022 •
edited
Loading

Murdock results

❌ FAILED

8066eba cpu/esp8266: fixes for compilation with GCC 12.2.0

Build failures (9)
Application Target Toolchain Runtime (s) Worker
examples/nimble_gatt esp32-wroom-32 gnu 12.40 mobi7
examples/nimble_heart_rate_sensor esp32-wroom-32 gnu 4.69 beast
examples/nimble_scanner esp32-wroom-32 gnu 10.39 mobi7
tests/nimble_autoconn_gnrc esp32-wroom-32 gnu 4.34 beast
tests/nimble_l2cap esp32-wroom-32 gnu 4.43 beast
tests/nimble_l2cap_server esp32-wroom-32 gnu 7.88 beast
tests/nimble_rpble_gnrc esp32-wroom-32 gnu 10.07 beast
tests/nimble_statconn_gnrc esp32-wroom-32 gnu 10.14 mobi7
tests/shell_ble esp32-wroom-32 gnu 4.68 beast

Artifacts

This 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.

@maribu maribu dismissed benpicco’s stale review October 10, 2022 12:32

There are many more compilation issues that I want to add to this PR. I should have added a WIP label :/

@maribu maribu added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 10, 2022
@maribu maribu force-pushed the cpu/esp32/GCC-12.2.0-fixes branch 2 times, most recently from 9a205bd to ef43890 Compare October 10, 2022 12:43
@maribu maribu added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Oct 10, 2022
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Oct 10, 2022

Wow, from where did you get this GCC version for ESP32? The official ESP32 GCC version is still 8.4.0.

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Oct 10, 2022

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

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Oct 10, 2022

It is hand baked; I'm intending to provide the packages for Alpine once they work as I hope for.

In the past I also compiled the toolchain (GCC version 5.2.0) especially for RIOT using crosstool-ng. I had also tried it with GCC version 8.4.0 while upgrading the RIOT port to ESP-IDF version 4.4. But I had a lot of trouble with the ESP-IDF when the settings in crosstool-ng were not exactly the same as used by Espressif, especially regarding the retargetable locking, the thread model and the POSIX threads. Therefore, I am really glad that we are now using the toolchain as provided by the vendor.

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.

For Arch Linux 12.2.0 is already available via the AUR: https://aur.archlinux.org/packages/xtensa-esp32-elf-gcc

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.

gschorcht
gschorcht reviewed Oct 10, 2022
View reviewed changes
pkg/esp32_sdk/patches/0028-components-fix-format-specifier.patch
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
Copy link
Copy Markdown
Contributor

@gschorcht gschorcht Oct 10, 2022

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?

Copy link
Copy Markdown
Member Author

@maribu maribu Oct 11, 2022

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.

Copy link
Copy Markdown
Contributor

@gschorcht gschorcht Oct 11, 2022 •
edited
Loading

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.

@gschorcht gschorcht mentioned this pull request Oct 11, 2022
Merged
@maribu maribu force-pushed the cpu/esp32/GCC-12.2.0-fixes branch from ef43890 to 4dc9b87 Compare October 11, 2022 12:21
@maribu maribu requested a review from kaspar030 as a code owner October 11, 2022 12:21
@maribu maribu changed the title cpu/esp32,pkg/esp32_sdk: fix compilation with GCC 12.2.0 cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 Oct 11, 2022
@maribu maribu force-pushed the cpu/esp32/GCC-12.2.0-fixes branch 2 times, most recently from ab1527e to 8066eba Compare October 12, 2022 14:42
@maribu maribu removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 12, 2022
@maribu
Copy link
Copy Markdown
Member Author

maribu commented Oct 12, 2022

This is too much WIP to be rushed into the next release anyway. So lets give other PRs the CI time instead.

@maribu maribu added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Oct 14, 2022
@maribu maribu changed the title cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 cpu/esp*,pkg/esp*: fix compilation with GCC 12.2.0 and newlib 4.x Oct 14, 2022
@maribu
pkg/esp32_sdk: fix compilation with GCC 12.2.0
287f2fc 
See the commit messages in the patches for details.
@maribu maribu force-pushed the cpu/esp32/GCC-12.2.0-fixes branch from 8066eba to efd279f Compare October 14, 2022 17:15
@gschorcht gschorcht mentioned this pull request Apr 12, 2023
Merged
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Apr 12, 2023

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 pthread_* 😟

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:

  1. build and provide the latest version with crosstool-NG without POSIX threads
  2. use the latest version from Espressif and provide a pthread library as the SDK does
  3. use the Ubuntu toolchain version (Ubuntu 22.04 comes with xtensa-lx106-elf version 10.3 and the picolibc)

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.

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Apr 12, 2023

Too bad, Ubuntu 22.04 doesn't provide a libstdc++-dev package for xtensa-lx106-elf.

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Feb 19, 2024

@gschorcht has already fixed theses issues, so this is no longer relevant

@maribu maribu closed this Feb 19, 2024
@maribu maribu deleted the cpu/esp32/GCC-12.2.0-fixes branch February 19, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gschorcht gschorcht gschorcht left review comments

@benpicco benpicco benpicco left review comments

@kaspar030 kaspar030 Awaiting requested review from kaspar030 kaspar030 is a code owner

Assignees

No one assigned

Labels

Area: cpu Area: CPU/MCU ports Area: pkg Area: External package ports Platform: ESP Platform: This PR/issue effects ESP-based platforms State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maribu @riot-ci @gschorcht @benpicco