Only use -ffreestanding with minimal C library

[keith-packard]

As Zephyr applications are "hosted" according to the C language spec, we should be telling the compiler to perform error checking and optimizations based upon the hosted language specification instead of the freestanding one.

However, this depends on having a C library which conforms with the language specification, and the minimal C library does not. Continue to use freestanding mode when that library is in use.

See discussion: #54336 (comment)

Signed-off-by: Keith Packard [email protected]

[keith-packard]

I've split this PR into two commits, the first adds COMPILER_FREESTANDING without changing current semantics, the second adopts the semantics proposed in the PR description. Happy to split this PR apart if that makes things easier in any way.

[tejlmand]

although from a logical viewpoint it makes sense to first decide whether hosted or freestanding is to be used, then we also have a wish to have picolibc default C-library in future.

So probably we should either default COMPILER_FREESTANDING to n, or have a helper symbol, such as SUPPORTS_FREESTANDING, so that a C-library, such as PICOLIBC can do:

config SUPPORTS_FREESTANDING
    default n if PICOLIBC

and then have:

config COMPILER_FREESTANDING
	bool "Build in a freestanding compiler mode"
	depends on SUPPORTS_FREESTANDING

from user perspective, I think I prefer user should select freestanding mode first, and then can choose available C-libs, but am open to alternative.
In such case, default n is probably best for freestanding mode.

[tejlmand]

Is this a good choice ?

If COMPILER_FREESTANDING is default y, then it means that PICOLIBC cannot be selected per default.
In order to use picolibs, users / samples must explicitly disable COMPILER_FREESTANDING befora being able to select picolibc.

I don't think that is what we want.

[keith-packard]

Thanks for the help here -- I don't know how to set things up so that freestanding is used unless you're using picolibc.

[keith-packard]

although from a logical viewpoint it makes sense to first decide whether hosted or freestanding is to be used, then we also have a wish to have picolibc default C-library in future.

I'm not sure -- the only reason freestanding mode should ever be used in Zephyr is when the system cannot support hosted mode due to bugs or gaps in the implementation. Even the Linux kernel is built in hosted mode to gain all of the compiler benefits.

So probably we should either default COMPILER_FREESTANDING to n, or have a helper symbol, such as SUPPORTS_FREESTANDING, so that a C-library, such as PICOLIBC can do:

Oh, a helper symbol is probably a good idea, but I'd invert the sense -- 'SUPPORTS_HOSTED' -- so that the system can move away from freestanding mode when the underlying environment supports hosted mode.

from user perspective, I think I prefer user should select freestanding mode first, and then can choose available C-libs, but am open to alternative. In such case, default n is probably best for freestanding mode.

I disagree -- I think freestanding mode is entirely a consequence of having bugs/gaps which prevent the use of hosted mode; the user shouldn't generally care if it has to be enabled. However, it's pretty clear I don't understand the Zephyr Kconfig system well enough to get the result I'm after -- having hosted mode be selected iff the environment supports it.

[tejlmand]

I'm not sure

I agree that hosted is the optimal choice, I was more referring to the following two scenarios:

• User decides toolchain, then decides hosted (default) or freestanding -> following C-libraries are available
  or
• User decides toolchain, then decide C-library -> hosted or freestanding options are available.

unfortunately Kconfig doesn't allow for a real mutual exclusive functionality where both C-libraries and hosted/freestanding choices are available and the the other automatically becomes unavailable if the other is selected.
One will always be available as option, and then the other can be available based on whether first is enabled or not.

Like this:

config FOO
    bool "Foo"

config BAR
    bool "Bar"
    depends on !FOO

where BAR can only be enabled as long as FOO is disabled.

Oh, a helper symbol is probably a good idea, but I'd invert the sense -- 'SUPPORTS_HOSTED'

No problem, but we need to decide what choice the user should make first / always be able to make.
Should the user always be able to choose C-library, and then hosted/freestanding choices are available based on that.
Or should the user always be able between hosted/freestanding and then C-lib choices are available based on that.

having hosted mode be selected iff the environment supports it.

Do I understand this correctly, so that if the environment supports host, then hosted should always be used ?
Then it sounds like you prefer the:

• User decides toolchain, then decide C-library -> hosted or freestanding options are available.

possibility

However, it's pretty clear I don't understand the Zephyr Kconfig system well enough to get the result I'm afte

