Skip to content

libc: Default to picolibc where supported#57340

Merged
nashif merged 2 commits intozephyrproject-rtos:mainfrom
keith-packard:picolibc-default
Sep 4, 2023
Merged

libc: Default to picolibc where supported#57340
nashif merged 2 commits intozephyrproject-rtos:mainfrom
keith-packard:picolibc-default

Conversation

@keith-packard
Copy link
Copy Markdown
Contributor

@keith-packard keith-packard commented Apr 27, 2023

This switches the default C library to picolibc for all targets which support it.

Closes #49922

@keith-packard keith-packard added area: C Library C Standard Library DNM This PR should not be merged (Do Not Merge) area: picolibc Picolibc C Standard Library labels Apr 27, 2023
@keith-packard keith-packard requested a review from nashif as a code owner April 27, 2023 13:33
@carlescufi carlescufi added the TSC Topics that need TSC discussion label Apr 27, 2023
@carlescufi
Copy link
Copy Markdown
Member

@keith-packard labeling this as TSC since this requires a bit more discussion.

@zephyrbot
Copy link
Copy Markdown

zephyrbot commented Apr 27, 2023

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

Name Old Revision New Revision Diff
mcuboot zephyrproject-rtos/mcuboot@1558e7a (main) zephyrproject-rtos/mcuboot@2878eb4 (upstream-sync) zephyrproject-rtos/mcuboot@1558e7ab..2878eb4e
picolibc zephyrproject-rtos/picolibc@93b5d5f (main) zephyrproject-rtos/picolibc@zephyr-source zephyrproject-rtos/picolibc@93b5d5f2..zephyr-source
sof zephyrproject-rtos/sof@ffbf9c2 (zephyr) zephyrproject-rtos/sof#28 zephyrproject-rtos/sof#28/files

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

@marc-hb
Copy link
Copy Markdown
Contributor

marc-hb commented May 1, 2023

One of the commit messages is:

modules/audio/sof: Pull fix for main return type

Upstream PR #28 switches return type of 'main' to 'int'.

Signed-off-by: Keith Packard keithp@keithp.com

Pull request 28 does not seem related. It's from 2017 :-)

EDIT: got it, you meant zephyrproject-rtos/sof/pull/28, not #28

Could you please share a link with more information about this change of the main prototype? It seems related to picolibc but so are many other changes so it does not narrow it down.


Screenshot 2023-05-01 at 10 20 17

@keith-packard
Copy link
Copy Markdown
Contributor Author

EDIT: got it, you meant zephyrproject-rtos/sof/pull/28, not #28

Sorry for the mis-leading link.

Could you please share a link with more information about this change of the main prototype? It seems related to picolibc but so are many other changes so it does not narrow it down.

Yeah, this is only vaguely related to picolibc itself -- as a complete C library, it's best to compile applications using picolibc without the -ffreestanding compiler flag as that improves code generation and error detection. But, with that option not present, clang makes mis-matching main type a fatal error. So, that means we needed to switch the return type to int. That series, #54628 was merged a while ago.

@stephanosio
Copy link
Copy Markdown
Member

As per #49922 (comment), this needs more documentation and process-related work before it can be approved by the TSC.

I will look into that and create a new PR based on this with the necessary changes.

tejlmand
tejlmand previously approved these changes Aug 22, 2023
Copy link
Copy Markdown
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm 🎉

This switches the default C library to picolibc for all targets which
support it.

Signed-off-by: Keith Packard <keithp@keithp.com>
With picolibc being the default C library, we need to explicitly include
testing against the minimal C library for kernel components.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
Copy link
Copy Markdown
Contributor Author

sorry for the update -- I rebased on top of an unrelated fix which was blocking CI. Now that it's passing, it would be a good time to get this merged :-)

@nashif nashif removed the TSC Topics that need TSC discussion label Sep 4, 2023
default PICOLIBC
default NEWLIB_LIBC if REQUIRES_FULL_LIBC
default PICOLIBC if REQUIRES_FULL_LIBC
default MINIMAL_LIBC
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dont we end up with both picolibc and minimal libc set as default? or does order here play a role?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Kconfig appears to pick the first which matches and which can be enabled -- the goal is to use picolibc where supported otherwise use minimal C library.

@carlescufi
Copy link
Copy Markdown
Member

@keith-packard, now that we have a migration guide, would you mind adding a note there? I assume under "Required changes", given that users may see their applications break depending on what they are using?

@keith-packard
Copy link
Copy Markdown
Contributor Author

@keith-packard, now that we have a migration guide, would you mind adding a note there? I assume under "Required changes", given that users may see their applications break depending on what they are using?

#62261 has some possible things to check. I'm not sure how these should be integrated into the rest of the migration notes though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Make picolibc the default C library for Zephyr