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.

Sign up for GitHub

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

Jump to bottom

picolibc: Update to version 1.8.7 #78354

Merged
merged 18 commits into from
Sep 16, 2024
Merged

picolibc: Update to version 1.8.7 #78354

merged 18 commits into from
Sep 16, 2024

Conversation

keith-packard
Copy link
Collaborator

Integrate new upstream release

The associated sdk-ng work is happening on https://github.com/zephyrproject-rtos/sdk-ng/tree/topic-picolibc-1.8.7

@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Sep 12, 2024
@keith-packard keith-packard added area: C Library C Standard Library area: picolibc Picolibc C Standard Library labels Sep 12, 2024
@zephyrbot
Copy link
Collaborator

zephyrbot commented Sep 12, 2024 •
edited
Loading

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

Name Old Revision New Revision Diff
picolibc zephyrproject-rtos/picolibc@764ef4e zephyrproject-rtos/picolibc@06bde1f (aarch64-cmake-fix) zephyrproject-rtos/picolibc@764ef4e4..06bde1fd

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

@keith-packard keith-packard removed the size: XS A PR changing only a single line of code label Sep 12, 2024
@zephyrbot zephyrbot added the area: POSIX POSIX API Library label Sep 12, 2024
@zephyrbot zephyrbot requested review from cfriedt and ycsin September 12, 2024 19:13
cfriedt
cfriedt reviewed Sep 12, 2024
View reviewed changes
lib/libc/common/source/time/ctime.c
@@ -4,6 +4,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

#undef _POSIX_C_SOURCE
Copy link
Member

@cfriedt cfriedt Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I understand why the kludge is necessary although I was hoping to address this particular issue preemptively almost 9 months ago :'(

#67132

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to approve as-is, but @keith-packard, if you feel like we could use the Kconfig, I would be more than happy to pursue that PR again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Up to you; we can revert this change if that happens.

@keith-packard
net: dns: Ignore GCC warnings in mdns_responder.c
8c0d361 
GCC complains about struct sockaddr accesses due to the various
address-family-specific variants being of differing sizes. Let's not
mess with code (which looks correct), just silence the compiler.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
subsys/llext: Allow .exported_sym to be between other data sections
a1f531a 
There's no reason for the .exported_sym data to always land at one end of a
data region; the order of sections depends on the whim of the compiler and
assembler.

Ignore overlaps between that region and other data regions.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
subsys/shell: cmd_date_set uses gmtime_r, a POSIX function
6f97c63 
Define _POSIX_C_SOURCE for this file to make sure gmtime_r is visible.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: Avoid stepping on picolibc clockid_t and timer_t
c5156c3 
picolibc sets _CLOCKID_T_DECLARED and _TIMER_T_DECLARED, not
__clockid_t_defined and __timer_t_defined.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: Avoid redefining TZNAME_MAX
b08f03f 
When _POSIX_C_SOURCE is defined by a source file, a POSIX-conforming C
library will define TZNAME_MAX in limits.h. Avoid redefining this
in Zephyr's posix_features.h.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: Don't require struct itimerspec in posix/time.h
02c6544 
struct itimerspec is only required to be available when building with
_POSIX_C_SOURCE defined. Applications including posix/time.h without
defining _POSIX_C_SOURCE will need to have a forward declaration of struct
itimerspec even if they don't use the functions requiring that type.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: Define _POSIX_C_SOURCE to gain access to POSIX functions
471c772 
options/fs.c and options/timer.c both use POSIX-only functions. To ensure
those symbols are visible from the underlying C library, define
_POSIX_C_SOURCE.

Signed-off-by: Keith Packard <keithp@keithp.com>
jukkar
jukkar previously approved these changes Sep 13, 2024
View reviewed changes
@keith-packard
Copy link
Collaborator Author

Given the number of changes in the POSIX code, I'd love to get @cfriedt to give this another glance before it gets merged?