it not so much about the Zephyr Kconfig, but more a limitation in Kconfig which doesn't support config loops that results in us having to decide which symbol is always available for configuration, and then the other symbol is available depending on the value of first symbol.
And that's a decision we must make, and there will always be someone who would like the opposite solution.

[galak]

Is there a reason we need this to be user selectable?

[keith-packard]

I don't think so -- it's really just a consequence of an environment which is lacking a standard C library.

[galak]

So lets make this just bool and drop the user selectable nature

[stephanosio]

User selection may still be desirable since dropping -ffreestanding enables a few optimisations using the libc functions, which user may or may not want.

[galak]

I think it best to wait for someone to come ask for this to be user selectable. 99.99% of users are not going to know how to set it and I don't think we should expose it until someone comes with a real need.

[stephanosio]

I think it best to wait for someone to come ask for this to be user selectable. 99.99% of users are not going to know how to set it and I don't think we should expose it until someone comes with a real need.

Why? Based on that reasoning, we should not have anything user configurable until someone comes with a need.

And I can already see the usefulness of having this user configurable (e.g. debugging a "mysterious" struct assignment failure to see whether it is due to the compiler making use of the libc memcpy by toggling this config in the menuconfig).

[keith-packard]

As long as the defaults are correct, having this configurable by the user seems reasonable, it's something they "shouldn't" have to set, but it's also not something that Zephyr requires to be set in any particular way. This will also allow people to experiment with this flag, which might help them become more comfortable working in a 'hosted' environment.

[keith-packard]

Do I understand this correctly, so that if the environment supports host, then hosted should always be used ? Then it sounds like you prefer the:

• User decides toolchain, then decide C-library -> hosted or freestanding options are available.

Definitely. Hosted mode makes the compiler able to detect many common programming mistakes and generate better code. As I've said a few times, even the Linux kernel uses hosted mode to gain these benefits. Hosted mode only means you have a C library, nothing more.

[tejlmand]

Oh, a helper symbol is probably a good idea, but I'd invert the sense -- 'SUPPORTS_HOSTED' -- so that the system can move away from freestanding mode when the underlying environment supports hosted mode.

@keith-packard I noticed you updated the last commit to remove the default y on freestanding.

I have a small proposal on how we can achieve the following in Kconfig without causing Kconfig loop and have both the CONFIG_COMPILER_FREESTANDING and CONFIG_COMPILER_HOSTED available, but the advantage is that CMake / code can test for if(CONFIG_COMPILER_HOSTED)/ #ifdef CONFIG_COMPILER_HOSTED, or if(CONFIG_COMPILER_FREESTANDING)/ #ifdef CONFIG_COMPILER_FREESTANDING, no not testing is required.

+config COMPILER_HOSTED_FORCED
+       bool
+
+config COMPILER_HOSTED
+       bool
+       default y if !COMPILER_FREESTANDING
+
+config COMPILER_FREESTANDING
+       bool "Build in a freestanding compiler mode"
+       depends on !COMPILER_HOSTED_FORCED
+       help
+         <help text>
+

then various c-libraries can enforce freestanding or hosted mode.
Or let the user decide.

Like this:

config MINIMAL_LIBC
	bool "Minimal C library"
	select COMPILER_FREESTANDING

config PICOLIBC
       bool "Picolibc library"
      select COMPILER_HOSTED_FORCED

This has the following behavior.
No Kconfig setting loops.
COMPILER_HOSTED will always has the opposite value of COMPILER_FREESTANDING.

A C library can force the use of freestanding mode by selecting COMPILER_FREESTANDING
A C library can force the use of hosted mode by selecting COMPILER_HOSTED_FORCED, by which COMPILER_FREESTANDING=n and thereby COMPILER_HOSTED=y

If a c-library doesn't enforce a particular mode, then the COMPILER_FREESTANDING is available for user selection, with a default value of n.

I assume this is the behavior we want to achieve.

[stephanosio]

We are way over-complicating this for no good reason.

The solution should be as simple as adding config COMPILER_FREESTANDING with prompt and selecting it from MINIMAL_LIBC such that it is always enabled when MINIMAL_LIBC is used and is user-configurable otherwise.

[galak]

The solution should be as simple as adding config COMPILER_FREESTANDING with prompt and selecting it from MINIMAL_LIBC such that it is always enabled when MINIMAL_LIBC is used and is user-configurable otherwise.

Agree with the first half, I still disagree with making this user-configurable.

[tejlmand]

