libc: Cleanup/fix setting of system include paths #14312
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@SteveOss this should hopefully fix your issue. |
galak
added
bug
Codecov Report
@@ Coverage Diff @@
## master #14312 +/- ##
=========================================
Coverage ? 52%
=========================================
Files ? 308
Lines ? 45554
Branches ? 10545
=========================================
Hits ? 23691
Misses ? 17063
Partials ? 4800Continue to review full report at Codecov.
|
andrewboie
approved these changes
Mar 12, 2019
Contributor
|
There is something wrong with this sentence:
|
mped-oticon
approved these changes
Mar 13, 2019
Contributor
|
LGTM, however this should be done for clang too |
SebastianBoe
requested changes
Mar 13, 2019
SebastianBoe
approved these changes
Mar 13, 2019
When we build with newlib we don't set -nostdinc. In that case make sure that we leave it to the toolchain to set the system include paths. The one exception to leaving to the toolchain to set the system include paths is the path to the newlib headers. Since we build with -ffreestanding we need to make sure the newlib header path is the before the toolchain headers. Otherwise the toolchain's 'freestanding' headers get picked up and that causes issues (for example getting PRI*64 defined properly from inttypes.h due to __STDC_HOSTED__ being '0'). For newlib we accomplish this by having the only system header specified by zephyr_system_include_directories() being just the newlib headers. Note: for minlibc we leave things alone as things just happen to work as the -I include of the libc headers takes precedence over -isystem so we get the libc headers over the toolchain ones. For the newlib case it appears that setting both -I and -isystem for the same dir causes the -I to be ignored. Fixes zephyrproject-rtos#14310 Signed-off-by: Kumar Gala <[email protected]>
|
Can confirm fixes my original issue. |
pabigot
added a commit
to pabigot/zephyr
that referenced
this pull request
Aug 14, 2019
The solution from zephyrproject-rtos#14312 of using -isystem to prioritize the position of the libc directory bypasses the effect of -ffreestanding with respect to libc symbols expected to be present in a non-hosted environment. Further, it breaks C++ with the ARM Embedded toolchain as the system fails to find the right file with #include_next. Use a more fine-grained solution that explicitly includes the underlying newlib header required for <inttypes.h> support before moving on to include the next available one, whether system or non-system. Closes zephyrproject-rtos#17564 Signed-off-by: Peter Bigot <[email protected]>
galak
pushed a commit
that referenced
this pull request
Aug 20, 2019
The solution from #14312 of using -isystem to prioritize the position of the libc directory bypasses the effect of -ffreestanding with respect to libc symbols expected to be present in a non-hosted environment. Further, it breaks C++ with the ARM Embedded toolchain as the system fails to find the right file with #include_next. Use a more fine-grained solution that explicitly includes the underlying newlib header required for <inttypes.h> support before moving on to include the next available one, whether system or non-system. Closes #17564 Signed-off-by: Peter Bigot <[email protected]>
pabigot
added a commit
to pabigot/zephyr
that referenced
this pull request
Aug 21, 2019
The solution from zephyrproject-rtos#14312 of using -isystem to prioritize the position of the libc directory bypasses the effect of -ffreestanding with respect to libc symbols expected to be present in a non-hosted environment. Further, it breaks C++ with the ARM Embedded toolchain as the system fails to find the right file with #include_next. Use a more fine-grained solution that explicitly includes the underlying newlib header required for <inttypes.h> support before moving on to include the next available one, whether system or non-system. Closes zephyrproject-rtos#17564 Backport: 96c1b05 Signed-off-by: Peter Bigot <[email protected]>
LeiW000
pushed a commit
to LeiW000/zephyr
that referenced
this pull request
Sep 2, 2019
The solution from zephyrproject-rtos#14312 of using -isystem to prioritize the position of the libc directory bypasses the effect of -ffreestanding with respect to libc symbols expected to be present in a non-hosted environment. Further, it breaks C++ with the ARM Embedded toolchain as the system fails to find the right file with #include_next. Use a more fine-grained solution that explicitly includes the underlying newlib header required for <inttypes.h> support before moving on to include the next available one, whether system or non-system. Closes zephyrproject-rtos#17564 Signed-off-by: Peter Bigot <[email protected]>
nashif
pushed a commit
that referenced
this pull request
Sep 25, 2019
The solution from #14312 of using -isystem to prioritize the position of the libc directory bypasses the effect of -ffreestanding with respect to libc symbols expected to be present in a non-hosted environment. Further, it breaks C++ with the ARM Embedded toolchain as the system fails to find the right file with #include_next. Use a more fine-grained solution that explicitly includes the underlying newlib header required for <inttypes.h> support before moving on to include the next available one, whether system or non-system. Closes #17564 Backport: 96c1b05 Signed-off-by: Peter Bigot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We use to build with -nostdinc and thus we explicitly set the system
include paths. Instead we should let the compiler do that and remove
doing it outselves.
Since we build with -ffreestanding we do need to explicitly set the a
system include path for the libc headers. Thus we change the libc
cases from zephyr_include_directories to
zephyr_system_include_directories. We need to ensure that the libc
headers are picked up instead of the 'freestanding' toolchain headers.
Note for whatever reason the SDK 0.9.5 version of the tools was treating
-I and -isystem similarly and thus would find the libc headers first
before the 'freestanding' headers from the toolchain.
Fixes #14310
Signed-off-by: Kumar Gala [email protected]