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

picolibc: long-long print support not enabled on prebuilt library #684

Closed
JordanYates opened this issue Jun 9, 2023 · 5 comments
Closed

Comments

@JordanYates
Copy link

Picolibc depends on io-long-long=true to support %lld format specifiers, which does not appear to be enabled in the prebuilt library.
As a result, the following code (all with CONFIG_CBPRINTF_FULL_INTEGRAL=y):

uint64_t test = 123456789123456789;
printk(" Test: %" PRIu64 "\n", test);
printk("Anded: %" PRIu32 "\n", (uint32_t)(test & 0xFFFFFFFF));

Outputs this with CONFIG_PICOLIBC:

*** Booting Zephyr OS build zephyr-v3.3.0-69-g6cc0071dba5d ***
 Test: 2899336981
Anded: 2899336981

This with CONFIG_PICOLIBC=y CONFIG_PICOLIBC_USE_MODULE=y CONFIG_PICOLIBC_IO_LONG_LONG=y

*** Booting Zephyr OS build zephyr-v3.3.0-69-g6cc0071dba5d ***
 Test: 123456789123456789
Anded: 2899336981

And this with CONFIG_NEWLIB_LIBC:

*** Booting Zephyr OS build zephyr-v3.3.0-69-g6cc0071dba5d ***
 Test: 123456789123456789
Anded: 2899336981

I think that the support should be enabled by default for the pre-built library.

Tested on Zephyr SDK v0.16.0, but I haven't seen any notes about it on newer releases and this line doesn't have the option enabled: https://github.com/zephyrproject-rtos/sdk-ng/blob/4a824f648c39c494bfc016241c44243c7a599048/configs/common.config#LL75

https://github.com/picolibc/picolibc/blob/main/doc/printf.md

@keith-packard
Copy link
Collaborator

keith-packard commented Jun 9, 2023

The pre-built library follows the default configuration for picolibc so that it matches other toolchains that include picolibc, including ARM LLVM, Debian, crosstool-ng and the ARM GNU toolchains. Note that if you select the floating point printf option, that version does include long long support. We could change that easily enough; it would increase the size of printf modestly, but still be smaller than the minimal C library. Alternatively, selecting IO_LONG_LONG could switch to the floating point version of the code, but that is pretty large (several kB).

@JordanYates
Copy link
Author

JordanYates commented Jun 9, 2023

I can appreciate the complexities of managing a default configuration for all users, but isn't sdk-ng building picolibc from source with the express purpose of Zephyr support?
The current behaviour is not great. The Zephyr build system requests all the appropriate options, but the SDK doesn't care and we end up with truncation to 32bits (silently!).
My expectation for how this was being handled is that the pre-built library is fully-featured (with a few floating point variants), and if footprint is super important, you drop back to PICOLIBC_USE_MODULE and tune out the features you don't need.
Requiring FP support to be selected to print 64 bit integers is super non-intuitive, and as far as I can see not documented anywhere.

@keith-packard
Copy link
Collaborator

Yup, I'm not saying the sdk config shouldn't be changed to match what Zephyr users expect, just explaining where the current settings came from.

@JordanYates
Copy link
Author

Looks like this will potentially be fixed with picolibc 1.8.5 https://github.com/picolibc/picolibc#picolibc-version-185

@keith-packard
Copy link
Collaborator

Yup, version 1.8.5 provides pre-built versions of all of the printf modes supported by current Zephyr configurations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants