Skip to content

Fix newlib include path and always enable C99 format specifier support #469

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

Merged
merged 2 commits into from
May 10, 2022

Conversation

stephanosio
Copy link
Member

@stephanosio stephanosio commented May 9, 2022

This commit enables the C99 format specifier support for the newlib
nano variant since it is required for C99 standard compatibility.

Without this, Zephyr applications cannot make use of the format
specifiers newly added in the C99 standard such as `%hhu` and `%hhd`.

Moreover, the newlib `inttypes.h` defines `PRI*8` macros as `%hh*` even
when this configuration is disabled, effectively making the abstraction
provided by the `inttypes.h` useless due to lack of universality; for
this reason, the C99 format specifier support must always be enabled
for both full and nano variants of the newlib.

For more details, refer to the issue https://github.com/zephyrproject-rtos/zephyr/issues/45336.

In addition, this commit disables the nano-formatted I/O build option
for the newlib nano variant because this option limits the formatted
I/O features to that of the C89 standard and overrides the C99 format
specifier support option -- refer to the newlib documentation of the
`--enable-newlib-nano-formatted-io` configuration.

Signed-off-by: Stephanos Ioannidis <[email protected]>
    crosstool-ng: Pull in newlib nano.specs include path patch

    This commit pulls in the crosstool-ng patch to fix the incorrect
    newlib-nano include path specified by the toolchain `nano.specs` file.

    For more details, refer to the issue #468.

    Signed-off-by: Stephanos Ioannidis <[email protected]>

crosstool-ng PR: zephyrproject-rtos/crosstool-ng#16

Fixes #468
Fixes zephyrproject-rtos/zephyr#45336

@stephanosio stephanosio added the DNM DO NOT MERGE label May 9, 2022
@stephanosio
Copy link
Member Author

DNM until zephyrproject-rtos/crosstool-ng#16 is merged and the submodule commit is updated to that of the merged commit.

@stephanosio
Copy link
Member Author

TODO: Check and document the footprint increase in the newlib-nano variant after enabling the C99 format specifier support.

Even if the footprint increase is significant however, as noted in zephyrproject-rtos/zephyr#45336, we must proceed with this change because this is required for us to be C99 compliant and universally make use of the C99 inttypes in the Zephyr codebase:

we should not be optimising the newlib footprint at the cost of breaking C99 compatibility.

This commit pulls in the crosstool-ng patch to fix the incorrect
newlib-nano include path specified by the toolchain `nano.specs` file.

For more details, refer to the issue zephyrproject-rtos#468.

Signed-off-by: Stephanos Ioannidis <[email protected]>
@stephanosio stephanosio force-pushed the newlib_nano_fix branch 2 times, most recently from 7e7defc to 65caa6c Compare May 10, 2022 11:39
This commit enables the C99 format specifier support for the newlib
nano variant since it is required for C99 standard compatibility.

Without this, Zephyr applications cannot make use of the format
specifiers newly added in the C99 standard such as `%hhu` and `%hhd`.

Moreover, the newlib `inttypes.h` defines `PRI*8` macros as `%hh*` even
when this configuration is disabled, effectively making the abstraction
provided by the `inttypes.h` useless due to lack of universality; for
this reason, the C99 format specifier support must always be enabled
for both full and nano variants of the newlib.

For more details, refer to the issue zephyrproject-rtos/zephyr#45336.

In addition, this commit disables the nano-formatted I/O build option
for the newlib nano variant because this option limits the formatted
I/O features to that of the C89 standard and overrides the C99 format
specifier support option -- refer to the newlib documentation of the
`--enable-newlib-nano-formatted-io` configuration.

Signed-off-by: Stephanos Ioannidis <[email protected]>
@stephanosio
Copy link
Member Author

stephanosio commented May 10, 2022

Building a modified samples/hello_world with printf instead of printk for qemu_cortex_m3:

Newlib-nano, before this PR

Memory region         Used Size  Region Size  %age Used
           FLASH:       14064 B       256 KB      5.36%
            SRAM:        4120 B        64 KB      6.29%
        IDT_LIST:          0 GB         2 KB      0.00%

Newlib-nano, after this PR

Memory region         Used Size  Region Size  %age Used
           FLASH:       29236 B       256 KB     11.15%
            SRAM:        4480 B        64 KB      6.84%
        IDT_LIST:          0 GB         2 KB      0.00%

Note that, by enabling the C99 format specifier support in the newlib nano variant, the flash footprint almost doubled (increased from 14064 bytes to 29236 bytes).

While this may seem like a very bad outcome at first glance, as noted earlier, this is a necessary change for us to be able to universally make use of the C99 format specifiers in the Zephyr codebase, which is necessary for C99 compliance as well as writing and supporting portable code.

To reiterate the statement made in the original issue:

we should not be optimising the newlib footprint at the cost of breaking C99 compatibility.

We could potentially further reduce the newlib nano footprint, even with the C99 format specifier support enabled, by making a number of breaking code changes to the newlib, but these changes are likely never going to be accepted by the upstream newlib project, and will be very high maintenance in the long run.

As noted in zephyrproject-rtos/zephyr#45336, with the introduction of the picolibc as a replacement for the minimal libc, the lowest end requirement of the newlib nano C library can be loosened, so the aforementioned effort to further reduce the newlib nano footprint with the C99 format specifier support enabled may not be worth the investment.

For these reasons, in spite of the significant footprint increase, we are going to proceed with this change so that the Zephyr codebase can readily make use of the C99 features and be free of the kind of issues observed in zephyrproject-rtos/zephyr#16645.

As an additional note, the newlib nano variant footprint is still considerably smaller than that of the newlib full variant even with the C99 format specifier support enabled, so the effectiveness of the nano variant is by no means nullified by this change.

Newlib-full

Memory region         Used Size  Region Size  %age Used
           FLASH:       38624 B       256 KB     14.73%
            SRAM:        6112 B        64 KB      9.33%
        IDT_LIST:          0 GB         2 KB      0.00%

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

Successfully merging this pull request may close these issues.

Incorrect newlib.h is used when nano.specs is used newlib: PRIx8 inttype incorrectly resolves to hh with newlib-nano
1 participant