toolchain: make toochain include path available to zephyr #8944

nashif · 2018-07-16T13:14:53Z

The include path is not available to Zephyr by default, this is contrary
to any other toolchain we use. For example unistd.h and friends are
available to Zephyr when building using xtools or embedded arm
toolchain, but not with the Zephyr SDK. This has been forcing us to
carry our own headers in the minimal c library.

Signed-off-by: Anas Nashif [email protected]

carlescufi

Looks definitely like an oversight when we were overwriting TOOLCHAIN_INCLUDES

codecov-io · 2018-07-16T13:45:07Z

Codecov Report

Merging #8944 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8944   +/-   ##
=======================================
  Coverage   53.23%   53.23%           
=======================================
  Files         210      210           
  Lines       25800    25800           
  Branches     5677     5677           
=======================================
  Hits        13735    13735           
  Misses       9755     9755           
  Partials     2310     2310

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed5139a...967bd01. Read the comment docs.

nashif · 2018-07-16T14:19:28Z

That one failure we get is exactly the problem we have where things work with one toolchain and not the other because of the header confusion.

galak · 2018-07-16T19:44:00Z

That one failure we get is exactly the problem we have where things work with one toolchain and not the other because of the header confusion.

So I'm not sure this makes sense. We should be using the headers associated with the libc we are using. So if we are building with newlib we'd use those, if we are building with minimal libc we should grab those headers. It seems with this change we are mixing the two.

galak

This change doesn't seem right. We are mixing headers from two different libc which seems like just subject to problems.

nashif · 2018-07-16T20:02:38Z

So I'm not sure this makes sense. We should be using the headers associated with the libc we are using.

We should be using the headers that come with the toolchain. Now the implementation of the libc (minimal lib or anything else) whether it is the one in the toolchain or a local one is a different story. This is how toolchains (other than the one we have in the zephyr sdk) are configured.

This change doesn't seem right. We are mixing headers from two different libc which seems like just subject to problems.

Most of the headers we have as part minimal libc should be removed, we have been adding empty files and duplication of headers due to this bug.

nashif · 2018-07-16T20:32:31Z

btw, try building samples/subsys/ipc/openamp with gccarmemb, it will fail because the arm toolchain even with minimal libc enabled does provide the headers of the libc built with the toolchain, i.e. newlib.

This all sounds and looks wrong, but we have been adding ad-hoc headers to support features, for example posix APIs (in include/posix), empty headers such as lib/libc/minimal/include/sys/cdefs.h and incomplete headers in other places.

galak · 2018-07-17T13:42:11Z

btw, try building samples/subsys/ipc/openamp with gccarmemb, it will fail because the arm toolchain even with minimal libc enabled does provide the headers of the libc built with the toolchain, i.e. newlib.

This all sounds and looks wrong, but we have been adding ad-hoc headers to support features, for example posix APIs (in include/posix), empty headers such as lib/libc/minimal/include/sys/cdefs.h and incomplete headers in other places.

Agree, this seems really wrong. What happens if you configure gcc w/o newlib? Do you still get a set of C headers?

nashif · 2018-07-17T14:11:36Z

Agree, this seems really wrong. What happens if you configure gcc w/o newlib? Do you still get a set of C headers?

There are a few headers that come from gcc, without newlib being built, you will have among others:

float.h
gcov.h
iso646.h
stdalign.h
stdarg.h
stdatomic.h
stdbool.h
stddef.h
stdfix.h
stdint-gcc.h
stdint.h
stdnoreturn.h
tgmath.h
unwind.h
varargs.h

with newlib, you will get all the newlib headers obviously.

aescolar

Nothing against from my side.

aescolar · 2018-07-25T17:11:06Z

arch/posix/include/posix_cheats.h

@@ -25,7 +25,6 @@

 #ifdef CONFIG_PTHREAD_IPC

-#define timespec zap_timespec


Why did you chose to remove this one? (it should work both with and without it)

The include path is not available to Zephyr by default, this is contrary to any other toolchain we use. For example unistd.h and friends are available to Zephyr when building using xtools or embedded arm toolchain, but not with the Zephyr SDK. This has been forcing us to carry our own headers in the minimal c library. Signed-off-by: Anas Nashif <[email protected]>

