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

Recent GCC v14 conflicts with stdatomic.h from the xtensa toolchain (esp-13.2.0_20240305) (IDFGH-13033) #13979

Closed
3 tasks done
M-Bab opened this issue Jun 14, 2024 · 9 comments
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally

Comments

@M-Bab
Copy link
Contributor

M-Bab commented Jun 14, 2024

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

The whole setup is a bit tricky: In some cases you will build xtensa toolchain code with another GCC than provided from the toolchain. In our case it is unit tests we run on x86 platforms compiled with the local GCC. But even outside this scope the xtensa toolchain will run into the problem when upgrading its GCC.

The stdatomic.h (in ~/.espressif/tools/xtensa-esp-elf/esp-13.2.0_20240305/xtensa-esp-elf/xtensa-esp-elf/include/stdatomic.h) tries to determine the right setup for different compilers:

#if __has_extension(c_atomic) || __has_extension(cxx_atomic)
#define	__CLANG_ATOMICS
#elif __GNUC_PREREQ__(4, 7)
#define	__GNUC_ATOMICS
#elif defined(__GNUC__)
#define	__SYNC_ATOMICS
#else
#error "stdatomic.h does not support your compiler"
#endif

Unfortunately the most recent GCC also replies __has_extension(c_atomic) with true. So we get the define __CLANG_ATOMICS which is obviously for Clang. Consequently the build also fails with:

error: implicit declaration of function__c11_atomic_thread_fence’; did you mean__atomic_thread_fence’? [-Wimplicit-function-declaration]
  143 |         __c11_atomic_thread_fence(__order);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~
      |         __atomic_thread_fence

And many other errors if stdatomic.h is somehow included. For now I will workaround that by modifying the xtensa toolchain:

diff stdatomic.h stdatomic-fixed.h 
37c37
< #define       __CLANG_ATOMICS
---
> #define       __GNUC_ATOMICS

But maybe you got a better solution. This issue is also a hint for other users having a similar problem. And to make you aware there is a change necessary when upgrading GCC.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jun 14, 2024
@github-actions github-actions bot changed the title Recent GCC v14 conflicts with stdatomic.h from the xtensa toolchain (esp-13.2.0_20240305) Recent GCC v14 conflicts with stdatomic.h from the xtensa toolchain (esp-13.2.0_20240305) (IDFGH-13033) Jun 14, 2024
@Lapshin
Copy link
Collaborator

Lapshin commented Jun 17, 2024

Hi @M-Bab !

In some cases you will build xtensa toolchain code with another GCC than provided from the toolchain.

Could you please explain what you want to achieve?

There are my thoughts regarding the ticket:

@M-Bab
Copy link
Contributor Author

M-Bab commented Jun 17, 2024

Well there might be other use-cases but ours is unit testing our code. We run the tests on a local x86 platform with Ceedling/Unity/CMock. So far we never had to care about mismatches of the local x86 gcc in the system and the gcc version of the xtensa toolchain.

This is the first time it breaks.

@M-Bab
Copy link
Contributor Author

M-Bab commented Jun 17, 2024

What do you think about

#if defined(__clang__) && (__has_extension(c_atomic) || __has_extension(cxx_atomic))
#define	__CLANG_ATOMICS
#elif __GNUC_PREREQ__(4, 7)
...

@Lapshin
Copy link
Collaborator

Lapshin commented Jun 18, 2024

@M-Bab , your change looks good!

BTW, did not catch why you need to mix toolchain internals. Anyway, you may be interested in idf.py --preview set-target linux build monitor command. It's still under development but could be useful for unit testing that does not require esp32 hardware..

@M-Bab
Copy link
Contributor Author

M-Bab commented Jun 19, 2024

BTW, did not catch why you need to mix toolchain internals.

Our unit-tests only run on the host platform - this is an x86 system. And of course it uses the GCC compiler which is installed on the host system. And this can be virtually any modern GCC. We are not mixing toolchains.

@M-Bab
Copy link
Contributor Author

M-Bab commented Jul 25, 2024

Any updates on this? Did you notify xtensa to fix this?

Other projects also started to recognize that this is an issue:

@Lapshin
Copy link
Collaborator

Lapshin commented Jul 29, 2024

@M-Bab , I sent a patch to newlib, but seems they are not interested in applying it. Thank you for posting the link to freebsd patch, it will be a good argument for dispute :)

There is a mailing list thread: https://sourceware.org/pipermail/newlib/2024/021163.html

@Lapshin
Copy link
Collaborator

Lapshin commented Jul 29, 2024

@M-Bab , don't worry. Our next release will have all the required patches

@Lapshin
Copy link
Collaborator

Lapshin commented Jul 30, 2024

@M-Bab , change is in newlib now https://sourceware.org/git/?p=newlib-cygwin.git;a=commit;h=d001c01a7c5277194e7f8891f0228a2e3c4c64e1

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Aug 15, 2024
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: NA Issue resolution is unavailable and removed Status: In Progress Work is in progress labels Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

3 participants