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

modules/picolibc: Include clang+cmake and old GCC changes #54394

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

keith-packard
Copy link
Collaborator

@keith-packard keith-packard commented Feb 3, 2023

This updates west.yml to pull a version of picolibc that includes both the patches in #54391 as well as a proposed fix that provides some compatibility with GCC versions before 4.5, such as the GCC based XCC compiler.

Signed-off-by: Keith Packard [email protected]

Fixes #54336

@keith-packard keith-packard added DNM This PR should not be merged (Do Not Merge) area: picolibc Picolibc C Standard Library labels Feb 3, 2023
@zephyrbot zephyrbot added manifest manifest-picolibc and removed DNM This PR should not be merged (Do Not Merge) labels Feb 3, 2023
@zephyrbot
Copy link
Collaborator

zephyrbot commented Feb 3, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
picolibc zephyrproject-rtos/picolibc@2097f26 (cmake-no-tls) zephyrproject-rtos/picolibc@0694a78 (main) zephyrproject-rtos/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

nordicjm
nordicjm previously approved these changes Feb 3, 2023
stephanosio
stephanosio previously approved these changes Feb 3, 2023
@keith-packard keith-packard added the DNM This PR should not be merged (Do Not Merge) label Feb 3, 2023
@keith-packard
Copy link
Collaborator Author

This patch is still under discussion for merging to picolibc; when this feature is merged to picolibc main, I'll remove the DNM tag.

@keith-packard keith-packard removed the DNM This PR should not be merged (Do Not Merge) label Feb 4, 2023
@keith-packard
Copy link
Collaborator Author

This now references the picolibc patch which was reviewed and merged upstream.

@stephanosio
Copy link
Member

There are some failures.

@keith-packard
Copy link
Collaborator Author

There are some failures.

Zephyr and picolibc were both trying to use a macro __deprecated, but picolibc had an argument for the optional message (which old GCC didn't support). Thanks for noticing, let's see if that's fixed now.

@keith-packard
Copy link
Collaborator Author

keith-packard commented Feb 7, 2023

There are some failures.

Zephyr and picolibc were both trying to use a macro __deprecated, but picolibc had an argument for the optional message (which old GCC didn't support). Thanks for noticing, let's see if that's fixed now.

Yup, looks fixed now -- the one remaining failure I'm seeing also on main, and that test isn't using picolibc.

@keith-packard
Copy link
Collaborator Author

Yup, looks fixed now -- the one remaining failure I'm seeing also on main, and that test isn't using picolibc.

And it's transient; re-running the test resolved the issue. Sigh.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried this with qemu_x86 w/llvm-14 or llvm-15 on ubuntu and get the following (or some form it):

<inline asm>:2:1: error: __getauxval changed binding to STB_GLOBAL

when trying:

./scripts/twister -p qemu_x86 -T tests/lib/c_lib/

branch: https://github.com/galak/zephyr/pull/new/llvm-x86-pico

@galak
Copy link
Collaborator

galak commented Feb 7, 2023

Tried this with qemu_x86 w/llvm-14 or llvm-15 on ubuntu and get the following (or some form it):

<inline asm>:2:1: error: __getauxval changed binding to STB_GLOBAL

when trying:

./scripts/twister -p qemu_x86 -T tests/lib/c_lib/

branch: https://github.com/galak/zephyr/pull/new/llvm-x86-pico

Seems to be related to __weak_reference is expanding to:

unsigned long getauxval(unsigned long type)
{
 (void) type;
 (*z_errno_wrap()) = 22;
 return 0;
}
__asm__(".weak_reference " "__getauxval");
__asm__(".globl " "__getauxval");
__asm__(".set " "__getauxval" ", " "getauxval");

@keith-packard
Copy link
Collaborator Author

Seems to be related to __weak_reference is expanding to:

unsigned long getauxval(unsigned long type)
{
 (void) type;
 (*z_errno_wrap()) = 22;
 return 0;
}
__asm__(".weak_reference " "__getauxval");
__asm__(".globl " "__getauxval");
__asm__(".set " "__getauxval" ", " "getauxval");

Yeah, newlib (and picolibc by extension) use inline asm for weak references instead of using attributes. I'm working on a picolibc patch to make it use __attribute__((__weak__, __alias__...)) instead. Seems to fix this, but needs testing.

@keith-packard
Copy link
Collaborator Author

Yeah, newlib (and picolibc by extension) use inline asm for weak references instead of using attributes. I'm working on a picolibc patch to make it use __attribute__((__weak__, __alias__...)) instead. Seems to fix this, but needs testing.

I've updated this series to pull in picolibc with this fix from upstream.

stephanosio
stephanosio previously approved these changes Feb 9, 2023
@keith-packard
Copy link
Collaborator Author

Updated past other recent picolibc merge, does not change target manifest revision

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on TSC conversation today (Mar 15, 2023), please also update the SDK to point at this version of picolibc.

Whenever we update west.yml picolibc, we should also update the SDK to match.

This updates west.yml to pull the version of picolibc that includes both
the patches in zephyrproject-rtos#54391 as well as the fix providing compatibility for the
'deprecated' attribute when using GCC versions before 4.5, such as the GCC
based XCC compiler. Changes for 'weak' attributes were added to avoid using
inline asm statements for compilers that support the necessary attributes.

This commit is synchronized with Zephyr SDK:

zephyrproject-rtos/sdk-ng#650

Signed-off-by: Keith Packard <[email protected]>
@keith-packard
Copy link
Collaborator Author

Based on TSC conversation today (Mar 15, 2023), please also update the SDK to point at this version of picolibc.

Whenever we update west.yml picolibc, we should also update the SDK to match.

Yup, been running tests to make sure updating both will work as expected.

@galak galak merged commit 047ee8c into zephyrproject-rtos:main Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

picolibc is incompatible with xcc / xcc-clang toolchains
5 participants