Remove headers that are available through toolchain and integrated libc. Signed-off-by: Anas Nashif <[email protected]>

board_output_number expects u32_t Signed-off-by: Anas Nashif <[email protected]>

Fix int type causing issues when built with newlib headers. Signed-off-by: Anas Nashif <[email protected]>

Fix printf specifiers with newlib. Signed-off-by: Anas Nashif <[email protected]>

nashif · 2019-01-10T12:34:01Z

will be resolved with sdk-ng

nashif requested a review from SebastianBoe as a code owner July 16, 2018 13:14

nashif mentioned this pull request Jul 16, 2018

Enhance 3rd party libc support #8922

Closed

carlescufi requested review from ioannisg, galak and MaureenHelm July 16, 2018 13:28

carlescufi approved these changes Jul 16, 2018

View reviewed changes

ioannisg approved these changes Jul 16, 2018

View reviewed changes

galak requested changes Jul 16, 2018

View reviewed changes

andrewboie approved these changes Jul 16, 2018

View reviewed changes

nashif force-pushed the toolchain_include_path branch from b356387 to ee1fc21 Compare July 25, 2018 14:19

nashif requested review from aescolar, jhedberg, jukkar, pfalcon, sjanc, tarunkum, tbursztyka and Vudentz as code owners July 25, 2018 14:19

nashif added the DNM This PR should not be merged (Do Not Merge) label Jul 25, 2018

aescolar reviewed Jul 25, 2018

View reviewed changes

nashif added 3 commits October 9, 2018 22:48

libc: minimal: remove headers available through toolchain

8dd14ac

Remove headers that are available through toolchain and integrated libc. Signed-off-by: Anas Nashif <[email protected]>

tests: mesh: fix int type

564e8be

board_output_number expects u32_t Signed-off-by: Anas Nashif <[email protected]>

nashif added 2 commits October 9, 2018 22:49

tests: benchmarks: fix type in console message

a5020ad

Fix int type causing issues when built with newlib headers. Signed-off-by: Anas Nashif <[email protected]>

samples: soc_flash_nrf: fix format specifier

967bd01

Fix printf specifiers with newlib. Signed-off-by: Anas Nashif <[email protected]>

nashif force-pushed the toolchain_include_path branch from ee1fc21 to 967bd01 Compare October 10, 2018 02:50

nashif mentioned this pull request Oct 10, 2018

ext: hal: altera: Don't depend on NEWLIB_LIBC #10468

Merged

nashif closed this Jan 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

toolchain: make toochain include path available to zephyr #8944

toolchain: make toochain include path available to zephyr #8944

nashif commented Jul 16, 2018

carlescufi left a comment

codecov-io commented Jul 16, 2018 •

edited

Loading

nashif commented Jul 16, 2018

galak commented Jul 16, 2018

galak left a comment

nashif commented Jul 16, 2018

nashif commented Jul 16, 2018

galak commented Jul 17, 2018

nashif commented Jul 17, 2018

aescolar left a comment

aescolar Jul 25, 2018

nashif commented Jan 10, 2019

		@@ -25,7 +25,6 @@

		#ifdef CONFIG_PTHREAD_IPC

		#define timespec zap_timespec

toolchain: make toochain include path available to zephyr #8944

toolchain: make toochain include path available to zephyr #8944

Conversation

nashif commented Jul 16, 2018

carlescufi left a comment

Choose a reason for hiding this comment

codecov-io commented Jul 16, 2018 • edited Loading

Codecov Report

nashif commented Jul 16, 2018

galak commented Jul 16, 2018

galak left a comment

Choose a reason for hiding this comment

nashif commented Jul 16, 2018

nashif commented Jul 16, 2018

galak commented Jul 17, 2018

nashif commented Jul 17, 2018

aescolar left a comment

Choose a reason for hiding this comment

aescolar Jul 25, 2018

Choose a reason for hiding this comment

nashif commented Jan 10, 2019

codecov-io commented Jul 16, 2018 •

edited

Loading