The solution should be as simple as adding config COMPILER_FREESTANDING with prompt and selecting it from MINIMAL_LIBC such that it is always enabled when MINIMAL_LIBC is used and is user-configurable otherwise.

if I understand @keith-packard correctly and this commit 92ae8bc

config PICOLIBC
	bool "Picolibc library"
	...
	depends on !COMPILER_FREESTANDING

then freestanding mode should never be chosen for picolibc.

If that is the case, we have three situations, forcing freestanding, forcing hosted, let user decide.

The COMPILER_HOSTED is only to allow straight forward testing, like if(CONFIG_COMPILER_HOSTED) instead of if(NOT CONFIG_COMPILER_HOSTED), and can be left out, but I don't see a invert symbol as over-complicated.

[keith-packard]

then freestanding mode should never be chosen for picolibc.

yeah, I just went and tried to build picolibc with -ffreestanding and there were compile warnings and some test suite failures. I'll treat those as bugs -- this "should" work, but it pretty clearly doesn't right now.

[stephanosio]

then freestanding mode should never be chosen for picolibc.

yeah, I just went and tried to build picolibc with -ffreestanding and there were compile warnings and some test suite failures. I'll treat those as bugs -- this "should" work, but it pretty clearly doesn't right now.

Agreed. Picolibc should be able to build for both "hosted" and "freestanding" environments; there is no technical reason why PICOLIBC depends on !COMPILER_FREESTANDING.

[keith-packard]

Agreed. Picolibc should be able to build for both "hosted" and "freestanding" environments; there is no technical reason why PICOLIBC depends on !COMPILER_FREESTANDING.

I've fixed the bugs upstream and am working on a 1.8.1 (or maybe 1.9) release in the next month or so. However, picolibc CI isn't running tests in -ffreestanding mode currently. I need to add a meson configuration option to add -ffreestanding to the compile options to make this easy; otherwise it would require separate cross compilation control scripts.

[cfriedt]

Looks like there are some minor clang warnings that come up about using abs() on float types. They seem to be scattered throughout our code

[keith-packard]

I've fixed the bugs upstream and am working on a 1.8.1 (or maybe 1.9) release in the next month or so.

With picolibc/picolibc#438 picolibc will get regular CI validation of -ffreestanding to ensure the library builds and operates correctly when that flag is used. To be clear, I don't think that's a good idea because of the reduction in compiler optimization and error detection, but until Zephyr users get comfortable with the transition, it's good to know the C library won't have faults as a result. I can submit a PR to move the picolibc module forward to this point; that's not required for the SDK as that doesn't ever build with -ffreestanding.

[keith-packard]

Looks like there are some minor clang warnings that come up about using abs() on float types. They seem to be scattered throughout our code

That sounds a bit like a request for changes; I'll see if I can clean these up.

[keith-packard]

Looks like there are some minor clang warnings that come up about using abs() on float types. They seem to be scattered throughout our code

That sounds a bit like a request for changes; I'll see if I can clean these up.

Here's what I've got:

• vl53l0x: Cast unsigned abs param to signed type to fix clang warning hal_st#14
• Clang fixes when building without -ffreestanding #56396

[carlescufi]

@keith-packard is this PR blocked by #54628 ? i.e. will CI fail here until we change the return type of main?

[keith-packard]

@keith-packard is this PR blocked by #54628 ? i.e. will CI fail here until we change the return type of main?

Thanks for checking. Yes, I think it is blocked by that; with the other remaining PR merged, I think that's all we're waiting for. I've rebased to verify -- the clang build should fail on newlib and picolibc test cases.

[keith-packard]

@keith-packard is this PR blocked by #54628 ? i.e. will CI fail here until we change the return type of main?

While the build here is failing with clang, that doesn't appear related to the main return type. Running the clang tests locally on native_posix, they all pass. That's not surprising as the native_posix architecture has it's own main entry point; any Zephyr main entry point is renamed via "preprocessor trickery".

So, while this PR should pass CI, I think we should block on #54628 as anyone using clang on other architectures will be impacted.

Help figuring out why the clang build here is failing would be appreciate; I re-tried the tests once and they still failed with mysterious errors.

[keith-packard]

I've switched the minimal libc Kconfig entry to use imply COMPILER_FREESTANDING as that will allow users to override the default and build without -ffreestanding even when using the minimal C library.

[keith-packard]

Help figuring out why the clang build here is failing would be appreciate; I re-tried the tests once and they still failed with mysterious errors.

ok, looks like the build errors have mysteriously resolved themselves. I think the code here is ready to go.