cfriedt
cfriedt reviewed Sep 13, 2024
View reviewed changes
lib/libc/common/Kconfig Outdated Show resolved Hide resolved
@keith-packard
libc/common: Remove redundant Kconfig POSIX_THREAD_SAFE_FUNCTIONS uses
331a477 
POSIX_THREAD_SAFE_FUNCTIONS already selects POSIX_C_LANG_SUPPORT_R and
POSIX_C_LANG_SUPPORT_R alread selects all of the COMMON_LIBC_*_R values.
That makes the 'default y if POSIX_THREAD_SAFE_FUNCTIONS' clauses in these
symbols redundant.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
lib/posix: Only enable Zephyr _r APIs when toolchain doesn't
d95adbe 
Check TC_PROVIDES_POSIX_C_LANG_SUPPORT_R before selecting
the COMMON_LIBC_*_R APIs.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
libc/picolibc: Don't replace picolibc time _r functions
3daa9dc 
Select TC_PROVIDES_POSIX_C_LANG_SUPPORT_R to keep Zephyr from
including the common libc implementation of the various _r APIs.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: Need sys/types.h to get suseconds_t defined
af6ac51 
Picolibc's sys/_timeval.h (which Zephyr shouldn't be using) needs
sys/types.h to be included first so that __suseconds_t is defined.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: options/shm.c needs _POSIX_C_SOURCE
40430e8 
This file uses PATH_MAX, which is defined in limits.h when _POSIX_C_SOURCE
is set.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
posix: sysconf.c uses POSIX values
c8865c5 
We need to add _POSIX_C_SOURCE to this file as it uses POSIX values from
limits.h including IOV_MAX, CHILD_MAX and ARG_MAX.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
tests/c_lib: Don't test undefined behavior of asctime and asctime_r
b4bc493 
These functions requre non-null parameters; passing them NULL
values is undefined. Remove tests which are not valid.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
tests/c_lib: Ensure TZ value is UTC for ctime/ctime_r tests
2f12271 
When testing these functions with CONFIG_NATIVE_LIBC, force the TZ
environment variable to UTC to ensure they produce the expected results.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
tests, samples: Define _POSIX_C_SOURCE where needed
01ff029 
Various tests and samples use POSIX-only functions. Define _POSIX_C_SOURCE
to ensure they are visible.

Signed-off-by: Keith Packard <keithp@keithp.com>
@keith-packard
picolibc: Update to version 1.8.7
8406cbb 
Integrate new upstream release.
Include strnlen_s and aarch64 cmake fixes.

Signed-off-by: Keith Packard <keithp@keithp.com>
cfriedt
cfriedt approved these changes Sep 14, 2024
View reviewed changes
Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

LGTM.

I'll respin #67132 as well, so that we can have a way to avoid peppering _POSIX_C_SOURCE throughout the code, for consistency.

@keith-packard keith-packard mentioned this pull request Sep 14, 2024
Merged
@carlescufi carlescufi merged commit 2803d4b into zephyrproject-rtos:main Sep 16, 2024
27 checks passed
@keith-packard keith-packard deleted the picolibc-1.8.7 branch September 28, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ycsin ycsin ycsin approved these changes

@jukkar jukkar jukkar approved these changes

@cfriedt cfriedt cfriedt approved these changes

@aescolar aescolar aescolar left review comments

@stephanosio stephanosio Awaiting requested review from stephanosio

@nashif nashif Awaiting requested review from nashif

@carlescufi carlescufi Awaiting requested review from carlescufi

@jakub-uC jakub-uC Awaiting requested review from jakub-uC

@kartben kartben Awaiting requested review from kartben

@pdgendt pdgendt Awaiting requested review from pdgendt

@rlubos rlubos Awaiting requested review from rlubos

@ssharks ssharks Awaiting requested review from ssharks

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@teburd teburd Awaiting requested review from teburd

Assignees

@cfriedt cfriedt

Labels
area: C Library C Standard Library area: llext Linkable Loadable Extensions area: Networking area: picolibc Picolibc C Standard Library area: POSIX POSIX API Library area: Samples Samples area: Shell Shell subsystem manifest manifest-picolibc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@keith-packard @zephyrbot @cfriedt @jukkar @ycsin @aescolar @carlescufi